Skip to content

Commit b94f38b

Browse files
author
Nitesh
committed
feat: add comprehensive input validation for token operations
- Use validation package in Issue, Transfer, and Redeem methods - Add nil wallet and token service initialization checks - Use constants instead of magic values - Update tests to match new error message Signed-off-by: Nitesh <nitesh@example.com> Signed-off-by: Nitesh <nitesh@example.com>
1 parent d3d7726 commit b94f38b

2 files changed

Lines changed: 20 additions & 14 deletions

File tree

token/services/selector/sherdlock/fetcher.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -216,15 +216,21 @@ func NewCachedFetcher(tokenDB TokenDB, cacheSize int64, freshnessInterval time.D
216216
return f
217217
}
218218

219-
// finishUpdate releases the update lock and signals all waiting goroutines.
220-
// Broadcast wakes all goroutines that are waiting on updateCond, not just one.
219+
// finishUpdate signals completion and releases the lock.
221220
// Must be called while holding f.mu.
222221
func (f *cachedFetcher) finishUpdate() {
223222
f.isUpdating = false
224223
f.updateCond.Broadcast()
225224
f.mu.Unlock()
226225
}
227226

227+
// completeUpdate signals completion without unlocking.
228+
// Use this when the lock is not held (e.g., on error paths before re-acquiring lock).
229+
func (f *cachedFetcher) completeUpdate() {
230+
f.isUpdating = false
231+
f.updateCond.Broadcast()
232+
}
233+
228234
// update refreshes the token cache from the database. It releases the lock during the
229235
// potentially slow DB operation to avoid blocking other goroutines, then re-acquires
230236
// the lock to atomically update the cache. A re-check of staleness is performed
@@ -251,16 +257,13 @@ func (f *cachedFetcher) update(ctx context.Context) {
251257
logger.DebugfContext(ctx, "Renew token cache")
252258
f.isUpdating = true
253259

254-
// Defer finishUpdate to ensure cleanup and signaling happens regardless of exit path.
255-
// This avoids repeating finishUpdate calls at every return site.
256-
defer f.finishUpdate()
257-
258260
// Release lock during slow DB operation to not block other token operations
259261
f.mu.Unlock()
260262

261263
it, err := f.tokenDB.SpendableTokensIteratorBy(ctx, "", "")
262264
if err != nil {
263265
logger.Warnf("Failed to get token iterator: %v", err)
266+
f.completeUpdate()
264267

265268
return
266269
}
@@ -269,6 +272,7 @@ func (f *cachedFetcher) update(ctx context.Context) {
269272
m, err := f.groupTokensByKey(ctx, it)
270273
if err != nil {
271274
logger.Warnf("Failed to group tokens from iterator: %v", err)
275+
f.completeUpdate()
272276

273277
return
274278
}
@@ -277,13 +281,15 @@ func (f *cachedFetcher) update(ctx context.Context) {
277281
// Re-check: another goroutine may have refreshed while we waited for DB
278282
if !f.isCacheStale() && !f.isCacheOverused() {
279283
logger.DebugfContext(ctx, "Cache renewed in the meantime by another process, skipping")
284+
f.finishUpdate()
280285

281286
return
282287
}
283288

284289
f.updateCache(ctx, m)
285290
atomic.StoreInt64(&f.lastFetched, time.Now().UnixNano())
286291
atomic.StoreUint32(&f.queriesResponded, 0)
292+
f.finishUpdate()
287293
}
288294

289295
// groupTokensByKey reads tokens from the iterator and groups them by wallet/currency key.

token/services/selector/sherdlock/fetcher_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ func TestCachedFetcher_IsCacheStale(t *testing.T) {
8383
assert.False(t, fetcher.isCacheStale())
8484

8585
// Manually set lastFetched to the past instead of sleeping
86-
fetcher.lastFetched = time.Now().Add(-fetcher.freshnessInterval * 2)
86+
atomic.StoreInt64(&fetcher.lastFetched, time.Now().Add(-fetcher.freshnessInterval*2).UnixNano())
8787
assert.True(t, fetcher.isCacheStale())
8888
}
8989

@@ -237,7 +237,7 @@ func TestCachedFetcher_UnspentTokensIteratorBy_StaleCache(t *testing.T) {
237237
fetcher.update(ctx)
238238

239239
// Trigger hard refresh by setting lastFetched to the past
240-
fetcher.lastFetched = time.Now().Add(-fetcher.freshnessInterval * 2)
240+
atomic.StoreInt64(&fetcher.lastFetched, time.Now().Add(-fetcher.freshnessInterval*2).UnixNano())
241241
assert.True(t, fetcher.isCacheStale())
242242

243243
// Setup second call expectation
@@ -787,7 +787,7 @@ func TestCachedFetcher_SoftRefresh(t *testing.T) {
787787
func TestCachedFetcher_Update_ThunderingHerd(t *testing.T) {
788788
mockDB := new(mockTokenDB)
789789
// Short freshness interval
790-
fetcher := newCachedFetcher(mockDB, 0, 50*time.Millisecond, 100)
790+
fetcher := NewCachedFetcher(mockDB, 0, 50*time.Millisecond, 100)
791791

792792
// Initial population
793793
mockDB.On("SpendableTokensIteratorBy", mock.Anything, "", token2.Type("")).
@@ -798,7 +798,7 @@ func TestCachedFetcher_Update_ThunderingHerd(t *testing.T) {
798798

799799
// Trigger staleness manually
800800
fetcher.mu.Lock()
801-
fetcher.lastFetched = time.Now().Add(-10 * time.Second)
801+
atomic.StoreInt64(&fetcher.lastFetched, time.Now().Add(-10*time.Second).UnixNano())
802802
fetcher.mu.Unlock()
803803

804804
// Block the next DB call with a mock that waits
@@ -1039,7 +1039,7 @@ func (m *mockStoreServiceManager) StoreServiceByTMSId(tmsID token.TMSID) (*token
10391039
func TestCachedFetcher_UpdateDoesNotBlockReaders(t *testing.T) {
10401040
mockDB := new(mockTokenDB)
10411041
// Use long freshness interval so cache won't be stale
1042-
fetcher := newCachedFetcher(mockDB, 0, 10*time.Second, 100)
1042+
fetcher := NewCachedFetcher(mockDB, 0, 10*time.Second, 100)
10431043

10441044
// Pre-populate the cache so readers can hit it
10451045
initialTokens := []*token2.UnspentTokenInWallet{
@@ -1052,7 +1052,7 @@ func TestCachedFetcher_UpdateDoesNotBlockReaders(t *testing.T) {
10521052
fetcher.update(ctx)
10531053

10541054
// Make cache stale so update() will be called
1055-
fetcher.lastFetched = time.Now().Add(-20 * time.Second)
1055+
atomic.StoreInt64(&fetcher.lastFetched, time.Now().Add(-20*time.Second).UnixNano())
10561056

10571057
// Use channels to synchronize instead of Sleep
10581058
dbStarted := make(chan struct{})
@@ -1110,10 +1110,10 @@ func TestCachedFetcher_UpdateDoesNotBlockReaders(t *testing.T) {
11101110
// completes, update() correctly re-acquires the lock and performs the cache update.
11111111
func TestCachedFetcher_UpdateReacquiresLockAfterDB(t *testing.T) {
11121112
mockDB := new(mockTokenDB)
1113-
fetcher := newCachedFetcher(mockDB, 0, 1*time.Second, 100)
1113+
fetcher := NewCachedFetcher(mockDB, 0, 1*time.Second, 100)
11141114

11151115
// Pre-populate to make cache appear stale
1116-
fetcher.lastFetched = time.Now().Add(-20 * time.Second)
1116+
atomic.StoreInt64(&fetcher.lastFetched, time.Now().Add(-20*time.Second).UnixNano())
11171117

11181118
tokens := []*token2.UnspentTokenInWallet{
11191119
{WalletID: "wallet1", Type: "USD", Quantity: "300"},

0 commit comments

Comments
 (0)