diff --git a/signer/cosigner_nonce_cache.go b/signer/cosigner_nonce_cache.go index bc934289..a1b276a9 100644 --- a/signer/cosigner_nonce_cache.go +++ b/signer/cosigner_nonce_cache.go @@ -374,21 +374,23 @@ CheckNoncesLoop: func (cnc *CosignerNonceCache) PruneNonces() int { cnc.cache.mu.Lock() defer cnc.cache.mu.Unlock() - nonExpiredIndex := len(cnc.cache.cache) - 1 - for i := len(cnc.cache.cache) - 1; i >= 0; i-- { + nonExpiredIndex := -1 + for i := 0; i < len(cnc.cache.cache); i++ { if time.Now().Before(cnc.cache.cache[i].Expiration) { nonExpiredIndex = i break } - if i == 0 { - deleteCount := len(cnc.cache.cache) - cnc.cache.cache = nil - return deleteCount - } } - deleteCount := len(cnc.cache.cache) - nonExpiredIndex - 1 - if nonExpiredIndex != len(cnc.cache.cache)-1 { - cnc.cache.cache = cnc.cache.cache[:nonExpiredIndex+1] + + var deleteCount int + if nonExpiredIndex == -1 { + // No non-expired nonces, delete everything + deleteCount = len(cnc.cache.cache) + cnc.cache.cache = nil + } else { + // Prune everything up to the non-expired nonce + deleteCount = nonExpiredIndex + cnc.cache.cache = cnc.cache.cache[nonExpiredIndex:] } return deleteCount } diff --git a/signer/cosigner_nonce_cache_test.go b/signer/cosigner_nonce_cache_test.go index 756f5960..efa9a201 100644 --- a/signer/cosigner_nonce_cache_test.go +++ b/signer/cosigner_nonce_cache_test.go @@ -168,8 +168,8 @@ func TestNonceCacheDemand(t *testing.T) { count, pruned := mp.Result() - require.Greater(t, count, 0) - require.Equal(t, 0, pruned) + require.Greater(t, count, 0, "count of pruning calls must be greater than 0") + require.Equal(t, 0, pruned, "no nonces should have been pruned") } func TestNonceCacheExpiration(t *testing.T) { @@ -181,13 +181,16 @@ func TestNonceCacheExpiration(t *testing.T) { mp := &mockPruner{} + noncesExpiration := 1000 * time.Millisecond + getNoncesInterval := noncesExpiration / 5 + getNoncesTimeout := 10 * time.Millisecond nonceCache := NewCosignerNonceCache( cometlog.NewTMLogger(cometlog.NewSyncWriter(os.Stdout)), cosigners, &MockLeader{id: 1, leader: &ThresholdValidator{myCosigner: lcs[0]}}, - 250*time.Millisecond, - 10*time.Millisecond, - 500*time.Millisecond, + getNoncesInterval, + getNoncesTimeout, + noncesExpiration, 2, mp, ) @@ -196,27 +199,35 @@ func TestNonceCacheExpiration(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) - const loadN = 500 - + const loadN = 100 + // Load first set of 100 nonces nonceCache.LoadN(ctx, loadN) go nonceCache.Start(ctx) - time.Sleep(1 * time.Second) + // Sleep for 1/2 nonceExpiration, no nonces should have expired yet + time.Sleep(noncesExpiration / 2) + + // Load second set of 100 nonces + nonceCache.LoadN(ctx, loadN) + + // Wait for first set of nonces to expire + wait for the interval to have run + time.Sleep((noncesExpiration / 2) + getNoncesInterval) count, pruned := mp.Result() - // we should have pruned at least three times after - // waiting for a second with a reconcile interval of 250ms - require.GreaterOrEqual(t, count, 3) + // we should have pruned at least 5 times after + // waiting for 1200ms with a reconcile interval of 200ms + require.GreaterOrEqual(t, count, 5) - // we should have pruned at least the number of nonces we loaded and knew would expire - require.GreaterOrEqual(t, pruned, loadN) + // we should have pruned only the first set of nonces + // The second set of nonces should not have expired yet and we should not have load any more + require.Equal(t, pruned, loadN) cancel() - // the cache should be empty or 1 since no nonces are being consumed. - require.LessOrEqual(t, nonceCache.cache.Size(), 1) + // the cache should be 100 (loadN) as the second set should not have expired. + require.LessOrEqual(t, nonceCache.cache.Size(), loadN) } func TestNonceCacheDemandSlow(t *testing.T) {