-
Notifications
You must be signed in to change notification settings - Fork 365
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
Cancun self destruct changes (EIP 6780) #281
base: master
Are you sure you want to change the base?
Cancun self destruct changes (EIP 6780) #281
Conversation
interpreter/src/runtime.rs
Outdated
@@ -17,6 +17,8 @@ pub struct RuntimeState { | |||
pub transaction_context: Rc<TransactionContext>, | |||
/// Return data buffer. | |||
pub retbuf: Vec<u8>, | |||
/// EVM Fork. | |||
pub fork: Fork, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any way we can do without this? It honestly breaks a core premise of rust-evm
to support custom impls..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can probably just put the check in handler.mark_delete
. When created(address)
is false, mark_delete
just does nothing.
Let me know if this works or if something is not fully compatible with EIP-6780.
Otherwise, the way to do it is to add another methods in handler that properly generalize this different behavior.
The handler can have access to fork config, but the core interpreter shouldn't. Mainnet is not the only thing we have to support..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll need to merge handler.mark_delete
and handler.reset_balance
to the same function. Perhaps mark_delete_reset
. I briefly checked the EIP. I think this should work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the review and apologies for the delayed response. I believe your proposal should work and does not conflict with the EIP. I will apply the suggested changes and see how it goes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sorpaas I applied the suggested changes, please let met now if this works for you.
This PR implements the new changes for the
selfdestruct
function as outlined in EIP-6780.The new EIP allows for contract deletion only within the same transaction that created it. Otherwise, the
selfdestruct
function will simply transfer all Ether in the contract account to the target address.Another change introduced by this EIP is that when
selfdestruct
is not called in the same transaction that created the contract, if the target address is the same as the contract address, there is no net change in balances. Unlike the previous specification, Ether will not be burned in this case.