Refactor IR #1050

New issue

Open

opened 2025-12-28 17:21:36 +00:00 by sami · 2 comments

sami commented

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

Owner

Originally created by @notimetoname on GitHub (Jun 28, 2023).

Some questions:

Why does it have workers, starters and runners? Also, all of them are anonymous, it makes it difficult to understand what "services" (well, just func ()) are running/will be running.
Why does it read config everywhere, everywhere, everywhere, etc it wants? SN has been using a separate place for config reading for years already. After config is parsed it passes (and modifies its types if need be) in every component that requires it.
Its constructor is 650+ lines long. It does many things (converting types, creating new types, reading configs, making some RPC, logging, filling DB, deciding the full IR work mode in a huge if ... else, etc). I mean that is not bad that everything is happening at the New step, I mean that most of that is done literally inside that func.
https://github.com/nspcc-dev/neofs-node/issues/2309 will require changes that should drop processors, listeners all related things. #2309 says "Each service can do Receive* for things it needs..." but IR has only two services in fact: timers (not services TBH, they only can accept Tick calls and be Reseted), and a Listener that should be dropped. It also makes anything related to https://github.com/nspcc-dev/neofs-node/issues/1337 hard to implement easily.

Originally created by @notimetoname on GitHub (Jun 28, 2023). Some questions: 1. Why does it have [workers](https://github.com/nspcc-dev/neofs-node/blob/5ca26a877c1ad5c21b416a00d292a50bd40964c7/pkg/innerring/innerring.go#L107), [starters](https://github.com/nspcc-dev/neofs-node/blob/5ca26a877c1ad5c21b416a00d292a50bd40964c7/pkg/innerring/innerring.go#L115) and [runners](https://github.com/nspcc-dev/neofs-node/blob/5ca26a877c1ad5c21b416a00d292a50bd40964c7/pkg/innerring/innerring.go#L129C3-L129C10)? Also, all of them are anonymous, it makes it difficult to understand what "services" (well, just `func ()`) are running/will be running. 2. Why does it read config [everywhere](https://github.com/nspcc-dev/neofs-node/blob/5ca26a877c1ad5c21b416a00d292a50bd40964c7/pkg/innerring/innerring.go#L358-L359), [everywhere](https://github.com/nspcc-dev/neofs-node/blob/5ca26a877c1ad5c21b416a00d292a50bd40964c7/pkg/innerring/innerring.go#L431), [everywhere](https://github.com/nspcc-dev/neofs-node/blob/5ca26a877c1ad5c21b416a00d292a50bd40964c7/pkg/innerring/innerring.go#L1021), etc it wants? SN has been using a separate place for [config reading](https://github.com/nspcc-dev/neofs-node/blob/5ca26a877c1ad5c21b416a00d292a50bd40964c7/cmd/neofs-node/config.go#L176) for years already. After config is parsed it [passes](https://github.com/nspcc-dev/neofs-node/blob/5ca26a877c1ad5c21b416a00d292a50bd40964c7/cmd/neofs-node/config.go#L532) (and [modifies](https://github.com/nspcc-dev/neofs-node/blob/5ca26a877c1ad5c21b416a00d292a50bd40964c7/cmd/neofs-node/config.go#L529) its types if need be) in every component that requires it. 3. Its [constructor](https://github.com/nspcc-dev/neofs-node/blob/5ca26a877c1ad5c21b416a00d292a50bd40964c7/pkg/innerring/innerring.go#L325) is 650+ lines long. It does many things (converting types, creating new types, reading configs, making some RPC, logging, filling DB, deciding the full IR work mode in a huge `if ... else`, etc). I mean that is not bad that everything is happening at the `New` step, I mean that most of that is done literally inside that func. 4. https://github.com/nspcc-dev/neofs-node/issues/2309 will require changes that should drop processors, listeners all related things. #2309 says "Each service can do Receive* for things it needs..." but IR has only two services in fact: timers (not services TBH, they only can accept `Tick` calls and be `Reset`ed), and a `Listener` that should be dropped. It also makes anything related to https://github.com/nspcc-dev/neofs-node/issues/1337 hard to implement easily.

sami added the

labels

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

sami commented

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

Author

Owner

@roman-khimov commented on GitHub (Jun 29, 2023):

The way I see it working is:

IR main reads config and keeps it internally forever
it creates connections
it passes them to innerring.New()
which can do whatever it needs to initialialize (read various data) and set up event listeners
it works until some connection breaks
when it breaks IR stops completely (including additional goroutines if any)
main kicks in, reconnects to RPC
and restarts IR again

The rationale is rather simple: IR state is completely stored on blockchain, it's a service that doesn't differ much from Oracle or Notary services, so it MUST NOT deal with connections internally at all, it should take them as a given. It also shouldn't care about configuration files, these are cmd things by definition.

@roman-khimov commented on GitHub (Jun 29, 2023): The way I see it working is: * IR `main` reads config and keeps it internally forever * it creates connections * it passes them to `innerring.New()` * which can do whatever it needs to initialialize (read various data) and set up event listeners * it works until some connection breaks * when it breaks IR stops completely (including additional goroutines if any) * `main` kicks in, reconnects to RPC * and restarts IR again The rationale is rather simple: IR state is completely stored on blockchain, it's a service that doesn't differ much from Oracle or Notary services, so it MUST NOT deal with connections internally at all, it should take them as a given. It also shouldn't care about configuration files, these are `cmd` things by definition.

sami commented

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

Author

Owner

@cthulhu-rider commented on GitHub (Jul 3, 2023):

if Inner Ring is based on and works around blockchain, then in my mind it should be constructed from type Blockchain interface providing all IR-expected stuff. From IR pov, Blockchain wont deal with connections (no endpoint, conn parameters).

@cthulhu-rider commented on GitHub (Jul 3, 2023): if Inner Ring is based on and works around blockchain, then in my mind it should be constructed from `type Blockchain interface` providing all IR-expected stuff. From IR pov, `Blockchain` wont deal with connections (no endpoint, conn parameters).