From 28e28f2a7ba2c0a1a0f40ae6bd0dcddad80fdcb4 Mon Sep 17 00:00:00 2001 From: noot <36753753+noot@users.noreply.github.com> Date: Mon, 22 Jun 2020 12:07:35 -0400 Subject: [PATCH] x/evm: fix EndBlock consensus failure (#334) * add test for sending tx w/ 21000 gas * improve rpc transfer test * use ctx in EndBlock * UpdateAccounts and ClearStateObjects with passed in context * log ethereum address on error Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Co-authored-by: Federico Kunze --- app/ante/eth.go | 7 ++++--- tests/rpc_test.go | 24 +++++++++++++++++++++++- x/evm/abci.go | 6 +++--- x/evm/handler_test.go | 25 ++++++++++++++++++++++--- x/evm/keeper/statedb.go | 10 ++++++++++ 5 files changed, 62 insertions(+), 10 deletions(-) diff --git a/app/ante/eth.go b/app/ante/eth.go index 4f1297abd..f5a18de94 100644 --- a/app/ante/eth.go +++ b/app/ante/eth.go @@ -14,6 +14,7 @@ import ( emint "github.com/cosmos/ethermint/types" evmtypes "github.com/cosmos/ethermint/x/evm/types" + "github.com/ethereum/go-ethereum/common" ethcore "github.com/ethereum/go-ethereum/core" ) @@ -180,7 +181,7 @@ func (avd AccountVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, s acc := avd.ak.GetAccount(ctx, address) if acc == nil { - return ctx, fmt.Errorf("account %s is nil", address) + return ctx, fmt.Errorf("account %s (%s) is nil", common.BytesToAddress(address.Bytes()), address) } // on InitChain make sure account number == 0 @@ -232,7 +233,7 @@ func (nvd NonceVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim acc := nvd.ak.GetAccount(ctx, address) if acc == nil { - return ctx, fmt.Errorf("account %s is nil", address) + return ctx, fmt.Errorf("account %s (%s) is nil", common.BytesToAddress(address.Bytes()), address) } seq := acc.GetSequence() @@ -287,7 +288,7 @@ func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula } if senderAcc == nil { - return ctx, fmt.Errorf("sender account %s is nil", address) + return ctx, fmt.Errorf("sender account %s (%s) is nil", common.BytesToAddress(address.Bytes()), address) } gasLimit := msgEthTx.GetGas() diff --git a/tests/rpc_test.go b/tests/rpc_test.go index 1c7845087..a215d8270 100644 --- a/tests/rpc_test.go +++ b/tests/rpc_test.go @@ -221,7 +221,29 @@ func getAddress(t *testing.T) []byte { return res[0] } -func TestEth_SendTransaction(t *testing.T) { +func TestEth_SendTransaction_Transfer(t *testing.T) { + from := getAddress(t) + + param := make([]map[string]string, 1) + param[0] = make(map[string]string) + param[0]["from"] = "0x" + fmt.Sprintf("%x", from) + param[0]["to"] = "0x0000000000000000000000000000000012341234" + param[0]["value"] = "0x16345785d8a0000" + param[0]["gasLimit"] = "0x5208" + param[0]["gasPrice"] = "0x55ae82600" + + rpcRes := call(t, "eth_sendTransaction", param) + + var hash hexutil.Bytes + err := json.Unmarshal(rpcRes.Result, &hash) + require.NoError(t, err) + + receipt := waitForReceipt(t, hash) + require.NotNil(t, receipt) + require.Equal(t, "0x1", receipt["status"].(string)) +} + +func TestEth_SendTransaction_ContractDeploy(t *testing.T) { from := getAddress(t) param := make([]map[string]string, 1) diff --git a/x/evm/abci.go b/x/evm/abci.go index 2b6f19d0c..626a199a3 100644 --- a/x/evm/abci.go +++ b/x/evm/abci.go @@ -31,16 +31,16 @@ func EndBlock(k Keeper, ctx sdk.Context, req abci.RequestEndBlock) []abci.Valida ctx = ctx.WithBlockGasMeter(sdk.NewInfiniteGasMeter()) // Update account balances before committing other parts of state - k.CommitStateDB.UpdateAccounts() + k.UpdateAccounts(ctx) // Commit state objects to KV store - _, err := k.CommitStateDB.WithContext(ctx).Commit(true) + _, err := k.Commit(ctx, true) if err != nil { panic(err) } // Clear accounts cache after account data has been committed - k.CommitStateDB.ClearStateObjects() + k.ClearStateObjects(ctx) bloom := ethtypes.BytesToBloom(k.Bloom.Bytes()) k.SetBlockBloom(ctx, ctx.BlockHeight(), bloom) diff --git a/x/evm/handler_test.go b/x/evm/handler_test.go index 5095689d1..5cd5c6e9b 100644 --- a/x/evm/handler_test.go +++ b/x/evm/handler_test.go @@ -1,6 +1,7 @@ package evm_test import ( + "crypto/ecdsa" "fmt" "math/big" "testing" @@ -10,6 +11,7 @@ import ( "github.com/ethereum/go-ethereum/common" ethcmn "github.com/ethereum/go-ethereum/common" + ethcrypto "github.com/ethereum/go-ethereum/crypto" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" @@ -258,9 +260,6 @@ func (suite *EvmTestSuite) TestQueryTxLogs() { err = tx.Sign(big.NewInt(3), priv.ToECDSA()) suite.Require().NoError(err) - // result, err := evm.HandleEthTxMsg(suite.ctx, suite.app.EvmKeeper, tx) - // suite.Require().NoError(err, "failed to handle eth tx msg") - result, err := suite.handler(suite.ctx, tx) suite.Require().NoError(err) suite.Require().NotNil(result) @@ -291,3 +290,23 @@ func (suite *EvmTestSuite) TestQueryTxLogs() { resultData.Logs[0].Data = []byte{} suite.Require().Equal(txLogs.Logs[0], resultData.Logs[0]) } + +func (suite *EvmTestSuite) TestSendTransaction() { + gasLimit := uint64(21000) + gasPrice := big.NewInt(1) + + priv, err := crypto.GenerateKey() + suite.Require().NoError(err, "failed to create key") + pub := priv.ToECDSA().Public().(*ecdsa.PublicKey) + + suite.app.EvmKeeper.SetBalance(suite.ctx, ethcrypto.PubkeyToAddress(*pub), big.NewInt(100)) + + // send simple value transfer with gasLimit=21000 + tx := types.NewMsgEthereumTx(1, ðcmn.Address{0x1}, big.NewInt(1), gasLimit, gasPrice, nil) + err = tx.Sign(big.NewInt(3), priv.ToECDSA()) + suite.Require().NoError(err) + + result, err := suite.handler(suite.ctx, tx) + suite.Require().NoError(err) + suite.Require().NotNil(result) +} diff --git a/x/evm/keeper/statedb.go b/x/evm/keeper/statedb.go index 8a7394bf1..d0f01742f 100644 --- a/x/evm/keeper/statedb.go +++ b/x/evm/keeper/statedb.go @@ -228,6 +228,16 @@ func (k *Keeper) CreateAccount(ctx sdk.Context, addr ethcmn.Address) { k.CommitStateDB.WithContext(ctx).CreateAccount(addr) } +// UpdateAccounts calls CommitStateDB.UpdateAccounts using the passed in context +func (k *Keeper) UpdateAccounts(ctx sdk.Context) { + k.CommitStateDB.WithContext(ctx).UpdateAccounts() +} + +// ClearStateObjects calls CommitStateDB.ClearStateObjects using the passed in context +func (k *Keeper) ClearStateObjects(ctx sdk.Context) { + k.CommitStateDB.WithContext(ctx).ClearStateObjects() +} + // Copy calls CommitStateDB.Copy using the passed in context func (k *Keeper) Copy(ctx sdk.Context) ethvm.StateDB { return k.CommitStateDB.WithContext(ctx).Copy()