The "deepCopy" function from "StateObject" makes pointer copy instead of value copy

[summerpro]

• Here is a pointer copy of the "account" variable in "stateObject"

type stateObject struct {
	// DB error
	dbErr   error
	stateDB *CommitStateDB
	account *ethermint.EthAccount

}

func (so *stateObject) deepCopy(db *CommitStateDB) *stateObject {
	newStateObj := newStateObject(db, so.account)

	newStateObj.code = so.code
	newStateObj.dirtyStorage = so.dirtyStorage.Copy()
	newStateObj.originStorage = so.originStorage.Copy()
	newStateObj.suicided = so.suicided
	newStateObj.dirtyCode = so.dirtyCode
	newStateObj.deleted = so.deleted

	return newStateObj
}

func newStateObject(db *CommitStateDB, accProto authexported.Account) *stateObject {
	// func newStateObject(db *CommitStateDB, accProto authexported.Account, balance sdk.Int) *stateObject {
	ethermintAccount, ok := accProto.(*ethermint.EthAccount)
	if !ok {
		panic(fmt.Sprintf("invalid account type for state object: %T", accProto))
	}

	// set empty code hash
	if ethermintAccount.CodeHash == nil {
		ethermintAccount.CodeHash = emptyCodeHash
	}

	return &stateObject{
		stateDB:                 db,
		account:                 ethermintAccount,
		address:                 ethermintAccount.EthAddress(),
		originStorage:           Storage{},
		dirtyStorage:            Storage{},
		keyToOriginStorageIndex: make(map[ethcmn.Hash]int),
		keyToDirtyStorageIndex:  make(map[ethcmn.Hash]int),
	}
}

risk

• When the CommitStateDB is rolled back through the Copy function, dirty data is generated due to pointer copy

// NewHandler returns a handler for Ethermint type messages.
func NewHandler(k *Keeper) sdk.Handler {
	return func(ctx sdk.Context, msg sdk.Msg) (result *sdk.Result, err error) {
		snapshotStateDB := k.CommitStateDB.Copy()

		// The "recover" code here is used to solve the problem of dirty data
		// in CommitStateDB due to insufficient gas.

		// The following is a detailed description:
		// If the gas is insufficient during the execution of the "handler",
		// panic will be thrown from the function "ConsumeGas" and finally
		// caught by the function "runTx" from Cosmos. The function "runTx"
		// will think that the execution of Msg has failed and the modified
		// data in the Store will not take effect.

		// Stacktrace：runTx->runMsgs->handler->...->gaskv.Store.Set->ConsumeGas

		// The problem is that when the modified data in the Store does not take
		// effect, the data in the modified CommitStateDB is not rolled back,
		// they take effect, and dirty data is generated.
		// Therefore, the code here specifically deals with this situation.
		// See https://github.com/cosmos/ethermint/issues/668 for more information.
		defer func() {
			if r := recover(); r != nil {
				// We first used "k.CommitStateDB = snapshotStateDB" to roll back
				// CommitStateDB, but this can only change the CommitStateDB in the
				// current Keeper object, but the Keeper object will be destroyed
				// soon, it is not a global variable, so the content pointed to by
				// the CommitStateDB pointer can be modified to take effect.
				types.CopyCommitStateDB(snapshotStateDB, k.CommitStateDB)
				panic(r)
			}
		}()
		ctx = ctx.WithEventManager(sdk.NewEventManager())
		switch msg := msg.(type) {
		case types.MsgEthereumTx:
			result, err = handleMsgEthereumTx(ctx, k, msg)
		case types.MsgEthermint:
			result, err = handleMsgEthermint(ctx, k, msg)
		default:
			return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "unrecognized %s message type: %T", ModuleName, msg)
		}
		if err != nil {
			types.CopyCommitStateDB(snapshotStateDB, k.CommitStateDB)
		}
		return result, err
	}
}

[summerpro]

solution1

• Use MarshalJSON for deepCopy

func (so *stateObject) deepCopy(db *CommitStateDB) *stateObject {
	newAccount := ethermint.ProtoAccount().(*ethermint.EthAccount)
	jsonAccount, err := so.account.MarshalJSON()
	if err != nil {
		panic(err)
	}
	err = newAccount.UnmarshalJSON(jsonAccount)
	if err != nil {
		panic(err)
	}
	newStateObj := newStateObject(db, newAccount)

	newStateObj.code = so.code
	newStateObj.dirtyStorage = so.dirtyStorage.Copy()
	newStateObj.originStorage = so.originStorage.Copy()
	newStateObj.suicided = so.suicided
	newStateObj.dirtyCode = so.dirtyCode
	newStateObj.deleted = so.deleted

	return newStateObj
}

solution2

• use reflect for deepCopy of interface type

func (so *stateObject) deepCopy(db *CommitStateDB) *stateObject {
	pub := so.account.PubKey
	if reflect.TypeOf(so.account.PubKey).Kind() == reflect.Ptr {
		// Pointer:
		pub = reflect.New(reflect.ValueOf(pub).Elem().Type()).Interface().(crypto.PubKey)
	} else {
		// Not pointer:
		pub = reflect.New(reflect.TypeOf(pub)).Elem().Interface().(crypto.PubKey)
	}
	var newAccount types.EthAccount
	newAccount.PubKey = pub
	copy(newAccount.Address, so.account.Address)
	copy(newAccount.CodeHash, so.account.CodeHash)
	copy(newAccount.Coins, so.account.Coins)
	newAccount.AccountNumber = so.account.AccountNumber
	newAccount.Sequence = so.account.Sequence

	newStateObj := newStateObject(db, newAccount)

	newStateObj.code = so.code
	newStateObj.dirtyStorage = so.dirtyStorage.Copy()
	newStateObj.originStorage = so.originStorage.Copy()
	newStateObj.suicided = so.suicided
	newStateObj.dirtyCode = so.dirtyCode
	newStateObj.deleted = so.deleted

	return newStateObj
}

[github-actions]

This issue is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[github-actions]

This issue is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 7 days.