From 97aacd9b359035d6cfeebf186f4420ccc19c7ecc Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Thu, 22 Jul 2021 15:39:40 +0200 Subject: [PATCH] core: fix pre-check for account balance under EIP-1559 (#23244) 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 --- cmd/evm/testdata/12/alloc.json | 11 ++++++++++ cmd/evm/testdata/12/env.json | 10 +++++++++ cmd/evm/testdata/12/readme.md | 40 ++++++++++++++++++++++++++++++++++ cmd/evm/testdata/12/txs.json | 20 +++++++++++++++++ core/state_processor_test.go | 2 +- core/state_transition.go | 1 + eth/tracers/api_test.go | 2 +- 7 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 cmd/evm/testdata/12/alloc.json create mode 100644 cmd/evm/testdata/12/env.json create mode 100644 cmd/evm/testdata/12/readme.md create mode 100644 cmd/evm/testdata/12/txs.json diff --git a/cmd/evm/testdata/12/alloc.json b/cmd/evm/testdata/12/alloc.json new file mode 100644 index 0000000000..3ed96894fb --- /dev/null +++ b/cmd/evm/testdata/12/alloc.json @@ -0,0 +1,11 @@ +{ + "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b" : { + "balance" : "84000000", + "code" : "0x", + "nonce" : "0x00", + "storage" : { + "0x00" : "0x00" + } + } +} + diff --git a/cmd/evm/testdata/12/env.json b/cmd/evm/testdata/12/env.json new file mode 100644 index 0000000000..8ae5465369 --- /dev/null +++ b/cmd/evm/testdata/12/env.json @@ -0,0 +1,10 @@ +{ + "currentCoinbase" : "0x2adc25665018aa1fe0e6bc666dac8fc2697ff9ba", + "currentDifficulty" : "0x020000", + "currentNumber" : "0x01", + "currentTimestamp" : "0x03e8", + "previousHash" : "0xfda4419b3660e99f37e536dae1ab081c180136bb38c837a93e93d9aab58553b2", + "currentGasLimit" : "0x0f4240", + "currentBaseFee" : "0x20" +} + diff --git a/cmd/evm/testdata/12/readme.md b/cmd/evm/testdata/12/readme.md new file mode 100644 index 0000000000..b0177ecc24 --- /dev/null +++ b/cmd/evm/testdata/12/readme.md @@ -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. \ No newline at end of file diff --git a/cmd/evm/testdata/12/txs.json b/cmd/evm/testdata/12/txs.json new file mode 100644 index 0000000000..cd683f271c --- /dev/null +++ b/cmd/evm/testdata/12/txs.json @@ -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" : [ + ] + } +] + diff --git a/core/state_processor_test.go b/core/state_processor_test.go index a57663914a..ce6d2bcdde 100644 --- a/core/state_processor_test.go +++ b/core/state_processor_test.go @@ -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{ diff --git a/core/state_transition.go b/core/state_transition.go index fec50028c6..b5b0ae990a 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -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) diff --git a/eth/tracers/api_test.go b/eth/tracers/api_test.go index dd9b61a1ae..4a4e5bbc33 100644 --- a/eth/tracers/api_test.go +++ b/eth/tracers/api_test.go @@ -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