Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: fix pre-check for account balance under EIP-1559 #23244

Merged
merged 6 commits into from
Jul 22, 2021
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
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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI this change makes this comment stale

// 2. caller has enough balance to cover transaction fee(gaslimit * gasprice)

}
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