Skip to content

Commit

Permalink
Fix JSON-RPC response's ID flow
Browse files Browse the repository at this point in the history
  • Loading branch information
evgeniy-scherbina committed Oct 20, 2023
1 parent f9fbfe5 commit 03ee2ff
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 12 deletions.
93 changes: 84 additions & 9 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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,
}
}

Expand Down
26 changes: 23 additions & 3 deletions service/cachemdw/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package cachemdw

import (
"context"
"encoding/json"
"errors"
"fmt"
"strconv"
"time"

"github.com/kava-labs/kava-proxy-service/clients/cache"
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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 {
Expand Down

0 comments on commit 03ee2ff

Please sign in to comment.