From 03ee2ff2bf8cb6468cb56aabdbf82ee1321b1127 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Fri, 20 Oct 2023 11:15:10 -0400 Subject: [PATCH] Fix JSON-RPC response's ID flow --- main_test.go | 93 +++++++++++++++++++++++++++++++++++---- service/cachemdw/cache.go | 26 +++++++++-- 2 files changed, 107 insertions(+), 12 deletions(-) diff --git a/main_test.go b/main_test.go index c73c268..a50161a 100644 --- a/main_test.go +++ b/main_test.go @@ -480,7 +480,7 @@ func TestE2ETestCachingMdwWithBlockNumberParam(t *testing.T) { // check that cached and non-cached responses are equal // eth_getBlockByNumber - cache MISS - resp1 := mkJsonRpcRequest(t, proxyServiceURL, tc.method, tc.params) + resp1 := mkJsonRpcRequest(t, proxyServiceURL, 1, tc.method, tc.params) require.Equal(t, cachemdw.CacheMissHeaderValue, resp1.Header[cachemdw.CacheHeaderKey][0]) body1, err := io.ReadAll(resp1.Body) require.NoError(t, err) @@ -491,7 +491,7 @@ func TestE2ETestCachingMdwWithBlockNumberParam(t *testing.T) { containsKey(t, redisClient, expectedKey) // eth_getBlockByNumber - cache HIT - resp2 := mkJsonRpcRequest(t, proxyServiceURL, tc.method, tc.params) + resp2 := mkJsonRpcRequest(t, proxyServiceURL, 1, tc.method, tc.params) require.Equal(t, cachemdw.CacheHitHeaderValue, resp2.Header[cachemdw.CacheHeaderKey][0]) body2, err := io.ReadAll(resp2.Body) require.NoError(t, err) @@ -563,7 +563,7 @@ func TestE2ETestCachingMdwWithBlockNumberParam_EmptyResult(t *testing.T) { // check that responses are equal // eth_getBlockByNumber - cache MISS - resp1 := mkJsonRpcRequest(t, proxyServiceURL, tc.method, tc.params) + resp1 := mkJsonRpcRequest(t, proxyServiceURL, 1, tc.method, tc.params) require.Equal(t, cachemdw.CacheMissHeaderValue, resp1.Header[cachemdw.CacheHeaderKey][0]) body1, err := io.ReadAll(resp1.Body) require.NoError(t, err) @@ -572,7 +572,7 @@ func TestE2ETestCachingMdwWithBlockNumberParam_EmptyResult(t *testing.T) { expectKeysNum(t, redisClient, tc.keysNum) // eth_getBlockByNumber - cache MISS again (empty results aren't cached) - resp2 := mkJsonRpcRequest(t, proxyServiceURL, tc.method, tc.params) + resp2 := mkJsonRpcRequest(t, proxyServiceURL, 1, tc.method, tc.params) require.Equal(t, cachemdw.CacheMissHeaderValue, resp2.Header[cachemdw.CacheHeaderKey][0]) body2, err := io.ReadAll(resp2.Body) require.NoError(t, err) @@ -603,6 +603,81 @@ func TestE2ETestCachingMdwWithBlockNumberParam_EmptyResult(t *testing.T) { cleanUpRedis(t, redisClient) } +func TestE2ETestCachingMdwWithBlockNumberParam_DiffJsonRpcReqIDs(t *testing.T) { + redisClient := redis.NewClient(&redis.Options{ + Addr: redisURL, + Password: redisPassword, + DB: 0, + }) + cleanUpRedis(t, redisClient) + expectKeysNum(t, redisClient, 0) + + for _, tc := range []struct { + desc string + method string + params []interface{} + keysNum int + }{ + { + desc: "test case #1", + method: "eth_getBlockByNumber", + params: []interface{}{"0x1", true}, + keysNum: 1, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + // test cache MISS and cache HIT scenarios for specified method + // check corresponding values in cachemdw.CacheHeaderKey HTTP header + // NOTE: JSON-RPC request IDs are different + // check that cached and non-cached responses differ only in response ID + + // eth_getBlockByNumber - cache MISS + resp1 := mkJsonRpcRequest(t, proxyServiceURL, 1, tc.method, tc.params) + require.Equal(t, cachemdw.CacheMissHeaderValue, resp1.Header[cachemdw.CacheHeaderKey][0]) + body1, err := io.ReadAll(resp1.Body) + require.NoError(t, err) + err = checkJsonRpcErr(body1) + require.NoError(t, err) + expectKeysNum(t, redisClient, tc.keysNum) + expectedKey := "local-chain:evm-request:eth_getBlockByNumber:sha256:d08b426164eacf6646fb1817403ec0af5d37869a0f32a01ebfab3096fa4999be" + containsKey(t, redisClient, expectedKey) + + // eth_getBlockByNumber - cache HIT + resp2 := mkJsonRpcRequest(t, proxyServiceURL, 2, tc.method, tc.params) + require.Equal(t, cachemdw.CacheHitHeaderValue, resp2.Header[cachemdw.CacheHeaderKey][0]) + body2, err := io.ReadAll(resp2.Body) + require.NoError(t, err) + err = checkJsonRpcErr(body2) + require.NoError(t, err) + expectKeysNum(t, redisClient, tc.keysNum) + containsKey(t, redisClient, expectedKey) + + rpcResp1, err := cachemdw.UnmarshalJsonRpcResponse(body1) + require.NoError(t, err) + rpcResp2, err := cachemdw.UnmarshalJsonRpcResponse(body2) + require.NoError(t, err) + + // JSON-RPC Version and Result should be equal + require.Equal(t, rpcResp1.Version, rpcResp2.Version) + require.Equal(t, rpcResp1.Result, rpcResp2.Result) + + // JSON-RPC response ID should correspond to JSON-RPC request ID + require.Equal(t, string(rpcResp1.ID), "1") + require.Equal(t, string(rpcResp2.ID), "2") + + // JSON-RPC error should be empty + require.Empty(t, rpcResp1.JsonRpcError) + require.Empty(t, rpcResp2.JsonRpcError) + + // Double-check that JSON-RPC responses differ only in response ID + rpcResp2.ID = []byte("1") + require.Equal(t, rpcResp1, rpcResp2) + }) + } + + cleanUpRedis(t, redisClient) +} + func expectKeysNum(t *testing.T, redisClient *redis.Client, keysNum int) { keys, err := redisClient.Keys(context.Background(), "*").Result() require.NoError(t, err) @@ -626,8 +701,8 @@ func cleanUpRedis(t *testing.T, redisClient *redis.Client) { } } -func mkJsonRpcRequest(t *testing.T, proxyServiceURL, method string, params []interface{}) *http.Response { - req := newJsonRpcRequest(method, params) +func mkJsonRpcRequest(t *testing.T, proxyServiceURL string, id int, method string, params []interface{}) *http.Response { + req := newJsonRpcRequest(id, method, params) reqInJSON, err := json.Marshal(req) require.NoError(t, err) reqReader := bytes.NewBuffer(reqInJSON) @@ -640,17 +715,17 @@ func mkJsonRpcRequest(t *testing.T, proxyServiceURL, method string, params []int type jsonRpcRequest struct { JsonRpc string `json:"jsonrpc"` + Id int `json:"id"` Method string `json:"method"` Params []interface{} `json:"params"` - Id int `json:"id"` } -func newJsonRpcRequest(method string, params []interface{}) *jsonRpcRequest { +func newJsonRpcRequest(id int, method string, params []interface{}) *jsonRpcRequest { return &jsonRpcRequest{ JsonRpc: "2.0", + Id: id, Method: method, Params: params, - Id: 1, } } diff --git a/service/cachemdw/cache.go b/service/cachemdw/cache.go index 0790166..7115554 100644 --- a/service/cachemdw/cache.go +++ b/service/cachemdw/cache.go @@ -2,8 +2,10 @@ package cachemdw import ( "context" + "encoding/json" "errors" "fmt" + "strconv" "time" "github.com/kava-labs/kava-proxy-service/clients/cache" @@ -82,6 +84,8 @@ func IsCacheable( } // GetCachedQueryResponse calculates cache key for request and then tries to get it from cache. +// NOTE: only JSON-RPC response's result will be taken from the cache. +// JSON-RPC response's ID and Version will be constructed on the fly to match JSON-RPC request. func (c *ServiceCache) GetCachedQueryResponse( ctx context.Context, req *decode.EVMRPCRequestEnvelope, @@ -91,15 +95,31 @@ func (c *ServiceCache) GetCachedQueryResponse( return nil, err } - value, err := c.cacheClient.Get(ctx, key) + // get JSON-RPC response's result from the cache + result, err := c.cacheClient.Get(ctx, key) if err != nil { return nil, err } - return value, nil + // JSON-RPC response's ID and Version should match JSON-RPC request + id := strconv.Itoa(int(req.ID)) + response := JsonRpcResponse{ + Version: req.JSONRPCVersion, + ID: []byte(id), + Result: result, + } + responseInJSON, err := json.Marshal(response) + if err != nil { + return nil, err + } + + return responseInJSON, nil } // CacheQueryResponse calculates cache key for request and then saves response to the cache. +// NOTE: only JSON-RPC response's result is cached. +// There is no point to cache JSON-RPC response's ID (because it should correspond to request's ID, which constantly changes). +// Same with JSON-RPC response's Version. func (c *ServiceCache) CacheQueryResponse( ctx context.Context, req *decode.EVMRPCRequestEnvelope, @@ -124,7 +144,7 @@ func (c *ServiceCache) CacheQueryResponse( return err } - return c.cacheClient.Set(ctx, key, responseInBytes, c.cacheTTL) + return c.cacheClient.Set(ctx, key, response.Result, c.cacheTTL) } func (c *ServiceCache) Healthcheck(ctx context.Context) error {