Skip to content

Conversation

@smartprogrammer93
Copy link
Contributor

context here:
ethereum/EIPs#7308

Changes

  • disable eth burn on preexisting contract when inheritor equals contract address for cancun

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Documentation

Requires documentation update

  • Yes
  • No

If yes, link the PR to the docs update or the issue with the details labeled docs. Remove if not applicable.

Requires explanation in Release Notes

  • Yes
  • No

@smartprogrammer93 smartprogrammer93 marked this pull request as ready for review August 16, 2023 13:21
@smartprogrammer93 smartprogrammer93 merged commit 833415e into master Aug 17, 2023
@smartprogrammer93 smartprogrammer93 deleted the fix/apply_6780_clarification_no_burn_with_pre_existing branch August 17, 2023 08:38
Comment on lines +2369 to 2373
if (spec.SelfdestructOnlyOnSameTransaction && !createInSameTx && inheritor.Equals(executingAccount))
return true; // dont burn eth when contract is not destroyed per EIP clarification

_state.SubtractFromBalance(executingAccount, result, spec);
return true;
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't:

        if (!spec.SelfdestructOnlyOnSameTransaction || createInSameTx || !inheritor.Equals(executingAccount))
        {
            _state.SubtractFromBalance(executingAccount, result, spec);
        }

        return true;

be more readable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like to specify only the condition where the burn does not happen. I find it less confusing than specifying all the conditions that it does burn.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants