Bugfix: Incorrect processing of self-destructed, new contract

Fixes #868 "Gas usage consensus error at Mainnet block 6001128", and equivalent
on other networks.  Mainnet sync is able to continue past 6001128 after this.

Here's a trace:

```
TRC 2021-09-29 15:13:21.532+01:00 Persisting blocks                  file=persist_blocks.nim:43 fromBlock=6000961 toBlock=6001152
...
DBG 2021-09-29 15:14:35.925+01:00 gasUsed neq cumulativeGasUsed      file=process_block.nim:68 gasUsed=7999726 cumulativeGasUsed=7989726
TRC 2021-09-29 15:14:35.925+01:00 peer disconnected                  file=blockchain_sync.nim:407 peer=<PEER:IP>
```

Similar output is seen at many blocks in the range 6001128..6001204.

The bug is when handling a combination of `CREATE` or `CREATE2`, along with
`SELFDESTRUCT` applied to the new contract address.

Init code for a contract can't return non-empty code and do `SELFDESTRUCT` at
the same time, because `SELFDESTRUCT` returns empty data.

But it is possible to return non-empty code in a newly created, self-destructed
account if the init code calls `DELEGATECALL` or `CALLCODE` to other code which
uses `SELFDESTRUCT`.

In this case we must still charge gas and write the code.  This shows on
Mainnet blocks 6001128..6001204, where the gas difference matters.  The code
must be written because the new code can be called later in the transaction
too, before self-destruction wipes the account at the end.

There are actually three semantic changes here for a self-destructed, new
contract:

- Gas is charged.
- The code is written to the account.
- It can fail due to insufficient gas.

This patch almost exactly reverts a15805e4 "fix applyCreateMessage" from
2019-02-28.  I wonder what that fixed.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
This commit is contained in:
Jamie Lokier 2021-10-19 12:52:01 +01:00
parent 242dfdd5ac
commit 5a5edb392a
2 changed files with 14 additions and 6 deletions

View File

@ -241,15 +241,19 @@ proc writeContract*(c: Computation, fork: Fork): bool {.gcsafe.} =
debug "Contract code can't start with 0xEF byte"
return false
let storageAddr = c.msg.contractAddress
if c.isSelfDestructed(storageAddr): return
# Charge gas and write the code even if the code address is self-destructed.
# Non-empty code in a newly created, self-destructed account is possible if
# the init code calls `DELEGATECALL` or `CALLCODE` to other code which uses
# `SELFDESTRUCT`. This shows on Mainnet blocks 6001128..6001204, where the
# gas difference matters. The new code can be called later in the
# transaction too, before self-destruction wipes the account at the end.
let gasParams = GasParams(kind: Create, cr_memLength: contractCode.len)
let codeCost = c.gasCosts[Create].c_handler(0.u256, gasParams).gasCost
if c.gasMeter.gasRemaining >= codeCost:
c.gasMeter.consumeGas(codeCost, reason = "Write contract code for CREATE")
c.vmState.mutateStateDb:
db.setCode(storageAddr, contractCode)
db.setCode(c.msg.contractAddress, contractCode)
result = true
else:
if fork < FkHomestead or fork >= FkByzantium: c.output = @[]

View File

@ -178,15 +178,19 @@ proc writeContract*(c: Computation, fork: Fork): bool {.gcsafe.} =
debug "Contract code can't start with 0xEF byte"
return false
let storageAddr = c.msg.contractAddress
if c.isSelfDestructed(storageAddr): return
# Charge gas and write the code even if the code address is self-destructed.
# Non-empty code in a newly created, self-destructed account is possible if
# the init code calls `DELEGATECALL` or `CALLCODE` to other code which uses
# `SELFDESTRUCT`. This shows on Mainnet blocks 6001128..6001204, where the
# gas difference matters. The new code can be called later in the
# transaction too, before self-destruction wipes the account at the end.
let gasParams = GasParams(kind: Create, cr_memLength: contractCode.len)
let codeCost = c.gasCosts[Create].c_handler(0.u256, gasParams).gasCost
if c.gasMeter.gasRemaining >= codeCost:
c.gasMeter.consumeGas(codeCost, reason = "Write contract code for CREATE")
c.vmState.mutateStateDb:
db.setCode(storageAddr, contractCode)
db.setCode(c.msg.contractAddress, contractCode)
result = true
else:
if fork < FkHomestead or FkByzantium <= fork: