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

Integrate NeoGo CN into IR node #2194

Closed
roman-khimov opened this issue Jan 9, 2023 · 0 comments · Fixed by #2277
Closed

Integrate NeoGo CN into IR node #2194

roman-khimov opened this issue Jan 9, 2023 · 0 comments · Fixed by #2277
Assignees
Labels
config Configuration format update or breaking change enhancement Improving existing functionality neofs-ir Inner Ring node application issues
Milestone

Comments

@roman-khimov
Copy link
Member

Is your feature request related to a problem? Please describe.

I'm always frustrated when I have to update NeoFS because it has so many moving parts and links:
Screenshot_20221215_152853

One of the problems specifically is IR/CN relationship. On one hand they're different components performing somewhat different tasks and theoretically they could be deployed to different nodes, use different keys and so on. On the other we always have 1:1 relationship between them (for Alphabet), they always use the same key and they're always on the same machine.

Describe the solution you'd like

We can simplify deployment/updates by merging them into one service (binary). IR node would get a ProtocolConfiguration section of the NeoGo config initially, will run an instance of core.Blockchain along with network.Server and consensus.Service inside and IR code itself can be just another service managed by network.Server (see https://github.com/nspcc-dev/neo-go/blob/v0.100.1/cli/server/server.go#L428 also).

IR will then subscribe to events internally via https://pkg.go.dev/github.com/nspcc-dev/neo-go/pkg/core#Blockchain.SubscribeForNotifications and could rely on this channel to always work. Transactions could be pushed via https://pkg.go.dev/github.com/nspcc-dev/neo-go/pkg/network#Server.RelayTxn and https://pkg.go.dev/github.com/nspcc-dev/neo-go/pkg/network#Server.RelayP2PNotaryRequest. Initially I though that RPC configuration can still be useful, but in fact it can be removed, even public networks use 1:1 IR/CN setups, so we can simplify the code wrt this.

Of course we can also have non-Alphabet IR nodes, but those could just run a regular RPC NeoGo instance internally.

Pros:

  • no need to manage/configure another service
  • CN will always have the right version compatible with IR (no problems with outdated/too new CNs)
  • faster CN/IR communication
  • more reliable CN/IR communication (this link can't go down)

Cons:

  • theoretical scalability issues

Describe alternatives you've considered

The old model as is. But it's hard to manage even in presence of Ansible roles/neofs-adm tools.

Additional context

We'll get this picture instead of the previous one:
Notes_230109_155422

Notice that M nodes can be omitted as well in many cases, leaving us with just two binaries.

@roman-khimov roman-khimov added enhancement Improving existing functionality triage neofs-ir Inner Ring node application issues config Configuration format update or breaking change labels Jan 9, 2023
@cthulhu-rider cthulhu-rider self-assigned this Jan 10, 2023
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Jan 26, 2023
…us node

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Jan 30, 2023
In previous implementation `distillNotaryRequests` and
`distillNotifications` could lock on destination channel write if caller
aborted reading it by external signal.

Remove select-case with abort channel in `listenNotifications` and
`listenNotaryRequests` to avoid potential locks.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Feb 9, 2023
…us node

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Feb 9, 2023
In previous implementation `distillNotaryRequests` and
`distillNotifications` could lock on destination channel write if caller
aborted reading it by external signal.

Remove select-case with abort channel in `listenNotifications` and
`listenNotaryRequests` to avoid potential locks.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Feb 9, 2023
…us node

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Feb 9, 2023
In previous implementation `distillNotaryRequests` and
`distillNotifications` could lock on destination channel write if caller
aborted reading it by external signal.

Remove select-case with abort channel in `listenNotifications` and
`listenNotaryRequests` to avoid potential locks.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Feb 9, 2023
Inner Ring always waits for synchronization to complete before before
opening RPC interfaces.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Feb 9, 2023
…us node

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Feb 9, 2023
In previous implementation `distillNotaryRequests` and
`distillNotifications` could lock on destination channel write if caller
aborted reading it by external signal.

Remove select-case with abort channel in `listenNotifications` and
`listenNotaryRequests` to avoid potential locks.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Feb 9, 2023
Inner Ring always waits for synchronization to complete before before
opening RPC interfaces.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Feb 9, 2023
…us node

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Feb 9, 2023
In previous implementation `distillNotaryRequests` and
`distillNotifications` could lock on destination channel write if caller
aborted reading it by external signal.

Remove select-case with abort channel in `listenNotifications` and
`listenNotaryRequests` to avoid potential locks.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Feb 9, 2023
Inner Ring always waits for synchronization to complete before before
opening RPC interfaces.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Feb 9, 2023
There is no need to specify some Neo Go configurations in Inner Ring
node, on the contrary, it makes sense to make them static. At the same
time, using the Neo Go config engine is still handy.

Make `blockchain.New` to accept wallet filepath and password of the
node's account and provide these value as `UnlockWallet` config.
Determine presence of Consensus and Notary services by the committee
lookup. Always enable relay mode.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Feb 9, 2023
Inner Ring always waits for synchronization to complete before before
opening RPC interfaces.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Feb 9, 2023
There is no need to specify some Neo Go configurations in Inner Ring
node, on the contrary, it makes sense to make them static. At the same
time, using the Neo Go config engine is still handy.

Make `blockchain.New` to accept wallet filepath and password of the
node's account and provide these value as `UnlockWallet` config.
Determine presence of Consensus and Notary services by the committee
lookup. Always enable relay mode.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Feb 14, 2023
Instructions from previous implementation can be done using actor
framework provided by Neo Go, so there is no need to dig into
transaction.

Add `Blockchain.CallContractNotarized` which creates a Notary service
request to call a contract from scratch. Signers of such transaction are
Alphabet members, Proxy and Notary contracts. Share actor construction
with the `SignAndSendTransactionNotary` method. Make `ApproveWithdrawal`
to just call `CallContractNotarized` on `Blockchain` instance of the
NeoFS Sidechain.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Feb 20, 2023
…y Neo Go

Replace `directRPCActor` with `rpcclient.Internal` one which is based on
the `rpcsrv.Server`. Remove no longer used type.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Feb 20, 2023
`Blockchain` almost definitely uses `notary.RPCActor` which should be
closed (`rpcclient` package products). While actor interface itself
doesn't container closing method, direct usage of `rpcclient.Internal`
enhances type coupling.

Call `Close()` method on `notary.RPCActor` if implemented.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
@roman-khimov roman-khimov added this to the v0.36.0 milestone Mar 28, 2023
roman-khimov added a commit that referenced this issue Mar 31, 2023
* closes #2194 
* reborn of #2211 

## Summary
New launch mode is a pure feature activated when `morph.client.endpoint`
is omitted only.

Local blockchain is configured in `morph.consensus` section.

NeoGo module is upgraded to revision with meaningful fix making
consensus switch loop work.

Only meaningful Sidechain settings are configured. Some NeoGo defaults
are overridden according to the specifics of NeoFS Sidechains
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config Configuration format update or breaking change enhancement Improving existing functionality neofs-ir Inner Ring node application issues
Projects
None yet
2 participants