Skip to content

Commit

Permalink
Fix balance behavior for selfdestructed accounts (#1526)
Browse files Browse the repository at this point in the history
Emit event for contract registration (#1518)
  • Loading branch information
codchen authored and udpatil committed Apr 17, 2024
1 parent a480088 commit 9c5f643
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 30 deletions.
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 @@ import (
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 @@ func (s *DBImpl) RevertToSnapshot(rev int) {
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)
}
}

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

0 comments on commit 9c5f643

Please sign in to comment.