Allow gas estimation for DynamicFeeTx in transactor (#5521)

* fix_: allow gas estimation for DynamicFeeTx

* test_: added tests for BuildAndValidateTransaction

* fix_: using IsDynamicTx for gasPrice suggestion

* fix_: hash transaction check

Co-authored-by: Stefan Dunca <47554641+stefandunca@users.noreply.github.com>

---------

Co-authored-by: Stefan Dunca <47554641+stefandunca@users.noreply.github.com>
This commit is contained in:
Lungu Cristian 2024-07-17 11:11:34 +03:00 committed by GitHub
parent 9403475572
commit 7e4322b45a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 140 additions and 17 deletions

View File

@ -281,7 +281,7 @@ func (t *Transactor) HashTransaction(args SendTxArgs) (validatedArgs SendTxArgs,
gasPrice := (*big.Int)(args.GasPrice)
gasFeeCap := (*big.Int)(args.MaxFeePerGas)
gasTipCap := (*big.Int)(args.MaxPriorityFeePerGas)
if args.GasPrice == nil && args.MaxFeePerGas == nil {
if args.GasPrice == nil && !args.IsDynamicFeeTx() {
ctx, cancel := context.WithTimeout(context.Background(), t.rpcCallTimeout)
defer cancel()
gasPrice, err = t.rpcWrapper.SuggestGasPrice(ctx)
@ -306,7 +306,7 @@ func (t *Transactor) HashTransaction(args SendTxArgs) (validatedArgs SendTxArgs,
gethTo = common.Address(*args.To)
gethToPtr = &gethTo
}
if args.GasPrice == nil {
if args.IsDynamicFeeTx() {
gas, err = t.rpcWrapper.EstimateGas(ctx, ethereum.CallMsg{
From: common.Address(args.From),
To: gethToPtr,
@ -334,7 +334,7 @@ func (t *Transactor) HashTransaction(args SendTxArgs) (validatedArgs SendTxArgs,
newNonce := hexutil.Uint64(nonce)
newGas := hexutil.Uint64(gas)
validatedArgs.Nonce = &newNonce
if args.GasPrice != nil {
if !args.IsDynamicFeeTx() {
validatedArgs.GasPrice = (*hexutil.Big)(gasPrice)
} else {
validatedArgs.MaxPriorityFeePerGas = (*hexutil.Big)(gasTipCap)
@ -380,6 +380,7 @@ func (t *Transactor) validateAndBuildTransaction(rpcWrapper *rpcWrapper, args Se
defer cancel()
gasPrice := (*big.Int)(args.GasPrice)
// GasPrice should be estimated only for LegacyTx
if !args.IsDynamicFeeTx() && args.GasPrice == nil {
gasPrice, err = rpcWrapper.SuggestGasPrice(ctx)
if err != nil {
@ -391,7 +392,7 @@ func (t *Transactor) validateAndBuildTransaction(rpcWrapper *rpcWrapper, args Se
var gas uint64
if args.Gas != nil {
gas = uint64(*args.Gas)
} else if args.Gas == nil && !args.IsDynamicFeeTx() {
} else {
ctx, cancel = context.WithTimeout(context.Background(), t.rpcCallTimeout)
defer cancel()
@ -403,13 +404,26 @@ func (t *Transactor) validateAndBuildTransaction(rpcWrapper *rpcWrapper, args Se
gethTo = common.Address(*args.To)
gethToPtr = &gethTo
}
gas, err = rpcWrapper.EstimateGas(ctx, ethereum.CallMsg{
From: common.Address(args.From),
To: gethToPtr,
GasPrice: gasPrice,
Value: value,
Data: args.GetInput(),
})
if args.IsDynamicFeeTx() {
gasFeeCap := (*big.Int)(args.MaxFeePerGas)
gasTipCap := (*big.Int)(args.MaxPriorityFeePerGas)
gas, err = t.rpcWrapper.EstimateGas(ctx, ethereum.CallMsg{
From: common.Address(args.From),
To: gethToPtr,
GasFeeCap: gasFeeCap,
GasTipCap: gasTipCap,
Value: value,
Data: args.GetInput(),
})
} else {
gas, err = t.rpcWrapper.EstimateGas(ctx, ethereum.CallMsg{
From: common.Address(args.From),
To: gethToPtr,
GasPrice: gasPrice,
Value: value,
Data: args.GetInput(),
})
}
if err != nil {
return tx, err
}

View File

@ -92,12 +92,13 @@ func (s *TransactorSuite) setupTransactionPoolAPI(args SendTxArgs, returnNonce,
} else {
usedGasPrice = (*big.Int)(args.GasPrice)
}
if args.Gas == nil {
s.txServiceMock.EXPECT().EstimateGas(gomock.Any(), gomock.Any()).Return(testGas, nil)
usedGas = testGas
} else {
usedGas = *args.Gas
}
}
if args.Gas == nil {
s.txServiceMock.EXPECT().EstimateGas(gomock.Any(), gomock.Any()).Return(testGas, nil)
usedGas = testGas
} else {
usedGas = *args.Gas
}
// Prepare the transaction and RLP encode it.
data := s.rlpEncodeTx(args, s.nodeConfig, account, &resultNonce, usedGas, usedGasPrice)
@ -213,6 +214,114 @@ func (s *TransactorSuite) TestGasValues() {
}
}
func (s *TransactorSuite) setupBuildTransactionMocks(args SendTxArgs, account *account.SelectedExtKey) {
s.txServiceMock.EXPECT().GetTransactionCount(gomock.Any(), gomock.Eq(common.Address(account.Address)), gethrpc.PendingBlockNumber).Return(&testNonce, nil)
if !args.IsDynamicFeeTx() && args.GasPrice == nil {
s.txServiceMock.EXPECT().GasPrice(gomock.Any()).Return(testGasPrice, nil)
}
if args.Gas == nil {
s.txServiceMock.EXPECT().EstimateGas(gomock.Any(), gomock.Any()).Return(testGas, nil)
}
}
func (s *TransactorSuite) TestBuildAndValidateTransaction() {
key, _ := gethcrypto.GenerateKey()
selectedAccount := &account.SelectedExtKey{
Address: account.FromAddress(utils.TestConfig.Account1.WalletAddress),
AccountKey: &types.Key{PrivateKey: key},
}
chainID := s.nodeConfig.NetworkID
fromAddress := account.FromAddress(utils.TestConfig.Account1.WalletAddress)
toAddress := account.ToAddress(utils.TestConfig.Account2.WalletAddress)
value := (*hexutil.Big)(big.NewInt(10))
expectedGasPrice := (*big.Int)(testGasPrice)
expectedGas := uint64(testGas)
expectedNonce := uint64(testNonce)
s.T().Run("DynamicFeeTransaction", func(t *testing.T) {
s.SetupTest()
gas := hexutil.Uint64(21000)
args := SendTxArgs{
From: fromAddress,
To: toAddress,
Gas: &gas,
Value: value,
MaxFeePerGas: testGasPrice,
MaxPriorityFeePerGas: testGasPrice,
}
s.setupBuildTransactionMocks(args, selectedAccount)
tx, err := s.manager.ValidateAndBuildTransaction(chainID, args)
s.NoError(err)
s.Equal(tx.Gas(), uint64(gas), "The gas shouldn't be estimated, but should use the gas from the Tx")
s.Equal(tx.GasFeeCap(), expectedGasPrice, "The maxFeePerGas should be the same as in the original Tx")
s.Equal(tx.GasTipCap(), expectedGasPrice, "The maxPriorityFeePerGas should be the same as in the original Tx")
s.Equal(tx.Type(), uint8(gethtypes.DynamicFeeTxType), "The transaction type should be DynamicFeeTxType")
})
s.T().Run("DynamicFeeTransaction with gas estimation", func(t *testing.T) {
s.SetupTest()
args := SendTxArgs{
From: fromAddress,
To: toAddress,
Value: value,
MaxFeePerGas: testGasPrice,
MaxPriorityFeePerGas: testGasPrice,
}
s.setupBuildTransactionMocks(args, selectedAccount)
tx, err := s.manager.ValidateAndBuildTransaction(chainID, args)
s.NoError(err)
s.Equal(tx.Gas(), expectedGas, "The gas should be estimated if not present in the original Tx")
s.Equal(tx.Nonce(), expectedNonce, "The nonce should be added if not present in the original Tx")
s.Equal(tx.GasFeeCap(), expectedGasPrice, "The maxFeePerGas should be the same as in the original Tx")
s.Equal(tx.GasTipCap(), expectedGasPrice, "The maxPriorityFeePerGas should be the same as in the original Tx")
s.Equal(tx.Type(), uint8(gethtypes.DynamicFeeTxType), "The transaction type should be DynamicFeeTxType")
})
s.T().Run("LegacyTransaction", func(t *testing.T) {
s.SetupTest()
gas := hexutil.Uint64(21000)
gasPrice := (*hexutil.Big)(big.NewInt(10))
args := SendTxArgs{
From: fromAddress,
To: toAddress,
Value: value,
Gas: &gas,
GasPrice: gasPrice,
}
s.setupBuildTransactionMocks(args, selectedAccount)
tx, err := s.manager.ValidateAndBuildTransaction(chainID, args)
s.NoError(err)
s.Equal(tx.Gas(), uint64(gas), "The gas shouldn't be estimated, but should use the gas from the Tx")
s.Equal(tx.GasPrice(), expectedGasPrice, "The gasPrice should be the same as in the original Tx")
s.Equal(tx.Type(), uint8(gethtypes.LegacyTxType), "The transaction type should be LegacyTxType")
})
s.T().Run("LegacyTransaction without gas estimation", func(t *testing.T) {
s.SetupTest()
args := SendTxArgs{
From: fromAddress,
To: toAddress,
Value: value,
}
s.setupBuildTransactionMocks(args, selectedAccount)
tx, err := s.manager.ValidateAndBuildTransaction(chainID, args)
s.NoError(err)
s.Equal(tx.Gas(), expectedGas, "The gas should be estimated if not present in the original Tx")
s.Equal(tx.GasPrice(), expectedGasPrice, "The gasPrice should be estimated if not present in the original Tx")
s.Equal(tx.Type(), uint8(gethtypes.LegacyTxType), "The transaction type should be LegacyTxType")
})
}
func (s *TransactorSuite) TestArgsValidation() {
args := SendTxArgs{
From: account.FromAddress(utils.TestConfig.Account1.WalletAddress),