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

rpc: ensure that (*Client).AddNetworkFee can properly calculate network fee for main Notary transaction #1799

Closed
AnnaShaleva opened this issue Mar 3, 2021 · 2 comments · Fixed by #1825
Assignees
Labels
I3 Minimal impact

Comments

@AnnaShaleva
Copy link
Member

Consider the following incomplete Notary main transaction signers:

  1. Deployed contract with verify method
  2. Multisig signer
  3. Notary contract

We need to ensure that (*Client).AddNetworkFee is able to calculate the network fee for signers 1 and 2 properly.

The resulting transaction network fee (which is AddNetworkFee(...) + CalculateNotaryFee(...)) should be enough to complete the transaction and to send it to the chain.

@AnnaShaleva AnnaShaleva added the I3 Minimal impact label Mar 3, 2021
@AnnaShaleva AnnaShaleva self-assigned this Mar 3, 2021
@roman-khimov
Copy link
Member

The problem is that deployed contract can demand any kind of invocation script. It can be empty script, it can have some signature inside, it can have multiple signatures inside, it can have hash/hashes, whatever. Invocation script obviously affects size/GAS spent on verification, so it's impossible to solve in generic way.

@AnnaShaleva
Copy link
Member Author

AnnaShaleva commented Mar 11, 2021

@roman-khimov, I think that at least we can solve this problem for empty invocation scripts (which is what our AddNetworkFee does). And this solves the problem with extra GAS addition for NeoFS proxy-contract witness verification (see the comment in pkg/morph/client/notary.go, L21-23)

But with the changes from #1825 it's not hard to extend the functionality of the AddNeworkFee to work with non-empty invocation scripts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I3 Minimal impact
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants