diff --git a/commit/factory.go b/commit/factory.go index 12ad57c49..d40cdc470 100644 --- a/commit/factory.go +++ b/commit/factory.go @@ -231,7 +231,7 @@ func (p *PluginFactory) NewReportingPlugin(ctx context.Context, config ocr3types offchainConfig.TokenInfo, ccipReader, offchainConfig.PriceFeedChainSelector, - cache.NewCache[string, uint8](cache.NeverExpirePolicy{}), + cache.NewInMemoryCache[string, uint8](cache.NewNeverExpirePolicy()), ) metricsReporter, err := metrics.NewPromReporter(lggr, p.ocrConfig.Config.ChainSelector) diff --git a/internal/cache/cache.go b/internal/cache/cache.go index c6f85e7c3..a933e94dd 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -8,11 +8,16 @@ import ( // Cache defines the interface for cache operations type Cache[K comparable, V any] interface { // Get retrieves a value from the cache + // Returns the value and true if found, zero value and false if not found Get(key K) (V, bool) + // Set adds or updates a value in the cache - Set(key K, value V) + // Returns true if a new entry was created, false if an existing entry was updated + Set(key K, value V) bool + // Delete removes a value from the cache - Delete(key K) + // Returns true if an entry was deleted, false if the key wasn't found + Delete(key K) bool } // EvictionPolicy defines how entries should be evicted from the cache @@ -33,8 +38,9 @@ type inMemoryCache[K comparable, V any] struct { mutex sync.RWMutex } -// NewCache creates a new cache with the specified eviction policy -func NewCache[K comparable, V any](policy EvictionPolicy) Cache[K, V] { +// NewInMemoryCache creates a new cache with the specified eviction policy +// The cache is thread-safe and can be used concurrently +func NewInMemoryCache[K comparable, V any](policy EvictionPolicy) Cache[K, V] { return &inMemoryCache[K, V]{ data: make(map[K]*cacheEntry), policy: policy, @@ -42,14 +48,16 @@ func NewCache[K comparable, V any](policy EvictionPolicy) Cache[K, V] { } // Set adds or updates a value in the cache -func (c *inMemoryCache[K, V]) Set(key K, value V) { +func (c *inMemoryCache[K, V]) Set(key K, value V) bool { c.mutex.Lock() defer c.mutex.Unlock() + _, exists := c.data[key] c.data[key] = &cacheEntry{ value: value, createdAt: time.Now(), } + return !exists } // Get retrieves a value from the cache @@ -68,46 +76,60 @@ func (c *inMemoryCache[K, V]) Get(key K) (V, bool) { return zero, false } - return entry.value.(V), true + value, ok := entry.value.(V) + if !ok { + var zero V + return zero, false + } + + return value, true } // Delete removes a value from the cache -func (c *inMemoryCache[K, V]) Delete(key K) { +func (c *inMemoryCache[K, V]) Delete(key K) bool { c.mutex.Lock() defer c.mutex.Unlock() - delete(c.data, key) + _, exists := c.data[key] + if exists { + delete(c.data, key) + } + return exists } -// NeverExpirePolicy implements EvictionPolicy for entries that should never expire -type NeverExpirePolicy struct{} +// neverExpirePolicy implements EvictionPolicy for entries that should never expire +type neverExpirePolicy struct{} + +func NewNeverExpirePolicy() EvictionPolicy { + return neverExpirePolicy{} +} -func (p NeverExpirePolicy) ShouldEvict(_ *cacheEntry) bool { +func (p neverExpirePolicy) ShouldEvict(_ *cacheEntry) bool { return false } -// TimeBasedPolicy implements EvictionPolicy for time-based expiration -type TimeBasedPolicy struct { +// timeBasedPolicy implements EvictionPolicy for time-based expiration +type timeBasedPolicy struct { ttl time.Duration } -func NewTimeBasedPolicy(ttl time.Duration) *TimeBasedPolicy { - return &TimeBasedPolicy{ttl: ttl} +func NewTimeBasedPolicy(ttl time.Duration) EvictionPolicy { + return &timeBasedPolicy{ttl: ttl} } -func (p TimeBasedPolicy) ShouldEvict(entry *cacheEntry) bool { +func (p timeBasedPolicy) ShouldEvict(entry *cacheEntry) bool { return time.Since(entry.createdAt) > p.ttl } -// CustomPolicy implements EvictionPolicy with a custom eviction function -type CustomPolicy struct { +// customPolicy implements EvictionPolicy with a custom eviction function +type customPolicy struct { evictFunc func(interface{}) bool } -func NewCustomPolicy(evictFunc func(interface{}) bool) *CustomPolicy { - return &CustomPolicy{evictFunc: evictFunc} +func NewCustomPolicy(evictFunc func(interface{}) bool) EvictionPolicy { + return &customPolicy{evictFunc: evictFunc} } -func (p CustomPolicy) ShouldEvict(entry *cacheEntry) bool { +func (p customPolicy) ShouldEvict(entry *cacheEntry) bool { return p.evictFunc(entry.value) } diff --git a/internal/cache/keys/keys.go b/internal/cache/keys/keys.go new file mode 100644 index 000000000..317259808 --- /dev/null +++ b/internal/cache/keys/keys.go @@ -0,0 +1,12 @@ +package cachekeys + +import ( + "fmt" + + "github.com/smartcontractkit/chainlink-ccip/pkg/types/ccipocr3" +) + +// TokenDecimals creates a cache key for token decimals +func TokenDecimals(token ccipocr3.UnknownEncodedAddress, address string) string { + return fmt.Sprintf("token-decimals:%s:%s", token, address) +} diff --git a/mocks/internal_/cache/cache.go b/mocks/internal_/cache/cache.go index 2ce091547..603330b71 100644 --- a/mocks/internal_/cache/cache.go +++ b/mocks/internal_/cache/cache.go @@ -18,8 +18,21 @@ func (_m *MockCache[K, V]) EXPECT() *MockCache_Expecter[K, V] { } // Delete provides a mock function with given fields: key -func (_m *MockCache[K, V]) Delete(key K) { - _m.Called(key) +func (_m *MockCache[K, V]) Delete(key K) bool { + ret := _m.Called(key) + + if len(ret) == 0 { + panic("no return value specified for Delete") + } + + var r0 bool + if rf, ok := ret.Get(0).(func(K) bool); ok { + r0 = rf(key) + } else { + r0 = ret.Get(0).(bool) + } + + return r0 } // MockCache_Delete_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Delete' @@ -40,12 +53,12 @@ func (_c *MockCache_Delete_Call[K, V]) Run(run func(key K)) *MockCache_Delete_Ca return _c } -func (_c *MockCache_Delete_Call[K, V]) Return() *MockCache_Delete_Call[K, V] { - _c.Call.Return() +func (_c *MockCache_Delete_Call[K, V]) Return(_a0 bool) *MockCache_Delete_Call[K, V] { + _c.Call.Return(_a0) return _c } -func (_c *MockCache_Delete_Call[K, V]) RunAndReturn(run func(K)) *MockCache_Delete_Call[K, V] { +func (_c *MockCache_Delete_Call[K, V]) RunAndReturn(run func(K) bool) *MockCache_Delete_Call[K, V] { _c.Call.Return(run) return _c } @@ -107,8 +120,21 @@ func (_c *MockCache_Get_Call[K, V]) RunAndReturn(run func(K) (V, bool)) *MockCac } // Set provides a mock function with given fields: key, value -func (_m *MockCache[K, V]) Set(key K, value V) { - _m.Called(key, value) +func (_m *MockCache[K, V]) Set(key K, value V) bool { + ret := _m.Called(key, value) + + if len(ret) == 0 { + panic("no return value specified for Set") + } + + var r0 bool + if rf, ok := ret.Get(0).(func(K, V) bool); ok { + r0 = rf(key, value) + } else { + r0 = ret.Get(0).(bool) + } + + return r0 } // MockCache_Set_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Set' @@ -130,12 +156,12 @@ func (_c *MockCache_Set_Call[K, V]) Run(run func(key K, value V)) *MockCache_Set return _c } -func (_c *MockCache_Set_Call[K, V]) Return() *MockCache_Set_Call[K, V] { - _c.Call.Return() +func (_c *MockCache_Set_Call[K, V]) Return(_a0 bool) *MockCache_Set_Call[K, V] { + _c.Call.Return(_a0) return _c } -func (_c *MockCache_Set_Call[K, V]) RunAndReturn(run func(K, V)) *MockCache_Set_Call[K, V] { +func (_c *MockCache_Set_Call[K, V]) RunAndReturn(run func(K, V) bool) *MockCache_Set_Call[K, V] { _c.Call.Return(run) return _c } diff --git a/pkg/reader/price_reader.go b/pkg/reader/price_reader.go index 313c16a6c..c8fc19917 100644 --- a/pkg/reader/price_reader.go +++ b/pkg/reader/price_reader.go @@ -11,6 +11,8 @@ import ( commontypes "github.com/smartcontractkit/chainlink-common/pkg/types" "github.com/smartcontractkit/chainlink-common/pkg/types/query/primitives" + cachekeys "github.com/smartcontractkit/chainlink-ccip/internal/cache/keys" + "github.com/smartcontractkit/chainlink-ccip/internal/cache" typeconv "github.com/smartcontractkit/chainlink-ccip/internal/libs/typeconv" "github.com/smartcontractkit/chainlink-ccip/internal/plugintypes" @@ -195,7 +197,7 @@ func (pr *priceReader) getFeedDecimals( feedChainReader contractreader.ContractReaderFacade, ) (uint8, error) { // Create cache key using token and contract address - cacheKey := string(token) + "-" + boundContract.Address + cacheKey := cachekeys.TokenDecimals(token, boundContract.Address) // Try to get from cache first if decimals, found := pr.decimalsCache.Get(cacheKey); found { @@ -219,12 +221,13 @@ func (pr *priceReader) getFeedDecimals( return 0, fmt.Errorf("decimals call failed for token %s: %w", token, err) } - // Store in cache - pr.decimalsCache.Set(cacheKey, decimals) + // Store in cache and log whether it was a new entry + isNew := pr.decimalsCache.Set(cacheKey, decimals) pr.lggr.Debugw("Cached token decimals", "token", token, "contract", boundContract.Address, - "decimals", decimals) + "decimals", decimals, + "isNewEntry", isNew) return decimals, nil } diff --git a/pkg/reader/price_reader_test.go b/pkg/reader/price_reader_test.go index 17e79acab..9860b34b0 100644 --- a/pkg/reader/price_reader_test.go +++ b/pkg/reader/price_reader_test.go @@ -97,7 +97,7 @@ func TestOnchainTokenPricesReader_GetTokenPricesUSD(t *testing.T) { // Setup cache expectations - it should return cache miss for any key mockCache.On("Get", mock.Anything).Return(uint8(0), false) // Expect Set to be called for each successful decimal fetch - mockCache.On("Set", mock.Anything, mock.Anything).Return() + mockCache.On("Set", mock.Anything, mock.Anything).Return(false) feedChain := cciptypes.ChainSelector(1) tokenPricesReader := priceReader{