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

Use morph/client package in inner ring instead of invoke #496

Closed
alexvanin opened this issue Apr 28, 2021 · 1 comment · Fixed by #574
Closed

Use morph/client package in inner ring instead of invoke #496

alexvanin opened this issue Apr 28, 2021 · 1 comment · Fixed by #574
Assignees
Labels
neofs-ir Inner Ring node application issues

Comments

@alexvanin
Copy link
Contributor

Right now inner ring has two ways to invoke chain methods:

  • by using wrapper functions in innerring/invoke package,
  • by using wrapper structures from morph/client package.

(1) functions were designed to be light weight wrappers around client. (2) structures were designed with complex logic inside: taking and marshaling structures from arguments, using predefined invocation fee, etc.

(2) already used in storage node and has some adoption in inner ring for various processors. I suppose we can completely drop wrapper functions from innerring/invoke and use only morph/client wrapper instances to avoid this duality.

@alexvanin
Copy link
Contributor Author

alexvanin commented May 25, 2021

After #505 we need to:

  • use morph/client wrappers in inner ring processors,
  • simplify static client methods.

Wrappers

As for wrappers right now there are many places where low level morphClient used to fetch and parse data.

// pkg/innerring/innerring.go
func (s *Server) initConfigFromBlockchain() error {
	// get current epoch
	val, err := s.morphClient.TestInvoke(s.contracts.netmap, getEpochMethod)
	if err != nil {
		return fmt.Errorf("can't read epoch: %w", err)
	}

	epoch, err := client.IntFromStackItem(val[0])
	if err != nil {
		return fmt.Errorf("can't parse epoch: %w", err)
	}

Instead this should be offloaded to wrappers in morph/client package. These wrappers can be created in-place in processors, so there won't be wrapper initialization section in inner ring server.

Static client methods

As for static clients, we've decided to add boolean flag if notary enalbed in static client constructors. Depending on flag, static client will call underlying client.InvokeNotary or client.Invoke methods. Static client itself (and it's wrappers) won't have two function for single contract methods invocation.

cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue May 25, 2021
There are two scenarios of invocation of contract methods:
  1. do not invoke notary contract;
  2. try to invoke notary contract if it is enabled in Client.

Taking this into account, `StaticClient` can work in one of the two described
modes. Based on this, it makes sense at the stage of creating `StaticClient`
to fix the call mode, and the further abstract from it.

Define `StaticClientOption` setters of `StaticClient` optional parameters.
Add `TryNotary` constructor of option which enables notary tries. Call
`NotaryInvoke` on underlying `Client` if the option is provided, otherwise
call `Invoke`. Mark `NotaryInvoke` method of `StaticClient` as deprecated.

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue May 25, 2021
Some of the client wrapper's methods should produce notary contract's
invocations. In previous implementation all wrappers provided separate
methods to do it. Since notary and non-notary invocation scenarios have very
different goals, it makes sense to separate the scenarios of using the
client wrapper  at the stage of its creation.

Define `Option` constructor for container client wrapper. Add `TryNotary`
option which enables tries of the notary invocations on underlying static
client. Mark all notary-dedicated methods as deprecated.

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue May 25, 2021
All client wrappers should use underlying static client with enabled notary
work mode in order to produce invocations of notary contract.

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue May 25, 2021
There are two scenarios of invocation of contract methods:
  1. do not invoke notary contract;
  2. try to invoke notary contract if it is enabled in Client.

Taking this into account, `StaticClient` can work in one of the two described
modes. Based on this, it makes sense at the stage of creating `StaticClient`
to fix the call mode, and the further abstract from it.

Define `StaticClientOption` setters of `StaticClient` optional parameters.
Add `TryNotary` constructor of option which enables notary tries. Call
`NotaryInvoke` on underlying `Client` if the option is provided, otherwise
call `Invoke`. Mark `NotaryInvoke` method of `StaticClient` as deprecated.

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue May 25, 2021
Some of the client wrapper's methods should produce notary contract's
invocations. In previous implementation all wrappers provided separate
methods to do it. Since notary and non-notary invocation scenarios have very
different goals, it makes sense to separate the scenarios of using the
client wrapper  at the stage of its creation.

Define `Option` constructor for container client wrapper. Add `TryNotary`
option which enables tries of the notary invocations on underlying static
client. Mark all notary-dedicated methods as deprecated.

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue May 25, 2021
All client wrappers should use underlying static client with enabled notary
work mode in order to produce invocations of notary contract.

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
neofs-ir Inner Ring node application issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants