From ce65ac91606932a9b486d4c8ccc3c19a00a6eee3 Mon Sep 17 00:00:00 2001 From: Andrew Gouin Date: Mon, 11 Dec 2023 17:14:26 -0700 Subject: [PATCH] add unit tests for prune fix test --- signer/cosigner_nonce_cache.go | 53 ++++----- signer/cosigner_nonce_cache_test.go | 162 +++++++++++++++++++++++++++- 2 files changed, 185 insertions(+), 30 deletions(-) diff --git a/signer/cosigner_nonce_cache.go b/signer/cosigner_nonce_cache.go index a1b276a9..f9b34b89 100644 --- a/signer/cosigner_nonce_cache.go +++ b/signer/cosigner_nonce_cache.go @@ -32,7 +32,7 @@ type CosignerNonceCache struct { threshold uint8 - cache NonceCache + cache *NonceCache pruner NonceCachePruner @@ -112,6 +112,30 @@ func (nc *NonceCache) Delete(index int) { nc.cache = append(nc.cache[:index], nc.cache[index+1:]...) } +func (nc *NonceCache) PruneNonces() int { + nc.mu.Lock() + defer nc.mu.Unlock() + nonExpiredIndex := -1 + for i := 0; i < len(nc.cache); i++ { + if time.Now().Before(nc.cache[i].Expiration) { + nonExpiredIndex = i + break + } + } + + var deleteCount int + if nonExpiredIndex == -1 { + // No non-expired nonces, delete everything + deleteCount = len(nc.cache) + nc.cache = nil + } else { + // Prune everything up to the non-expired nonce + deleteCount = nonExpiredIndex + nc.cache = nc.cache[nonExpiredIndex:] + } + return deleteCount +} + type CosignerNoncesRel struct { Cosigner Cosigner Nonces CosignerNonces @@ -152,13 +176,14 @@ func NewCosignerNonceCache( nonceExpiration: nonceExpiration, threshold: threshold, pruner: pruner, + cache: new(NonceCache), // buffer up to 1000 empty events so that we don't ever block empty: make(chan struct{}, 1000), movingAverage: newMovingAverage(4 * getNoncesInterval), // weighted average over 4 intervals } // the only time pruner is expected to be non-nil is during tests, otherwise we use the cache logic. if pruner == nil { - cnc.pruner = cnc + cnc.pruner = cnc.cache } return cnc @@ -371,30 +396,6 @@ CheckNoncesLoop: return nil, fmt.Errorf("no nonces found involving cosigners %+v", cosignerInts) } -func (cnc *CosignerNonceCache) PruneNonces() int { - cnc.cache.mu.Lock() - defer cnc.cache.mu.Unlock() - nonExpiredIndex := -1 - for i := 0; i < len(cnc.cache.cache); i++ { - if time.Now().Before(cnc.cache.cache[i].Expiration) { - nonExpiredIndex = i - break - } - } - - 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 -} - func (cnc *CosignerNonceCache) ClearNonces(cosigner Cosigner) { cnc.cache.mu.Lock() defer cnc.cache.mu.Unlock() diff --git a/signer/cosigner_nonce_cache_test.go b/signer/cosigner_nonce_cache_test.go index efa9a201..ea65696a 100644 --- a/signer/cosigner_nonce_cache_test.go +++ b/signer/cosigner_nonce_cache_test.go @@ -64,6 +64,7 @@ func TestClearNonces(t *testing.T) { cnc := CosignerNonceCache{ threshold: 2, + cache: new(NonceCache), } for i := 0; i < 10; i++ { @@ -102,14 +103,14 @@ func TestClearNonces(t *testing.T) { } type mockPruner struct { - cnc *CosignerNonceCache + cache *NonceCache count int pruned int mu sync.Mutex } func (mp *mockPruner) PruneNonces() int { - pruned := mp.cnc.PruneNonces() + pruned := mp.cache.PruneNonces() mp.mu.Lock() defer mp.mu.Unlock() mp.count++ @@ -143,7 +144,7 @@ func TestNonceCacheDemand(t *testing.T) { mp, ) - mp.cnc = nonceCache + mp.cache = nonceCache.cache ctx, cancel := context.WithCancel(context.Background()) @@ -195,7 +196,7 @@ func TestNonceCacheExpiration(t *testing.T) { mp, ) - mp.cnc = nonceCache + mp.cache = nonceCache.cache ctx, cancel := context.WithCancel(context.Background()) @@ -230,6 +231,159 @@ func TestNonceCacheExpiration(t *testing.T) { require.LessOrEqual(t, nonceCache.cache.Size(), loadN) } +func TestNonceCachePrune(t *testing.T) { + type testCase struct { + name string + nonces []*CachedNonce + expected []*CachedNonce + } + + now := time.Now() + + testCases := []testCase{ + { + name: "no nonces", + nonces: nil, + expected: nil, + }, + { + name: "no expired nonces", + nonces: []*CachedNonce{ + { + UUID: uuid.MustParse("d6ef381f-6234-432d-b204-d8957fe60360"), + Expiration: now.Add(1 * time.Second), + }, + { + UUID: uuid.MustParse("cdc3673d-7946-459a-b458-cbbde0eecd04"), + Expiration: now.Add(2 * time.Second), + }, + { + UUID: uuid.MustParse("38c6a201-0b8b-46eb-ab69-c7b2716d408e"), + Expiration: now.Add(3 * time.Second), + }, + { + UUID: uuid.MustParse("5caf5ab2-d460-430f-87fa-8ed2983ae8fb"), + Expiration: now.Add(4 * time.Second), + }, + }, + expected: []*CachedNonce{ + { + UUID: uuid.MustParse("d6ef381f-6234-432d-b204-d8957fe60360"), + Expiration: now.Add(1 * time.Second), + }, + { + UUID: uuid.MustParse("cdc3673d-7946-459a-b458-cbbde0eecd04"), + Expiration: now.Add(2 * time.Second), + }, + { + UUID: uuid.MustParse("38c6a201-0b8b-46eb-ab69-c7b2716d408e"), + Expiration: now.Add(3 * time.Second), + }, + { + UUID: uuid.MustParse("5caf5ab2-d460-430f-87fa-8ed2983ae8fb"), + Expiration: now.Add(4 * time.Second), + }, + }, + }, + { + name: "first nonce is expired", + nonces: []*CachedNonce{ + { + UUID: uuid.MustParse("d6ef381f-6234-432d-b204-d8957fe60360"), + Expiration: now.Add(-1 * time.Second), + }, + { + UUID: uuid.MustParse("cdc3673d-7946-459a-b458-cbbde0eecd04"), + Expiration: now.Add(2 * time.Second), + }, + { + UUID: uuid.MustParse("38c6a201-0b8b-46eb-ab69-c7b2716d408e"), + Expiration: now.Add(3 * time.Second), + }, + { + UUID: uuid.MustParse("5caf5ab2-d460-430f-87fa-8ed2983ae8fb"), + Expiration: now.Add(4 * time.Second), + }, + }, + expected: []*CachedNonce{ + { + UUID: uuid.MustParse("cdc3673d-7946-459a-b458-cbbde0eecd04"), + Expiration: now.Add(2 * time.Second), + }, + { + UUID: uuid.MustParse("38c6a201-0b8b-46eb-ab69-c7b2716d408e"), + Expiration: now.Add(3 * time.Second), + }, + { + UUID: uuid.MustParse("5caf5ab2-d460-430f-87fa-8ed2983ae8fb"), + Expiration: now.Add(4 * time.Second), + }, + }, + }, + { + name: "all but last nonce expired", + nonces: []*CachedNonce{ + { + UUID: uuid.MustParse("d6ef381f-6234-432d-b204-d8957fe60360"), + Expiration: now.Add(-1 * time.Second), + }, + { + UUID: uuid.MustParse("cdc3673d-7946-459a-b458-cbbde0eecd04"), + Expiration: now.Add(-1 * time.Second), + }, + { + UUID: uuid.MustParse("38c6a201-0b8b-46eb-ab69-c7b2716d408e"), + Expiration: now.Add(-1 * time.Second), + }, + { + UUID: uuid.MustParse("5caf5ab2-d460-430f-87fa-8ed2983ae8fb"), + Expiration: now.Add(4 * time.Second), + }, + }, + expected: []*CachedNonce{ + { + UUID: uuid.MustParse("5caf5ab2-d460-430f-87fa-8ed2983ae8fb"), + Expiration: now.Add(4 * time.Second), + }, + }, + }, + { + name: "all nonces expired", + nonces: []*CachedNonce{ + { + UUID: uuid.MustParse("d6ef381f-6234-432d-b204-d8957fe60360"), + Expiration: now.Add(-1 * time.Second), + }, + { + UUID: uuid.MustParse("cdc3673d-7946-459a-b458-cbbde0eecd04"), + Expiration: now.Add(-1 * time.Second), + }, + { + UUID: uuid.MustParse("38c6a201-0b8b-46eb-ab69-c7b2716d408e"), + Expiration: now.Add(-1 * time.Second), + }, + { + UUID: uuid.MustParse("5caf5ab2-d460-430f-87fa-8ed2983ae8fb"), + Expiration: now.Add(-1 * time.Second), + }, + }, + expected: nil, + }, + } + + for _, tc := range testCases { + nc := NonceCache{ + cache: tc.nonces, + } + + pruned := nc.PruneNonces() + + require.Equal(t, len(tc.nonces)-len(tc.expected), pruned, tc.name) + + require.Equal(t, tc.expected, nc.cache, tc.name) + } +} + func TestNonceCacheDemandSlow(t *testing.T) { lcs, _ := getTestLocalCosigners(t, 2, 3) cosigners := make([]Cosigner, len(lcs))