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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions x/evm/state/balance.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ func (s *DBImpl) SubBalance(evmAddr common.Address, amt *big.Int, reason tracing
s.AddBalance(evmAddr, new(big.Int).Neg(amt), reason)
return
}
if s.HasSelfDestructed(evmAddr) {
// redirect coins to fee collector, since simply burning here would cause coin supply mismatch
evmAddr, _ = s.k.GetFeeCollectorAddress(s.ctx)
}

ctx := s.ctx

Expand Down Expand Up @@ -62,11 +58,6 @@ func (s *DBImpl) AddBalance(evmAddr common.Address, amt *big.Int, reason tracing
return
}

if s.HasSelfDestructed(evmAddr) {
// redirect coins to fee collector, since simply burning here would cause coin supply mismatch
evmAddr, _ = s.k.GetFeeCollectorAddress(s.ctx)
}

ctx := s.ctx
// this avoids emitting cosmos events for ephemeral bookkeeping transfers like send_native
if s.eventsSuppressed {
Expand Down Expand Up @@ -125,7 +116,7 @@ func (s *DBImpl) SetBalance(evmAddr common.Address, amt *big.Int, reason tracing
}

func (s *DBImpl) getSeiAddress(evmAddr common.Address) sdk.AccAddress {
if feeCollector, _ := s.k.GetFeeCollectorAddress(s.ctx); feeCollector == evmAddr {
if s.coinbaseEvmAddress.Cmp(evmAddr) == 0 {
return s.coinbaseAddress
}
return s.k.GetSeiAddressOrDefault(s.ctx, evmAddr)
Expand Down
20 changes: 19 additions & 1 deletion x/evm/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/tracing"
"github.com/sei-protocol/sei-chain/utils"
"github.com/sei-protocol/sei-chain/x/evm/types"
)

Expand Down Expand Up @@ -108,10 +109,27 @@
s.Snapshot()
}

func (s *DBImpl) handleResidualFundsInDestructedAccounts(st *TemporaryState) {
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)
}
Comment on lines +113 to +126

Check warning

Code scanning / CodeQL

Iteration over map Warning

Iteration over map may be a possible source of non-determinism
}

func (s *DBImpl) clearAccountStateIfDestructed(st *TemporaryState) {
for acc, status := range st.transientAccounts {
if !bytes.Equal(status, AccountDeleted) {
return
continue
}
s.clearAccountState(common.HexToAddress(acc))
}
Expand Down
6 changes: 6 additions & 0 deletions x/evm/state/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,17 @@ func TestSelfDestructAssociated(t *testing.T) {
require.Equal(t, big.NewInt(0), k.BankKeeper().GetBalance(ctx, seiAddr, k.GetBaseDenom(ctx)).Amount.BigInt())
require.True(t, statedb.HasSelfDestructed(evmAddr))
require.False(t, statedb.Created(evmAddr))
statedb.AddBalance(evmAddr, big.NewInt(1), tracing.BalanceChangeUnspecified)
require.Equal(t, big.NewInt(1), statedb.GetBalance(evmAddr))
statedb.Finalize()
require.Equal(t, common.Hash{}, statedb.GetState(evmAddr, key))
// association should also be removed
_, ok := k.GetSeiAddress(statedb.Ctx(), evmAddr)
require.False(t, ok)
// balance in destructed account should be cleared and transferred to coinbase
require.Equal(t, big.NewInt(0), statedb.GetBalance(evmAddr))
fc, _ := k.GetFeeCollectorAddress(statedb.Ctx())
require.Equal(t, big.NewInt(1), statedb.GetBalance(fc))
}

func TestSnapshot(t *testing.T) {
Expand Down
47 changes: 28 additions & 19 deletions x/evm/state/statedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ type DBImpl struct {
// a temporary address that collects fees for this particular transaction so that there is
// no single bottleneck for fee collection. Its account state and balance will be deleted
// before the block commits
coinbaseAddress sdk.AccAddress
coinbaseAddress sdk.AccAddress
coinbaseEvmAddress common.Address

k EVMKeeper
simulation bool
Expand All @@ -38,13 +39,15 @@ type DBImpl struct {
}

func NewDBImpl(ctx sdk.Context, k EVMKeeper, simulation bool) *DBImpl {
feeCollector, _ := k.GetFeeCollectorAddress(ctx)
s := &DBImpl{
ctx: ctx,
k: k,
snapshottedCtxs: []sdk.Context{},
coinbaseAddress: GetCoinbaseAddress(ctx.TxIndex()),
simulation: simulation,
tempStateCurrent: NewTemporaryState(),
ctx: ctx,
k: k,
snapshottedCtxs: []sdk.Context{},
coinbaseAddress: GetCoinbaseAddress(ctx.TxIndex()),
simulation: simulation,
tempStateCurrent: NewTemporaryState(),
coinbaseEvmAddress: feeCollector,
}
s.Snapshot() // take an initial snapshot for GetCommitted
return s
Expand Down Expand Up @@ -82,6 +85,14 @@ func (s *DBImpl) Finalize() (surplus sdk.Int, err error) {
return
}

// delete state of self-destructed accounts
s.handleResidualFundsInDestructedAccounts(s.tempStateCurrent)
s.clearAccountStateIfDestructed(s.tempStateCurrent)
for _, ts := range s.tempStatesHist {
s.handleResidualFundsInDestructedAccounts(ts)
s.clearAccountStateIfDestructed(ts)
}

// remove transient states
// write cache to underlying
s.flushCtx(s.ctx)
Expand All @@ -90,12 +101,9 @@ func (s *DBImpl) Finalize() (surplus sdk.Int, err error) {
s.flushCtx(s.snapshottedCtxs[i])
}

// delete state of self-destructed accoutns
s.clearAccountStateIfDestructed(s.tempStateCurrent)
surplus = s.tempStateCurrent.surplus
for _, ts := range s.tempStatesHist {
surplus = surplus.Add(ts.surplus)
s.clearAccountStateIfDestructed(ts)
}
if surplus.IsNegative() {
err = fmt.Errorf("negative surplus value: %s", surplus.String())
Expand All @@ -119,15 +127,16 @@ func (s *DBImpl) GetStorageRoot(common.Address) common.Hash {
func (s *DBImpl) Copy() vm.StateDB {
newCtx := s.ctx.WithMultiStore(s.ctx.MultiStore().CacheMultiStore())
return &DBImpl{
ctx: newCtx,
snapshottedCtxs: append(s.snapshottedCtxs, s.ctx),
tempStateCurrent: NewTemporaryState(),
tempStatesHist: append(s.tempStatesHist, s.tempStateCurrent),
k: s.k,
coinbaseAddress: s.coinbaseAddress,
simulation: s.simulation,
err: s.err,
logger: s.logger,
ctx: newCtx,
snapshottedCtxs: append(s.snapshottedCtxs, s.ctx),
tempStateCurrent: NewTemporaryState(),
tempStatesHist: append(s.tempStatesHist, s.tempStateCurrent),
k: s.k,
coinbaseAddress: s.coinbaseAddress,
coinbaseEvmAddress: s.coinbaseEvmAddress,
simulation: s.simulation,
err: s.err,
logger: s.logger,
}
}

Expand Down
Loading