From f4f50343e730ce58370b1a92f639a8a57ae273e9 Mon Sep 17 00:00:00 2001 From: Dmitry Shulyak Date: Tue, 2 Jan 2018 13:24:43 +0200 Subject: [PATCH] Use single codepath for sending transaction for local and remote nodes Previously procedure for sending transactions was different based on which type of node was used, local (light node) or remote. This change removes separate code path for working with light node, and the only difference will be backend for rpc client. In case of light node all of the communication will be done over inproc dial. Also there is a couple of related changes in this PR: - new EthereumTransactor that provides higher level API for working with ethereum network, and it is fully conformant with ethclient - new test rpc service that improves flexibility and coverage of txqueue manager tests --- geth/txqueue/ethtxclient.go | 93 ++++++++++++++ geth/txqueue/fake/mock.go | 91 ++++++++++++++ geth/txqueue/fake/txservice.go | 40 ++++++ geth/txqueue/txqueue_manager.go | 177 +++++++-------------------- geth/txqueue/txqueue_manager_test.go | 135 ++++++++++++-------- 5 files changed, 352 insertions(+), 184 deletions(-) create mode 100644 geth/txqueue/ethtxclient.go create mode 100644 geth/txqueue/fake/mock.go create mode 100644 geth/txqueue/fake/txservice.go diff --git a/geth/txqueue/ethtxclient.go b/geth/txqueue/ethtxclient.go new file mode 100644 index 00000000000..59084d851dc --- /dev/null +++ b/geth/txqueue/ethtxclient.go @@ -0,0 +1,93 @@ +package txqueue + +import ( + "context" + "math/big" + + ethereum "github.com/ethereum/go-ethereum" + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/rlp" + "github.com/status-im/status-go/geth/rpc" +) + +// EthereumTransactor provides methods to create transactions for ethereum network. +type EthereumTransactor interface { + PendingNonceAt(ctx context.Context, account common.Address) (uint64, error) + ethereum.GasEstimator + ethereum.GasPricer + ethereum.TransactionSender +} + +// EthTxClient wraps common API methods that are used to send transaction. +type EthTxClient struct { + c *rpc.Client +} + +func NewEthTxClient(client *rpc.Client) *EthTxClient { + return &EthTxClient{c: client} +} + +// PendingNonceAt returns the account nonce of the given account in the pending state. +// This is the nonce that should be used for the next transaction. +func (ec *EthTxClient) PendingNonceAt(ctx context.Context, account common.Address) (uint64, error) { + var result hexutil.Uint64 + err := ec.c.CallContext(ctx, &result, "eth_getTransactionCount", account, "pending") + return uint64(result), err +} + +// SuggestGasPrice retrieves the currently suggested gas price to allow a timely +// execution of a transaction. +func (ec *EthTxClient) SuggestGasPrice(ctx context.Context) (*big.Int, error) { + var hex hexutil.Big + if err := ec.c.CallContext(ctx, &hex, "eth_gasPrice"); err != nil { + return nil, err + } + return (*big.Int)(&hex), nil +} + +// EstimateGas tries to estimate the gas needed to execute a specific transaction based on +// the current pending state of the backend blockchain. There is no guarantee that this is +// the true gas limit requirement as other transactions may be added or removed by miners, +// but it should provide a basis for setting a reasonable default. +func (ec *EthTxClient) EstimateGas(ctx context.Context, msg ethereum.CallMsg) (*big.Int, error) { + var hex hexutil.Big + err := ec.c.CallContext(ctx, &hex, "eth_estimateGas", toCallArg(msg)) + if err != nil { + return nil, err + } + return (*big.Int)(&hex), nil +} + +// SendTransaction injects a signed transaction into the pending pool for execution. +// +// If the transaction was a contract creation use the TransactionReceipt method to get the +// contract address after the transaction has been mined. +func (ec *EthTxClient) SendTransaction(ctx context.Context, tx *types.Transaction) error { + data, err := rlp.EncodeToBytes(tx) + if err != nil { + return err + } + return ec.c.CallContext(ctx, nil, "eth_sendRawTransaction", common.ToHex(data)) +} + +func toCallArg(msg ethereum.CallMsg) interface{} { + arg := map[string]interface{}{ + "from": msg.From, + "to": msg.To, + } + if len(msg.Data) > 0 { + arg["data"] = hexutil.Bytes(msg.Data) + } + if msg.Value != nil { + arg["value"] = (*hexutil.Big)(msg.Value) + } + if msg.Gas != nil { + arg["gas"] = (*hexutil.Big)(msg.Gas) + } + if msg.GasPrice != nil { + arg["gasPrice"] = (*hexutil.Big)(msg.GasPrice) + } + return arg +} diff --git a/geth/txqueue/fake/mock.go b/geth/txqueue/fake/mock.go new file mode 100644 index 00000000000..13dce62a42f --- /dev/null +++ b/geth/txqueue/fake/mock.go @@ -0,0 +1,91 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/status-im/status-go/geth/txqueue/fake (interfaces: FakePublicTxApi) + +// Package fake is a generated GoMock package. +package fake + +import ( + context "context" + big "math/big" + reflect "reflect" + + common "github.com/ethereum/go-ethereum/common" + hexutil "github.com/ethereum/go-ethereum/common/hexutil" + rpc "github.com/ethereum/go-ethereum/rpc" + gomock "github.com/golang/mock/gomock" +) + +// MockFakePublicTxApi is a mock of FakePublicTxApi interface +type MockFakePublicTxApi struct { + ctrl *gomock.Controller + recorder *MockFakePublicTxApiMockRecorder +} + +// MockFakePublicTxApiMockRecorder is the mock recorder for MockFakePublicTxApi +type MockFakePublicTxApiMockRecorder struct { + mock *MockFakePublicTxApi +} + +// NewMockFakePublicTxApi creates a new mock instance +func NewMockFakePublicTxApi(ctrl *gomock.Controller) *MockFakePublicTxApi { + mock := &MockFakePublicTxApi{ctrl: ctrl} + mock.recorder = &MockFakePublicTxApiMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockFakePublicTxApi) EXPECT() *MockFakePublicTxApiMockRecorder { + return m.recorder +} + +// EstimateGas mocks base method +func (m *MockFakePublicTxApi) EstimateGas(arg0 context.Context, arg1 CallArgs) (*hexutil.Big, error) { + ret := m.ctrl.Call(m, "EstimateGas", arg0, arg1) + ret0, _ := ret[0].(*hexutil.Big) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// EstimateGas indicates an expected call of EstimateGas +func (mr *MockFakePublicTxApiMockRecorder) EstimateGas(arg0, arg1 interface{}) *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EstimateGas", reflect.TypeOf((*MockFakePublicTxApi)(nil).EstimateGas), arg0, arg1) +} + +// GasPrice mocks base method +func (m *MockFakePublicTxApi) GasPrice(arg0 context.Context) (*big.Int, error) { + ret := m.ctrl.Call(m, "GasPrice", arg0) + ret0, _ := ret[0].(*big.Int) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GasPrice indicates an expected call of GasPrice +func (mr *MockFakePublicTxApiMockRecorder) GasPrice(arg0 interface{}) *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GasPrice", reflect.TypeOf((*MockFakePublicTxApi)(nil).GasPrice), arg0) +} + +// GetTransactionCount mocks base method +func (m *MockFakePublicTxApi) GetTransactionCount(arg0 context.Context, arg1 common.Address, arg2 rpc.BlockNumber) (*hexutil.Uint64, error) { + ret := m.ctrl.Call(m, "GetTransactionCount", arg0, arg1, arg2) + ret0, _ := ret[0].(*hexutil.Uint64) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetTransactionCount indicates an expected call of GetTransactionCount +func (mr *MockFakePublicTxApiMockRecorder) GetTransactionCount(arg0, arg1, arg2 interface{}) *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTransactionCount", reflect.TypeOf((*MockFakePublicTxApi)(nil).GetTransactionCount), arg0, arg1, arg2) +} + +// SendRawTransaction mocks base method +func (m *MockFakePublicTxApi) SendRawTransaction(arg0 context.Context, arg1 hexutil.Bytes) (common.Hash, error) { + ret := m.ctrl.Call(m, "SendRawTransaction", arg0, arg1) + ret0, _ := ret[0].(common.Hash) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// SendRawTransaction indicates an expected call of SendRawTransaction +func (mr *MockFakePublicTxApiMockRecorder) SendRawTransaction(arg0, arg1 interface{}) *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendRawTransaction", reflect.TypeOf((*MockFakePublicTxApi)(nil).SendRawTransaction), arg0, arg1) +} diff --git a/geth/txqueue/fake/txservice.go b/geth/txqueue/fake/txservice.go new file mode 100644 index 00000000000..80eb207f67e --- /dev/null +++ b/geth/txqueue/fake/txservice.go @@ -0,0 +1,40 @@ +package fake + +import ( + context "context" + big "math/big" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/ethereum/go-ethereum/rpc" + gomock "github.com/golang/mock/gomock" +) + +func NewTestServer(ctrl *gomock.Controller) (*rpc.Server, *MockFakePublicTxApi) { + srv := rpc.NewServer() + svc := NewMockFakePublicTxApi(ctrl) + if err := srv.RegisterName("eth", svc); err != nil { + panic(err) + } + return srv, svc +} + +// CallArgs copied from module go-ethereum/internal/ethapi +type CallArgs struct { + From common.Address `json:"from"` + To *common.Address `json:"to"` + Gas hexutil.Big `json:"gas"` + GasPrice hexutil.Big `json:"gasPrice"` + Value hexutil.Big `json:"value"` + Data hexutil.Bytes `json:"data"` +} + +// FakePublicTxApi used to generate mock by mockgen util. +// This was done because PublicTransactionPoolAPI is located in internal/ethapi module +// and there is no easy way to generate mocks from internal modules. +type FakePublicTxApi interface { + GasPrice(ctx context.Context) (*big.Int, error) + EstimateGas(ctx context.Context, args CallArgs) (*hexutil.Big, error) + GetTransactionCount(ctx context.Context, address common.Address, blockNr rpc.BlockNumber) (*hexutil.Uint64, error) + SendRawTransaction(ctx context.Context, encodedTx hexutil.Bytes) (common.Hash, error) +} diff --git a/geth/txqueue/txqueue_manager.go b/geth/txqueue/txqueue_manager.go index 71f133061e0..6331611f7f9 100644 --- a/geth/txqueue/txqueue_manager.go +++ b/geth/txqueue/txqueue_manager.go @@ -5,12 +5,10 @@ import ( "math/big" "time" + ethereum "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/accounts/keystore" gethcommon "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/core/types" - "github.com/ethereum/go-ethereum/les/status" - "github.com/ethereum/go-ethereum/rlp" "github.com/pborman/uuid" "github.com/status-im/status-go/geth/common" "github.com/status-im/status-go/geth/log" @@ -26,6 +24,10 @@ const ( // SendTxDefaultErrorCode is sent by default, when error is not nil, but type is unknown/unexpected. SendTxDefaultErrorCode = SendTransactionDefaultErrorCode + + defaultGas = 90000 + + cancelTimeout = time.Minute ) // Send transaction response codes @@ -49,6 +51,7 @@ type Manager struct { nodeManager common.NodeManager accountManager common.AccountManager txQueue *TxQueue + ethTxClient EthereumTransactor } // NewManager returns a new Manager. @@ -57,6 +60,7 @@ func NewManager(nodeManager common.NodeManager, accountManager common.AccountMan nodeManager: nodeManager, accountManager: accountManager, txQueue: NewTransactionQueue(), + ethTxClient: NewEthTxClient(nodeManager.RPCClient()), } } @@ -157,21 +161,8 @@ func (m *Manager) CompleteTransaction(id common.QueuedTxID, password string) (ge m.NotifyOnQueuedTxReturn(queuedTx, ErrInvalidCompleteTxSender) return gethcommon.Hash{}, ErrInvalidCompleteTxSender } - - config, err := m.nodeManager.NodeConfig() - if err != nil { - log.Warn("could not get a node config", "err", err) - return gethcommon.Hash{}, err - } - // Send the transaction finally. - var hash gethcommon.Hash - - if config.UpstreamConfig.Enabled { - hash, err = m.completeRemoteTransaction(queuedTx, password) - } else { - hash, err = m.completeLocalTransaction(queuedTx, password) - } + hash, err := m.completeTransaction(selectedAccount, queuedTx, password) // when incorrect sender tries to complete the account, // notify and keep tx in queue (so that correct sender can complete) @@ -190,79 +181,64 @@ func (m *Manager) CompleteTransaction(id common.QueuedTxID, password string) (ge return hash, err } -const cancelTimeout = time.Minute - -func (m *Manager) completeLocalTransaction(queuedTx *common.QueuedTx, password string) (gethcommon.Hash, error) { - log.Info("complete transaction using local node", "id", queuedTx.ID) - - les, err := m.nodeManager.LightEthereumService() - if err != nil { - return gethcommon.Hash{}, err - } - - ctx, cancel := context.WithTimeout(context.Background(), cancelTimeout) - defer cancel() - - return les.StatusBackend.SendTransaction(ctx, status.SendTxArgs(queuedTx.Args), password) -} - -func (m *Manager) completeRemoteTransaction(queuedTx *common.QueuedTx, password string) (gethcommon.Hash, error) { - log.Info("complete transaction using upstream node", "id", queuedTx.ID) - +func (m *Manager) completeTransaction(selectedAccount *common.SelectedExtKey, queuedTx *common.QueuedTx, password string) (gethcommon.Hash, error) { + log.Info("complete transaction", "id", queuedTx.ID) var emptyHash gethcommon.Hash - + log.Info("verifying account password for transaction", "id", queuedTx.ID) config, err := m.nodeManager.NodeConfig() if err != nil { return emptyHash, err } - - selectedAcct, err := m.accountManager.SelectedAccount() - if err != nil { - return emptyHash, err - } - - _, err = m.accountManager.VerifyAccountPassword(config.KeyStoreDir, selectedAcct.Address.String(), password) + _, err = m.accountManager.VerifyAccountPassword(config.KeyStoreDir, selectedAccount.Address.String(), password) if err != nil { - log.Warn("failed to verify account", "account", selectedAcct.Address.String(), "error", err.Error()) + log.Warn("failed to verify account", "account", selectedAccount.Address.String(), "error", err.Error()) return emptyHash, err } - // We need to request a new transaction nounce from upstream node. + // update transaction with nonce, gas price and gas estimates ctx, cancel := context.WithTimeout(context.Background(), cancelTimeout) defer cancel() - - var txCount hexutil.Uint - client := m.nodeManager.RPCClient() - err = client.CallContext(ctx, &txCount, "eth_getTransactionCount", queuedTx.Args.From, "pending") + nonce, err := m.ethTxClient.PendingNonceAt(ctx, queuedTx.Args.From) if err != nil { return emptyHash, err } - args := queuedTx.Args - + var gasPrice *big.Int if args.GasPrice == nil { - value, gasPriceErr := m.gasPrice() - if gasPriceErr != nil { - return emptyHash, gasPriceErr + ctx2, cancel2 := context.WithTimeout(context.Background(), cancelTimeout) + defer cancel2() + gasPrice, err = m.ethTxClient.SuggestGasPrice(ctx2) + if err != nil { + return emptyHash, err } - args.GasPrice = value + } else { + gasPrice = (*big.Int)(args.GasPrice) } chainID := big.NewInt(int64(config.NetworkID)) - nonce := uint64(txCount) - gasPrice := (*big.Int)(args.GasPrice) data := []byte(args.Data) value := (*big.Int)(args.Value) toAddr := gethcommon.Address{} if args.To != nil { toAddr = *args.To } - - gas, err := m.estimateGas(args) + ctx3, cancel3 := context.WithTimeout(context.Background(), cancelTimeout) + defer cancel3() + gas, err := m.ethTxClient.EstimateGas(ctx3, ethereum.CallMsg{ + From: args.From, + To: args.To, + GasPrice: gasPrice, + Value: value, + Data: data, + }) if err != nil { return emptyHash, err } + if gas.Cmp(big.NewInt(defaultGas)) == -1 { + log.Info("default gas will be used. estimated gas", gas, "is lower than", defaultGas) + gas = big.NewInt(defaultGas) + } log.Info( "preparing raw transaction", @@ -272,90 +248,19 @@ func (m *Manager) completeRemoteTransaction(queuedTx *common.QueuedTx, password "gasPrice", gasPrice, "value", value, ) - - tx := types.NewTransaction(nonce, toAddr, value, (*big.Int)(gas), gasPrice, data) - signedTx, err := types.SignTx(tx, types.NewEIP155Signer(chainID), selectedAcct.AccountKey.PrivateKey) - if err != nil { - return emptyHash, err - } - - txBytes, err := rlp.EncodeToBytes(signedTx) + tx := types.NewTransaction(nonce, toAddr, value, gas, gasPrice, data) + signedTx, err := types.SignTx(tx, types.NewEIP155Signer(chainID), selectedAccount.AccountKey.PrivateKey) if err != nil { return emptyHash, err } - - ctx2, cancel2 := context.WithTimeout(context.Background(), cancelTimeout) - defer cancel2() - - if err := client.CallContext(ctx2, nil, "eth_sendRawTransaction", gethcommon.ToHex(txBytes)); err != nil { + ctx4, cancel4 := context.WithTimeout(context.Background(), cancelTimeout) + defer cancel4() + if err := m.ethTxClient.SendTransaction(ctx4, tx); err != nil { return emptyHash, err } - return signedTx.Hash(), nil } -func (m *Manager) estimateGas(args common.SendTxArgs) (*hexutil.Big, error) { - if args.Gas != nil { - return args.Gas, nil - } - - client := m.nodeManager.RPCClient() - ctx, cancel := context.WithTimeout(context.Background(), cancelTimeout) - defer cancel() - - var gasPrice hexutil.Big - if args.GasPrice != nil { - gasPrice = *args.GasPrice - } - - var value hexutil.Big - if args.Value != nil { - value = *args.Value - } - - params := struct { - From gethcommon.Address `json:"from"` - To *gethcommon.Address `json:"to"` - Gas hexutil.Big `json:"gas"` - GasPrice hexutil.Big `json:"gasPrice"` - Value hexutil.Big `json:"value"` - Data hexutil.Bytes `json:"data"` - }{ - From: args.From, - To: args.To, - GasPrice: gasPrice, - Value: value, - Data: []byte(args.Data), - } - - var estimatedGas hexutil.Big - if err := client.CallContext( - ctx, - &estimatedGas, - "eth_estimateGas", - params, - ); err != nil { - log.Warn("failed to estimate gas", "err", err) - return nil, err - } - - return &estimatedGas, nil -} - -func (m *Manager) gasPrice() (*hexutil.Big, error) { - client := m.nodeManager.RPCClient() - ctx, cancel := context.WithTimeout(context.Background(), cancelTimeout) - defer cancel() - - var gasPrice hexutil.Big - if err := client.CallContext(ctx, &gasPrice, "eth_gasPrice"); err != nil { - log.Warn("failed to get gas price", "err", err) - return nil, err - } - - return &gasPrice, nil -} - // CompleteTransactions instructs backend to complete sending of multiple transactions func (m *Manager) CompleteTransactions(ids []common.QueuedTxID, password string) map[common.QueuedTxID]common.RawCompleteTransactionResult { results := make(map[common.QueuedTxID]common.RawCompleteTransactionResult) diff --git a/geth/txqueue/txqueue_manager_test.go b/geth/txqueue/txqueue_manager_test.go index 606768186f6..f8a39eb5e90 100644 --- a/geth/txqueue/txqueue_manager_test.go +++ b/geth/txqueue/txqueue_manager_test.go @@ -3,16 +3,22 @@ package txqueue import ( "context" "errors" + "math/big" "sync" "testing" "github.com/ethereum/go-ethereum/accounts/keystore" - "github.com/stretchr/testify/suite" - + gethcommon "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/ethereum/go-ethereum/crypto" + gethrpc "github.com/ethereum/go-ethereum/rpc" "github.com/golang/mock/gomock" + "github.com/stretchr/testify/suite" "github.com/status-im/status-go/geth/common" "github.com/status-im/status-go/geth/params" + "github.com/status-im/status-go/geth/rpc" + "github.com/status-im/status-go/geth/txqueue/fake" . "github.com/status-im/status-go/testing" ) @@ -28,34 +34,53 @@ type TxQueueTestSuite struct { nodeManagerMock *common.MockNodeManager accountManagerMockCtrl *gomock.Controller accountManagerMock *common.MockAccountManager + server *gethrpc.Server + client *gethrpc.Client + txServiceMockCtrl *gomock.Controller + txServiceMock *fake.MockFakePublicTxApi } func (s *TxQueueTestSuite) SetupTest() { s.nodeManagerMockCtrl = gomock.NewController(s.T()) s.accountManagerMockCtrl = gomock.NewController(s.T()) + s.txServiceMockCtrl = gomock.NewController(s.T()) s.nodeManagerMock = common.NewMockNodeManager(s.nodeManagerMockCtrl) s.accountManagerMock = common.NewMockAccountManager(s.accountManagerMockCtrl) + + s.server, s.txServiceMock = fake.NewTestServer(s.txServiceMockCtrl) + s.client = gethrpc.DialInProc(s.server) + rpclient, _ := rpc.NewClient(s.client, params.UpstreamRPCConfig{}) + s.nodeManagerMock.EXPECT().RPCClient().Return(rpclient) } func (s *TxQueueTestSuite) TearDownTest() { s.nodeManagerMockCtrl.Finish() s.accountManagerMockCtrl.Finish() + s.txServiceMockCtrl.Finish() + s.server.Stop() + s.client.Close() } func (s *TxQueueTestSuite) TestCompleteTransaction() { - s.accountManagerMock.EXPECT().SelectedAccount().Return(&common.SelectedExtKey{ - Address: common.FromAddress(TestConfig.Account1.Address), - }, nil) - - s.nodeManagerMock.EXPECT().NodeConfig().Return( - params.NewNodeConfig("/tmp", params.RopstenNetworkID, true), - ) - - // TODO(adam): StatusBackend as an interface would allow a better solution. - // As we want to avoid network connection, we mock LES with a known error - // and treat as success. - s.nodeManagerMock.EXPECT().LightEthereumService().Return(nil, errTxAssumedSent) + nodeConfig, nodeErr := params.NewNodeConfig("/tmp", params.RopstenNetworkID, true) + password := TestConfig.Account1.Password + key, _ := crypto.GenerateKey() + account := &common.SelectedExtKey{ + Address: common.FromAddress(TestConfig.Account1.Address), + AccountKey: &keystore.Key{PrivateKey: key}, + } + s.accountManagerMock.EXPECT().SelectedAccount().Return(account, nil) + s.accountManagerMock.EXPECT().VerifyAccountPassword(nodeConfig.KeyStoreDir, account.Address.String(), password).Return( + nil, nil) + s.nodeManagerMock.EXPECT().NodeConfig().Return(nodeConfig, nodeErr) + + nonce := hexutil.Uint64(10) + s.txServiceMock.EXPECT().GetTransactionCount(gomock.Any(), account.Address, gethrpc.PendingBlockNumber).Return(&nonce, nil) + s.txServiceMock.EXPECT().GasPrice(gomock.Any()).Return(big.NewInt(10), nil) + gas := hexutil.Big(*big.NewInt(defaultGas + 1)) + s.txServiceMock.EXPECT().EstimateGas(gomock.Any(), gomock.Any()).Return(&gas, nil) + s.txServiceMock.EXPECT().SendRawTransaction(gomock.Any(), gomock.Any()).Return(gethcommon.Hash{}, nil) txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock) @@ -81,31 +106,37 @@ func (s *TxQueueTestSuite) TestCompleteTransaction() { s.NoError(err) go func() { - _, errCompleteTransaction := txQueueManager.CompleteTransaction(tx.ID, TestConfig.Account1.Password) - s.Equal(errTxAssumedSent, errCompleteTransaction) + _, errCompleteTransaction := txQueueManager.CompleteTransaction(tx.ID, password) + s.NoError(errCompleteTransaction) }() err = txQueueManager.WaitForTransaction(tx) - s.Equal(errTxAssumedSent, err) + s.NoError(err) // Check that error is assigned to the transaction. - s.Equal(errTxAssumedSent, tx.Err) + s.NoError(tx.Err) // Transaction should be already removed from the queue. s.False(txQueueManager.TransactionQueue().Has(tx.ID)) } func (s *TxQueueTestSuite) TestCompleteTransactionMultipleTimes() { - s.accountManagerMock.EXPECT().SelectedAccount().Return(&common.SelectedExtKey{ - Address: common.FromAddress(TestConfig.Account1.Address), - }, nil) - - s.nodeManagerMock.EXPECT().NodeConfig().Return( - params.NewNodeConfig("/tmp", params.RopstenNetworkID, true), - ) - - // TODO(adam): StatusBackend as an interface would allow a better solution. - // As we want to avoid network connection, we mock LES with a known error - // and treat as success. - s.nodeManagerMock.EXPECT().LightEthereumService().Return(nil, errTxAssumedSent) + nodeConfig, nodeErr := params.NewNodeConfig("/tmp", params.RopstenNetworkID, true) + password := TestConfig.Account1.Password + key, _ := crypto.GenerateKey() + account := &common.SelectedExtKey{ + Address: common.FromAddress(TestConfig.Account1.Address), + AccountKey: &keystore.Key{PrivateKey: key}, + } + s.accountManagerMock.EXPECT().SelectedAccount().Return(account, nil) + s.accountManagerMock.EXPECT().VerifyAccountPassword(nodeConfig.KeyStoreDir, account.Address.String(), password).Return( + nil, nil) + s.nodeManagerMock.EXPECT().NodeConfig().Return(nodeConfig, nodeErr) + + nonce := hexutil.Uint64(10) + s.txServiceMock.EXPECT().GetTransactionCount(gomock.Any(), account.Address, gethrpc.PendingBlockNumber).Return(&nonce, nil) + s.txServiceMock.EXPECT().GasPrice(gomock.Any()).Return(big.NewInt(10), nil) + gas := hexutil.Big(*big.NewInt(defaultGas + 1)) + s.txServiceMock.EXPECT().EstimateGas(gomock.Any(), gomock.Any()).Return(&gas, nil) + s.txServiceMock.EXPECT().SendRawTransaction(gomock.Any(), gomock.Any()).Return(gethcommon.Hash{}, nil) txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock) @@ -124,7 +155,7 @@ func (s *TxQueueTestSuite) TestCompleteTransactionMultipleTimes() { txQueueManager.SetTransactionReturnHandler(func(queuedTx *common.QueuedTx, err error) { s.Equal(tx.ID, queuedTx.ID) - s.Equal(errTxAssumedSent, err) + s.NoError(err) }) err := txQueueManager.QueueTransaction(tx) @@ -132,12 +163,12 @@ func (s *TxQueueTestSuite) TestCompleteTransactionMultipleTimes() { var wg sync.WaitGroup var mu sync.Mutex - completeTxErrors := make(map[error]int) + completeTxErrors := map[error]int{} for i := 0; i < 3; i++ { wg.Add(1) go func() { defer wg.Done() - _, errCompleteTransaction := txQueueManager.CompleteTransaction(tx.ID, TestConfig.Account1.Password) + _, errCompleteTransaction := txQueueManager.CompleteTransaction(tx.ID, password) mu.Lock() completeTxErrors[errCompleteTransaction]++ mu.Unlock() @@ -145,15 +176,15 @@ func (s *TxQueueTestSuite) TestCompleteTransactionMultipleTimes() { } err = txQueueManager.WaitForTransaction(tx) - s.Equal(errTxAssumedSent, err) + s.NoError(err) // Check that error is assigned to the transaction. - s.Equal(errTxAssumedSent, tx.Err) + s.NoError(tx.Err) // Transaction should be already removed from the queue. s.False(txQueueManager.TransactionQueue().Has(tx.ID)) // Wait for all CompleteTransaction calls. wg.Wait() - s.Equal(completeTxErrors[errTxAssumedSent], 1) + s.Equal(completeTxErrors[nil], 1) } func (s *TxQueueTestSuite) TestAccountMismatch() { @@ -196,16 +227,24 @@ func (s *TxQueueTestSuite) TestAccountMismatch() { } func (s *TxQueueTestSuite) TestInvalidPassword() { - s.accountManagerMock.EXPECT().SelectedAccount().Return(&common.SelectedExtKey{ - Address: common.FromAddress(TestConfig.Account1.Address), - }, nil) - - s.nodeManagerMock.EXPECT().NodeConfig().Return( - params.NewNodeConfig("/tmp", params.RopstenNetworkID, true), - ) - - // Set ErrDecrypt error response as expected with a wrong password. - s.nodeManagerMock.EXPECT().LightEthereumService().Return(nil, keystore.ErrDecrypt) + nodeConfig, nodeErr := params.NewNodeConfig("/tmp", params.RopstenNetworkID, true) + password := "invalid-password" + key, _ := crypto.GenerateKey() + account := &common.SelectedExtKey{ + Address: common.FromAddress(TestConfig.Account1.Address), + AccountKey: &keystore.Key{PrivateKey: key}, + } + s.accountManagerMock.EXPECT().SelectedAccount().Return(account, nil) + s.accountManagerMock.EXPECT().VerifyAccountPassword(nodeConfig.KeyStoreDir, account.Address.String(), password).Return( + nil, nil) + s.nodeManagerMock.EXPECT().NodeConfig().Return(nodeConfig, nodeErr) + + nonce := hexutil.Uint64(10) + s.txServiceMock.EXPECT().GetTransactionCount(gomock.Any(), account.Address, gethrpc.PendingBlockNumber).Return(&nonce, nil) + s.txServiceMock.EXPECT().GasPrice(gomock.Any()).Return(big.NewInt(10), nil) + gas := hexutil.Big(*big.NewInt(defaultGas + 1)) + s.txServiceMock.EXPECT().EstimateGas(gomock.Any(), gomock.Any()).Return(&gas, nil) + s.txServiceMock.EXPECT().SendRawTransaction(gomock.Any(), gomock.Any()).Return(gethcommon.Hash{}, keystore.ErrDecrypt) txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock) @@ -233,8 +272,8 @@ func (s *TxQueueTestSuite) TestInvalidPassword() { err := txQueueManager.QueueTransaction(tx) s.NoError(err) - _, err = txQueueManager.CompleteTransaction(tx.ID, "invalid-password") - s.Equal(err, keystore.ErrDecrypt) + _, err = txQueueManager.CompleteTransaction(tx.ID, password) + s.Equal(err.Error(), keystore.ErrDecrypt.Error()) // Transaction should stay in the queue as mismatched accounts // is a recoverable error.