Skip to content
This repository has been archived by the owner on Dec 4, 2024. It is now read-only.

estimateGas fix for simple value transactions and contract creations #1998

Merged
merged 3 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions e2e-polybft/e2e/jsonrpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/0xPolygon/polygon-edge/consensus/polybft/contractsapi"
"github.com/0xPolygon/polygon-edge/e2e-polybft/framework"
"github.com/0xPolygon/polygon-edge/helper/hex"
"github.com/0xPolygon/polygon-edge/state"
"github.com/0xPolygon/polygon-edge/types"
)

Expand Down Expand Up @@ -126,6 +127,40 @@ func TestE2E_JsonRPC(t *testing.T) {
require.Equal(t, uint64(0x56a3), resp)
})

t.Run("eth_estimateGas by zero-balance account - simple value transfer", func(t *testing.T) {
acctZeroBalance, err := wallet.GenerateKey()
require.NoError(t, err)

fundedAccountAddress := acct.Address()
nonFundedAccountAddress := acctZeroBalance.Address()

estimateGasFn := func(value *big.Int) {
resp, err := client.EstimateGas(&ethgo.CallMsg{
From: nonFundedAccountAddress,
To: &fundedAccountAddress,
Value: value,
})

require.NoError(t, err)
require.Equal(t, state.TxGas, resp)
}

estimateGasFn(ethgo.Gwei(1))

// transfer some funds to zero balance account
valueTransferTxn := cluster.SendTxn(t, acct, &ethgo.Transaction{
From: fundedAccountAddress,
To: &nonFundedAccountAddress,
Value: ethgo.Gwei(10),
})

require.NoError(t, valueTransferTxn.Wait())
require.True(t, valueTransferTxn.Succeed())

// now call estimate gas again for the now funded account
estimateGasFn(ethgo.Gwei(1))
})

t.Run("eth_getBalance", func(t *testing.T) {
key1, err := wallet.GenerateKey()
require.NoError(t, err)
Expand Down
26 changes: 14 additions & 12 deletions jsonrpc/eth_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -551,13 +551,24 @@ func (e *Eth) EstimateGas(arg *txnArgs, rawNum *BlockNumber) (interface{}, error
return nil, err
}

forksInTime := e.store.GetForksInTime(header.Number)

if transaction.IsValueTransfer() {
// if it is a simple value transfer or a contract creation,
// we already know what is the transaction gas cost, no need to apply transaction
gasCost, err := state.TransactionGasCost(transaction, forksInTime.Homestead, forksInTime.Istanbul)
if err != nil {
return nil, err
}

return argUint64(gasCost), nil
}

// Force transaction gas price if empty
if err = e.fillTransactionGasPrice(transaction); err != nil {
return nil, err
}

forksInTime := e.store.GetForksInTime(header.Number)

var standardGas uint64
if transaction.IsContractCreation() && forksInTime.Homestead {
standardGas = state.TxGasContractCreation
Expand All @@ -579,7 +590,6 @@ func (e *Eth) EstimateGas(arg *txnArgs, rawNum *BlockNumber) (interface{}, error
}

gasPriceInt := new(big.Int).Set(transaction.GasPrice)
valueInt := new(big.Int).Set(transaction.Value)

var availableBalance *big.Int

Expand All @@ -602,14 +612,6 @@ func (e *Eth) EstimateGas(arg *txnArgs, rawNum *BlockNumber) (interface{}, error
}

availableBalance = new(big.Int).Set(accountBalance)

if transaction.Value != nil {
if valueInt.Cmp(availableBalance) > 0 {
return 0, ErrInsufficientFunds
}

availableBalance.Sub(availableBalance, valueInt)
}
}

// Recalculate the gas ceiling based on the available funds (if any)
Expand Down Expand Up @@ -663,7 +665,7 @@ func (e *Eth) EstimateGas(arg *txnArgs, rawNum *BlockNumber) (interface{}, error

transaction.Gas = gas

result, applyErr := e.store.ApplyTxn(header, transaction, nil, false)
result, applyErr := e.store.ApplyTxn(header, transaction, nil, true)

if result != nil {
data = []byte(hex.EncodeToString(result.ReturnValue))
Expand Down
44 changes: 38 additions & 6 deletions jsonrpc/eth_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -772,27 +772,59 @@ func TestEth_EstimateGas_Reverts(t *testing.T) {
}
}

func TestEth_EstimateGas_Errors(t *testing.T) {
func TestEth_EstimateGas_ValueTransfer(t *testing.T) {
store := getExampleStore()
ethEndpoint := newTestEthEndpoint(store)

// Account doesn't have any balance
store.account.account.Balance = big.NewInt(0)

// The transaction has a value > 0
from := types.StringToAddress("0xSenderAddress")
to := types.StringToAddress("0xReceiverAddress")
mockTx := constructMockTx(nil, nil)
mockTx.Value = argBytesPtr([]byte{0x1})
mockTx.From = &from
mockTx.To = &to

// Run the estimation
estimate, estimateErr := ethEndpoint.EstimateGas(
estimate, err := ethEndpoint.EstimateGas(
mockTx,
nil,
)

assert.Equal(t, 0, estimate)
assert.NotNil(t, estimate)
estimateUint64, ok := estimate.(argUint64)
assert.True(t, ok)
assert.NoError(t, err)
assert.Equal(t, state.TxGas, uint64(estimateUint64)) // simple value transfers are 21000wei always
}

func TestEth_EstimateGas_ContractCreation(t *testing.T) {
store := getExampleStore()
ethEndpoint := newTestEthEndpoint(store)

// Account doesn't have any balance
store.account.account.Balance = big.NewInt(0)

// The transaction has a value > 0
from := types.StringToAddress("0xSenderAddress")
mockTx := constructMockTx(nil, nil)
mockTx.From = &from
mockTx.Input = argBytesPtr([]byte{})
mockTx.To = nil

// Run the estimation
estimate, err := ethEndpoint.EstimateGas(
mockTx,
nil,
)

// Make sure the insufficient funds error message is contained
assert.ErrorIs(t, estimateErr, ErrInsufficientFunds)
assert.NotNil(t, estimate)
estimateUint64, ok := estimate.(argUint64)
assert.True(t, ok)
assert.NoError(t, err)
assert.Equal(t, state.TxGasContractCreation, uint64(estimateUint64))
}

type mockSpecialStore struct {
Expand Down Expand Up @@ -867,7 +899,7 @@ func (m *mockSpecialStore) GetCode(root types.Hash, addr types.Address) ([]byte,
}

func (m *mockSpecialStore) GetForksInTime(blockNumber uint64) chain.ForksInTime {
return chain.ForksInTime{}
return chain.AllForksEnabled.At(0)
}

func (m *mockSpecialStore) ApplyTxn(header *types.Header, txn *types.Transaction, _ types.StateOverride, _ bool) (*runtime.ExecutionResult, error) {
Expand Down
8 changes: 8 additions & 0 deletions types/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,14 @@ func (t *Transaction) IsContractCreation() bool {
return t.To == nil
}

// IsValueTransfer checks if tx is a value transfer
func (t *Transaction) IsValueTransfer() bool {
return t.Value != nil &&
t.Value.Sign() > 0 &&
len(t.Input) == 0 &&
!t.IsContractCreation()
}

// ComputeHash computes the hash of the transaction
func (t *Transaction) ComputeHash(blockNumber uint64) *Transaction {
GetTransactionHashHandler(blockNumber).ComputeHash(t)
Expand Down