Refactor node validation in Inner Ring code #1127

New issue

Open

opened 2025-12-28 17:21:53 +00:00 by sami · 3 comments

sami commented

2025-12-28 17:21:53 +00:00

Owner

Originally created by @cthulhu-rider on GitHub (Sep 14, 2023).

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:

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 4c55307421
previous point also happens because different validators work with the same parts. For example, locode, structure and attribute (coming with #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

Originally created by @cthulhu-rider on GitHub (Sep 14, 2023). 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 4c55307421e75e81a098490eada5dd48e2e20fc2 2. previous point also happens because different validators work with the same parts. For example, `locode`, `structure` and `attribute` (coming with #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

sami added the

enhancement

good first issue

neofs-ir

labels

2025-12-28 17:21:53 +00:00

sami commented

2025-12-28 17:21:53 +00:00

Author

Owner

@roman-khimov commented on GitHub (Sep 14, 2023):

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.

@roman-khimov commented on GitHub (Sep 14, 2023): 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.

sami commented

2025-12-28 17:21:53 +00:00

Author

Owner

@carpawell commented on GitHub (Sep 14, 2023):

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.

@carpawell commented on GitHub (Sep 14, 2023): > 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.

sami commented

2025-12-28 17:21:53 +00:00

Author

Owner

@cthulhu-rider commented on GitHub (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

@cthulhu-rider commented on GitHub (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