Skip to content

Commit

Permalink
estimateGas fix for simple value transactions and contract creations (
Browse files Browse the repository at this point in the history
  • Loading branch information
goran-ethernal committed Oct 19, 2023
1 parent 73469c6 commit 5515483
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 18 deletions.
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

0 comments on commit 5515483

Please sign in to comment.