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

Fix balance behavior for selfdestructed accounts #1526

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

codchen
Copy link
Collaborator

@codchen codchen commented Apr 10, 2024

Describe your changes and provide context

Instead of ignoring balance changes to self-destructed accounts in the same block, we want to keep track of the balance changes (just in case the account is recreated in the same block) and burn any remaining tokens at the end of the transaction

Testing performed to validate your change

unit test

@codchen codchen force-pushed the selfdestruct-balance-behavior branch from 129b158 to 28fee50 Compare April 10, 2024 02:25
@codchen codchen changed the title Emit event for contract registration (#1518) Fix balance behavior for selfdestructed accounts Apr 10, 2024
@codchen codchen requested a review from Kbhat1 April 10, 2024 02:27
Comment on lines +113 to +126
for a, status := range st.transientAccounts {
if !bytes.Equal(status, AccountDeleted) {
continue
}
acc := common.HexToAddress(a)
residual := s.GetBalance(acc)
if residual.Cmp(utils.Big0) == 0 {
continue
}
s.SubBalance(acc, residual, tracing.BalanceDecreaseSelfdestructBurn)
// we don't want to really "burn" the token since it will mess up
// total supply calculation, so we send them to fee collector instead
s.AddBalance(s.coinbaseEvmAddress, residual, tracing.BalanceDecreaseSelfdestructBurn)
}

Check warning

Code scanning / CodeQL

Iteration over map Warning

Iteration over map may be a possible source of non-determinism
@codecov-commenter
Copy link

codecov-commenter commented Apr 10, 2024

Codecov Report

Attention: Patch coverage is 73.68421% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 60.44%. Comparing base (37251b2) to head (6fc9540).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##            seiv2    #1526      +/-   ##
==========================================
+ Coverage   60.12%   60.44%   +0.32%     
==========================================
  Files         365      365              
  Lines       25833    25843      +10     
==========================================
+ Hits        15532    15621      +89     
+ Misses       9314     9233      -81     
- Partials      987      989       +2     
Files Coverage Δ
x/evm/state/balance.go 69.89% <100.00%> (+8.50%) ⬆️
x/evm/state/state.go 95.45% <100.00%> (+0.41%) ⬆️
x/evm/state/statedb.go 52.57% <58.33%> (+2.57%) ⬆️

... and 2 files with indirect coverage changes

@codchen codchen force-pushed the selfdestruct-balance-behavior branch from 28fee50 to a6208c9 Compare April 10, 2024 04:07
@codchen codchen force-pushed the selfdestruct-balance-behavior branch from a6208c9 to 82a5630 Compare April 10, 2024 14:47
@codchen codchen force-pushed the selfdestruct-balance-behavior branch from 82a5630 to 6fc9540 Compare April 11, 2024 03:32
@codchen codchen merged commit e2514a8 into seiv2 Apr 11, 2024
44 checks passed
@codchen codchen deleted the selfdestruct-balance-behavior branch April 11, 2024 13:22
udpatil pushed a commit that referenced this pull request Apr 17, 2024
udpatil pushed a commit that referenced this pull request Apr 17, 2024
udpatil pushed a commit that referenced this pull request Apr 17, 2024
udpatil pushed a commit that referenced this pull request Apr 18, 2024
udpatil pushed a commit that referenced this pull request Apr 19, 2024
udpatil pushed a commit that referenced this pull request Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants