addressing comments

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)

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,23 +38,26 @@ 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,
 	}
 }
 
 // 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)
 }

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)
+}

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
 }

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{