From d81554b42f5243035b2f8ce9cfc37fc2c6242b03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caner=20=C3=87=C4=B1dam?= Date: Thu, 1 Feb 2018 03:43:51 +0300 Subject: [PATCH 1/8] more configuration for setupTransactionPoolAPI --- geth/transactions/txqueue_manager_test.go | 100 +++++++++++++++++----- 1 file changed, 79 insertions(+), 21 deletions(-) diff --git a/geth/transactions/txqueue_manager_test.go b/geth/transactions/txqueue_manager_test.go index 3fa4af31430..9aae9cd3ff0 100644 --- a/geth/transactions/txqueue_manager_test.go +++ b/geth/transactions/txqueue_manager_test.go @@ -10,7 +10,9 @@ import ( "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/crypto" + "github.com/ethereum/go-ethereum/rlp" gethrpc "github.com/ethereum/go-ethereum/rpc" "github.com/golang/mock/gomock" "github.com/stretchr/testify/suite" @@ -61,19 +63,46 @@ func (s *TxQueueTestSuite) TearDownTest() { s.client.Close() } -func (s *TxQueueTestSuite) setupTransactionPoolAPI(account *common.SelectedExtKey, nonce hexutil.Uint64, gas hexutil.Big, txErr error) { - s.txServiceMock.EXPECT().GetTransactionCount(gomock.Any(), account.Address, gethrpc.PendingBlockNumber).Return(&nonce, nil) - s.txServiceMock.EXPECT().GasPrice(gomock.Any()).Return(big.NewInt(10), nil) - s.txServiceMock.EXPECT().EstimateGas(gomock.Any(), gomock.Any()).Return(&gas, nil) - s.txServiceMock.EXPECT().SendRawTransaction(gomock.Any(), gomock.Any()).Return(gethcommon.Hash{}, txErr) +func (s *TxQueueTestSuite) setupTransactionPoolAPI(tx *common.QueuedTx, config *params.NodeConfig, account *common.SelectedExtKey, nonce uint64, gas hexutil.Big, txErr error) { + non := hexutil.Uint64(nonce) + // Expect calls to gas functions only if there are no user defined values. + s.txServiceMock.EXPECT().GetTransactionCount(gomock.Any(), account.Address, gethrpc.PendingBlockNumber).Return(&non, nil) + if tx.Args.GasPrice == nil { + s.txServiceMock.EXPECT().GasPrice(gomock.Any()).Return(big.NewInt(10), nil) + } + if tx.Args.Gas == nil { + s.txServiceMock.EXPECT().EstimateGas(gomock.Any(), gomock.Any()).Return(&gas, nil) + } + // Prepare the transaction an RLP encode it. + data := s.rlpEncodeTx(tx, config, account, nonce, gas) + // Expect the RLP encoded transaction. + s.txServiceMock.EXPECT().SendRawTransaction(gomock.Any(), data).Return(gethcommon.Hash{}, txErr) +} + +func (s *TxQueueTestSuite) rlpEncodeTx(tx *common.QueuedTx, config *params.NodeConfig, account *common.SelectedExtKey, nonce uint64, gas hexutil.Big) []byte { + newTx := types.NewTransaction( + nonce, + gethcommon.Address(*tx.Args.To), + tx.Args.Value.ToInt(), + gas.ToInt(), + tx.Args.GasPrice.ToInt(), + tx.Args.Data, + ) + chainID := big.NewInt(int64(config.NetworkID)) + signedTx, err := types.SignTx(newTx, types.NewEIP155Signer(chainID), account.AccountKey.PrivateKey) + s.NoError(err) + data, err := rlp.EncodeToBytes(signedTx) + s.NoError(err) + return data } -func (s *TxQueueTestSuite) setupStatusBackend(account *common.SelectedExtKey, password string, passwordErr error) { +func (s *TxQueueTestSuite) setupStatusBackend(account *common.SelectedExtKey, password string, passwordErr error) *params.NodeConfig { nodeConfig, nodeErr := params.NewNodeConfig("/tmp", params.RopstenNetworkID, true) s.nodeManagerMock.EXPECT().NodeConfig().Return(nodeConfig, nodeErr) s.accountManagerMock.EXPECT().SelectedAccount().Return(account, nil) s.accountManagerMock.EXPECT().VerifyAccountPassword(nodeConfig.KeyStoreDir, account.Address.String(), password).Return( nil, passwordErr) + return nodeConfig } func (s *TxQueueTestSuite) TestCompleteTransaction() { @@ -83,21 +112,22 @@ func (s *TxQueueTestSuite) TestCompleteTransaction() { Address: common.FromAddress(TestConfig.Account1.Address), AccountKey: &keystore.Key{PrivateKey: key}, } - s.setupStatusBackend(account, password, nil) + config := s.setupStatusBackend(account, password, nil) - nonce := hexutil.Uint64(10) + tx := common.CreateTransaction(context.Background(), common.SendTxArgs{ + From: common.FromAddress(TestConfig.Account1.Address), + To: common.ToAddress(TestConfig.Account2.Address), + }) + + nonce := uint64(10) gas := hexutil.Big(*big.NewInt(defaultGas + 1)) - s.setupTransactionPoolAPI(account, nonce, gas, nil) + s.setupTransactionPoolAPI(tx, config, account, nonce, gas, nil) txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock) txQueueManager.Start() defer txQueueManager.Stop() - tx := common.CreateTransaction(context.Background(), common.SendTxArgs{ - From: common.FromAddress(TestConfig.Account1.Address), - To: common.ToAddress(TestConfig.Account2.Address), - }) err := txQueueManager.QueueTransaction(tx) s.NoError(err) @@ -125,22 +155,22 @@ func (s *TxQueueTestSuite) TestCompleteTransactionMultipleTimes() { Address: common.FromAddress(TestConfig.Account1.Address), AccountKey: &keystore.Key{PrivateKey: key}, } - s.setupStatusBackend(account, password, nil) + config := s.setupStatusBackend(account, password, nil) - nonce := hexutil.Uint64(10) + tx := common.CreateTransaction(context.Background(), common.SendTxArgs{ + From: common.FromAddress(TestConfig.Account1.Address), + To: common.ToAddress(TestConfig.Account2.Address), + }) + + nonce := uint64(10) gas := hexutil.Big(*big.NewInt(defaultGas + 1)) - s.setupTransactionPoolAPI(account, nonce, gas, nil) + s.setupTransactionPoolAPI(tx, config, account, nonce, gas, nil) txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock) txQueueManager.DisableNotificactions() txQueueManager.Start() defer txQueueManager.Stop() - tx := common.CreateTransaction(context.Background(), common.SendTxArgs{ - From: common.FromAddress(TestConfig.Account1.Address), - To: common.ToAddress(TestConfig.Account2.Address), - }) - err := txQueueManager.QueueTransaction(tx) s.NoError(err) @@ -181,6 +211,34 @@ func (s *TxQueueTestSuite) TestCompleteTransactionMultipleTimes() { s.Equal(txCount-1, inprogressTx, "txs expected to be reported as inprogress") } +/* +func (s *TxQueueTestSuite) TestCompleteTransactionSignature() { + password := TestConfig.Account1.Password + key, _ := crypto.GenerateKey() + account := &common.SelectedExtKey{ + Address: common.FromAddress(TestConfig.Account1.Address), + AccountKey: &keystore.Key{PrivateKey: key}, + } + s.setupStatusBackend(account, password, nil) + + nonce := hexutil.Uint64(10) + gas := hexutil.Big(*big.NewInt(defaultGas + 1)) + + txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock) + + txQueueManager.Start() + defer txQueueManager.Stop() + + tx := common.CreateTransaction(context.Background(), common.SendTxArgs{ + From: common.FromAddress(TestConfig.Account1.Address), + To: common.ToAddress(TestConfig.Account2.Address), + }) + err := txQueueManager.QueueTransaction(tx) + s.NoError(err) + +} +*/ + func (s *TxQueueTestSuite) TestAccountMismatch() { s.accountManagerMock.EXPECT().SelectedAccount().Return(&common.SelectedExtKey{ Address: common.FromAddress(TestConfig.Account2.Address), From 4c3d4be49f1c35cce38c63c0c8db9c87ce63fab5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caner=20=C3=87=C4=B1dam?= Date: Thu, 1 Feb 2018 17:26:22 +0300 Subject: [PATCH 2/8] change timeout --- geth/transactions/txqueue_manager.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/geth/transactions/txqueue_manager.go b/geth/transactions/txqueue_manager.go index 984e066fd39..ad3a9f3cc6b 100644 --- a/geth/transactions/txqueue_manager.go +++ b/geth/transactions/txqueue_manager.go @@ -99,7 +99,9 @@ func (m *Manager) WaitForTransaction(tx *common.QueuedTx) error { // - or times out select { case <-tx.Done: - case <-time.After(DefaultTxSendCompletionTimeout * time.Second): + // TODO: Please reconsider this before merging. + //case <-time.After(DefaultTxSendCompletionTimeout * time.Second): + case <-time.After(time.Millisecond * 100): m.txDone(tx, gethcommon.Hash{}, queue.ErrQueuedTxTimedOut) } return tx.Err From 155a424622623f578d3572026835a6df29cd3a78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caner=20=C3=87=C4=B1dam?= Date: Thu, 1 Feb 2018 23:15:36 +0300 Subject: [PATCH 3/8] fix bad expected values --- geth/transactions/txqueue_manager.go | 2 +- geth/transactions/txqueue_manager_test.go | 67 ++++++++--------------- 2 files changed, 24 insertions(+), 45 deletions(-) diff --git a/geth/transactions/txqueue_manager.go b/geth/transactions/txqueue_manager.go index ad3a9f3cc6b..0ec7e7cb3d3 100644 --- a/geth/transactions/txqueue_manager.go +++ b/geth/transactions/txqueue_manager.go @@ -101,7 +101,7 @@ func (m *Manager) WaitForTransaction(tx *common.QueuedTx) error { case <-tx.Done: // TODO: Please reconsider this before merging. //case <-time.After(DefaultTxSendCompletionTimeout * time.Second): - case <-time.After(time.Millisecond * 100): + case <-time.After(time.Second): m.txDone(tx, gethcommon.Hash{}, queue.ErrQueuedTxTimedOut) } return tx.Err diff --git a/geth/transactions/txqueue_manager_test.go b/geth/transactions/txqueue_manager_test.go index 9aae9cd3ff0..a52555c0b1b 100644 --- a/geth/transactions/txqueue_manager_test.go +++ b/geth/transactions/txqueue_manager_test.go @@ -63,29 +63,36 @@ func (s *TxQueueTestSuite) TearDownTest() { s.client.Close() } -func (s *TxQueueTestSuite) setupTransactionPoolAPI(tx *common.QueuedTx, config *params.NodeConfig, account *common.SelectedExtKey, nonce uint64, gas hexutil.Big, txErr error) { - non := hexutil.Uint64(nonce) +func (s *TxQueueTestSuite) setupTransactionPoolAPI(tx *common.QueuedTx, config *params.NodeConfig, account *common.SelectedExtKey, nonce *hexutil.Uint64, gas *hexutil.Big, txErr error) { // Expect calls to gas functions only if there are no user defined values. - s.txServiceMock.EXPECT().GetTransactionCount(gomock.Any(), account.Address, gethrpc.PendingBlockNumber).Return(&non, nil) + // And also set the expected gas and gas price for RLP encoding the expected tx. + var usedGas, usedGasPrice *big.Int + s.txServiceMock.EXPECT().GetTransactionCount(gomock.Any(), account.Address, gethrpc.PendingBlockNumber).Return(nonce, nil) if tx.Args.GasPrice == nil { - s.txServiceMock.EXPECT().GasPrice(gomock.Any()).Return(big.NewInt(10), nil) + usedGasPrice = big.NewInt(10) + s.txServiceMock.EXPECT().GasPrice(gomock.Any()).Return(usedGasPrice, nil) + } else { + usedGasPrice = (*big.Int)(tx.Args.GasPrice) } if tx.Args.Gas == nil { - s.txServiceMock.EXPECT().EstimateGas(gomock.Any(), gomock.Any()).Return(&gas, nil) + s.txServiceMock.EXPECT().EstimateGas(gomock.Any(), gomock.Any()).Return(gas, nil) + usedGas = (*big.Int)(gas) + } else { + usedGas = (*big.Int)(tx.Args.Gas) } - // Prepare the transaction an RLP encode it. - data := s.rlpEncodeTx(tx, config, account, nonce, gas) + // Prepare the transaction anD RLP encode it. + data := s.rlpEncodeTx(tx, config, account, nonce, usedGas, usedGasPrice) // Expect the RLP encoded transaction. s.txServiceMock.EXPECT().SendRawTransaction(gomock.Any(), data).Return(gethcommon.Hash{}, txErr) } -func (s *TxQueueTestSuite) rlpEncodeTx(tx *common.QueuedTx, config *params.NodeConfig, account *common.SelectedExtKey, nonce uint64, gas hexutil.Big) []byte { +func (s *TxQueueTestSuite) rlpEncodeTx(tx *common.QueuedTx, config *params.NodeConfig, account *common.SelectedExtKey, nonce *hexutil.Uint64, gas, gasPrice *big.Int) hexutil.Bytes { newTx := types.NewTransaction( - nonce, + uint64(*nonce), gethcommon.Address(*tx.Args.To), tx.Args.Value.ToInt(), - gas.ToInt(), - tx.Args.GasPrice.ToInt(), + gas, + gasPrice, tx.Args.Data, ) chainID := big.NewInt(int64(config.NetworkID)) @@ -93,7 +100,7 @@ func (s *TxQueueTestSuite) rlpEncodeTx(tx *common.QueuedTx, config *params.NodeC s.NoError(err) data, err := rlp.EncodeToBytes(signedTx) s.NoError(err) - return data + return hexutil.Bytes(data) } func (s *TxQueueTestSuite) setupStatusBackend(account *common.SelectedExtKey, password string, passwordErr error) *params.NodeConfig { @@ -119,9 +126,9 @@ func (s *TxQueueTestSuite) TestCompleteTransaction() { To: common.ToAddress(TestConfig.Account2.Address), }) - nonce := uint64(10) + nonce := hexutil.Uint64(10) gas := hexutil.Big(*big.NewInt(defaultGas + 1)) - s.setupTransactionPoolAPI(tx, config, account, nonce, gas, nil) + s.setupTransactionPoolAPI(tx, config, account, &nonce, &gas, nil) txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock) @@ -162,9 +169,9 @@ func (s *TxQueueTestSuite) TestCompleteTransactionMultipleTimes() { To: common.ToAddress(TestConfig.Account2.Address), }) - nonce := uint64(10) + nonce := hexutil.Uint64(10) gas := hexutil.Big(*big.NewInt(defaultGas + 1)) - s.setupTransactionPoolAPI(tx, config, account, nonce, gas, nil) + s.setupTransactionPoolAPI(tx, config, account, &nonce, &gas, nil) txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock) txQueueManager.DisableNotificactions() @@ -211,34 +218,6 @@ func (s *TxQueueTestSuite) TestCompleteTransactionMultipleTimes() { s.Equal(txCount-1, inprogressTx, "txs expected to be reported as inprogress") } -/* -func (s *TxQueueTestSuite) TestCompleteTransactionSignature() { - password := TestConfig.Account1.Password - key, _ := crypto.GenerateKey() - account := &common.SelectedExtKey{ - Address: common.FromAddress(TestConfig.Account1.Address), - AccountKey: &keystore.Key{PrivateKey: key}, - } - s.setupStatusBackend(account, password, nil) - - nonce := hexutil.Uint64(10) - gas := hexutil.Big(*big.NewInt(defaultGas + 1)) - - txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock) - - txQueueManager.Start() - defer txQueueManager.Stop() - - tx := common.CreateTransaction(context.Background(), common.SendTxArgs{ - From: common.FromAddress(TestConfig.Account1.Address), - To: common.ToAddress(TestConfig.Account2.Address), - }) - err := txQueueManager.QueueTransaction(tx) - s.NoError(err) - -} -*/ - func (s *TxQueueTestSuite) TestAccountMismatch() { s.accountManagerMock.EXPECT().SelectedAccount().Return(&common.SelectedExtKey{ Address: common.FromAddress(TestConfig.Account2.Address), From 35a2a313d668b7a130730e72dfba7a04e9e735c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caner=20=C3=87=C4=B1dam?= Date: Thu, 1 Feb 2018 23:55:36 +0300 Subject: [PATCH 4/8] revert timeout --- geth/transactions/txqueue_manager.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/geth/transactions/txqueue_manager.go b/geth/transactions/txqueue_manager.go index 0ec7e7cb3d3..984e066fd39 100644 --- a/geth/transactions/txqueue_manager.go +++ b/geth/transactions/txqueue_manager.go @@ -99,9 +99,7 @@ func (m *Manager) WaitForTransaction(tx *common.QueuedTx) error { // - or times out select { case <-tx.Done: - // TODO: Please reconsider this before merging. - //case <-time.After(DefaultTxSendCompletionTimeout * time.Second): - case <-time.After(time.Second): + case <-time.After(DefaultTxSendCompletionTimeout * time.Second): m.txDone(tx, gethcommon.Hash{}, queue.ErrQueuedTxTimedOut) } return tx.Err From 16a05ebb133f12bb1dfd9827903a7a48479f0010 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caner=20=C3=87=C4=B1dam?= Date: Fri, 2 Feb 2018 16:45:14 +0300 Subject: [PATCH 5/8] fix hanging test --- geth/transactions/txqueue_manager.go | 10 ++++++---- geth/transactions/txqueue_manager_test.go | 4 +++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/geth/transactions/txqueue_manager.go b/geth/transactions/txqueue_manager.go index 502a6444c92..26e176ffbad 100644 --- a/geth/transactions/txqueue_manager.go +++ b/geth/transactions/txqueue_manager.go @@ -32,6 +32,7 @@ type Manager struct { addrLock *AddrLocker notify bool completionTimeout time.Duration + ctxTimeout time.Duration } // NewManager returns a new Manager. @@ -43,6 +44,7 @@ func NewManager(nodeManager common.NodeManager, accountManager common.AccountMan addrLock: &AddrLocker{}, notify: true, completionTimeout: DefaultTxSendCompletionTimeout, + ctxTimeout: defaultTimeout, } } @@ -166,7 +168,7 @@ func (m *Manager) completeTransaction(queuedTx *common.QueuedTx, selectedAccount } m.addrLock.LockAddr(queuedTx.Args.From) defer m.addrLock.UnlockAddr(queuedTx.Args.From) - ctx, cancel := context.WithTimeout(context.Background(), defaultTimeout) + ctx, cancel := context.WithTimeout(context.Background(), m.ctxTimeout) defer cancel() nonce, err := m.ethTxClient.PendingNonceAt(ctx, queuedTx.Args.From) if err != nil { @@ -175,7 +177,7 @@ func (m *Manager) completeTransaction(queuedTx *common.QueuedTx, selectedAccount args := queuedTx.Args gasPrice := (*big.Int)(args.GasPrice) if args.GasPrice == nil { - ctx, cancel = context.WithTimeout(context.Background(), defaultTimeout) + ctx, cancel = context.WithTimeout(context.Background(), m.ctxTimeout) defer cancel() gasPrice, err = m.ethTxClient.SuggestGasPrice(ctx) if err != nil { @@ -193,7 +195,7 @@ func (m *Manager) completeTransaction(queuedTx *common.QueuedTx, selectedAccount gas := (*big.Int)(args.Gas) if args.Gas == nil { - ctx, cancel = context.WithTimeout(context.Background(), defaultTimeout) + ctx, cancel = context.WithTimeout(context.Background(), m.ctxTimeout) defer cancel() gas, err = m.ethTxClient.EstimateGas(ctx, ethereum.CallMsg{ From: args.From, @@ -224,7 +226,7 @@ func (m *Manager) completeTransaction(queuedTx *common.QueuedTx, selectedAccount if err != nil { return hash, err } - ctx, cancel = context.WithTimeout(context.Background(), defaultTimeout) + ctx, cancel = context.WithTimeout(context.Background(), m.ctxTimeout) defer cancel() if err := m.ethTxClient.SendTransaction(ctx, signedTx); err != nil { return hash, err diff --git a/geth/transactions/txqueue_manager_test.go b/geth/transactions/txqueue_manager_test.go index a2376b98c8c..8a2092dd758 100644 --- a/geth/transactions/txqueue_manager_test.go +++ b/geth/transactions/txqueue_manager_test.go @@ -132,6 +132,7 @@ func (s *TxQueueTestSuite) TestCompleteTransaction() { txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock) txQueueManager.completionTimeout = time.Second + txQueueManager.ctxTimeout = time.Second txQueueManager.Start() defer txQueueManager.Stop() @@ -177,6 +178,7 @@ func (s *TxQueueTestSuite) TestCompleteTransactionMultipleTimes() { txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock) txQueueManager.completionTimeout = time.Second + txQueueManager.ctxTimeout = time.Second txQueueManager.DisableNotificactions() txQueueManager.Start() defer txQueueManager.Stop() @@ -197,6 +199,7 @@ func (s *TxQueueTestSuite) TestCompleteTransactionMultipleTimes() { defer wg.Done() _, err := txQueueManager.CompleteTransaction(tx.ID, password) mu.Lock() + defer mu.Unlock() if err == nil { completedTx++ } else if err == queue.ErrQueuedTxInProgress { @@ -204,7 +207,6 @@ func (s *TxQueueTestSuite) TestCompleteTransactionMultipleTimes() { } else { s.Fail("tx failed with unexpected error: ", err.Error()) } - mu.Unlock() }() } From 1b3dbdec53b8a6b6f5e574ff3c57d711dec19bba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caner=20=C3=87=C4=B1dam?= Date: Fri, 2 Feb 2018 18:31:15 +0300 Subject: [PATCH 6/8] table-driven gas and gas price tests --- geth/transactions/txqueue_manager_test.go | 93 +++++++++++++++-------- 1 file changed, 61 insertions(+), 32 deletions(-) diff --git a/geth/transactions/txqueue_manager_test.go b/geth/transactions/txqueue_manager_test.go index 8a2092dd758..eb384f65d92 100644 --- a/geth/transactions/txqueue_manager_test.go +++ b/geth/transactions/txqueue_manager_test.go @@ -119,43 +119,72 @@ func (s *TxQueueTestSuite) TestCompleteTransaction() { Address: common.FromAddress(TestConfig.Account1.Address), AccountKey: &keystore.Key{PrivateKey: key}, } - config := s.setupStatusBackend(account, password, nil) - - tx := common.CreateTransaction(context.Background(), common.SendTxArgs{ - From: common.FromAddress(TestConfig.Account1.Address), - To: common.ToAddress(TestConfig.Account2.Address), - }) nonce := hexutil.Uint64(10) gas := hexutil.Big(*big.NewInt(defaultGas + 1)) - s.setupTransactionPoolAPI(tx, config, account, &nonce, &gas, nil) - txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock) - txQueueManager.completionTimeout = time.Second - txQueueManager.ctxTimeout = time.Second - - txQueueManager.Start() - defer txQueueManager.Stop() - - s.NoError(txQueueManager.QueueTransaction(tx)) - w := make(chan struct{}) - var ( - hash gethcommon.Hash - err error - ) - go func() { - hash, err = txQueueManager.CompleteTransaction(tx.ID, password) - s.NoError(err) - close(w) - }() + testCases := []struct { + name string + gas *hexutil.Big + gasPrice *hexutil.Big + }{ + { + "noGasDef", + nil, + nil, + }, + { + "gasDefined", + (*hexutil.Big)(big.NewInt(100)), + nil, + }, + { + "gasPriceDefined", + nil, + (*hexutil.Big)(big.NewInt(100)), + }, + } - rst := txQueueManager.WaitForTransaction(tx) - // Check that error is assigned to the transaction. - s.NoError(rst.Error) - // Transaction should be already removed from the queue. - s.False(txQueueManager.TransactionQueue().Has(tx.ID)) - s.NoError(WaitClosed(w, time.Second)) - s.Equal(hash, rst.Hash) + for _, testCase := range testCases { + s.T().Run(testCase.name, func(t *testing.T) { + s.SetupTest() + config := s.setupStatusBackend(account, password, nil) + tx := common.CreateTransaction(context.Background(), common.SendTxArgs{ + From: common.FromAddress(TestConfig.Account1.Address), + To: common.ToAddress(TestConfig.Account2.Address), + Gas: testCase.gas, + GasPrice: testCase.gasPrice, + }) + s.setupTransactionPoolAPI(tx, config, account, &nonce, &gas, nil) + + txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock) + txQueueManager.completionTimeout = time.Second + txQueueManager.ctxTimeout = time.Second + + txQueueManager.Start() + defer txQueueManager.Stop() + + s.NoError(txQueueManager.QueueTransaction(tx)) + w := make(chan struct{}) + var ( + hash gethcommon.Hash + err error + ) + go func() { + hash, err = txQueueManager.CompleteTransaction(tx.ID, password) + s.NoError(err) + close(w) + }() + + rst := txQueueManager.WaitForTransaction(tx) + // Check that error is assigned to the transaction. + s.NoError(rst.Error) + // Transaction should be already removed from the queue. + s.False(txQueueManager.TransactionQueue().Has(tx.ID)) + s.NoError(WaitClosed(w, time.Second)) + s.Equal(hash, rst.Hash) + }) + } } func (s *TxQueueTestSuite) TestCompleteTransactionMultipleTimes() { From 70e00f71ea75ed4f092a76f0e91ad2ac9a6237eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caner=20=C3=87=C4=B1dam?= Date: Mon, 5 Feb 2018 13:31:20 +0300 Subject: [PATCH 7/8] rename ctxTimeout to rpcCallTimeout --- geth/transactions/txqueue_manager.go | 12 ++++++------ geth/transactions/txqueue_manager_test.go | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/geth/transactions/txqueue_manager.go b/geth/transactions/txqueue_manager.go index 26e176ffbad..1f135b15f5e 100644 --- a/geth/transactions/txqueue_manager.go +++ b/geth/transactions/txqueue_manager.go @@ -32,7 +32,7 @@ type Manager struct { addrLock *AddrLocker notify bool completionTimeout time.Duration - ctxTimeout time.Duration + rpcCallTimeout time.Duration } // NewManager returns a new Manager. @@ -44,7 +44,7 @@ func NewManager(nodeManager common.NodeManager, accountManager common.AccountMan addrLock: &AddrLocker{}, notify: true, completionTimeout: DefaultTxSendCompletionTimeout, - ctxTimeout: defaultTimeout, + rpcCallTimeout: defaultTimeout, } } @@ -168,7 +168,7 @@ func (m *Manager) completeTransaction(queuedTx *common.QueuedTx, selectedAccount } m.addrLock.LockAddr(queuedTx.Args.From) defer m.addrLock.UnlockAddr(queuedTx.Args.From) - ctx, cancel := context.WithTimeout(context.Background(), m.ctxTimeout) + ctx, cancel := context.WithTimeout(context.Background(), m.rpcCallTimeout) defer cancel() nonce, err := m.ethTxClient.PendingNonceAt(ctx, queuedTx.Args.From) if err != nil { @@ -177,7 +177,7 @@ func (m *Manager) completeTransaction(queuedTx *common.QueuedTx, selectedAccount args := queuedTx.Args gasPrice := (*big.Int)(args.GasPrice) if args.GasPrice == nil { - ctx, cancel = context.WithTimeout(context.Background(), m.ctxTimeout) + ctx, cancel = context.WithTimeout(context.Background(), m.rpcCallTimeout) defer cancel() gasPrice, err = m.ethTxClient.SuggestGasPrice(ctx) if err != nil { @@ -195,7 +195,7 @@ func (m *Manager) completeTransaction(queuedTx *common.QueuedTx, selectedAccount gas := (*big.Int)(args.Gas) if args.Gas == nil { - ctx, cancel = context.WithTimeout(context.Background(), m.ctxTimeout) + ctx, cancel = context.WithTimeout(context.Background(), m.rpcCallTimeout) defer cancel() gas, err = m.ethTxClient.EstimateGas(ctx, ethereum.CallMsg{ From: args.From, @@ -226,7 +226,7 @@ func (m *Manager) completeTransaction(queuedTx *common.QueuedTx, selectedAccount if err != nil { return hash, err } - ctx, cancel = context.WithTimeout(context.Background(), m.ctxTimeout) + ctx, cancel = context.WithTimeout(context.Background(), m.rpcCallTimeout) defer cancel() if err := m.ethTxClient.SendTransaction(ctx, signedTx); err != nil { return hash, err diff --git a/geth/transactions/txqueue_manager_test.go b/geth/transactions/txqueue_manager_test.go index eb384f65d92..138bd76deb2 100644 --- a/geth/transactions/txqueue_manager_test.go +++ b/geth/transactions/txqueue_manager_test.go @@ -159,7 +159,7 @@ func (s *TxQueueTestSuite) TestCompleteTransaction() { txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock) txQueueManager.completionTimeout = time.Second - txQueueManager.ctxTimeout = time.Second + txQueueManager.rpcCallTimeout = time.Second txQueueManager.Start() defer txQueueManager.Stop() @@ -207,7 +207,7 @@ func (s *TxQueueTestSuite) TestCompleteTransactionMultipleTimes() { txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock) txQueueManager.completionTimeout = time.Second - txQueueManager.ctxTimeout = time.Second + txQueueManager.rpcCallTimeout = time.Second txQueueManager.DisableNotificactions() txQueueManager.Start() defer txQueueManager.Stop() From 347f726a939198f4dd60d2f05973ea45e5559896 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caner=20=C3=87=C4=B1dam?= Date: Mon, 5 Feb 2018 17:28:40 +0300 Subject: [PATCH 8/8] reduce args --- geth/transactions/txqueue_manager_test.go | 31 ++++++++++++----------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/geth/transactions/txqueue_manager_test.go b/geth/transactions/txqueue_manager_test.go index 138bd76deb2..78c271b886a 100644 --- a/geth/transactions/txqueue_manager_test.go +++ b/geth/transactions/txqueue_manager_test.go @@ -63,25 +63,31 @@ func (s *TxQueueTestSuite) TearDownTest() { s.client.Close() } -func (s *TxQueueTestSuite) setupTransactionPoolAPI(tx *common.QueuedTx, config *params.NodeConfig, account *common.SelectedExtKey, nonce *hexutil.Uint64, gas *hexutil.Big, txErr error) { +var ( + testGas = (*hexutil.Big)(big.NewInt(defaultGas + 1)) + testGasPrice = (*hexutil.Big)(big.NewInt(10)) + testNonce = hexutil.Uint64(10) +) + +func (s *TxQueueTestSuite) setupTransactionPoolAPI(tx *common.QueuedTx, config *params.NodeConfig, account *common.SelectedExtKey, txErr error) { // Expect calls to gas functions only if there are no user defined values. // And also set the expected gas and gas price for RLP encoding the expected tx. var usedGas, usedGasPrice *big.Int - s.txServiceMock.EXPECT().GetTransactionCount(gomock.Any(), account.Address, gethrpc.PendingBlockNumber).Return(nonce, nil) + s.txServiceMock.EXPECT().GetTransactionCount(gomock.Any(), account.Address, gethrpc.PendingBlockNumber).Return(&testNonce, nil) if tx.Args.GasPrice == nil { - usedGasPrice = big.NewInt(10) + usedGasPrice = (*big.Int)(testGasPrice) s.txServiceMock.EXPECT().GasPrice(gomock.Any()).Return(usedGasPrice, nil) } else { usedGasPrice = (*big.Int)(tx.Args.GasPrice) } if tx.Args.Gas == nil { - s.txServiceMock.EXPECT().EstimateGas(gomock.Any(), gomock.Any()).Return(gas, nil) - usedGas = (*big.Int)(gas) + s.txServiceMock.EXPECT().EstimateGas(gomock.Any(), gomock.Any()).Return(testGas, nil) + usedGas = (*big.Int)(testGas) } else { usedGas = (*big.Int)(tx.Args.Gas) } // Prepare the transaction anD RLP encode it. - data := s.rlpEncodeTx(tx, config, account, nonce, usedGas, usedGasPrice) + data := s.rlpEncodeTx(tx, config, account, &testNonce, usedGas, usedGasPrice) // Expect the RLP encoded transaction. s.txServiceMock.EXPECT().SendRawTransaction(gomock.Any(), data).Return(gethcommon.Hash{}, txErr) } @@ -120,9 +126,6 @@ func (s *TxQueueTestSuite) TestCompleteTransaction() { AccountKey: &keystore.Key{PrivateKey: key}, } - nonce := hexutil.Uint64(10) - gas := hexutil.Big(*big.NewInt(defaultGas + 1)) - testCases := []struct { name string gas *hexutil.Big @@ -135,13 +138,13 @@ func (s *TxQueueTestSuite) TestCompleteTransaction() { }, { "gasDefined", - (*hexutil.Big)(big.NewInt(100)), + testGas, nil, }, { "gasPriceDefined", nil, - (*hexutil.Big)(big.NewInt(100)), + testGasPrice, }, } @@ -155,7 +158,7 @@ func (s *TxQueueTestSuite) TestCompleteTransaction() { Gas: testCase.gas, GasPrice: testCase.gasPrice, }) - s.setupTransactionPoolAPI(tx, config, account, &nonce, &gas, nil) + s.setupTransactionPoolAPI(tx, config, account, nil) txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock) txQueueManager.completionTimeout = time.Second @@ -201,9 +204,7 @@ func (s *TxQueueTestSuite) TestCompleteTransactionMultipleTimes() { To: common.ToAddress(TestConfig.Account2.Address), }) - nonce := hexutil.Uint64(10) - gas := hexutil.Big(*big.NewInt(defaultGas + 1)) - s.setupTransactionPoolAPI(tx, config, account, &nonce, &gas, nil) + s.setupTransactionPoolAPI(tx, config, account, nil) txQueueManager := NewManager(s.nodeManagerMock, s.accountManagerMock) txQueueManager.completionTimeout = time.Second