Skip to content

Commit

Permalink
core: fix pre-check for account balance under EIP-1559 (#23244)
Browse files Browse the repository at this point in the history
When processing a transaction with London fork rules, EIP-1559 mandates
checking that the sender must have sufficient balance to cover gas * gasFeeCap.

In the EIP's pseudocode, this check happens after the value transferred by the
transaction has already been deducted. However, in go-ethereum, the balance
has not yet been updated when the check happens, and therefore needs to be
added explicitly.

Co-authored-by: Martin Holst Swende <martin@swende.se>
  • Loading branch information
MariusVanDerWijden and holiman committed Jul 22, 2021
1 parent f05419f commit 97aacd9
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 2 deletions.
11 changes: 11 additions & 0 deletions cmd/evm/testdata/12/alloc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b" : {
"balance" : "84000000",
"code" : "0x",
"nonce" : "0x00",
"storage" : {
"0x00" : "0x00"
}
}
}

10 changes: 10 additions & 0 deletions cmd/evm/testdata/12/env.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"currentCoinbase" : "0x2adc25665018aa1fe0e6bc666dac8fc2697ff9ba",
"currentDifficulty" : "0x020000",
"currentNumber" : "0x01",
"currentTimestamp" : "0x03e8",
"previousHash" : "0xfda4419b3660e99f37e536dae1ab081c180136bb38c837a93e93d9aab58553b2",
"currentGasLimit" : "0x0f4240",
"currentBaseFee" : "0x20"
}

40 changes: 40 additions & 0 deletions cmd/evm/testdata/12/readme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
## Test 1559 balance + gasCap

This test contains an EIP-1559 consensus issue which happened on Ropsten, where
`geth` did not properly account for the value transfer while doing the check on `max_fee_per_gas * gas_limit`.

Before the issue was fixed, this invocation allowed the transaction to pass into a block:
```
dir=./testdata/12 && ./evm t8n --state.fork=London --input.alloc=$dir/alloc.json --input.txs=$dir/txs.json --input.env=$dir/env.json --output.alloc=stdout --output.result=stdout
```

With the fix applied, the result is:
```
dir=./testdata/12 && ./evm t8n --state.fork=London --input.alloc=$dir/alloc.json --input.txs=$dir/txs.json --input.env=$dir/env.json --output.alloc=stdout --output.result=stdout
INFO [07-21|19:03:50.276] rejected tx index=0 hash=ccc996..d83435 from=0xa94f5374Fce5edBC8E2a8697C15331677e6EbF0B error="insufficient funds for gas * price + value: address 0xa94f5374Fce5edBC8E2a8697C15331677e6EbF0B have 84000000 want 84000032"
INFO [07-21|19:03:50.276] Trie dumping started root=e05f81..6597a5
INFO [07-21|19:03:50.276] Trie dumping complete accounts=1 elapsed="39.549µs"
{
"alloc": {
"0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b": {
"balance": "0x501bd00"
}
},
"result": {
"stateRoot": "0xe05f81f8244a76503ceec6f88abfcd03047a612a1001217f37d30984536597a5",
"txRoot": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",
"receiptRoot": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",
"logsHash": "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347",
"logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
"receipts": [],
"rejected": [
{
"index": 0,
"error": "insufficient funds for gas * price + value: address 0xa94f5374Fce5edBC8E2a8697C15331677e6EbF0B have 84000000 want 84000032"
}
]
}
}
```

The transaction is rejected.
20 changes: 20 additions & 0 deletions cmd/evm/testdata/12/txs.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
[
{
"input" : "0x",
"gas" : "0x5208",
"nonce" : "0x0",
"to" : "0x1111111111111111111111111111111111111111",
"value" : "0x20",
"v" : "0x0",
"r" : "0x0",
"s" : "0x0",
"secretKey" : "0x45a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8",
"chainId" : "0x1",
"type" : "0x2",
"maxFeePerGas" : "0xfa0",
"maxPriorityFeePerGas" : "0x20",
"accessList" : [
]
}
]

2 changes: 1 addition & 1 deletion core/state_processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func TestStateProcessorErrors(t *testing.T) {
txs: []*types.Transaction{
makeTx(0, common.Address{}, big.NewInt(1000000000000000000), params.TxGas, big.NewInt(875000000), nil),
},
want: "could not apply tx 0 [0x98c796b470f7fcab40aaef5c965a602b0238e1034cce6fb73823042dd0638d74]: insufficient funds for transfer: address 0x71562b71999873DB5b286dF957af199Ec94617F7",
want: "could not apply tx 0 [0x98c796b470f7fcab40aaef5c965a602b0238e1034cce6fb73823042dd0638d74]: insufficient funds for gas * price + value: address 0x71562b71999873DB5b286dF957af199Ec94617F7 have 1000000000000000000 want 1000018375000000000",
},
{ // ErrInsufficientFunds
txs: []*types.Transaction{
Expand Down
1 change: 1 addition & 0 deletions core/state_transition.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ func (st *StateTransition) buyGas() error {
if st.gasFeeCap != nil {
balanceCheck = new(big.Int).SetUint64(st.msg.Gas())
balanceCheck = balanceCheck.Mul(balanceCheck, st.gasFeeCap)
balanceCheck.Add(balanceCheck, st.value)
}
if have, want := st.state.GetBalance(st.msg.From()), balanceCheck; have.Cmp(want) < 0 {
return fmt.Errorf("%w: address %v have %v want %v", ErrInsufficientFunds, st.msg.From().Hex(), have, want)
Expand Down
2 changes: 1 addition & 1 deletion eth/tracers/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ func TestOverriddenTraceCall(t *testing.T) {
config: &TraceCallConfig{
Tracer: &tracer,
},
expectErr: core.ErrInsufficientFundsForTransfer,
expectErr: core.ErrInsufficientFunds,
expect: nil,
},
// Successful simple contract call
Expand Down

0 comments on commit 97aacd9

Please sign in to comment.