evm: update msgs to accept zero gas price (#299)

* update keeper to accept gas price=0; default pending to latest

* cleanup

* more cleanup

* more cleanup

* more cleanup

* more cleanup

* ante: copy IncrementSequenceDecorator from SDK's AnteHandler

* improve bloom test

* lint

* update msg error

Co-authored-by: Federico Kunze <federico.kunze94@gmail.com>
Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

app/ante/ante.go

@@ -43,7 +43,9 @@ func NewAnteHandler(ak auth.AccountKeeper, bk bank.Keeper, sk types.SupplyKeeper
 				authante.NewDeductFeeDecorator(ak, sk),
 				authante.NewSigGasConsumeDecorator(ak, sigGasConsumer),
 				authante.NewSigVerificationDecorator(ak),
-				authante.NewIncrementSequenceDecorator(ak), // innermost AnteDecorator
+				// TODO: remove once SDK is updated to v0.39.
+				// This fixes an issue that account sequence wasn't being updated on CheckTx.
+				NewIncrementSequenceDecorator(ak), // innermost AnteDecorator
 			)
 
 		case evmtypes.MsgEthereumTx:
@@ -80,3 +82,46 @@ func sigGasConsumer(
 		return sdkerrors.Wrapf(sdkerrors.ErrInvalidPubKey, "unrecognized public key type: %T", pubkey)
 	}
 }
+
+// IncrementSequenceDecorator handles incrementing sequences of all signers.
+// Use the IncrementSequenceDecorator decorator to prevent replay attacks. Note,
+// there is no need to execute IncrementSequenceDecorator on RecheckTX since
+// CheckTx would already bump the sequence number.
+//
+// NOTE: Since CheckTx and DeliverTx state are managed separately, subsequent and
+// sequential txs orginating from the same account cannot be handled correctly in
+// a reliable way unless sequence numbers are managed and tracked manually by a
+// client. It is recommended to instead use multiple messages in a tx.
+type IncrementSequenceDecorator struct {
+	ak auth.AccountKeeper
+}
+
+func NewIncrementSequenceDecorator(ak auth.AccountKeeper) IncrementSequenceDecorator {
+	return IncrementSequenceDecorator{
+		ak: ak,
+	}
+}
+
+func (isd IncrementSequenceDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) {
+	// no need to increment sequence on RecheckTx
+	if ctx.IsReCheckTx() && !simulate {
+		return next(ctx, tx, simulate)
+	}
+
+	sigTx, ok := tx.(authante.SigVerifiableTx)
+	if !ok {
+		return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type")
+	}
+
+	// increment sequence of all signers
+	for _, addr := range sigTx.GetSigners() {
+		acc := isd.ak.GetAccount(ctx, addr)
+		if err := acc.SetSequence(acc.GetSequence() + 1); err != nil {
+			panic(err)
+		}
+
+		isd.ak.SetAccount(ctx, acc)
+	}
+
+	return next(ctx, tx, simulate)
+}

go.sum

@@ -329,6 +329,7 @@ github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoA
 github.com/gorilla/websocket v1.4.1-0.20190629185528-ae1634f6a989/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=
 github.com/gorilla/websocket v1.4.1 h1:q7AeDBpnBk8AogcD4DSag/Ukw/KV+YhzLj2bP5HvKCM=
 github.com/gorilla/websocket v1.4.1/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
+github.com/gorilla/websocket v1.4.2 h1:+/TMaTYc4QFitKJxsQ7Yye35DkWvkdLcvGKqM+x0Ufc=
 github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
 github.com/gostaticanalysis/analysisutil v0.0.0-20190318220348-4088753ea4d3 h1:JVnpOZS+qxli+rgVl98ILOXVNbW+kb5wcxeGx8ShUIw=
 github.com/gostaticanalysis/analysisutil v0.0.0-20190318220348-4088753ea4d3/go.mod h1:eEOZF4jCKGi+aprrirO9e7WKB3beBRtWgqGunKl6pKE=

rpc/types.go

@@ -44,9 +44,8 @@ func (bn *BlockNumber) UnmarshalJSON(data []byte) error {
 		*bn = LatestBlockNumber
 		return nil
 	case "pending":
-		return fmt.Errorf("pending queries not implemented")
+		*bn = LatestBlockNumber
-		// *bn = PendingBlockNumber
+		return nil
-		// return nil
 	}
 
 	blckNum, err := hexutil.DecodeUint64(input)

tests/rpc_test.go

