From 4a45c400eae8a43f2b832825b28c7ee2ba2873af Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Wed, 5 Jul 2023 17:30:59 -0400 Subject: [PATCH 01/10] Feat: add a client-side session cache --- app/client/cli/cache/session.go | 73 +++++++++++++++++++++++++++ app/client/cli/cache/session_test.go | 75 ++++++++++++++++++++++++++++ app/client/cli/servicer.go | 45 ++++++++++++++++- 3 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 app/client/cli/cache/session.go create mode 100644 app/client/cli/cache/session_test.go diff --git a/app/client/cli/cache/session.go b/app/client/cli/cache/session.go new file mode 100644 index 000000000..c9109bb8f --- /dev/null +++ b/app/client/cli/cache/session.go @@ -0,0 +1,73 @@ +package cache + +import ( + "encoding/json" + "errors" + "fmt" + + "github.com/pokt-network/pocket/persistence/kvstore" + "github.com/pokt-network/pocket/rpc" +) + +var errSessionNotFound = errors.New("session not found in cache") + +// SessionCache stores and retrieves sessions for application+relaychain pairs +// +// It uses a key-value store as backing storage +type SessionCache struct { + // store is the local store for cached sessions + store kvstore.KVStore +} + +// NewSessionCache returns a session cache backed by a kvstore using the provided database path. +func NewSessionCache(databasePath string) (*SessionCache, error) { + store, err := kvstore.NewKVStore(databasePath) + if err != nil { + return nil, fmt.Errorf("Error initializing key-value store using path %s: %w", databasePath, err) + } + + return &SessionCache{ + store: store, + }, nil +} + +// Get returns the cached session, if found, for an app+chain combination. +// The caller is responsible to verify that the returned session is valid for the current block height. +// Get is NOT safe to use concurrently +func (s *SessionCache) Get(appAddr, chain string) (*rpc.Session, error) { + key := sessionKey(appAddr, chain) + bz, err := s.store.Get(key) + if err != nil { + return nil, fmt.Errorf("error getting session from the store: %s %w", err.Error(), errSessionNotFound) + } + + var session rpc.Session + if err := json.Unmarshal(bz, &session); err != nil { + return nil, fmt.Errorf("error unmarshalling session from store: %w", err) + } + + return &session, nil +} + +// Set stores the provided session in the cache with the key being the app+chain combination. +// For each app+chain combination, a single session will be stored. Subsequent calls to Set will overwrite the entry for the provided app and chain. +// Set is NOT safe to use concurrently +func (s *SessionCache) Set(session *rpc.Session) error { + bz, err := json.Marshal(*session) + if err != nil { + return fmt.Errorf("error marshalling session for app: %s, chain: %s, session height: %d: %w", session.Application.Address, session.Chain, session.SessionHeight, err) + } + + key := sessionKey(session.Application.Address, session.Chain) + if s.store.Set(key, bz); err != nil { + return fmt.Errorf("error storing session for app: %s, chain: %s, session height: %d in the cache: %w", session.Application.Address, session.Chain, session.SessionHeight, err) + } + return nil +} + +// sessionKey returns a key to get/set a session, based on application's address and the relay chain. +// +// The height is not used as part of the key, because for each app+chain combination only one session, i.e. the current one, is of interest. +func sessionKey(appAddr, chain string) []byte { + return []byte(fmt.Sprintf("%s-%s", appAddr, chain)) +} diff --git a/app/client/cli/cache/session_test.go b/app/client/cli/cache/session_test.go new file mode 100644 index 000000000..4b5afbaec --- /dev/null +++ b/app/client/cli/cache/session_test.go @@ -0,0 +1,75 @@ +package cache + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/pokt-network/pocket/rpc" +) + +func TestGet(t *testing.T) { + const ( + app1 = "app1Addr" + relaychainEth = "ETH-Goerli" + numSessionBlocks = 4 + sessionHeight = 8 + sessionNumber = 2 + ) + + session1 := &rpc.Session{ + Application: rpc.ProtocolActor{ + ActorType: rpc.Application, + Address: "app1Addr", + Chains: []string{relaychainEth}, + }, + Chain: relaychainEth, + NumSessionBlocks: numSessionBlocks, + SessionHeight: sessionHeight, + SessionNumber: sessionNumber, + } + + testCases := []struct { + name string + cacheContents []*rpc.Session + app string + chain string + expected *rpc.Session + expectedErr error + }{ + { + name: "Return cached session", + cacheContents: []*rpc.Session{session1}, + app: app1, + chain: relaychainEth, + expected: session1, + }, + { + name: "Error returned for session not found in cache", + app: "foo", + chain: relaychainEth, + expectedErr: errSessionNotFound, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + dbPath, err := os.MkdirTemp("", "cacheStoragePath") + require.NoError(t, err) + defer os.RemoveAll(dbPath) + + cache, err := NewSessionCache(dbPath) + require.NoError(t, err) + + for _, s := range tc.cacheContents { + err := cache.Set(s) + require.NoError(t, err) + } + + got, err := cache.Get(tc.app, tc.chain) + require.ErrorIs(t, err, tc.expectedErr) + require.EqualValues(t, tc.expected, got) + }) + } +} diff --git a/app/client/cli/servicer.go b/app/client/cli/servicer.go index 5787d2d7e..f42b49e73 100644 --- a/app/client/cli/servicer.go +++ b/app/client/cli/servicer.go @@ -9,12 +9,16 @@ import ( "github.com/spf13/cobra" + "github.com/pokt-network/pocket/app/client/cli/cache" "github.com/pokt-network/pocket/app/client/cli/flags" + "github.com/pokt-network/pocket/logger" "github.com/pokt-network/pocket/rpc" coreTypes "github.com/pokt-network/pocket/shared/core/types" "github.com/pokt-network/pocket/shared/crypto" ) +const sessionCacheDBPath = "/tmp" + func init() { rootCmd.AddCommand(NewServicerCommand()) } @@ -115,6 +119,26 @@ func validateServicer(session *rpc.Session, servicerAddress string) (*rpc.Protoc return nil, fmt.Errorf("Error getting servicer: address %s does not match any servicers in the session %d", servicerAddress, session.SessionNumber) } +// ADDTEST: add either unit tests or integration tests using kvstore +// getSessionFrom uses the client-side session cache to retrieve a session for app/chain combination at the provided height, if one has already been retrieved and cached. +func getSessionFromCache(sessionCache *cache.SessionCache, appAddress, chain string, height int64) (*rpc.Session, error) { + if sessionCache == nil { + return nil, fmt.Errorf("got nil session cache") + } + + session, err := sessionCache.Get(appAddress, chain) + if err != nil { + return nil, err + } + + // verify the cached session is valid + if height >= session.SessionHeight && height < session.SessionHeight+session.NumSessionBlocks { + return session, nil + } + + return nil, fmt.Errorf("no valid session found") +} + func getCurrentSession(ctx context.Context, appAddress, chain string) (*rpc.Session, error) { // CONSIDERATION: passing 0 as the height value to get the current session seems more optimal than this. currentHeight, err := getCurrentHeight(ctx) @@ -122,6 +146,16 @@ func getCurrentSession(ctx context.Context, appAddress, chain string) (*rpc.Sess return nil, fmt.Errorf("Error getting current session: %w", err) } + sessionCache, err := cache.NewSessionCache(sessionCacheDBPath) + if err != nil { + logger.Global.Warn().Err(err).Msg("failed to initialize session cache") + } + + session, err := getSessionFromCache(sessionCache, appAddress, chain, currentHeight) + if err == nil { + return session, nil + } + req := rpc.SessionRequest{ AppAddress: appAddress, Chain: chain, @@ -148,7 +182,16 @@ func getCurrentSession(ctx context.Context, appAddress, chain string) (*rpc.Sess return nil, fmt.Errorf("Error getting current session: Unexpected response %v", resp) } - return resp.JSON200, nil + session = resp.JSON200 + if sessionCache == nil { + return session, nil + } + + if err := sessionCache.Set(session); err != nil { + logger.Global.Warn().Err(err).Msg("failed to store session in cache") + } + + return session, nil } // REFACTOR: reuse this function in all the query commands From 6e7a7a6f979fe24ba8d6b072688c93aabdeb977c Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Thu, 6 Jul 2023 09:40:03 -0400 Subject: [PATCH 02/10] Fix linter error --- app/client/cli/cache/session.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/client/cli/cache/session.go b/app/client/cli/cache/session.go index c9109bb8f..b8409f0a0 100644 --- a/app/client/cli/cache/session.go +++ b/app/client/cli/cache/session.go @@ -59,7 +59,7 @@ func (s *SessionCache) Set(session *rpc.Session) error { } key := sessionKey(session.Application.Address, session.Chain) - if s.store.Set(key, bz); err != nil { + if err := s.store.Set(key, bz); err != nil { return fmt.Errorf("error storing session for app: %s, chain: %s, session height: %d in the cache: %w", session.Application.Address, session.Chain, session.SessionHeight, err) } return nil From 95a629f93927f3bd07f0b0bc8309bf9764d83d9a Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Wed, 19 Jul 2023 17:28:29 -0400 Subject: [PATCH 03/10] Address review comments: add tests for getCachedSession --- app/client/cli/cache/session.go | 2 + app/client/cli/servicer.go | 39 +++++++++------ app/client/cli/servicer_test.go | 85 +++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 14 deletions(-) create mode 100644 app/client/cli/servicer_test.go diff --git a/app/client/cli/cache/session.go b/app/client/cli/cache/session.go index b8409f0a0..29dde99b8 100644 --- a/app/client/cli/cache/session.go +++ b/app/client/cli/cache/session.go @@ -1,5 +1,6 @@ package cache +// TODO: add a TTL for cached sessions, since we know the sessions' length import ( "encoding/json" "errors" @@ -34,6 +35,7 @@ func NewSessionCache(databasePath string) (*SessionCache, error) { // Get returns the cached session, if found, for an app+chain combination. // The caller is responsible to verify that the returned session is valid for the current block height. // Get is NOT safe to use concurrently +// DISCUSS: do we need concurrency here? func (s *SessionCache) Get(appAddr, chain string) (*rpc.Session, error) { key := sessionKey(appAddr, chain) bz, err := s.store.Get(key) diff --git a/app/client/cli/servicer.go b/app/client/cli/servicer.go index f42b49e73..7c73a424a 100644 --- a/app/client/cli/servicer.go +++ b/app/client/cli/servicer.go @@ -4,6 +4,7 @@ import ( "context" "encoding/hex" "encoding/json" + "errors" "fmt" "net/http" @@ -17,10 +18,25 @@ import ( "github.com/pokt-network/pocket/shared/crypto" ) +// IMPROVE: make this configurable const sessionCacheDBPath = "/tmp" +var ( + errNoSessionCache = errors.New("session cache not set up") + errSessionNotFoundInCache = errors.New("session not found in cache") + errNoMatchingSessionInCache = errors.New("no session matching the requested height found in cache") + + sessionCache *cache.SessionCache +) + func init() { rootCmd.AddCommand(NewServicerCommand()) + + var err error + sessionCache, err = cache.NewSessionCache(sessionCacheDBPath) + if err != nil { + logger.Global.Warn().Err(err).Msg("failed to initialize session cache") + } } func NewServicerCommand() *cobra.Command { @@ -119,24 +135,23 @@ func validateServicer(session *rpc.Session, servicerAddress string) (*rpc.Protoc return nil, fmt.Errorf("Error getting servicer: address %s does not match any servicers in the session %d", servicerAddress, session.SessionNumber) } -// ADDTEST: add either unit tests or integration tests using kvstore -// getSessionFrom uses the client-side session cache to retrieve a session for app/chain combination at the provided height, if one has already been retrieved and cached. -func getSessionFromCache(sessionCache *cache.SessionCache, appAddress, chain string, height int64) (*rpc.Session, error) { - if sessionCache == nil { - return nil, fmt.Errorf("got nil session cache") +// getSessionFromCache uses the client-side session cache to fetch a session for app+chain combination at the provided height, if one has already been retrieved and cached. +func getSessionFromCache(cache *cache.SessionCache, appAddress, chain string, height int64) (*rpc.Session, error) { + if cache == nil { + return nil, errNoSessionCache } - session, err := sessionCache.Get(appAddress, chain) + session, err := cache.Get(appAddress, chain) if err != nil { - return nil, err + return nil, fmt.Errorf("%w: %s", errSessionNotFoundInCache, err.Error()) } - // verify the cached session is valid + // verify the cached session matches the provided height if height >= session.SessionHeight && height < session.SessionHeight+session.NumSessionBlocks { return session, nil } - return nil, fmt.Errorf("no valid session found") + return nil, errNoMatchingSessionInCache } func getCurrentSession(ctx context.Context, appAddress, chain string) (*rpc.Session, error) { @@ -146,11 +161,6 @@ func getCurrentSession(ctx context.Context, appAddress, chain string) (*rpc.Sess return nil, fmt.Errorf("Error getting current session: %w", err) } - sessionCache, err := cache.NewSessionCache(sessionCacheDBPath) - if err != nil { - logger.Global.Warn().Err(err).Msg("failed to initialize session cache") - } - session, err := getSessionFromCache(sessionCache, appAddress, chain, currentHeight) if err == nil { return session, nil @@ -184,6 +194,7 @@ func getCurrentSession(ctx context.Context, appAddress, chain string) (*rpc.Sess session = resp.JSON200 if sessionCache == nil { + logger.Global.Warn().Msg("session cache not available: cannot cache the retrieved session") return session, nil } diff --git a/app/client/cli/servicer_test.go b/app/client/cli/servicer_test.go new file mode 100644 index 000000000..e483124c9 --- /dev/null +++ b/app/client/cli/servicer_test.go @@ -0,0 +1,85 @@ +package cli + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/pokt-network/pocket/app/client/cli/cache" + "github.com/pokt-network/pocket/rpc" +) + +const ( + testRelaychainEth = "ETH-Goerli" + testSessionHeight = 8 + testCurrentHeight = 9 +) + +func TestGetSessionFromCache(t *testing.T) { + const app1Addr = "app1Addr" + + testCases := []struct { + name string + cachedSessions []*rpc.Session + expected *rpc.Session + expectedErr error + }{ + { + name: "cached session is returned", + cachedSessions: []*rpc.Session{testSession(app1Addr, testSessionHeight)}, + expected: testSession(app1Addr, testSessionHeight), + }, + { + name: "nil session cache returns an error", + expectedErr: errNoSessionCache, + }, + { + name: "session not found in cache", + cachedSessions: []*rpc.Session{testSession("foo", testSessionHeight)}, + expectedErr: errSessionNotFoundInCache, + }, + { + name: "cached session does not match the provided height", + cachedSessions: []*rpc.Session{testSession(app1Addr, 9999999)}, + expectedErr: errNoMatchingSessionInCache, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var c *cache.SessionCache + if len(tc.cachedSessions) > 0 { + dbPath, err := os.MkdirTemp("", "cliCacheStoragePath") + require.NoError(t, err) + defer os.RemoveAll(dbPath) + + c, err = cache.NewSessionCache(dbPath) + require.NoError(t, err) + + for _, s := range tc.cachedSessions { + err := c.Set(s) + require.NoError(t, err) + } + } + + got, err := getSessionFromCache(c, app1Addr, testRelaychainEth, testCurrentHeight) + require.ErrorIs(t, err, tc.expectedErr) + require.EqualValues(t, tc.expected, got) + }) + } +} + +func testSession(appAddr string, height int64) *rpc.Session { + return &rpc.Session{ + Application: rpc.ProtocolActor{ + ActorType: rpc.Application, + Address: appAddr, + Chains: []string{testRelaychainEth}, + }, + Chain: testRelaychainEth, + NumSessionBlocks: 4, + SessionHeight: height, + SessionNumber: 2, + } +} From 688a6bfa01cf75b4a861286fce947133641fec7e Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Wed, 19 Jul 2023 17:44:58 -0400 Subject: [PATCH 04/10] Fix linter warning --- app/client/cli/servicer.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/client/cli/servicer.go b/app/client/cli/servicer.go index 7c73a424a..5f0d46457 100644 --- a/app/client/cli/servicer.go +++ b/app/client/cli/servicer.go @@ -136,12 +136,12 @@ func validateServicer(session *rpc.Session, servicerAddress string) (*rpc.Protoc } // getSessionFromCache uses the client-side session cache to fetch a session for app+chain combination at the provided height, if one has already been retrieved and cached. -func getSessionFromCache(cache *cache.SessionCache, appAddress, chain string, height int64) (*rpc.Session, error) { - if cache == nil { +func getSessionFromCache(c *cache.SessionCache, appAddress, chain string, height int64) (*rpc.Session, error) { + if c == nil { return nil, errNoSessionCache } - session, err := cache.Get(appAddress, chain) + session, err := c.Get(appAddress, chain) if err != nil { return nil, fmt.Errorf("%w: %s", errSessionNotFoundInCache, err.Error()) } From dbcb2c80b28c8535246c28f0ca160ce42fb88627 Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Wed, 19 Jul 2023 20:45:20 -0400 Subject: [PATCH 05/10] Fix formatting --- app/client/cli/servicer_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/client/cli/servicer_test.go b/app/client/cli/servicer_test.go index e483124c9..2daf049a5 100644 --- a/app/client/cli/servicer_test.go +++ b/app/client/cli/servicer_test.go @@ -11,9 +11,9 @@ import ( ) const ( - testRelaychainEth = "ETH-Goerli" - testSessionHeight = 8 - testCurrentHeight = 9 + testRelaychainEth = "ETH-Goerli" + testSessionHeight = 8 + testCurrentHeight = 9 ) func TestGetSessionFromCache(t *testing.T) { From 6dc17860729450fe5afa02d985edff3c162d513f Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Thu, 20 Jul 2023 06:48:41 -0400 Subject: [PATCH 06/10] Address review comment --- app/client/cli/cache/session.go | 5 +++++ app/client/cli/servicer.go | 2 ++ 2 files changed, 7 insertions(+) diff --git a/app/client/cli/cache/session.go b/app/client/cli/cache/session.go index 29dde99b8..5220919c5 100644 --- a/app/client/cli/cache/session.go +++ b/app/client/cli/cache/session.go @@ -67,6 +67,11 @@ func (s *SessionCache) Set(session *rpc.Session) error { return nil } +// Stop call stop on the backing store. No calls should be made to Get or Set after calling Stop. +func (s *SessionCache) Stop() error { + return s.store.Stop() +} + // sessionKey returns a key to get/set a session, based on application's address and the relay chain. // // The height is not used as part of the key, because for each app+chain combination only one session, i.e. the current one, is of interest. diff --git a/app/client/cli/servicer.go b/app/client/cli/servicer.go index 5f0d46457..6bfd167ed 100644 --- a/app/client/cli/servicer.go +++ b/app/client/cli/servicer.go @@ -72,6 +72,8 @@ Will prompt the user for the *application* account passphrase`, Aliases: []string{}, Args: cobra.ExactArgs(4), RunE: func(cmd *cobra.Command, args []string) error { + defer sessionCache.Stop() + applicationAddr := args[0] servicerAddr := args[1] chain := args[2] From 93f801f14dce6bc082b82d6305731566ad47d839 Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Thu, 20 Jul 2023 07:06:19 -0400 Subject: [PATCH 07/10] Fix Stop error check --- app/client/cli/servicer.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/client/cli/servicer.go b/app/client/cli/servicer.go index 6bfd167ed..4734f6b4d 100644 --- a/app/client/cli/servicer.go +++ b/app/client/cli/servicer.go @@ -72,7 +72,11 @@ Will prompt the user for the *application* account passphrase`, Aliases: []string{}, Args: cobra.ExactArgs(4), RunE: func(cmd *cobra.Command, args []string) error { - defer sessionCache.Stop() + defer func() { + if err := sessionCache.Stop(); err != nil { + logger.Global.Warn().Err(err).Msg("failed to stop session cache") + } + }() applicationAddr := args[0] servicerAddr := args[1] From 88567b65c21c913f6c801b21ccbea5469f00810a Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Mon, 24 Jul 2023 10:00:01 -0400 Subject: [PATCH 08/10] Address review comments --- app/client/cli/cache/session.go | 23 +++++++++++++++-------- app/client/cli/cache/session_test.go | 2 +- app/client/cli/servicer.go | 6 +++--- app/client/cli/servicer_test.go | 11 +++++++---- 4 files changed, 26 insertions(+), 16 deletions(-) diff --git a/app/client/cli/cache/session.go b/app/client/cli/cache/session.go index 5220919c5..a7c6a7ac7 100644 --- a/app/client/cli/cache/session.go +++ b/app/client/cli/cache/session.go @@ -12,22 +12,29 @@ import ( var errSessionNotFound = errors.New("session not found in cache") -// SessionCache stores and retrieves sessions for application+relaychain pairs +// SessionCache defines the set of methods used to interact with the client-side session cache +type SessionCache interface { + Get(appAddr, chain string) (*rpc.Session, error) + Set(session *rpc.Session) error + Stop() error +} + +// sessionCache stores and retrieves sessions for application+relaychain pairs // // It uses a key-value store as backing storage -type SessionCache struct { +type sessionCache struct { // store is the local store for cached sessions store kvstore.KVStore } -// NewSessionCache returns a session cache backed by a kvstore using the provided database path. -func NewSessionCache(databasePath string) (*SessionCache, error) { +// Create returns a session cache backed by a kvstore using the provided database path. +func Create(databasePath string) (SessionCache, error) { store, err := kvstore.NewKVStore(databasePath) if err != nil { return nil, fmt.Errorf("Error initializing key-value store using path %s: %w", databasePath, err) } - return &SessionCache{ + return &sessionCache{ store: store, }, nil } @@ -36,7 +43,7 @@ func NewSessionCache(databasePath string) (*SessionCache, error) { // The caller is responsible to verify that the returned session is valid for the current block height. // Get is NOT safe to use concurrently // DISCUSS: do we need concurrency here? -func (s *SessionCache) Get(appAddr, chain string) (*rpc.Session, error) { +func (s *sessionCache) Get(appAddr, chain string) (*rpc.Session, error) { key := sessionKey(appAddr, chain) bz, err := s.store.Get(key) if err != nil { @@ -54,7 +61,7 @@ func (s *SessionCache) Get(appAddr, chain string) (*rpc.Session, error) { // Set stores the provided session in the cache with the key being the app+chain combination. // For each app+chain combination, a single session will be stored. Subsequent calls to Set will overwrite the entry for the provided app and chain. // Set is NOT safe to use concurrently -func (s *SessionCache) Set(session *rpc.Session) error { +func (s *sessionCache) Set(session *rpc.Session) error { bz, err := json.Marshal(*session) if err != nil { return fmt.Errorf("error marshalling session for app: %s, chain: %s, session height: %d: %w", session.Application.Address, session.Chain, session.SessionHeight, err) @@ -68,7 +75,7 @@ func (s *SessionCache) Set(session *rpc.Session) error { } // Stop call stop on the backing store. No calls should be made to Get or Set after calling Stop. -func (s *SessionCache) Stop() error { +func (s *sessionCache) Stop() error { return s.store.Stop() } diff --git a/app/client/cli/cache/session_test.go b/app/client/cli/cache/session_test.go index 4b5afbaec..aae4ef5ea 100644 --- a/app/client/cli/cache/session_test.go +++ b/app/client/cli/cache/session_test.go @@ -59,7 +59,7 @@ func TestGet(t *testing.T) { require.NoError(t, err) defer os.RemoveAll(dbPath) - cache, err := NewSessionCache(dbPath) + cache, err := Create(dbPath) require.NoError(t, err) for _, s := range tc.cacheContents { diff --git a/app/client/cli/servicer.go b/app/client/cli/servicer.go index 4734f6b4d..6a653ee6f 100644 --- a/app/client/cli/servicer.go +++ b/app/client/cli/servicer.go @@ -26,14 +26,14 @@ var ( errSessionNotFoundInCache = errors.New("session not found in cache") errNoMatchingSessionInCache = errors.New("no session matching the requested height found in cache") - sessionCache *cache.SessionCache + sessionCache cache.SessionCache ) func init() { rootCmd.AddCommand(NewServicerCommand()) var err error - sessionCache, err = cache.NewSessionCache(sessionCacheDBPath) + sessionCache, err = cache.Create(sessionCacheDBPath) if err != nil { logger.Global.Warn().Err(err).Msg("failed to initialize session cache") } @@ -142,7 +142,7 @@ func validateServicer(session *rpc.Session, servicerAddress string) (*rpc.Protoc } // getSessionFromCache uses the client-side session cache to fetch a session for app+chain combination at the provided height, if one has already been retrieved and cached. -func getSessionFromCache(c *cache.SessionCache, appAddress, chain string, height int64) (*rpc.Session, error) { +func getSessionFromCache(c cache.SessionCache, appAddress, chain string, height int64) (*rpc.Session, error) { if c == nil { return nil, errNoSessionCache } diff --git a/app/client/cli/servicer_test.go b/app/client/cli/servicer_test.go index 2daf049a5..fa67d5a8f 100644 --- a/app/client/cli/servicer_test.go +++ b/app/client/cli/servicer_test.go @@ -48,13 +48,14 @@ func TestGetSessionFromCache(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - var c *cache.SessionCache + var c cache.SessionCache + // prepare cache with test session for this unit test if len(tc.cachedSessions) > 0 { dbPath, err := os.MkdirTemp("", "cliCacheStoragePath") require.NoError(t, err) defer os.RemoveAll(dbPath) - c, err = cache.NewSessionCache(dbPath) + c, err = cache.Create(dbPath) require.NoError(t, err) for _, s := range tc.cachedSessions { @@ -71,6 +72,8 @@ func TestGetSessionFromCache(t *testing.T) { } func testSession(appAddr string, height int64) *rpc.Session { + const numSessionBlocks = 4 + return &rpc.Session{ Application: rpc.ProtocolActor{ ActorType: rpc.Application, @@ -78,8 +81,8 @@ func testSession(appAddr string, height int64) *rpc.Session { Chains: []string{testRelaychainEth}, }, Chain: testRelaychainEth, - NumSessionBlocks: 4, + NumSessionBlocks: numSessionBlocks, SessionHeight: height, - SessionNumber: 2, + SessionNumber: (height / numSessionBlocks), } } From d96baeb64a3dcf465b5b55b57dba6f665d0dbe7e Mon Sep 17 00:00:00 2001 From: Arash <23505281+adshmh@users.noreply.github.com> Date: Mon, 24 Jul 2023 17:17:00 -0400 Subject: [PATCH 09/10] Update app/client/cli/servicer_test.go Co-authored-by: Daniel Olshansky --- app/client/cli/servicer_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/client/cli/servicer_test.go b/app/client/cli/servicer_test.go index fa67d5a8f..566adb822 100644 --- a/app/client/cli/servicer_test.go +++ b/app/client/cli/servicer_test.go @@ -83,6 +83,6 @@ func testSession(appAddr string, height int64) *rpc.Session { Chain: testRelaychainEth, NumSessionBlocks: numSessionBlocks, SessionHeight: height, - SessionNumber: (height / numSessionBlocks), + SessionNumber: (height / numSessionBlocks), // assumes numSessionBlocks never changed } } From ad0be9a393dfcf24652f9e642285e19a9b39ef4e Mon Sep 17 00:00:00 2001 From: Arash Deshmeh Date: Mon, 24 Jul 2023 17:21:36 -0400 Subject: [PATCH 10/10] Address review comments --- app/client/cli/cache/session.go | 4 ++-- app/client/cli/cache/session_test.go | 2 +- app/client/cli/servicer.go | 2 +- app/client/cli/servicer_test.go | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/client/cli/cache/session.go b/app/client/cli/cache/session.go index a7c6a7ac7..6412459fa 100644 --- a/app/client/cli/cache/session.go +++ b/app/client/cli/cache/session.go @@ -27,8 +27,8 @@ type sessionCache struct { store kvstore.KVStore } -// Create returns a session cache backed by a kvstore using the provided database path. -func Create(databasePath string) (SessionCache, error) { +// NewSessionCache returns a session cache backed by a kvstore using the provided database path. +func NewSessionCache(databasePath string) (SessionCache, error) { store, err := kvstore.NewKVStore(databasePath) if err != nil { return nil, fmt.Errorf("Error initializing key-value store using path %s: %w", databasePath, err) diff --git a/app/client/cli/cache/session_test.go b/app/client/cli/cache/session_test.go index aae4ef5ea..4b5afbaec 100644 --- a/app/client/cli/cache/session_test.go +++ b/app/client/cli/cache/session_test.go @@ -59,7 +59,7 @@ func TestGet(t *testing.T) { require.NoError(t, err) defer os.RemoveAll(dbPath) - cache, err := Create(dbPath) + cache, err := NewSessionCache(dbPath) require.NoError(t, err) for _, s := range tc.cacheContents { diff --git a/app/client/cli/servicer.go b/app/client/cli/servicer.go index 6a653ee6f..0ed35dff4 100644 --- a/app/client/cli/servicer.go +++ b/app/client/cli/servicer.go @@ -33,7 +33,7 @@ func init() { rootCmd.AddCommand(NewServicerCommand()) var err error - sessionCache, err = cache.Create(sessionCacheDBPath) + sessionCache, err = cache.NewSessionCache(sessionCacheDBPath) if err != nil { logger.Global.Warn().Err(err).Msg("failed to initialize session cache") } diff --git a/app/client/cli/servicer_test.go b/app/client/cli/servicer_test.go index 566adb822..cd84a1e87 100644 --- a/app/client/cli/servicer_test.go +++ b/app/client/cli/servicer_test.go @@ -55,7 +55,7 @@ func TestGetSessionFromCache(t *testing.T) { require.NoError(t, err) defer os.RemoveAll(dbPath) - c, err = cache.Create(dbPath) + c, err = cache.NewSessionCache(dbPath) require.NoError(t, err) for _, s := range tc.cachedSessions {