From d2a81daf576fa16f500ded87903dd51bf7f150f7 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Thu, 25 Mar 2021 11:38:15 +0300 Subject: [PATCH 1/3] rpc: refactor TestCalculateNetworkFee We need to ensure that we don't add extra network fee during calculation. --- pkg/rpc/server/client_test.go | 171 +++++++++++++++++++++++----------- 1 file changed, 118 insertions(+), 53 deletions(-) diff --git a/pkg/rpc/server/client_test.go b/pkg/rpc/server/client_test.go index 41305f332..4fb084ac4 100644 --- a/pkg/rpc/server/client_test.go +++ b/pkg/rpc/server/client_test.go @@ -71,6 +71,7 @@ func TestAddNetworkFee(t *testing.T) { defer chain.Close() defer rpcSrv.Shutdown() const extraFee = 10 + var nonce uint32 c, err := client.New(context.Background(), httpSrv.URL, client.Options{}) require.NoError(t, err) @@ -98,41 +99,105 @@ func TestAddNetworkFee(t *testing.T) { require.Error(t, c.AddNetworkFee(tx, extraFee, accs[0], accs[1])) }) t.Run("Simple", func(t *testing.T) { - tx := transaction.New([]byte{byte(opcode.PUSH1)}, 0) - accs := getAccounts(t, 1) - tx.Signers = []transaction.Signer{{ - Account: accs[0].PrivateKey().GetScriptHash(), - Scopes: transaction.CalledByEntry, - }} - require.NoError(t, c.AddNetworkFee(tx, 10, accs[0])) - require.NoError(t, accs[0].SignTx(testchain.Network(), tx)) - cFee, _ := fee.Calculate(chain.GetBaseExecFee(), accs[0].Contract.Script) - require.Equal(t, int64(io.GetVarSize(tx))*feePerByte+cFee+extraFee, tx.NetworkFee) + acc0 := wallet.NewAccountFromPrivateKey(testchain.PrivateKeyByID(0)) + check := func(t *testing.T, extraFee int64) { + tx := transaction.New([]byte{byte(opcode.PUSH1)}, 0) + tx.ValidUntilBlock = 20 + tx.Signers = []transaction.Signer{{ + Account: acc0.PrivateKey().GetScriptHash(), + Scopes: transaction.CalledByEntry, + }} + tx.Nonce = nonce + nonce++ + + require.NoError(t, c.AddNetworkFee(tx, extraFee, acc0)) + actual := tx.NetworkFee + + require.NoError(t, acc0.SignTx(testchain.Network(), tx)) + cFee, _ := fee.Calculate(chain.GetBaseExecFee(), acc0.Contract.Script) + expected := int64(io.GetVarSize(tx))*feePerByte + cFee + extraFee + + require.Equal(t, expected, actual) + err := chain.VerifyTx(tx) + if extraFee < 0 { + require.Error(t, err) + } else { + require.NoError(t, err) + } + } + + t.Run("with extra fee", func(t *testing.T) { + // check that calculated network fee with extra value is enough + check(t, extraFee) + }) + t.Run("without extra fee", func(t *testing.T) { + // check that calculated network fee without extra value is enough + check(t, 0) + }) + t.Run("exactFee-1", func(t *testing.T) { + // check that we don't add unexpected extra GAS + check(t, -1) + }) }) t.Run("Multi", func(t *testing.T) { - tx := transaction.New([]byte{byte(opcode.PUSH1)}, 0) - accs := getAccounts(t, 4) - pubs := keys.PublicKeys{accs[1].PrivateKey().PublicKey(), accs[2].PrivateKey().PublicKey(), accs[3].PrivateKey().PublicKey()} - require.NoError(t, accs[1].ConvertMultisig(2, pubs)) - require.NoError(t, accs[2].ConvertMultisig(2, pubs)) - tx.Signers = []transaction.Signer{ - { - Account: accs[0].PrivateKey().GetScriptHash(), - Scopes: transaction.CalledByEntry, - }, - { - Account: hash.Hash160(accs[1].Contract.Script), - Scopes: transaction.Global, - }, + acc0 := wallet.NewAccountFromPrivateKey(testchain.PrivateKeyByID(0)) + acc1 := wallet.NewAccountFromPrivateKey(testchain.PrivateKeyByID(0)) + acc1.ConvertMultisig(3, keys.PublicKeys{ + testchain.PrivateKeyByID(0).PublicKey(), + testchain.PrivateKeyByID(1).PublicKey(), + testchain.PrivateKeyByID(2).PublicKey(), + testchain.PrivateKeyByID(3).PublicKey(), + }) + check := func(t *testing.T, extraFee int64) { + tx := transaction.New([]byte{byte(opcode.PUSH1)}, 0) + tx.ValidUntilBlock = 20 + tx.Signers = []transaction.Signer{ + { + Account: acc0.PrivateKey().GetScriptHash(), + Scopes: transaction.CalledByEntry, + }, + { + Account: hash.Hash160(acc1.Contract.Script), + Scopes: transaction.Global, + }, + } + tx.Nonce = nonce + nonce++ + + require.NoError(t, c.AddNetworkFee(tx, extraFee, acc0, acc1)) + actual := tx.NetworkFee + + require.NoError(t, acc0.SignTx(testchain.Network(), tx)) + tx.Scripts = append(tx.Scripts, transaction.Witness{ + InvocationScript: testchain.Sign(tx), + VerificationScript: acc1.Contract.Script, + }) + cFee, _ := fee.Calculate(chain.GetBaseExecFee(), acc0.Contract.Script) + cFeeM, _ := fee.Calculate(chain.GetBaseExecFee(), acc1.Contract.Script) + expected := int64(io.GetVarSize(tx))*feePerByte + cFee + cFeeM + extraFee + + require.Equal(t, expected, actual) + err := chain.VerifyTx(tx) + if extraFee < 0 { + require.Error(t, err) + } else { + require.NoError(t, err) + } } - require.NoError(t, c.AddNetworkFee(tx, extraFee, accs[0], accs[1])) - require.NoError(t, accs[0].SignTx(testchain.Network(), tx)) - require.NoError(t, accs[1].SignTx(testchain.Network(), tx)) - require.NoError(t, accs[2].SignTx(testchain.Network(), tx)) - cFee, _ := fee.Calculate(chain.GetBaseExecFee(), accs[0].Contract.Script) - cFeeM, _ := fee.Calculate(chain.GetBaseExecFee(), accs[1].Contract.Script) - require.Equal(t, int64(io.GetVarSize(tx))*feePerByte+cFee+cFeeM+extraFee, tx.NetworkFee) + + t.Run("with extra fee", func(t *testing.T) { + // check that calculated network fee with extra value is enough + check(t, extraFee) + }) + t.Run("without extra fee", func(t *testing.T) { + // check that calculated network fee without extra value is enough + check(t, 0) + }) + t.Run("exactFee-1", func(t *testing.T) { + // check that we don't add unexpected extra GAS + check(t, -1) + }) }) t.Run("Contract", func(t *testing.T) { h, err := util.Uint160DecodeStringLE(verifyContractHash) @@ -142,16 +207,16 @@ func TestAddNetworkFee(t *testing.T) { acc1 := wallet.NewAccountFromPrivateKey(priv) // contract account acc1.Contract.Deployed = true acc1.Contract.Script, err = base64.StdEncoding.DecodeString(verifyContractAVM) + require.NoError(t, err) newTx := func(t *testing.T) *transaction.Transaction { tx := transaction.New([]byte{byte(opcode.PUSH1)}, 0) - require.NoError(t, err) tx.ValidUntilBlock = chain.BlockHeight() + 10 return tx } t.Run("Valid", func(t *testing.T) { - completeTx := func(t *testing.T) *transaction.Transaction { + check := func(t *testing.T, extraFee int64) { tx := newTx(t) tx.Signers = []transaction.Signer{ { @@ -164,28 +229,28 @@ func TestAddNetworkFee(t *testing.T) { }, } require.NoError(t, c.AddNetworkFee(tx, extraFee, acc0, acc1)) - return tx + require.NoError(t, acc0.SignTx(testchain.Network(), tx)) + tx.Scripts = append(tx.Scripts, transaction.Witness{}) + err = chain.VerifyTx(tx) + if extraFee < 0 { + require.Error(t, err) + } else { + require.NoError(t, err) + } } - // check that network fee with extra value is enough - tx1 := completeTx(t) - require.NoError(t, acc0.SignTx(testchain.Network(), tx1)) - tx1.Scripts = append(tx1.Scripts, transaction.Witness{}) - require.NoError(t, chain.VerifyTx(tx1)) - - // check that network fee without extra value is enough - tx2 := completeTx(t) - tx2.NetworkFee -= extraFee - require.NoError(t, acc0.SignTx(testchain.Network(), tx2)) - tx2.Scripts = append(tx2.Scripts, transaction.Witness{}) - require.NoError(t, chain.VerifyTx(tx2)) - - // check that we don't add unexpected extra GAS - tx3 := completeTx(t) - tx3.NetworkFee -= extraFee + 1 - require.NoError(t, acc0.SignTx(testchain.Network(), tx3)) - tx3.Scripts = append(tx3.Scripts, transaction.Witness{}) - require.Error(t, chain.VerifyTx(tx3)) + t.Run("with extra fee", func(t *testing.T) { + // check that calculated network fee with extra value is enough + check(t, extraFee) + }) + t.Run("without extra fee", func(t *testing.T) { + // check that calculated network fee without extra value is enough + check(t, 0) + }) + t.Run("exactFee-1", func(t *testing.T) { + // check that we don't add unexpected extra GAS + check(t, -1) + }) }) t.Run("Invalid", func(t *testing.T) { tx := newTx(t) From 252e03bc34fa37c056d77ee8d4c77156c74cf09e Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Wed, 24 Mar 2021 20:32:48 +0300 Subject: [PATCH 2/3] rpc: add CalculateNetworkFee RPC method --- pkg/core/blockchainer/policer.go | 1 + pkg/rpc/client/rpc.go | 14 ++++++ pkg/rpc/server/client_test.go | 34 ++++++++++++-- pkg/rpc/server/server.go | 79 ++++++++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 3 deletions(-) diff --git a/pkg/core/blockchainer/policer.go b/pkg/core/blockchainer/policer.go index 19eef7545..29c3a3ca5 100644 --- a/pkg/core/blockchainer/policer.go +++ b/pkg/core/blockchainer/policer.go @@ -5,4 +5,5 @@ type Policer interface { GetBaseExecFee() int64 GetMaxVerificationGAS() int64 GetStoragePrice() int64 + FeePerByte() int64 } diff --git a/pkg/rpc/client/rpc.go b/pkg/rpc/client/rpc.go index 9465badb3..884482c32 100644 --- a/pkg/rpc/client/rpc.go +++ b/pkg/rpc/client/rpc.go @@ -28,6 +28,20 @@ import ( var errNetworkNotInitialized = errors.New("RPC client network is not initialized") +// CalculateNetworkFee calculates network fee for transaction. The transaction may +// have empty witnesses for contract signers and may have only verification scripts +// filled for standard sig/multisig signers. +func (c *Client) CalculateNetworkFee(tx *transaction.Transaction) (int64, error) { + var ( + params = request.NewRawParams(tx.Bytes()) + resp int64 + ) + if err := c.performRequest("calculatenetworkfee", params, &resp); err != nil { + return 0, err + } + return resp, nil +} + // GetApplicationLog returns the contract log based on the specified txid. func (c *Client) GetApplicationLog(hash util.Uint256, trig *trigger.Type) (*result.ApplicationLog, error) { var ( diff --git a/pkg/rpc/server/client_test.go b/pkg/rpc/server/client_test.go index 4fb084ac4..efa18d793 100644 --- a/pkg/rpc/server/client_test.go +++ b/pkg/rpc/server/client_test.go @@ -66,7 +66,7 @@ func TestClient_NEP17(t *testing.T) { }) } -func TestAddNetworkFee(t *testing.T) { +func TestAddNetworkFeeCalculateNetworkFee(t *testing.T) { chain, rpcSrv, httpSrv := initServerWithInMemoryChain(t) defer chain.Close() defer rpcSrv.Shutdown() @@ -110,6 +110,13 @@ func TestAddNetworkFee(t *testing.T) { tx.Nonce = nonce nonce++ + tx.Scripts = []transaction.Witness{ + {VerificationScript: acc0.GetVerificationScript()}, + } + actualCalculatedNetFee, err := c.CalculateNetworkFee(tx) + require.NoError(t, err) + + tx.Scripts = nil require.NoError(t, c.AddNetworkFee(tx, extraFee, acc0)) actual := tx.NetworkFee @@ -118,7 +125,8 @@ func TestAddNetworkFee(t *testing.T) { expected := int64(io.GetVarSize(tx))*feePerByte + cFee + extraFee require.Equal(t, expected, actual) - err := chain.VerifyTx(tx) + require.Equal(t, expected, actualCalculatedNetFee+extraFee) + err = chain.VerifyTx(tx) if extraFee < 0 { require.Error(t, err) } else { @@ -165,6 +173,15 @@ func TestAddNetworkFee(t *testing.T) { tx.Nonce = nonce nonce++ + tx.Scripts = []transaction.Witness{ + {VerificationScript: acc0.GetVerificationScript()}, + {VerificationScript: acc1.GetVerificationScript()}, + } + actualCalculatedNetFee, err := c.CalculateNetworkFee(tx) + require.NoError(t, err) + + tx.Scripts = nil + require.NoError(t, c.AddNetworkFee(tx, extraFee, acc0, acc1)) actual := tx.NetworkFee @@ -178,7 +195,8 @@ func TestAddNetworkFee(t *testing.T) { expected := int64(io.GetVarSize(tx))*feePerByte + cFee + cFeeM + extraFee require.Equal(t, expected, actual) - err := chain.VerifyTx(tx) + require.Equal(t, expected, actualCalculatedNetFee+extraFee) + err = chain.VerifyTx(tx) if extraFee < 0 { require.Error(t, err) } else { @@ -228,9 +246,19 @@ func TestAddNetworkFee(t *testing.T) { Scopes: transaction.Global, }, } + // we need to fill standard verification scripts to use CalculateNetworkFee. + tx.Scripts = []transaction.Witness{ + {VerificationScript: acc0.GetVerificationScript()}, + {}, + } + actual, err := c.CalculateNetworkFee(tx) + require.NoError(t, err) + tx.Scripts = nil + require.NoError(t, c.AddNetworkFee(tx, extraFee, acc0, acc1)) require.NoError(t, acc0.SignTx(testchain.Network(), tx)) tx.Scripts = append(tx.Scripts, transaction.Witness{}) + require.Equal(t, tx.NetworkFee, actual+extraFee) err = chain.VerifyTx(tx) if extraFee < 0 { require.Error(t, err) diff --git a/pkg/rpc/server/server.go b/pkg/rpc/server/server.go index 3479fc9b4..8f10088ca 100644 --- a/pkg/rpc/server/server.go +++ b/pkg/rpc/server/server.go @@ -20,6 +20,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/core" "github.com/nspcc-dev/neo-go/pkg/core/block" "github.com/nspcc-dev/neo-go/pkg/core/blockchainer" + "github.com/nspcc-dev/neo-go/pkg/core/fee" "github.com/nspcc-dev/neo-go/pkg/core/mpt" "github.com/nspcc-dev/neo-go/pkg/core/state" "github.com/nspcc-dev/neo-go/pkg/core/transaction" @@ -93,6 +94,7 @@ const ( ) var rpcHandlers = map[string]func(*Server, request.Params) (interface{}, *response.Error){ + "calculatenetworkfee": (*Server).calculateNetworkFee, "getapplicationlog": (*Server).getApplicationLog, "getbestblockhash": (*Server).getBestBlockHash, "getblock": (*Server).getBlock, @@ -549,6 +551,83 @@ func (s *Server) validateAddress(reqParams request.Params) (interface{}, *respon return validateAddress(param.Value), nil } +// calculateNetworkFee calculates network fee for the transaction. +func (s *Server) calculateNetworkFee(reqParams request.Params) (interface{}, *response.Error) { + if len(reqParams) < 1 { + return 0, response.ErrInvalidParams + } + byteTx, err := reqParams[0].GetBytesBase64() + if err != nil { + return 0, response.WrapErrorWithData(response.ErrInvalidParams, err) + } + tx, err := transaction.NewTransactionFromBytes(byteTx) + if err != nil { + return 0, response.WrapErrorWithData(response.ErrInvalidParams, err) + } + hashablePart, err := tx.EncodeHashableFields() + if err != nil { + return 0, response.WrapErrorWithData(response.ErrInvalidParams, fmt.Errorf("failed to compute tx size: %w", err)) + } + size := len(hashablePart) + io.GetVarSize(len(tx.Signers)) + var ( + ef int64 + netFee int64 + ) + for i, signer := range tx.Signers { + var verificationScript []byte + for _, w := range tx.Scripts { + if w.VerificationScript != nil && hash.Hash160(w.VerificationScript).Equals(signer.Account) { + // then it's a standard sig/multisig witness + verificationScript = w.VerificationScript + break + } + } + 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, []byte{}, 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 + } + 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() + if err != nil { + cause := fmt.Errorf("resulting stackitem cannot be converted to Boolean: %w", err) + return 0, response.NewRPCError(verificationErr, cause.Error(), cause) + } + if !isOK { + cause := errors.New("`verify` method returned `false` on stack") + return 0, response.NewRPCError(verificationErr, cause.Error(), cause) + } + netFee += res.GasConsumed + size += io.GetVarSize([]byte{}) * 2 // both scripts are empty + continue + } + + if ef == 0 { + ef = s.chain.GetPolicer().GetBaseExecFee() + } + fee, sizeDelta := fee.Calculate(ef, verificationScript) + netFee += fee + size += sizeDelta + } + fee := s.chain.GetPolicer().FeePerByte() + netFee += int64(size) * fee + return netFee, nil +} + // getApplicationLog returns the contract log based on the specified txid or blockid. func (s *Server) getApplicationLog(reqParams request.Params) (interface{}, *response.Error) { hash, err := reqParams.Value(0).GetUint256() From bcc5c055de327868b8d404d21f71ec09f63aea31 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Fri, 26 Mar 2021 18:52:51 +0300 Subject: [PATCH 3/3] rpc: allow to `calculatenetworkfee` for `verify` with args We can load invocation script for those contract-based witnesses which requires arguments for `verify` methods. --- pkg/rpc/server/client_test.go | 71 +++++++++++++++++++++++++++++++++++ pkg/rpc/server/server.go | 5 ++- 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/pkg/rpc/server/client_test.go b/pkg/rpc/server/client_test.go index efa18d793..112344349 100644 --- a/pkg/rpc/server/client_test.go +++ b/pkg/rpc/server/client_test.go @@ -18,6 +18,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm" + "github.com/nspcc-dev/neo-go/pkg/vm/emit" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/wallet" "github.com/stretchr/testify/require" @@ -314,6 +315,76 @@ func TestAddNetworkFeeCalculateNetworkFee(t *testing.T) { }) } +func TestCalculateNetworkFee(t *testing.T) { + chain, rpcSrv, httpSrv := initServerWithInMemoryChain(t) + defer chain.Close() + defer rpcSrv.Shutdown() + const extraFee = 10 + + c, err := client.New(context.Background(), httpSrv.URL, client.Options{}) + require.NoError(t, err) + require.NoError(t, c.Init()) + + t.Run("ContractWithArgs", func(t *testing.T) { + check := func(t *testing.T, extraFee int64) { + h, err := util.Uint160DecodeStringLE(verifyWithArgsContractHash) + require.NoError(t, err) + priv := testchain.PrivateKeyByID(0) + acc0 := wallet.NewAccountFromPrivateKey(priv) + tx := transaction.New([]byte{byte(opcode.PUSH1)}, 0) + require.NoError(t, err) + tx.ValidUntilBlock = chain.BlockHeight() + 10 + tx.Signers = []transaction.Signer{ + { + Account: acc0.PrivateKey().GetScriptHash(), + Scopes: transaction.CalledByEntry, + }, + { + Account: h, + Scopes: transaction.Global, + }, + } + + bw := io.NewBufBinWriter() + emit.Bool(bw.BinWriter, false) + emit.Int(bw.BinWriter, int64(4)) + emit.String(bw.BinWriter, "good_string") // contract's `verify` return `true` with this string + require.NoError(t, bw.Err) + contractInv := bw.Bytes() + // we need to fill standard verification scripts to use CalculateNetworkFee. + tx.Scripts = []transaction.Witness{ + {VerificationScript: acc0.GetVerificationScript()}, + {InvocationScript: contractInv}, + } + tx.NetworkFee, err = c.CalculateNetworkFee(tx) + require.NoError(t, err) + tx.NetworkFee += extraFee + tx.Scripts = nil + + require.NoError(t, acc0.SignTx(testchain.Network(), tx)) + tx.Scripts = append(tx.Scripts, transaction.Witness{InvocationScript: contractInv}) + err = chain.VerifyTx(tx) + if extraFee < 0 { + require.Error(t, err) + } else { + require.NoError(t, err) + } + } + + t.Run("with extra fee", func(t *testing.T) { + // check that calculated network fee with extra value is enough + check(t, extraFee) + }) + t.Run("without extra fee", func(t *testing.T) { + // check that calculated network fee without extra value is enough + check(t, 0) + }) + t.Run("exactFee-1", func(t *testing.T) { + // check that we don't add unexpected extra GAS + check(t, -1) + }) + }) +} func TestSignAndPushInvocationTx(t *testing.T) { chain, rpcSrv, httpSrv := initServerWithInMemoryChain(t) defer chain.Close() diff --git a/pkg/rpc/server/server.go b/pkg/rpc/server/server.go index 8f10088ca..a39bb4612 100644 --- a/pkg/rpc/server/server.go +++ b/pkg/rpc/server/server.go @@ -584,7 +584,7 @@ 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, []byte{}, signer.Account, tx) + 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 => @@ -612,7 +612,8 @@ func (s *Server) calculateNetworkFee(reqParams request.Params) (interface{}, *re return 0, response.NewRPCError(verificationErr, cause.Error(), cause) } netFee += res.GasConsumed - size += io.GetVarSize([]byte{}) * 2 // both scripts are empty + 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 }