Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor node validation in Inner Ring code #2578

Open
cthulhu-rider opened this issue Sep 14, 2023 · 3 comments
Open

Refactor node validation in Inner Ring code #2578

cthulhu-rider opened this issue Sep 14, 2023 · 3 comments
Labels
enhancement Improving existing functionality good first issue Good for newcomers I4 No visible changes neofs-ir Inner Ring node application issues S2 Regular significance U4 Nothing urgent

Comments

@cthulhu-rider
Copy link
Contributor

currently Inner Ring verifies and updates descriptors of the storage nodes to be set in the NeoFS network map via https://pkg.go.dev/github.com/nspcc-dev/neofs-node/pkg/innerring/processors/netmap#NodeValidator

it's implemented through several independed packages https://github.com/nspcc-dev/neofs-node/tree/master/pkg/innerring/processors/netmap/nodevalidation

there are two disadvantages in such approach:

  1. since each "small" validator may change descriptor through pointer (e.g. LOCODE one), they must be strictly ordered, so https://pkg.go.dev/github.com/nspcc-dev/neofs-node@v0.38.0/pkg/innerring/processors/netmap/nodevalidation#CompositeValidator becomes unusable. We've already encountered such problem in 4c55307
  2. previous point also happens because different validators work with the same parts. For example, locode, structure and attribute (coming with Verifiable attributes #2280) work with attributes but stay out of sync, so they must repeat similar checks

i propose to refactor this part of code to make it more simple (e.g. gather in one package) and safe. Power of interfaces will allow us to make it easy to understand and test

@cthulhu-rider cthulhu-rider added good first issue Good for newcomers triage neofs-ir Inner Ring node application issues labels Sep 14, 2023
@roman-khimov
Copy link
Member

Refs. #2279.

But specifically about changing attributes, this should be removed out of IR scope. Whatever SN sets is either taken or rejected as is.

@carpawell
Copy link
Member

this should be removed out of IR scope

Do we have an issue for that? I do not think that is as simple as "Refactor node validation". More components should be affected.

@cthulhu-rider
Copy link
Contributor Author

cthulhu-rider commented Sep 14, 2023

yep, this issue is about refactor only, it doesn't matter will it be resolved before or after mutation drop. But taking #2279 into account, first of all we could split verify and update stages first to easily drop verify one later

@roman-khimov roman-khimov added enhancement Improving existing functionality U4 Nothing urgent S2 Regular significance I4 No visible changes and removed triage labels Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improving existing functionality good first issue Good for newcomers I4 No visible changes neofs-ir Inner Ring node application issues S2 Regular significance U4 Nothing urgent
Projects
None yet
Development

No branches or pull requests

3 participants