From 2d196b3f3575eedc849b6e028f9376e769dd8b56 Mon Sep 17 00:00:00 2001 From: AnnaShaleva Date: Mon, 25 Oct 2021 17:42:20 +0300 Subject: [PATCH] rpc: refactor `calculatenetworkfee` handler Use (Blockchainer).VerifyWitness() to calculate network fee for contract-based witnesses. --- internal/fakechain/fakechain.go | 4 ++-- pkg/consensus/payload_test.go | 6 ++++-- pkg/core/bench_test.go | 2 +- pkg/core/blockchain.go | 11 +++++----- pkg/core/blockchainer/blockchainer.go | 2 +- pkg/core/stateroot/module.go | 3 ++- pkg/network/extpool/pool.go | 4 ++-- pkg/network/extpool/pool_test.go | 6 +++--- pkg/network/server.go | 2 +- pkg/network/server_test.go | 10 ++++----- pkg/rpc/server/server.go | 31 +++------------------------ pkg/services/notary/notary.go | 2 +- 12 files changed, 31 insertions(+), 52 deletions(-) diff --git a/internal/fakechain/fakechain.go b/internal/fakechain/fakechain.go index 73d0d9746..748315754 100644 --- a/internal/fakechain/fakechain.go +++ b/internal/fakechain/fakechain.go @@ -37,7 +37,7 @@ type FakeChain struct { blocks map[util.Uint256]*block.Block hdrHashes map[uint32]util.Uint256 txs map[util.Uint256]*transaction.Transaction - VerifyWitnessF func() error + VerifyWitnessF func() (int64, error) MaxVerificationGAS int64 NotaryContractScriptHash util.Uint160 NotaryDepositExpiration uint32 @@ -430,7 +430,7 @@ func (chain *FakeChain) VerifyTx(*transaction.Transaction) error { } // VerifyWitness implements Blockchainer interface. -func (chain *FakeChain) VerifyWitness(util.Uint160, hash.Hashable, *transaction.Witness, int64) error { +func (chain *FakeChain) VerifyWitness(util.Uint160, hash.Hashable, *transaction.Witness, int64) (int64, error) { if chain.VerifyWitnessF != nil { return chain.VerifyWitnessF() } diff --git a/pkg/consensus/payload_test.go b/pkg/consensus/payload_test.go index f5d6175b9..3101f6072 100644 --- a/pkg/consensus/payload_test.go +++ b/pkg/consensus/payload_test.go @@ -262,9 +262,11 @@ func TestPayload_Sign(t *testing.T) { p := randomPayload(t, prepareRequestType) h := priv.PublicKey().GetScriptHash() bc := newTestChain(t, false) - require.Error(t, bc.VerifyWitness(h, p, &p.Witness, payloadGasLimit)) + _, err = bc.VerifyWitness(h, p, &p.Witness, payloadGasLimit) + require.Error(t, err) require.NoError(t, p.Sign(priv)) - require.NoError(t, bc.VerifyWitness(h, p, &p.Witness, payloadGasLimit)) + _, err = bc.VerifyWitness(h, p, &p.Witness, payloadGasLimit) + require.NoError(t, err) } func TestMessageType_String(t *testing.T) { diff --git a/pkg/core/bench_test.go b/pkg/core/bench_test.go index 24beadb32..bf4ae9126 100644 --- a/pkg/core/bench_test.go +++ b/pkg/core/bench_test.go @@ -19,6 +19,6 @@ func BenchmarkVerifyWitness(t *testing.B) { t.ResetTimer() for n := 0; n < t.N; n++ { - _ = bc.VerifyWitness(tx.Signers[0].Account, tx, &tx.Scripts[0], 100000000) + _, _ = bc.VerifyWitness(tx.Signers[0].Account, tx, &tx.Scripts[0], 100000000) } } diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 77543467e..38172577e 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -2116,12 +2116,12 @@ func (bc *Blockchain) InitVerificationVM(v *vm.VM, getContract func(util.Uint160 return nil } -// VerifyWitness checks that w is a correct witness for c signed by h. -func (bc *Blockchain) VerifyWitness(h util.Uint160, c hash.Hashable, w *transaction.Witness, gas int64) error { +// VerifyWitness checks that w is a correct witness for c signed by h. It returns +// the amount of GAS consumed during verification and an error. +func (bc *Blockchain) VerifyWitness(h util.Uint160, c hash.Hashable, w *transaction.Witness, gas int64) (int64, error) { ic := bc.newInteropContext(trigger.Verification, bc.dao, nil, nil) ic.Container = c - _, err := bc.verifyHashAgainstScript(h, w, ic, gas) - return err + return bc.verifyHashAgainstScript(h, w, ic, gas) } // verifyHashAgainstScript verifies given hash against the given witness and returns the amount of GAS consumed. @@ -2197,7 +2197,8 @@ func (bc *Blockchain) verifyHeaderWitnesses(currHeader, prevHeader *block.Header } else { hash = prevHeader.NextConsensus } - return bc.VerifyWitness(hash, currHeader, &currHeader.Script, HeaderVerificationGasLimit) + _, err := bc.VerifyWitness(hash, currHeader, &currHeader.Script, HeaderVerificationGasLimit) + return err } // GoverningTokenHash returns the governing token (NEO) native contract hash. diff --git a/pkg/core/blockchainer/blockchainer.go b/pkg/core/blockchainer/blockchainer.go index b63bca04c..c4c24573f 100644 --- a/pkg/core/blockchainer/blockchainer.go +++ b/pkg/core/blockchainer/blockchainer.go @@ -73,7 +73,7 @@ type Blockchainer interface { SubscribeForNotifications(ch chan<- *subscriptions.NotificationEvent) SubscribeForTransactions(ch chan<- *transaction.Transaction) VerifyTx(*transaction.Transaction) error - VerifyWitness(util.Uint160, hash.Hashable, *transaction.Witness, int64) error + VerifyWitness(util.Uint160, hash.Hashable, *transaction.Witness, int64) (int64, error) GetMemPool() *mempool.Pool UnsubscribeFromBlocks(ch chan<- *block.Block) UnsubscribeFromExecutions(ch chan<- *state.AppExecResult) diff --git a/pkg/core/stateroot/module.go b/pkg/core/stateroot/module.go index fdc053595..e5e7a5439 100644 --- a/pkg/core/stateroot/module.go +++ b/pkg/core/stateroot/module.go @@ -238,5 +238,6 @@ func (s *Module) verifyWitness(r *state.MPTRoot) error { s.mtx.Lock() h := s.getKeyCacheForHeight(r.Index).validatorsHash s.mtx.Unlock() - return s.bc.VerifyWitness(h, r, &r.Witness[0], maxVerificationGAS) + _, err := s.bc.VerifyWitness(h, r, &r.Witness[0], maxVerificationGAS) + return err } diff --git a/pkg/network/extpool/pool.go b/pkg/network/extpool/pool.go index bbf315fd2..3044c461a 100644 --- a/pkg/network/extpool/pool.go +++ b/pkg/network/extpool/pool.go @@ -69,7 +69,7 @@ func (p *Pool) Add(e *payload.Extensible) (bool, error) { } func (p *Pool) verify(e *payload.Extensible) (bool, error) { - if err := p.chain.VerifyWitness(e.Sender, e, &e.Witness, extensibleVerifyMaxGAS); err != nil { + if _, err := p.chain.VerifyWitness(e.Sender, e, &e.Witness, extensibleVerifyMaxGAS); err != nil { return false, err } h := p.chain.BlockHeight() @@ -118,7 +118,7 @@ func (p *Pool) RemoveStale(index uint32) { lst.Remove(old) continue } - if err := p.chain.VerifyWitness(e.Sender, e, &e.Witness, extensibleVerifyMaxGAS); err != nil { + if _, err := p.chain.VerifyWitness(e.Sender, e, &e.Witness, extensibleVerifyMaxGAS); err != nil { delete(p.verified, h) lst.Remove(old) continue diff --git a/pkg/network/extpool/pool_test.go b/pkg/network/extpool/pool_test.go index 0919fff49..754f4f493 100644 --- a/pkg/network/extpool/pool_test.go +++ b/pkg/network/extpool/pool_test.go @@ -134,11 +134,11 @@ func newTestChain() *testChain { }, } } -func (c *testChain) VerifyWitness(u util.Uint160, _ hash.Hashable, _ *transaction.Witness, _ int64) error { +func (c *testChain) VerifyWitness(u util.Uint160, _ hash.Hashable, _ *transaction.Witness, _ int64) (int64, error) { if !c.verifyWitness(u) { - return errVerification + return 0, errVerification } - return nil + return 0, nil } func (c *testChain) IsExtensibleAllowed(u util.Uint160) bool { return c.isAllowed(u) diff --git a/pkg/network/server.go b/pkg/network/server.go index 9ec819978..c702a66ac 100644 --- a/pkg/network/server.go +++ b/pkg/network/server.go @@ -1048,7 +1048,7 @@ func (s *Server) verifyAndPoolNotaryRequest(r *payload.P2PNotaryRequest) error { func verifyNotaryRequest(bc blockchainer.Blockchainer, _ *transaction.Transaction, data interface{}) error { r := data.(*payload.P2PNotaryRequest) payer := r.FallbackTransaction.Signers[1].Account - if err := bc.VerifyWitness(payer, r, &r.Witness, bc.GetPolicer().GetMaxVerificationGAS()); err != nil { + if _, err := bc.VerifyWitness(payer, r, &r.Witness, bc.GetPolicer().GetMaxVerificationGAS()); err != nil { return fmt.Errorf("bad P2PNotaryRequest payload witness: %w", err) } notaryHash := bc.GetNotaryContractScriptHash() diff --git a/pkg/network/server_test.go b/pkg/network/server_test.go index 9b35c09db..51e9fe633 100644 --- a/pkg/network/server_test.go +++ b/pkg/network/server_test.go @@ -432,11 +432,11 @@ func TestConsensus(t *testing.T) { return NewMessage(CMDExtensible, pl) } - s.chain.(*fakechain.FakeChain).VerifyWitnessF = func() error { return errors.New("invalid") } + s.chain.(*fakechain.FakeChain).VerifyWitnessF = func() (int64, error) { return 0, errors.New("invalid") } msg := newConsensusMessage(0, s.chain.BlockHeight()+1) require.Error(t, s.handleMessage(p, msg)) - s.chain.(*fakechain.FakeChain).VerifyWitnessF = func() error { return nil } + s.chain.(*fakechain.FakeChain).VerifyWitnessF = func() (int64, error) { return 0, nil } require.NoError(t, s.handleMessage(p, msg)) require.Contains(t, s.consensus.(*fakeConsensus).payloads, msg.Payload.(*payload.Extensible)) @@ -726,7 +726,7 @@ func TestInv(t *testing.T) { }) t.Run("extensible", func(t *testing.T) { ep := payload.NewExtensible() - s.chain.(*fakechain.FakeChain).VerifyWitnessF = func() error { return nil } + s.chain.(*fakechain.FakeChain).VerifyWitnessF = func() (int64, error) { return 0, nil } ep.ValidBlockEnd = s.chain.(*fakechain.FakeChain).BlockHeight() + 1 ok, err := s.extensiblePool.Add(ep) require.NoError(t, err) @@ -1033,12 +1033,12 @@ func TestVerifyNotaryRequest(t *testing.T) { } t.Run("bad payload witness", func(t *testing.T) { - bc.VerifyWitnessF = func() error { return errors.New("bad witness") } + bc.VerifyWitnessF = func() (int64, error) { return 0, errors.New("bad witness") } require.Error(t, verifyNotaryRequest(bc, nil, newNotaryRequest())) }) t.Run("bad fallback sender", func(t *testing.T) { - bc.VerifyWitnessF = func() error { return nil } + bc.VerifyWitnessF = func() (int64, error) { return 0, nil } r := newNotaryRequest() r.FallbackTransaction.Signers[0] = transaction.Signer{Account: util.Uint160{7, 8, 9}} require.Error(t, verifyNotaryRequest(bc, nil, r)) diff --git a/pkg/rpc/server/server.go b/pkg/rpc/server/server.go index 051dda712..98f5aced4 100644 --- a/pkg/rpc/server/server.go +++ b/pkg/rpc/server/server.go @@ -613,36 +613,11 @@ func (s *Server) calculateNetworkFee(reqParams request.Params) (interface{}, *re } } if verificationScript == nil { // then it still might be a contract-based verification - verificationErr := fmt.Sprintf("contract verification for signer #%d failed", i) - res, respErr := s.runScriptInVM(trigger.Verification, tx.Scripts[i].InvocationScript, signer.Account, tx) - if respErr != nil && errors.Is(respErr.Cause, core.ErrUnknownVerificationContract) { - // it's neither a contract-based verification script nor a standard witness attached to - // the tx, so the user did not provide enough data to calculate fee for that witness => - // it's a user error - return 0, response.NewRPCError(verificationErr, respErr.Cause.Error(), respErr.Cause) - } - if respErr != nil { - return 0, respErr - } - res.Finalize() - if res.State != "HALT" { - cause := fmt.Errorf("invalid VM state %s due to an error: %s", res.State, res.FaultException) - return 0, response.NewRPCError(verificationErr, cause.Error(), cause) - } - if l := len(res.Stack); l != 1 { - cause := fmt.Errorf("result stack length should be equal to 1, got %d", l) - return 0, response.NewRPCError(verificationErr, cause.Error(), cause) - } - isOK, err := res.Stack[0].TryBool() + gasConsumed, err := s.chain.VerifyWitness(signer.Account, tx, &tx.Scripts[i], int64(s.config.MaxGasInvoke)) if err != nil { - cause := fmt.Errorf("resulting stackitem cannot be converted to Boolean: %w", err) - return 0, response.NewRPCError(verificationErr, cause.Error(), cause) + return 0, response.NewRPCError(fmt.Sprintf("contract verification for signer #%d failed", i), err.Error(), err) } - if !isOK { - cause := errors.New("`verify` method returned `false` on stack") - return 0, response.NewRPCError(verificationErr, cause.Error(), cause) - } - netFee += res.GasConsumed + netFee += gasConsumed size += io.GetVarSize([]byte{}) + // verification script is empty (contract-based witness) io.GetVarSize(tx.Scripts[i].InvocationScript) // invocation script might not be empty (args for `verify`) continue diff --git a/pkg/services/notary/notary.go b/pkg/services/notary/notary.go index af9b77b0a..9fd69fe22 100644 --- a/pkg/services/notary/notary.go +++ b/pkg/services/notary/notary.go @@ -227,7 +227,7 @@ func (n *Notary) OnNewRequest(payload *payload.P2PNotaryRequest) { switch r.witnessInfo[i].typ { case Contract: // Need to check even if r.main.Scripts[i].InvocationScript is already filled in. - err := n.Config.Chain.VerifyWitness(r.main.Signers[i].Account, r.main, &w, n.Config.Chain.GetPolicer().GetMaxVerificationGAS()) + _, err := n.Config.Chain.VerifyWitness(r.main.Signers[i].Account, r.main, &w, n.Config.Chain.GetPolicer().GetMaxVerificationGAS()) if err != nil { continue }