core: fix some incorrect EIP-6780 selfdestruct tracer behavior #32919
Conversation
jwasinger
changed the title
core/state/statedb.go
Outdated
|
|
||
| // ExistBeforeCurTx returns true if a contract exists and was not created | ||
| // in the current transaction. | ||
| func (s *StateDB) ExistBeforeCurTx(addr common.Address) bool { |
There was a problem hiding this comment.
I will rename this to reflect that the method returns true if the account exists and it is a contract.
| evm.StateDB.AddBalance(beneficiary.Bytes20(), balance, tracing.BalanceIncreaseSelfdestruct) | ||
| createdInTx := !evm.StateDB.ExistBeforeCurTx(scope.Contract.Address()) | ||
|
|
||
| if createdInTx { |
There was a problem hiding this comment.
I don't understand this section. Why is the balance burnt immediately and not at the tx end in case contract is new?
There was a problem hiding this comment.
It's part of the spec for 6780.
There was a problem hiding this comment.
We have a general case in statedb.Finalize: if the contract being destroyed has balance > 0, that balance is burnt. Does that not cover this?
There was a problem hiding this comment.
Basically: I think this part was fine before and the createdInTx is not strictly needed. Let me know if I'm missing something.
| s.hooks.OnBalanceChange(addr, bal.ToBig(), new(big.Int), tracing.BalanceDecreaseSelfdestructBurn) | ||
| } | ||
| } | ||
| if s.hooks.OnNonceChangeV2 != nil { |
There was a problem hiding this comment.
Please also add OnNonceChange (v1) even tho it doesn't take in a reason.
|
I've now gone through the PR, quite a few good fixes there. Left a comment for the section where I couldn't follow the logic, why we change it. |
2 participants
BalanceIncreaseSelfdestruct(transfer to recipient of selfdestruct) was duplicated in the hooked statedb and in the opcode handler for the selfdestruct opcode.