From 13a027b48e859a0c2c3179a70baa068803e19d68 Mon Sep 17 00:00:00 2001 From: Robert Pirtle Date: Wed, 11 Oct 2023 12:40:42 -0700 Subject: [PATCH] improve & refactor evm_rpc methods for reuse (#41) * add missing valid blocktags * test invalid/unsupported block tag * refactor ExtractBlockNumberFromEVMRPCRequest for reuse will need the validation and block number extraction for height-based routing. however, it will be in the critical path, so we need to not have the bit that makes an additional evm request. * add tests of cacheable param checks * support & handle routing always-latest methods some methods will always work if routed to a pruning cluster. do that! * track & encode "empty" block tag requests separately * refactor block tags to constants * refactor & rename NoHistoryMethods * refactor method param type checks to pure functions * add test coverage of public decode methods --- decode/evm_rpc.go | 138 +++++++++++++++++++++++--------- decode/evm_rpc_test.go | 178 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 273 insertions(+), 43 deletions(-) diff --git a/decode/evm_rpc.go b/decode/evm_rpc.go index c773792..5d0981c 100644 --- a/decode/evm_rpc.go +++ b/decode/evm_rpc.go @@ -12,6 +12,18 @@ import ( cosmosmath "cosmossdk.io/math" ) +// These block tags are special strings used to reference blocks in JSON-RPC +// see https://ethereum.org/en/developers/docs/apis/json-rpc/#default-block +const ( + BlockTagLatest = "latest" + BlockTagPending = "pending" + BlockTagEarliest = "earliest" + BlockTagFinalized = "finalized" + BlockTagSafe = "safe" + // "empty" is not in the spec, it is our encoding for requests made with a nil block tag param. + BlockTagEmpty = "empty" +) + // Errors that might result from decoding parts or the whole of // an EVM RPC request var ( @@ -37,6 +49,18 @@ var CacheableByBlockNumberMethods = []string{ "eth_call", } +// MethodHasBlockNumberParam returns true when the method expects a block number in the request parameters. +func MethodHasBlockNumberParam(method string) bool { + var includesBlockNumberParam bool + for _, cacheableByBlockNumberMethod := range CacheableByBlockNumberMethods { + if method == cacheableByBlockNumberMethod { + includesBlockNumberParam = true + break + } + } + return includesBlockNumberParam +} + // List of evm methods that can be cached by block hash // and so are useful for converting and tracking the block hash associated with // any requests invoking those methods to the matching block number @@ -48,6 +72,52 @@ var CacheableByBlockHashMethods = []string{ "eth_getTransactionByBlockHashAndIndex", } +// MethodHasBlockHashParam returns true when the method expects a block hash in the request parameters. +func MethodHasBlockHashParam(method string) bool { + var includesBlockHashParam bool + for _, cacheableByBlockHashMethod := range CacheableByBlockHashMethods { + if method == cacheableByBlockHashMethod { + includesBlockHashParam = true + break + } + } + return includesBlockHashParam +} + +// NoHistoryMethods is a list of JSON-RPC methods that rely only on the present state of the chain. +// They can always be safely routed to an up-to-date pruning cluster. +var NoHistoryMethods = []string{ + "web3_clientVersion", + "web3_sha3", + "net_version", + "net_listening", + "net_peerCount", + "eth_protocolVersion", + "eth_syncing", + "eth_coinbase", + "eth_chainId", + "eth_mining", + "eth_hashrate", + "eth_gasPrice", + "eth_accounts", + "eth_sign", + "eth_signTransaction", + "eth_sendTransaction", + "eth_sendRawTransaction", +} + +// MethodRequiresNoHistory returns true when the JSON-RPC method always functions correctly +// when sent to the latest block. +// This is useful for determining if a request can be routed to a pruning cluster. +func MethodRequiresNoHistory(method string) bool { + for _, nonHistoricalMethod := range NoHistoryMethods { + if method == nonHistoricalMethod { + return true + } + } + return false +} + // List of evm methods that can be cached independent // of block number (i.e. by block or transaction hash, filter id, or time period) // TODO: break these out into separate list for methods that can be cached using the same key type @@ -106,10 +176,18 @@ var MethodNameToBlockHashParamIndex = map[string]int{ // Mapping of string tag values used in the eth api to // normalized int values that can be stored as the block number // for the proxied request metric +// see https://ethereum.org/en/developers/docs/apis/json-rpc/#default-block var BlockTagToNumberCodec = map[string]int64{ - "latest": -1, - "pending": -2, - "earliest": -3, + BlockTagLatest: -1, + BlockTagPending: -2, + BlockTagEarliest: -3, + BlockTagFinalized: -4, + BlockTagSafe: -5, + // "empty" is not part of the evm json-rpc spec + // it is our encoding for when no parameter is passed in as a block tag param + // usually, clients interpret an empty block tag to mean "latest" + // we track it separately here to more accurately track how users make requests + BlockTagEmpty: -6, } // EVMRPCRequest wraps expected values present in a request @@ -143,37 +221,16 @@ func (r *EVMRPCRequestEnvelope) ExtractBlockNumberFromEVMRPCRequest(ctx context. if r.Method == "" { return 0, ErrInvalidEthAPIRequest } - - // validate this is a request with a block number param - var cacheableByBlockNumber bool - for _, cacheableByBlockNumberMethod := range CacheableByBlockNumberMethods { - if r.Method == cacheableByBlockNumberMethod { - cacheableByBlockNumber = true - break - } + // handle cacheable by block number + if MethodHasBlockNumberParam(r.Method) { + return ParseBlockNumberFromParams(r.Method, r.Params) } - - var cacheableByBlockHash bool - for _, cacheableByBlockHashMethod := range CacheableByBlockHashMethods { - if r.Method == cacheableByBlockHashMethod { - cacheableByBlockHash = true - break - } - } - - if !cacheableByBlockNumber && !cacheableByBlockHash { - return 0, ErrUncachaebleByBlockNumberEthRequest - } - - // parse block number using heuristics so byzantine - // they require their own consensus engine 😅 - // https://ethereum.org/en/developers/docs/apis/json-rpc - // or at least a healthy level of [code coverage](./evm_rpc_test.go) ;-) - if cacheableByBlockNumber { - return parseBlockNumberFromParams(r.Method, r.Params) + // handle cacheable by block hash + if MethodHasBlockHashParam(r.Method) { + return lookupBlockNumberFromHashParam(ctx, evmClient, r.Method, r.Params) } - - return lookupBlockNumberFromHashParam(ctx, evmClient, r.Method, r.Params) + // handle unable to cached + return 0, ErrUncachaebleByBlockNumberEthRequest } // Generic method to lookup the block number @@ -201,35 +258,36 @@ func lookupBlockNumberFromHashParam(ctx context.Context, evmClient *ethclient.Cl } // Generic method to parse the block number from a set of params -func parseBlockNumberFromParams(methodName string, params []interface{}) (int64, error) { +// errors if method does not have a block number in the param, or the param has an unexpected value +// block tags are encoded to an int64 according to the BlockTagToNumberCodec map. +func ParseBlockNumberFromParams(methodName string, params []interface{}) (int64, error) { paramIndex, exists := MethodNameToBlockNumberParamIndex[methodName] if !exists { return 0, ErrUncachaebleByBlockNumberEthRequest } + // capture requests made with empty block tag params + if params[paramIndex] == nil { + return BlockTagToNumberCodec["empty"], nil + } + tag, isString := params[paramIndex].(string) if !isString { return 0, fmt.Errorf(fmt.Sprintf("error decoding block number param from params %+v at index %d", params, paramIndex)) } - var blockNumber int64 - tagEncoding, exists := BlockTagToNumberCodec[tag] + blockNumber, exists := BlockTagToNumberCodec[tag] if !exists { spaceint, valid := cosmosmath.NewIntFromString(tag) - if !valid { return 0, fmt.Errorf(fmt.Sprintf("unable to parse tag %s to integer", tag)) } blockNumber = spaceint.Int64() - - return blockNumber, nil } - blockNumber = tagEncoding - return blockNumber, nil } diff --git a/decode/evm_rpc_test.go b/decode/evm_rpc_test.go index 6f770af..15125aa 100644 --- a/decode/evm_rpc_test.go +++ b/decode/evm_rpc_test.go @@ -6,6 +6,7 @@ import ( "github.com/ethereum/go-ethereum/ethclient" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var ( @@ -16,6 +17,60 @@ var ( }() ) +func TestUnitTest_MethodCategorization(t *testing.T) { + testCases := []struct { + name string + method string + hasBlockNumber bool + hasBlockHash bool + needsNoHistory bool + }{ + { + name: "block number method", + method: "eth_getBlockByNumber", + hasBlockNumber: true, + hasBlockHash: false, + needsNoHistory: false, + }, + { + name: "block hash method", + method: "eth_getBlockByHash", + hasBlockNumber: false, + hasBlockHash: true, + needsNoHistory: false, + }, + { + name: "needs no history", + method: "eth_sendTransaction", + hasBlockNumber: false, + hasBlockHash: false, + needsNoHistory: true, + }, + { + name: "invalid method", + method: "eth_notRealMethod", + hasBlockNumber: false, + hasBlockHash: false, + needsNoHistory: false, + }, + { + name: "empty method", + method: "", + hasBlockNumber: false, + hasBlockHash: false, + needsNoHistory: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.hasBlockNumber, MethodHasBlockNumberParam(tc.method), "unexpected MethodHasBlockNumberParam result") + require.Equal(t, tc.hasBlockHash, MethodHasBlockHashParam(tc.method), "unexpected MethodHasBlockHashParam result") + require.Equal(t, tc.needsNoHistory, MethodRequiresNoHistory(tc.method), "unexpected MethodRequiresNoHistory result") + }) + } +} + func TestUnitTestExtractBlockNumberFromEVMRPCRequestReturnsExpectedBlockForValidRequest(t *testing.T) { requestedBlockNumberHexEncoding := "0x2" expectedBlockNumber := int64(2) @@ -34,19 +89,48 @@ func TestUnitTestExtractBlockNumberFromEVMRPCRequestReturnsExpectedBlockForValid } func TestUnitTestExtractBlockNumberFromEVMRPCRequestReturnsExpectedBlockNumberForTag(t *testing.T) { - requestedBlockTag := "latest" + tags := []string{"latest", "pending", "earliest", "finalized", "safe"} + + for _, requestedBlockTag := range tags { + validRequest := EVMRPCRequestEnvelope{ + Method: "eth_getBlockByNumber", + Params: []interface{}{ + requestedBlockTag, false, + }, + } + + blockNumber, err := validRequest.ExtractBlockNumberFromEVMRPCRequest(testContext, dummyEthClient) + + assert.Nil(t, err) + assert.Equal(t, BlockTagToNumberCodec[requestedBlockTag], blockNumber) + } + // check empty block number validRequest := EVMRPCRequestEnvelope{ Method: "eth_getBlockByNumber", Params: []interface{}{ - requestedBlockTag, false, + nil, false, }, } blockNumber, err := validRequest.ExtractBlockNumberFromEVMRPCRequest(testContext, dummyEthClient) assert.Nil(t, err) - assert.Equal(t, BlockTagToNumberCodec[requestedBlockTag], blockNumber) + assert.Equal(t, BlockTagToNumberCodec["empty"], blockNumber) +} + +func TestUnitTestExtractBlockNumberFromEVMRPCRequestFailsForInvalidTag(t *testing.T) { + requestedBlockTag := "invalid-block-tag" + validRequest := EVMRPCRequestEnvelope{ + Method: "eth_getBlockByNumber", + Params: []interface{}{ + requestedBlockTag, false, + }, + } + + _, err := validRequest.ExtractBlockNumberFromEVMRPCRequest(testContext, dummyEthClient) + + assert.ErrorContains(t, err, "unable to parse tag") } func TestUnitTestExtractBlockNumberFromEVMRPCRequestReturnsErrorWhenRequestMethodEmpty(t *testing.T) { @@ -84,3 +168,91 @@ func TestUnitTestExtractBlockNumberFromEVMRPCRequestReturnsErrorWhenUnknownReque assert.Equal(t, ErrUncachaebleByBlockNumberEthRequest, err) } + +func TestUnitTest_ParseBlockNumberFromParams(t *testing.T) { + testCases := []struct { + name string + req EVMRPCRequestEnvelope + expectedBlockNumber int64 + expectedErr string + }{ + { + name: "method with block number", + req: EVMRPCRequestEnvelope{ + Method: "eth_getBlockByNumber", + Params: []interface{}{ + "0xd", false, + }, + }, + expectedBlockNumber: 13, + expectedErr: "", + }, + { + name: "method with block tag", + req: EVMRPCRequestEnvelope{ + Method: "eth_getBlockByNumber", + Params: []interface{}{ + "latest", false, + }, + }, + expectedBlockNumber: BlockTagToNumberCodec[BlockTagLatest], + expectedErr: "", + }, + { + name: "method with no block number in params", + req: EVMRPCRequestEnvelope{ + Method: "eth_getBlockByHash", + Params: []interface{}{ + "0xb8d6ffd1ebd2df7a735c72e755886c6dd6587e096ae788558c6f24f31469b271", false, + }, + }, + expectedBlockNumber: 0, + expectedErr: "request is not cache-able by block number", + }, + { + name: "method with empty block number", + req: EVMRPCRequestEnvelope{ + Method: "eth_getBlockByNumber", + Params: []interface{}{ + nil, false, + }, + }, + expectedBlockNumber: BlockTagToNumberCodec[BlockTagEmpty], + expectedErr: "", + }, + { + name: "method with invalid string block number param", + req: EVMRPCRequestEnvelope{ + Method: "eth_getBlockByNumber", + Params: []interface{}{ + "not-an-int", false, + }, + }, + expectedBlockNumber: 0, + expectedErr: "unable to parse tag not-an-int to integer", + }, + { + name: "method with non-string block number param", + req: EVMRPCRequestEnvelope{ + Method: "eth_getBlockByNumber", + Params: []interface{}{ + false, false, + }, + }, + expectedBlockNumber: 0, + expectedErr: "error decoding block number param from params", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + blockNumber, err := ParseBlockNumberFromParams(tc.req.Method, tc.req.Params) + if tc.expectedErr == "" { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, tc.expectedErr) + } + require.Equal(t, tc.expectedBlockNumber, blockNumber) + }) + } +}