evm: roll back CommitStateDB after failing to execute handler (#677)
* Roll back CommitStateDB after failing to execute handler in evm module * add function CopyCommitStateDB * add comment * add comment * Add ut about the dirty data generated by CommitStateDB * format code
This commit is contained in:
parent
ed313c9482
commit
153478eb69
@ -14,16 +14,50 @@ import (
|
||||
|
||||
// NewHandler returns a handler for Ethermint type messages.
|
||||
func NewHandler(k *Keeper) sdk.Handler {
|
||||
return func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
|
||||
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:
|
||||
return handleMsgEthereumTx(ctx, k, msg)
|
||||
result, err = handleMsgEthereumTx(ctx, k, msg)
|
||||
case types.MsgEthermint:
|
||||
return handleMsgEthermint(ctx, k, msg)
|
||||
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
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -2,6 +2,7 @@ package evm_test
|
||||
|
||||
import (
|
||||
"crypto/ecdsa"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"math/big"
|
||||
"strings"
|
||||
@ -415,3 +416,112 @@ func (suite *EvmTestSuite) TestSendTransaction() {
|
||||
suite.Require().NoError(err)
|
||||
suite.Require().NotNil(result)
|
||||
}
|
||||
|
||||
func (suite *EvmTestSuite) TestOutOfGasWhenDeployContract() {
|
||||
// Test contract:
|
||||
//http://remix.ethereum.org/#optimize=false&evmVersion=istanbul&version=soljson-v0.5.15+commit.6a57276f.js
|
||||
//2_Owner.sol
|
||||
//
|
||||
//pragma solidity >=0.4.22 <0.7.0;
|
||||
//
|
||||
///**
|
||||
// * @title Owner
|
||||
// * @dev Set & change owner
|
||||
// */
|
||||
//contract Owner {
|
||||
//
|
||||
// address private owner;
|
||||
//
|
||||
// // event for EVM logging
|
||||
// event OwnerSet(address indexed oldOwner, address indexed newOwner);
|
||||
//
|
||||
// // modifier to check if caller is owner
|
||||
// modifier isOwner() {
|
||||
// // If the first argument of 'require' evaluates to 'false', execution terminates and all
|
||||
// // changes to the state and to Ether balances are reverted.
|
||||
// // This used to consume all gas in old EVM versions, but not anymore.
|
||||
// // It is often a good idea to use 'require' to check if functions are called correctly.
|
||||
// // As a second argument, you can also provide an explanation about what went wrong.
|
||||
// require(msg.sender == owner, "Caller is not owner");
|
||||
// _;
|
||||
//}
|
||||
//
|
||||
// /**
|
||||
// * @dev Set contract deployer as owner
|
||||
// */
|
||||
// constructor() public {
|
||||
// owner = msg.sender; // 'msg.sender' is sender of current call, contract deployer for a constructor
|
||||
// emit OwnerSet(address(0), owner);
|
||||
//}
|
||||
//
|
||||
// /**
|
||||
// * @dev Change owner
|
||||
// * @param newOwner address of new owner
|
||||
// */
|
||||
// function changeOwner(address newOwner) public isOwner {
|
||||
// emit OwnerSet(owner, newOwner);
|
||||
// owner = newOwner;
|
||||
//}
|
||||
//
|
||||
// /**
|
||||
// * @dev Return owner address
|
||||
// * @return address of owner
|
||||
// */
|
||||
// function getOwner() external view returns (address) {
|
||||
// return owner;
|
||||
//}
|
||||
//}
|
||||
|
||||
// Deploy contract - Owner.sol
|
||||
gasLimit := uint64(1)
|
||||
suite.ctx = suite.ctx.WithGasMeter(sdk.NewGasMeter(gasLimit))
|
||||
gasPrice := big.NewInt(10000)
|
||||
|
||||
priv, err := ethsecp256k1.GenerateKey()
|
||||
suite.Require().NoError(err, "failed to create key")
|
||||
|
||||
bytecode := common.FromHex("0x608060405234801561001057600080fd5b50336000806101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff1602179055506000809054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16600073ffffffffffffffffffffffffffffffffffffffff167f342827c97908e5e2f71151c08502a66d44b6f758e3ac2f1de95f02eb95f0a73560405160405180910390a36102c4806100dc6000396000f3fe608060405234801561001057600080fd5b5060043610610053576000357c010000000000000000000000000000000000000000000000000000000090048063893d20e814610058578063a6f9dae1146100a2575b600080fd5b6100606100e6565b604051808273ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16815260200191505060405180910390f35b6100e4600480360360208110156100b857600080fd5b81019080803573ffffffffffffffffffffffffffffffffffffffff16906020019092919050505061010f565b005b60008060009054906101000a900473ffffffffffffffffffffffffffffffffffffffff16905090565b6000809054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff163373ffffffffffffffffffffffffffffffffffffffff16146101d1576040517f08c379a00000000000000000000000000000000000000000000000000000000081526004018080602001828103825260138152602001807f43616c6c6572206973206e6f74206f776e65720000000000000000000000000081525060200191505060405180910390fd5b8073ffffffffffffffffffffffffffffffffffffffff166000809054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff167f342827c97908e5e2f71151c08502a66d44b6f758e3ac2f1de95f02eb95f0a73560405160405180910390a3806000806101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff1602179055505056fea265627a7a72315820f397f2733a89198bc7fed0764083694c5b828791f39ebcbc9e414bccef14b48064736f6c63430005100032")
|
||||
tx := types.NewMsgEthereumTx(1, nil, big.NewInt(0), gasLimit, gasPrice, bytecode)
|
||||
tx.Sign(big.NewInt(3), priv.ToECDSA())
|
||||
suite.Require().NoError(err)
|
||||
|
||||
snapshotCommitStateDBJson, err := json.Marshal(suite.app.EvmKeeper.CommitStateDB)
|
||||
suite.Require().Nil(err)
|
||||
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
currentCommitStateDBJson, err := json.Marshal(suite.app.EvmKeeper.CommitStateDB)
|
||||
suite.Require().Nil(err)
|
||||
suite.Require().Equal(snapshotCommitStateDBJson, currentCommitStateDBJson)
|
||||
} else {
|
||||
suite.Require().Fail("panic did not happen")
|
||||
}
|
||||
}()
|
||||
|
||||
suite.handler(suite.ctx, tx)
|
||||
suite.Require().Fail("panic did not happen")
|
||||
}
|
||||
|
||||
func (suite *EvmTestSuite) TestErrorWhenDeployContract() {
|
||||
gasLimit := uint64(1000000)
|
||||
gasPrice := big.NewInt(10000)
|
||||
|
||||
priv, err := ethsecp256k1.GenerateKey()
|
||||
suite.Require().NoError(err, "failed to create key")
|
||||
|
||||
bytecode := common.FromHex("0xa6f9dae10000000000000000000000006a82e4a67715c8412a9114fbd2cbaefbc8181424")
|
||||
|
||||
tx := types.NewMsgEthereumTx(1, nil, big.NewInt(0), gasLimit, gasPrice, bytecode)
|
||||
tx.Sign(big.NewInt(3), priv.ToECDSA())
|
||||
suite.Require().NoError(err)
|
||||
|
||||
snapshotCommitStateDBJson, err := json.Marshal(suite.app.EvmKeeper.CommitStateDB)
|
||||
suite.Require().Nil(err)
|
||||
|
||||
_, sdkErr := suite.handler(suite.ctx, tx)
|
||||
suite.Require().NotNil(sdkErr)
|
||||
|
||||
currentCommitStateDBJson, err := json.Marshal(suite.app.EvmKeeper.CommitStateDB)
|
||||
suite.Require().Nil(err)
|
||||
suite.Require().Equal(snapshotCommitStateDBJson, currentCommitStateDBJson)
|
||||
}
|
||||
|
@ -760,59 +760,63 @@ func (csdb *CommitStateDB) CreateAccount(addr ethcmn.Address) {
|
||||
//
|
||||
// NOTE: Snapshots of the copied state cannot be applied to the copy.
|
||||
func (csdb *CommitStateDB) Copy() *CommitStateDB {
|
||||
csdb.lock.Lock()
|
||||
defer csdb.lock.Unlock()
|
||||
|
||||
// copy all the basic fields, initialize the memory ones
|
||||
state := &CommitStateDB{
|
||||
ctx: csdb.ctx,
|
||||
storeKey: csdb.storeKey,
|
||||
paramSpace: csdb.paramSpace,
|
||||
accountKeeper: csdb.accountKeeper,
|
||||
stateObjects: []stateEntry{},
|
||||
addressToObjectIndex: make(map[ethcmn.Address]int),
|
||||
stateObjectsDirty: make(map[ethcmn.Address]struct{}),
|
||||
refund: csdb.refund,
|
||||
logSize: csdb.logSize,
|
||||
preimages: make([]preimageEntry, len(csdb.preimages)),
|
||||
hashToPreimageIndex: make(map[ethcmn.Hash]int, len(csdb.hashToPreimageIndex)),
|
||||
journal: newJournal(),
|
||||
}
|
||||
state := &CommitStateDB{}
|
||||
CopyCommitStateDB(csdb, state)
|
||||
|
||||
return state
|
||||
}
|
||||
|
||||
func CopyCommitStateDB(from, to *CommitStateDB) {
|
||||
from.lock.Lock()
|
||||
defer from.lock.Unlock()
|
||||
|
||||
to.ctx = from.ctx
|
||||
to.storeKey = from.storeKey
|
||||
to.paramSpace = from.paramSpace
|
||||
to.accountKeeper = from.accountKeeper
|
||||
to.stateObjects = []stateEntry{}
|
||||
to.addressToObjectIndex = make(map[ethcmn.Address]int)
|
||||
to.stateObjectsDirty = make(map[ethcmn.Address]struct{})
|
||||
to.refund = from.refund
|
||||
to.logSize = from.logSize
|
||||
to.preimages = make([]preimageEntry, len(from.preimages))
|
||||
to.hashToPreimageIndex = make(map[ethcmn.Hash]int, len(from.hashToPreimageIndex))
|
||||
to.journal = newJournal()
|
||||
|
||||
// copy the dirty states, logs, and preimages
|
||||
for _, dirty := range csdb.journal.dirties {
|
||||
for _, dirty := range from.journal.dirties {
|
||||
// There is a case where an object is in the journal but not in the
|
||||
// stateObjects: OOG after touch on ripeMD prior to Byzantium. Thus, we
|
||||
// need to check for nil.
|
||||
//
|
||||
// Ref: https://github.com/ethereum/go-ethereum/pull/16485#issuecomment-380438527
|
||||
if idx, exist := csdb.addressToObjectIndex[dirty.address]; exist {
|
||||
state.stateObjects = append(state.stateObjects, stateEntry{
|
||||
if idx, exist := from.addressToObjectIndex[dirty.address]; exist {
|
||||
to.stateObjects = append(to.stateObjects, stateEntry{
|
||||
address: dirty.address,
|
||||
stateObject: csdb.stateObjects[idx].stateObject.deepCopy(state),
|
||||
stateObject: from.stateObjects[idx].stateObject.deepCopy(to),
|
||||
})
|
||||
state.addressToObjectIndex[dirty.address] = len(state.stateObjects) - 1
|
||||
state.stateObjectsDirty[dirty.address] = struct{}{}
|
||||
to.addressToObjectIndex[dirty.address] = len(to.stateObjects) - 1
|
||||
to.stateObjectsDirty[dirty.address] = struct{}{}
|
||||
}
|
||||
}
|
||||
|
||||
// Above, we don't copy the actual journal. This means that if the copy is
|
||||
// copied, the loop above will be a no-op, since the copy's journal is empty.
|
||||
// Thus, here we iterate over stateObjects, to enable copies of copies.
|
||||
for addr := range csdb.stateObjectsDirty {
|
||||
if idx, exist := state.addressToObjectIndex[addr]; !exist {
|
||||
state.setStateObject(csdb.stateObjects[idx].stateObject.deepCopy(state))
|
||||
state.stateObjectsDirty[addr] = struct{}{}
|
||||
for addr := range from.stateObjectsDirty {
|
||||
if idx, exist := to.addressToObjectIndex[addr]; !exist {
|
||||
to.setStateObject(from.stateObjects[idx].stateObject.deepCopy(to))
|
||||
to.stateObjectsDirty[addr] = struct{}{}
|
||||
}
|
||||
}
|
||||
|
||||
// copy pre-images
|
||||
for i, preimageEntry := range csdb.preimages {
|
||||
state.preimages[i] = preimageEntry
|
||||
state.hashToPreimageIndex[preimageEntry.hash] = i
|
||||
for i, preimageEntry := range from.preimages {
|
||||
to.preimages[i] = preimageEntry
|
||||
to.hashToPreimageIndex[preimageEntry.hash] = i
|
||||
}
|
||||
|
||||
return state
|
||||
}
|
||||
|
||||
// ForEachStorage iterates over each storage items, all invoke the provided
|
||||
|
Loading…
Reference in New Issue
Block a user