@@ -11,6 +11,7 @@ package tests
 
 import (
 	"bytes"
+	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"math/big"
@@ -107,6 +108,14 @@ func call(t *testing.T, method string, params interface{}) *Response {
 	return rpcRes
 }
 
+// turns a 0x prefixed hex string to a big.Int
+func hexToBigInt(t *testing.T, in string) *big.Int {
+	s := in[2:]
+	b, err := hex.DecodeString(s)
+	require.NoError(t, err)
+	return big.NewInt(0).SetBytes(b)
+}
+
 func TestEth_protocolVersion(t *testing.T) {
 	expectedRes := hexutil.Uint(version.ProtocolVersion)
 
@@ -590,3 +599,41 @@ func TestEth_PendingTransactionFilter(t *testing.T) {
 	require.True(t, len(txs) >= 2, "could not get any txs", "changesRes.Result", string(changesRes.Result))
 
 }
+
+func TestBlockBloom(t *testing.T) {
+	hash := deployTestContractWithFunction(t)
+	receipt := waitForReceipt(t, hash)
+
+	number := receipt["blockNumber"].(string)
+	t.Log(number)
+
+	param := []interface{}{number, false}
+	rpcRes := call(t, "eth_getBlockByNumber", param)
+
+	block := make(map[string]interface{})
+	err := json.Unmarshal(rpcRes.Result, &block)
+	require.NoError(t, err)
+
+	lb := hexToBigInt(t, block["logsBloom"].(string))
+	require.NotEqual(t, big.NewInt(0), lb)
+	require.Equal(t, hash.String(), block["transactions"].([]interface{})[0])
+}
+
+func TestBlockBloom_Hash(t *testing.T) {
+	t.Skip()
+	// TODO: get this to work
+	hash := deployTestContractWithFunction(t)
+	receipt := waitForReceipt(t, hash)
+
+	blockHash := receipt["blockHash"].(string)
+
+	param := []interface{}{blockHash, false}
+	rpcRes := call(t, "eth_getBlockByHash", param)
+
+	block := make(map[string]interface{})
+	err := json.Unmarshal(rpcRes.Result, &block)
+	require.NoError(t, err)
+
+	lb := hexToBigInt(t, block["logsBloom"].(string))
+	require.NotEqual(t, big.NewInt(0), lb)
+}

x/evm/abci.go

@@ -19,8 +19,8 @@ func BeginBlock(k Keeper, ctx sdk.Context, req abci.RequestBeginBlock) {
 
 	// Consider removing this when using evm as module without web3 API
 	bloom := ethtypes.BytesToBloom(k.Bloom.Bytes())
-	k.SetBlockBloomMapping(ctx, bloom, req.Header.GetHeight()-1)
+	k.SetBlockBloomMapping(ctx, bloom, req.Header.GetHeight())
-	k.SetBlockHashMapping(ctx, req.Header.LastBlockId.GetHash(), req.Header.GetHeight()-1)
+	k.SetBlockHashMapping(ctx, req.Header.LastBlockId.GetHash(), req.Header.GetHeight())
 	k.Bloom = big.NewInt(0)
 	k.TxCount = 0
 }

x/evm/keeper/keeper.go

@@ -61,7 +61,7 @@ func (k Keeper) GetBlockHashMapping(ctx sdk.Context, hash []byte) (int64, error)
 	store := ctx.KVStore(k.blockKey)
 	bz := store.Get(hash)
 	if len(bz) == 0 {
-		return 0, fmt.Errorf("block with hash '%s' not found", ethcmn.BytesToHash(hash))
+		return 0, fmt.Errorf("block with hash '%s' not found", ethcmn.BytesToHash(hash).Hex())
 	}
 
 	height := binary.BigEndian.Uint64(bz)

x/evm/types/msg.go

@@ -76,8 +76,8 @@ func (msg MsgEthermint) GetSignBytes() []byte {
 
 // ValidateBasic runs stateless checks on the message
 func (msg MsgEthermint) ValidateBasic() error {
-	if msg.Price.Sign() != 1 {
+	if msg.Price.Sign() == -1 {
-		return sdkerrors.Wrapf(types.ErrInvalidValue, "price must be positive %s", msg.Price)
+		return sdkerrors.Wrapf(types.ErrInvalidValue, "price cannot be negative %s", msg.Price)
 	}
 
 	// Amount can be 0
@@ -175,8 +175,8 @@ func (msg MsgEthereumTx) Type() string { return TypeMsgEthereumTx }
 // ValidateBasic implements the sdk.Msg interface. It performs basic validation
 // checks of a Transaction. If returns an error if validation fails.
 func (msg MsgEthereumTx) ValidateBasic() error {
-	if msg.Data.Price.Sign() != 1 {
+	if msg.Data.Price.Sign() == -1 {
-		return sdkerrors.Wrapf(types.ErrInvalidValue, "price must be positive %s", msg.Data.Price)
+		return sdkerrors.Wrapf(types.ErrInvalidValue, "price cannot be negative %s", msg.Data.Price)
 	}
 
 	// Amount can be 0