From 8742b9166a453a142d1f393404b4506829fdc227 Mon Sep 17 00:00:00 2001 From: Jonathan Weiss Date: Thu, 15 Sep 2022 10:30:21 +0300 Subject: [PATCH 01/25] fix: stateprf deletes keys after stateproof advances. --- stateproof/signer.go | 44 ++++++++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/stateproof/signer.go b/stateproof/signer.go index 02749890a3..394d23bb61 100644 --- a/stateproof/signer.go +++ b/stateproof/signer.go @@ -40,6 +40,8 @@ type sigFromAddr struct { func (spw *Worker) signer(latest basics.Round) { nextRnd := spw.nextStateProofRound(latest) + // at this point there isn't any known stateproof by the signer, set as 0 to ensure no keys will be deleted. + prevStateProof := basics.Round(0) for { // Start signing StateProofs from nextRnd onwards select { case <-spw.ledger.Wait(nextRnd): @@ -54,6 +56,12 @@ func (spw *Worker) signer(latest basics.Round) { spw.invokeBuilder(nextRnd) nextRnd++ + nxtstateProof := hdr.StateProofTracking[protocol.StateProofBasic].StateProofNextRound + if nxtstateProof > prevStateProof { + spw.attemptKeyDeletionPriorToRound(prevStateProof) + prevStateProof = nxtstateProof + } + case <-spw.ctx.Done(): spw.wg.Done() return @@ -61,6 +69,27 @@ func (spw *Worker) signer(latest basics.Round) { } } +func (spw *Worker) attemptKeyDeletionPriorToRound(prevStateproofRound basics.Round) { + for _, key := range spw.accts.StateProofKeys(prevStateproofRound) { + if key.ParticipationRecord.StateProof == nil { + continue + } + + FirstRoundInKeyLifetime, err := key.ParticipationRecord.StateProof.FirstRoundInKeyLifetime(uint64(prevStateproofRound)) + if err != nil { + continue + } + + if basics.Round(FirstRoundInKeyLifetime+key.StateProof.KeyLifetime) >= prevStateproofRound { + continue + } + + if err := spw.accts.DeleteStateProofKey(key.ParticipationID, prevStateproofRound-1); err != nil { + spw.log.Warnf("spw.signBlock(%d): Couldn't delete StateProof keys: %v", prevStateproofRound, err) + } + } +} + func (spw *Worker) nextStateProofRound(latest basics.Round) basics.Round { var nextrnd basics.Round @@ -160,25 +189,12 @@ func (spw *Worker) signStateProof(hdr bookkeeping.BlockHeader) { } // any error in handle sig indicates the signature wasn't stored in disk, thus we cannot delete the key. - for i, sfa := range sigs { + for _, sfa := range sigs { if _, err := spw.handleSig(sfa, nil); err != nil { spw.log.Warnf("spw.signBlock(%d): handleSig: %v", hdr.Round, err) continue } spw.log.Infof("spw.signBlock(%d): sp message was signed with address %v", hdr.Round, sfa.SignerAddress) - firstRoundInKeyLifetime, err := usedSigners[i].FirstRoundInKeyLifetime() // Calculate first round of the key in order to delete all previous keys (and keep the current one for now) - if err != nil { - spw.log.Warnf("spw.signBlock(%d): Signer.FirstRoundInKeyLifetime: %v", hdr.Round, err) - continue - } - if firstRoundInKeyLifetime == 0 { - continue // No previous keys to delete (also underflows when subtracting 1) - } - - // Safe to delete key for sfa.Round because the signature is now stored in the disk. - if err := spw.accts.DeleteStateProofKey(ids[i], basics.Round(firstRoundInKeyLifetime-1)); err != nil { // Subtract 1 to delete all keys up to this one - spw.log.Warnf("spw.signBlock(%d): DeleteStateProofKey: %v", hdr.Round, err) - } } } From b91d71c674929b3750904afedbcd5bb92fd62c23 Mon Sep 17 00:00:00 2001 From: Jonathan Weiss Date: Thu, 15 Sep 2022 13:02:18 +0300 Subject: [PATCH 02/25] fixed unit test to match the current code of key deletion --- stateproof/signer.go | 14 +++----------- stateproof/worker_test.go | 16 ++++++++++++++-- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/stateproof/signer.go b/stateproof/signer.go index 394d23bb61..d6fb522ba3 100644 --- a/stateproof/signer.go +++ b/stateproof/signer.go @@ -71,20 +71,12 @@ func (spw *Worker) signer(latest basics.Round) { func (spw *Worker) attemptKeyDeletionPriorToRound(prevStateproofRound basics.Round) { for _, key := range spw.accts.StateProofKeys(prevStateproofRound) { - if key.ParticipationRecord.StateProof == nil { + keyContext := key.StateProofSecrets.SignerContext + if basics.Round(keyContext.FirstValid+keyContext.KeyLifetime) > prevStateproofRound { continue } - FirstRoundInKeyLifetime, err := key.ParticipationRecord.StateProof.FirstRoundInKeyLifetime(uint64(prevStateproofRound)) - if err != nil { - continue - } - - if basics.Round(FirstRoundInKeyLifetime+key.StateProof.KeyLifetime) >= prevStateproofRound { - continue - } - - if err := spw.accts.DeleteStateProofKey(key.ParticipationID, prevStateproofRound-1); err != nil { + if err := spw.accts.DeleteStateProofKey(key.ParticipationID, prevStateproofRound); err != nil { spw.log.Warnf("spw.signBlock(%d): Couldn't delete StateProof keys: %v", prevStateproofRound, err) } } diff --git a/stateproof/worker_test.go b/stateproof/worker_test.go index c412faddaf..97fa5dbf39 100644 --- a/stateproof/worker_test.go +++ b/stateproof/worker_test.go @@ -246,6 +246,11 @@ func (s *testWorkerStubs) advanceLatest(delta uint64) { defer s.mu.Unlock() for r := uint64(0); r < delta; r++ { + interval := config.Consensus[s.blocks[s.latest].CurrentProtocol].StateProofInterval + if r%interval == 0 { + s.addBlock(s.blocks[s.latest].StateProofTracking[protocol.StateProofBasic].StateProofNextRound + basics.Round(interval)) + continue + } s.addBlock(s.blocks[s.latest].StateProofTracking[protocol.StateProofBasic].StateProofNextRound) } } @@ -570,12 +575,19 @@ func TestSignerDeletesUnneededStateProofKeys(t *testing.T) { defer w.Shutdown() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatest(3 * proto.StateProofInterval) + s.advanceLatest(1 * proto.StateProofInterval) // going to rnd 256 // Expect all signatures to be broadcast. require.Zero(t, s.GetNumDeletedKeys()) w.signStateProof(s.blocks[basics.Round(proto.StateProofInterval)]) - require.Equal(t, s.GetNumDeletedKeys(), nParticipants) + s.advanceLatest(2 * proto.StateProofInterval) // advancing rounds up to 768 + + // chose 513 because that is the next round, and the signer must've passed through the deletion function by now. + for w.lastSignedBlock() < 513 { + time.Sleep(time.Millisecond * 100) + } + + require.Equal(t, nParticipants, s.GetNumDeletedKeys()) } func TestSignerDoesntDeleteKeysWhenDBDoesntStoreSigs(t *testing.T) { From beb794a3b468c2a25526df5eb6159d5d2767039a Mon Sep 17 00:00:00 2001 From: Jonathan Weiss Date: Thu, 15 Sep 2022 13:20:59 +0300 Subject: [PATCH 03/25] fix: all unit tests pass, added TODO --- stateproof/worker_test.go | 48 +++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/stateproof/worker_test.go b/stateproof/worker_test.go index 97fa5dbf39..42b5a6650a 100644 --- a/stateproof/worker_test.go +++ b/stateproof/worker_test.go @@ -241,7 +241,17 @@ func (s *testWorkerStubs) BroadcastInternalSignedTxGroup(tx []transactions.Signe func (s *testWorkerStubs) RegisterHandlers([]network.TaggedMessageHandler) { } -func (s *testWorkerStubs) advanceLatest(delta uint64) { +// TODO: understand why using the other function fails multiple tests. +func (s *testWorkerStubs) advanceLatestWithoutStateProof(delta uint64) { + s.mu.Lock() + defer s.mu.Unlock() + + for r := uint64(0); r < delta; r++ { + s.addBlock(s.blocks[s.latest].StateProofTracking[protocol.StateProofBasic].StateProofNextRound) + } +} + +func (s *testWorkerStubs) advanceLatestAndStateProofs(delta uint64) { s.mu.Lock() defer s.mu.Unlock() @@ -327,12 +337,12 @@ func TestWorkerAllSigs(t *testing.T) { defer w.Shutdown() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatest(proto.StateProofInterval + proto.StateProofInterval/2) + s.advanceLatestWithoutStateProof(proto.StateProofInterval + proto.StateProofInterval/2) // Go through several iterations, making sure that we get // the signatures and certs broadcast at each round. for iter := 0; iter < 5; iter++ { - s.advanceLatest(proto.StateProofInterval) + s.advanceLatestWithoutStateProof(proto.StateProofInterval) for i := 0; i < len(keys); i++ { // Expect all signatures to be broadcast. @@ -396,8 +406,8 @@ func TestWorkerPartialSigs(t *testing.T) { defer w.Shutdown() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatest(proto.StateProofInterval + proto.StateProofInterval/2) - s.advanceLatest(proto.StateProofInterval) + s.advanceLatestWithoutStateProof(proto.StateProofInterval + proto.StateProofInterval/2) + s.advanceLatestWithoutStateProof(proto.StateProofInterval) for i := 0; i < len(keys); i++ { // Expect all signatures to be broadcast. @@ -413,7 +423,7 @@ func TestWorkerPartialSigs(t *testing.T) { } // Expect a state proof to be formed in the next StateProofInterval/2. - s.advanceLatest(proto.StateProofInterval / 2) + s.advanceLatestWithoutStateProof(proto.StateProofInterval / 2) tx, err := s.waitOnTxnWithTimeout(time.Second * 5) require.NoError(t, err) @@ -461,7 +471,7 @@ func TestWorkerInsufficientSigs(t *testing.T) { defer w.Shutdown() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatest(3 * proto.StateProofInterval) + s.advanceLatestWithoutStateProof(3 * proto.StateProofInterval) for i := 0; i < len(keys); i++ { // Expect all signatures to be broadcast. @@ -493,7 +503,7 @@ func TestWorkerRestart(t *testing.T) { s := newWorkerStubs(t, keys, 10) proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatest(3*proto.StateProofInterval - 1) + s.advanceLatestWithoutStateProof(3*proto.StateProofInterval - 1) dbRand := crypto.RandUint64() @@ -539,7 +549,7 @@ func TestWorkerHandleSig(t *testing.T) { defer w.Shutdown() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatest(3 * proto.StateProofInterval) + s.advanceLatestWithoutStateProof(3 * proto.StateProofInterval) for i := 0; i < len(keys); i++ { // Expect all signatures to be broadcast. @@ -575,12 +585,12 @@ func TestSignerDeletesUnneededStateProofKeys(t *testing.T) { defer w.Shutdown() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatest(1 * proto.StateProofInterval) // going to rnd 256 + s.advanceLatestAndStateProofs(1 * proto.StateProofInterval) // going to rnd 256 // Expect all signatures to be broadcast. require.Zero(t, s.GetNumDeletedKeys()) w.signStateProof(s.blocks[basics.Round(proto.StateProofInterval)]) - s.advanceLatest(2 * proto.StateProofInterval) // advancing rounds up to 768 + s.advanceLatestAndStateProofs(2 * proto.StateProofInterval) // advancing rounds up to 768 // chose 513 because that is the next round, and the signer must've passed through the deletion function by now. for w.lastSignedBlock() < 513 { @@ -613,7 +623,7 @@ func TestSignerDoesntDeleteKeysWhenDBDoesntStoreSigs(t *testing.T) { w.Start() defer w.Shutdown() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatest(3 * proto.StateProofInterval) + s.advanceLatestWithoutStateProof(3 * proto.StateProofInterval) // Expect all signatures to be broadcast. require.NoError(t, w.db.Atomic( @@ -647,10 +657,10 @@ func TestWorkerRemoveBuildersAndSignatures(t *testing.T) { defer w.Shutdown() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatest(proto.StateProofInterval + proto.StateProofInterval/2) + s.advanceLatestWithoutStateProof(proto.StateProofInterval + proto.StateProofInterval/2) for iter := 0; iter < expectedStateProofs; iter++ { - s.advanceLatest(proto.StateProofInterval) + s.advanceLatestWithoutStateProof(proto.StateProofInterval) tx := <-s.txmsg a.Equal(tx.Txn.Type, protocol.StateProofTx) } @@ -709,10 +719,10 @@ func TestWorkerBuildersRecoveryLimit(t *testing.T) { w.Start() defer w.Shutdown() - s.advanceLatest(proto.StateProofInterval + proto.StateProofInterval/2) + s.advanceLatestWithoutStateProof(proto.StateProofInterval + proto.StateProofInterval/2) for iter := uint64(0); iter < proto.StateProofMaxRecoveryIntervals+1; iter++ { - s.advanceLatest(proto.StateProofInterval) + s.advanceLatestWithoutStateProof(proto.StateProofInterval) tx := <-s.txmsg a.Equal(tx.Txn.Type, protocol.StateProofTx) } @@ -742,7 +752,7 @@ func TestWorkerBuildersRecoveryLimit(t *testing.T) { }) a.Equal(proto.StateProofMaxRecoveryIntervals+1, uint64(len(roundSigs))) - s.advanceLatest(proto.StateProofInterval) + s.advanceLatestWithoutStateProof(proto.StateProofInterval) tx := <-s.txmsg a.Equal(tx.Txn.Type, protocol.StateProofTx) @@ -945,9 +955,9 @@ func TestBuilderGeneratesValidStateProofTXN(t *testing.T) { defer w.Shutdown() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatest(proto.StateProofInterval + proto.StateProofInterval/2) + s.advanceLatestWithoutStateProof(proto.StateProofInterval + proto.StateProofInterval/2) - s.advanceLatest(proto.StateProofInterval) + s.advanceLatestWithoutStateProof(proto.StateProofInterval) for i := 0; i < len(keys); i++ { // Expect all signatures to be broadcast. From 019931933849a0fe5f8d359f7604234f20334f1b Mon Sep 17 00:00:00 2001 From: Jonathan Weiss Date: Wed, 28 Sep 2022 11:13:16 +0300 Subject: [PATCH 04/25] fx: comment --- stateproof/worker_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/stateproof/worker_test.go b/stateproof/worker_test.go index 42b5a6650a..736c9eeb15 100644 --- a/stateproof/worker_test.go +++ b/stateproof/worker_test.go @@ -241,7 +241,6 @@ func (s *testWorkerStubs) BroadcastInternalSignedTxGroup(tx []transactions.Signe func (s *testWorkerStubs) RegisterHandlers([]network.TaggedMessageHandler) { } -// TODO: understand why using the other function fails multiple tests. func (s *testWorkerStubs) advanceLatestWithoutStateProof(delta uint64) { s.mu.Lock() defer s.mu.Unlock() @@ -251,6 +250,7 @@ func (s *testWorkerStubs) advanceLatestWithoutStateProof(delta uint64) { } } +// used to simulate to workers that rounds have advanced, and stateproofs were created. func (s *testWorkerStubs) advanceLatestAndStateProofs(delta uint64) { s.mu.Lock() defer s.mu.Unlock() @@ -586,10 +586,8 @@ func TestSignerDeletesUnneededStateProofKeys(t *testing.T) { proto := config.Consensus[protocol.ConsensusCurrentVersion] s.advanceLatestAndStateProofs(1 * proto.StateProofInterval) // going to rnd 256 - // Expect all signatures to be broadcast. - require.Zero(t, s.GetNumDeletedKeys()) - w.signStateProof(s.blocks[basics.Round(proto.StateProofInterval)]) + s.advanceLatestAndStateProofs(2 * proto.StateProofInterval) // advancing rounds up to 768 // chose 513 because that is the next round, and the signer must've passed through the deletion function by now. From 5ad38a12822cae131295dc1880ac86fed1f2cff6 Mon Sep 17 00:00:00 2001 From: Jonathan Weiss Date: Wed, 28 Sep 2022 14:33:38 +0300 Subject: [PATCH 05/25] fx: deletion of previous rounds --- stateproof/signer.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/stateproof/signer.go b/stateproof/signer.go index d6fb522ba3..077b57cd97 100644 --- a/stateproof/signer.go +++ b/stateproof/signer.go @@ -69,14 +69,11 @@ func (spw *Worker) signer(latest basics.Round) { } } +// attempts to delete any key that its first valid is <= (prevStateproofRound - lifetime) func (spw *Worker) attemptKeyDeletionPriorToRound(prevStateproofRound basics.Round) { for _, key := range spw.accts.StateProofKeys(prevStateproofRound) { - keyContext := key.StateProofSecrets.SignerContext - if basics.Round(keyContext.FirstValid+keyContext.KeyLifetime) > prevStateproofRound { - continue - } - - if err := spw.accts.DeleteStateProofKey(key.ParticipationID, prevStateproofRound); err != nil { + latestRndToDelete := prevStateproofRound - basics.Round(key.StateProofSecrets.SignerContext.KeyLifetime) + if err := spw.accts.DeleteStateProofKey(key.ParticipationID, latestRndToDelete); err != nil { spw.log.Warnf("spw.signBlock(%d): Couldn't delete StateProof keys: %v", prevStateproofRound, err) } } From c8729b5e26237ca3b305d9946c3c15ffb2845b27 Mon Sep 17 00:00:00 2001 From: Jonathan Weiss Date: Wed, 28 Sep 2022 16:39:42 +0300 Subject: [PATCH 06/25] fx: unit-test --- stateproof/signer.go | 3 +++ stateproof/worker_test.go | 39 +++++++++++++++++++++++++++++---------- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/stateproof/signer.go b/stateproof/signer.go index 077b57cd97..c775e7938b 100644 --- a/stateproof/signer.go +++ b/stateproof/signer.go @@ -73,6 +73,9 @@ func (spw *Worker) signer(latest basics.Round) { func (spw *Worker) attemptKeyDeletionPriorToRound(prevStateproofRound basics.Round) { for _, key := range spw.accts.StateProofKeys(prevStateproofRound) { latestRndToDelete := prevStateproofRound - basics.Round(key.StateProofSecrets.SignerContext.KeyLifetime) + if latestRndToDelete > prevStateproofRound { + continue + } if err := spw.accts.DeleteStateProofKey(key.ParticipationID, latestRndToDelete); err != nil { spw.log.Warnf("spw.signBlock(%d): Couldn't delete StateProof keys: %v", prevStateproofRound, err) } diff --git a/stateproof/worker_test.go b/stateproof/worker_test.go index 736c9eeb15..a64cfa5dd9 100644 --- a/stateproof/worker_test.go +++ b/stateproof/worker_test.go @@ -256,12 +256,14 @@ func (s *testWorkerStubs) advanceLatestAndStateProofs(delta uint64) { defer s.mu.Unlock() for r := uint64(0); r < delta; r++ { - interval := config.Consensus[s.blocks[s.latest].CurrentProtocol].StateProofInterval - if r%interval == 0 { - s.addBlock(s.blocks[s.latest].StateProofTracking[protocol.StateProofBasic].StateProofNextRound + basics.Round(interval)) - continue - } + interval := basics.Round(config.Consensus[s.blocks[s.latest].CurrentProtocol].StateProofInterval) s.addBlock(s.blocks[s.latest].StateProofTracking[protocol.StateProofBasic].StateProofNextRound) + tmp := s.blocks[s.latest].StateProofTracking[protocol.StateProofBasic] + blk := s.blocks[s.latest] + if blk.Round%interval == 0 && tmp.StateProofNextRound-interval < blk.Round { + tmp.StateProofNextRound += interval + s.blocks[s.latest].StateProofTracking[protocol.StateProofBasic] = tmp + } } } @@ -585,17 +587,34 @@ func TestSignerDeletesUnneededStateProofKeys(t *testing.T) { defer w.Shutdown() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatestAndStateProofs(1 * proto.StateProofInterval) // going to rnd 256 + s.advanceLatestAndStateProofs(proto.StateProofInterval) // going to rnd 256 // the stproof is in 512 require.Zero(t, s.GetNumDeletedKeys()) - s.advanceLatestAndStateProofs(2 * proto.StateProofInterval) // advancing rounds up to 768 + s.advanceLatestAndStateProofs(2 * proto.StateProofInterval) // advancing rounds up to 768. + + ctx, cncl := context.WithTimeout(context.Background(), time.Second*5) + defer cncl() - // chose 513 because that is the next round, and the signer must've passed through the deletion function by now. + // this busy wait shouldn't take long, the signer starts from round 512... for w.lastSignedBlock() < 513 { - time.Sleep(time.Millisecond * 100) + select { + case <-ctx.Done(): + require.Fail(t, "test ran out of time.") + t.Fail() + return + default: + time.Sleep(time.Second) + } } - require.Equal(t, nParticipants, s.GetNumDeletedKeys()) + + s.mu.Lock() + defer s.mu.Unlock() + + require.NotZero(t, len(s.deletedStateProofKeys)) + for _, round := range s.deletedStateProofKeys { + require.LessOrEqual(t, int(round), 512) + } } func TestSignerDoesntDeleteKeysWhenDBDoesntStoreSigs(t *testing.T) { From 597008edfe93c01f7bee47782a04d401ad55622f Mon Sep 17 00:00:00 2001 From: Jonathan Weiss Date: Wed, 28 Sep 2022 16:58:14 +0300 Subject: [PATCH 07/25] fx: prevStateproof init value --- stateproof/signer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stateproof/signer.go b/stateproof/signer.go index c775e7938b..c72e37cf3b 100644 --- a/stateproof/signer.go +++ b/stateproof/signer.go @@ -41,7 +41,7 @@ type sigFromAddr struct { func (spw *Worker) signer(latest basics.Round) { nextRnd := spw.nextStateProofRound(latest) // at this point there isn't any known stateproof by the signer, set as 0 to ensure no keys will be deleted. - prevStateProof := basics.Round(0) + prevStateProof := nextRnd for { // Start signing StateProofs from nextRnd onwards select { case <-spw.ledger.Wait(nextRnd): From dd00c9d09b4d278a4fb457459ba1438080e6ccd2 Mon Sep 17 00:00:00 2001 From: Jonathan Weiss Date: Wed, 28 Sep 2022 17:23:06 +0300 Subject: [PATCH 08/25] fx: golangcli --- stateproof/signer.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/stateproof/signer.go b/stateproof/signer.go index c72e37cf3b..eeb8b2151c 100644 --- a/stateproof/signer.go +++ b/stateproof/signer.go @@ -21,7 +21,6 @@ import ( "github.com/algorand/go-algorand/config" "github.com/algorand/go-algorand/crypto/merklesignature" - "github.com/algorand/go-algorand/data/account" "github.com/algorand/go-algorand/data/basics" "github.com/algorand/go-algorand/data/bookkeeping" "github.com/algorand/go-algorand/protocol" @@ -138,8 +137,6 @@ func (spw *Worker) signStateProof(hdr bookkeeping.BlockHeader) { } sigs := make([]sigFromAddr, 0, len(keys)) - ids := make([]account.ParticipationID, 0, len(keys)) - usedSigners := make([]*merklesignature.Signer, 0, len(keys)) stateproofMessage, err := GenerateStateProofMessage(spw.ledger, uint64(votersHdr.Round), hdr) if err != nil { @@ -176,8 +173,6 @@ func (spw *Worker) signStateProof(hdr bookkeeping.BlockHeader) { Round: hdr.Round, Sig: sig, }) - ids = append(ids, key.ParticipationID) - usedSigners = append(usedSigners, key.StateProofSecrets) } // any error in handle sig indicates the signature wasn't stored in disk, thus we cannot delete the key. From cf1098660ec18f0bd8f11ef3c40dcf6ce6b70b98 Mon Sep 17 00:00:00 2001 From: algoidan Date: Sun, 23 Oct 2022 16:36:04 +0300 Subject: [PATCH 09/25] fix merge issues. --- stateproof/worker_test.go | 44 +++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/stateproof/worker_test.go b/stateproof/worker_test.go index a64cfa5dd9..1fccf157d9 100644 --- a/stateproof/worker_test.go +++ b/stateproof/worker_test.go @@ -241,7 +241,7 @@ func (s *testWorkerStubs) BroadcastInternalSignedTxGroup(tx []transactions.Signe func (s *testWorkerStubs) RegisterHandlers([]network.TaggedMessageHandler) { } -func (s *testWorkerStubs) advanceLatestWithoutStateProof(delta uint64) { +func (s *testWorkerStubs) advanceRoundsWithoutStateProof(delta uint64) { s.mu.Lock() defer s.mu.Unlock() @@ -251,7 +251,7 @@ func (s *testWorkerStubs) advanceLatestWithoutStateProof(delta uint64) { } // used to simulate to workers that rounds have advanced, and stateproofs were created. -func (s *testWorkerStubs) advanceLatestAndStateProofs(delta uint64) { +func (s *testWorkerStubs) advanceRoundsAndStateProofs(delta uint64) { s.mu.Lock() defer s.mu.Unlock() @@ -339,12 +339,12 @@ func TestWorkerAllSigs(t *testing.T) { defer w.Shutdown() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatestWithoutStateProof(proto.StateProofInterval + proto.StateProofInterval/2) + s.advanceRoundsWithoutStateProof(proto.StateProofInterval + proto.StateProofInterval/2) // Go through several iterations, making sure that we get // the signatures and certs broadcast at each round. for iter := 0; iter < 5; iter++ { - s.advanceLatestWithoutStateProof(proto.StateProofInterval) + s.advanceRoundsWithoutStateProof(proto.StateProofInterval) for i := 0; i < len(keys); i++ { // Expect all signatures to be broadcast. @@ -408,8 +408,8 @@ func TestWorkerPartialSigs(t *testing.T) { defer w.Shutdown() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatestWithoutStateProof(proto.StateProofInterval + proto.StateProofInterval/2) - s.advanceLatestWithoutStateProof(proto.StateProofInterval) + s.advanceRoundsWithoutStateProof(proto.StateProofInterval + proto.StateProofInterval/2) + s.advanceRoundsWithoutStateProof(proto.StateProofInterval) for i := 0; i < len(keys); i++ { // Expect all signatures to be broadcast. @@ -425,7 +425,7 @@ func TestWorkerPartialSigs(t *testing.T) { } // Expect a state proof to be formed in the next StateProofInterval/2. - s.advanceLatestWithoutStateProof(proto.StateProofInterval / 2) + s.advanceRoundsWithoutStateProof(proto.StateProofInterval / 2) tx, err := s.waitOnTxnWithTimeout(time.Second * 5) require.NoError(t, err) @@ -473,7 +473,7 @@ func TestWorkerInsufficientSigs(t *testing.T) { defer w.Shutdown() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatestWithoutStateProof(3 * proto.StateProofInterval) + s.advanceRoundsWithoutStateProof(3 * proto.StateProofInterval) for i := 0; i < len(keys); i++ { // Expect all signatures to be broadcast. @@ -505,7 +505,7 @@ func TestWorkerRestart(t *testing.T) { s := newWorkerStubs(t, keys, 10) proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatestWithoutStateProof(3*proto.StateProofInterval - 1) + s.advanceRoundsWithoutStateProof(3*proto.StateProofInterval - 1) dbRand := crypto.RandUint64() @@ -551,7 +551,7 @@ func TestWorkerHandleSig(t *testing.T) { defer w.Shutdown() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatestWithoutStateProof(3 * proto.StateProofInterval) + s.advanceRoundsWithoutStateProof(3 * proto.StateProofInterval) for i := 0; i < len(keys); i++ { // Expect all signatures to be broadcast. @@ -587,10 +587,10 @@ func TestSignerDeletesUnneededStateProofKeys(t *testing.T) { defer w.Shutdown() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatestAndStateProofs(proto.StateProofInterval) // going to rnd 256 // the stproof is in 512 + s.advanceRoundsAndStateProofs(proto.StateProofInterval) // going to rnd 256 // the stproof is in 512 require.Zero(t, s.GetNumDeletedKeys()) - s.advanceLatestAndStateProofs(2 * proto.StateProofInterval) // advancing rounds up to 768. + s.advanceRoundsAndStateProofs(2 * proto.StateProofInterval) // advancing rounds up to 768. ctx, cncl := context.WithTimeout(context.Background(), time.Second*5) defer cncl() @@ -640,7 +640,7 @@ func TestSignerDoesntDeleteKeysWhenDBDoesntStoreSigs(t *testing.T) { w.Start() defer w.Shutdown() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatestWithoutStateProof(3 * proto.StateProofInterval) + s.advanceRoundsWithoutStateProof(3 * proto.StateProofInterval) // Expect all signatures to be broadcast. require.NoError(t, w.db.Atomic( @@ -674,10 +674,10 @@ func TestWorkerRemoveBuildersAndSignatures(t *testing.T) { defer w.Shutdown() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatestWithoutStateProof(proto.StateProofInterval + proto.StateProofInterval/2) + s.advanceRoundsWithoutStateProof(proto.StateProofInterval + proto.StateProofInterval/2) for iter := 0; iter < expectedStateProofs; iter++ { - s.advanceLatestWithoutStateProof(proto.StateProofInterval) + s.advanceRoundsWithoutStateProof(proto.StateProofInterval) tx := <-s.txmsg a.Equal(tx.Txn.Type, protocol.StateProofTx) } @@ -736,10 +736,10 @@ func TestWorkerBuildersRecoveryLimit(t *testing.T) { w.Start() defer w.Shutdown() - s.advanceLatestWithoutStateProof(proto.StateProofInterval + proto.StateProofInterval/2) + s.advanceRoundsWithoutStateProof(proto.StateProofInterval + proto.StateProofInterval/2) for iter := uint64(0); iter < proto.StateProofMaxRecoveryIntervals+1; iter++ { - s.advanceLatestWithoutStateProof(proto.StateProofInterval) + s.advanceRoundsWithoutStateProof(proto.StateProofInterval) tx := <-s.txmsg a.Equal(tx.Txn.Type, protocol.StateProofTx) } @@ -769,7 +769,7 @@ func TestWorkerBuildersRecoveryLimit(t *testing.T) { }) a.Equal(proto.StateProofMaxRecoveryIntervals+1, uint64(len(roundSigs))) - s.advanceLatestWithoutStateProof(proto.StateProofInterval) + s.advanceRoundsWithoutStateProof(proto.StateProofInterval) tx := <-s.txmsg a.Equal(tx.Txn.Type, protocol.StateProofTx) @@ -972,9 +972,9 @@ func TestBuilderGeneratesValidStateProofTXN(t *testing.T) { defer w.Shutdown() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatestWithoutStateProof(proto.StateProofInterval + proto.StateProofInterval/2) + s.advanceRoundsWithoutStateProof(proto.StateProofInterval + proto.StateProofInterval/2) - s.advanceLatestWithoutStateProof(proto.StateProofInterval) + s.advanceRoundsWithoutStateProof(proto.StateProofInterval) for i := 0; i < len(keys); i++ { // Expect all signatures to be broadcast. @@ -1385,7 +1385,7 @@ func TestWorker_BuildersPersistenceAfterRestart(t *testing.T) { const firstExpectedStateproof = 512 proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatest((expectedStateproofs+1)*proto.StateProofInterval + proto.StateProofInterval/2) // 512, 768, 1024, ... (9 StateProofs) + s.advanceRoundsWithoutStateProof((expectedStateproofs+1)*proto.StateProofInterval + proto.StateProofInterval/2) // 512, 768, 1024, ... (9 StateProofs) err := waitForBuilderAndSignerToWaitOnRound(s) a.NoError(err) @@ -1427,7 +1427,7 @@ func TestWorker_OnlySignaturesInDatabase(t *testing.T) { w.Start() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceLatest((expectedStateproofs+1)*proto.StateProofInterval + proto.StateProofInterval/2) // 512, 768, 1024, ... (9 StateProofs) + s.advanceRoundsWithoutStateProof((expectedStateproofs+1)*proto.StateProofInterval + proto.StateProofInterval/2) // 512, 768, 1024, ... (9 StateProofs) err := waitForBuilderAndSignerToWaitOnRound(s) a.NoError(err) From c9d553f27203e568c04b595fe7c939e7e2a3194c Mon Sep 17 00:00:00 2001 From: algoidan Date: Sun, 23 Oct 2022 18:17:07 +0300 Subject: [PATCH 10/25] refactoring --- stateproof/builder.go | 21 ++++++++++++++++++++- stateproof/signer.go | 21 --------------------- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/stateproof/builder.go b/stateproof/builder.go index 42cc77f81e..cdab2f88c6 100644 --- a/stateproof/builder.go +++ b/stateproof/builder.go @@ -367,6 +367,7 @@ func (spw *Worker) builder(latest basics.Round) { } spw.deleteOldSigs(&hdr) + spw.deleteOldKeys(&hdr) spw.deleteOldBuilders(&hdr) // Broadcast signatures based on the previous block(s) that @@ -461,6 +462,24 @@ func (spw *Worker) deleteOldSigs(currentHdr *bookkeeping.BlockHeader) { } } +func (spw *Worker) deleteOldKeys(currentHdr *bookkeeping.BlockHeader) { + proto := config.Consensus[currentHdr.CurrentProtocol] + stateProofNextRound := currentHdr.StateProofTracking[protocol.StateProofBasic].StateProofNextRound + if proto.StateProofInterval == 0 || stateProofNextRound == 0 { + return + } + + oldestRoundToRemove := stateProofNextRound.SubSaturate(basics.Round(proto.StateProofInterval)) + + keys := spw.accts.StateProofKeys(oldestRoundToRemove) + for _, key := range keys { + if err := spw.accts.DeleteStateProofKey(key.ParticipationID, oldestRoundToRemove); err != nil { + spw.log.Warnf("deleteOldKeys: could not remove key for round %s %v %v", key.ParticipationID, oldestRoundToRemove, err) + } + } + +} + func (spw *Worker) deleteOldBuilders(currentHdr *bookkeeping.BlockHeader) { oldestRoundToRemove := GetOldestExpectedStateProof(currentHdr) @@ -477,8 +496,8 @@ func (spw *Worker) deleteOldBuilders(currentHdr *bookkeeping.BlockHeader) { return deleteBuilders(tx, oldestRoundToRemove) }) if err != nil { - spw.log.Warnf("deleteOldBuilders: failed to delete builders from database: %v", err) } + spw.log.Warnf("deleteOldBuilders: failed to delete builders from database: %v", err) } func (spw *Worker) tryBroadcast() { diff --git a/stateproof/signer.go b/stateproof/signer.go index eeb8b2151c..ce0e88c94e 100644 --- a/stateproof/signer.go +++ b/stateproof/signer.go @@ -39,8 +39,6 @@ type sigFromAddr struct { func (spw *Worker) signer(latest basics.Round) { nextRnd := spw.nextStateProofRound(latest) - // at this point there isn't any known stateproof by the signer, set as 0 to ensure no keys will be deleted. - prevStateProof := nextRnd for { // Start signing StateProofs from nextRnd onwards select { case <-spw.ledger.Wait(nextRnd): @@ -55,12 +53,6 @@ func (spw *Worker) signer(latest basics.Round) { spw.invokeBuilder(nextRnd) nextRnd++ - nxtstateProof := hdr.StateProofTracking[protocol.StateProofBasic].StateProofNextRound - if nxtstateProof > prevStateProof { - spw.attemptKeyDeletionPriorToRound(prevStateProof) - prevStateProof = nxtstateProof - } - case <-spw.ctx.Done(): spw.wg.Done() return @@ -68,19 +60,6 @@ func (spw *Worker) signer(latest basics.Round) { } } -// attempts to delete any key that its first valid is <= (prevStateproofRound - lifetime) -func (spw *Worker) attemptKeyDeletionPriorToRound(prevStateproofRound basics.Round) { - for _, key := range spw.accts.StateProofKeys(prevStateproofRound) { - latestRndToDelete := prevStateproofRound - basics.Round(key.StateProofSecrets.SignerContext.KeyLifetime) - if latestRndToDelete > prevStateproofRound { - continue - } - if err := spw.accts.DeleteStateProofKey(key.ParticipationID, latestRndToDelete); err != nil { - spw.log.Warnf("spw.signBlock(%d): Couldn't delete StateProof keys: %v", prevStateproofRound, err) - } - } -} - func (spw *Worker) nextStateProofRound(latest basics.Round) basics.Round { var nextrnd basics.Round From 0e8a1130111de2dd861ee05edb9683e7a6abbd10 Mon Sep 17 00:00:00 2001 From: algoidan Date: Mon, 24 Oct 2022 09:52:17 +0300 Subject: [PATCH 11/25] add test --- stateproof/builder.go | 3 +-- stateproof/worker_test.go | 53 +++++++++++++++++++++++++++++---------- 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/stateproof/builder.go b/stateproof/builder.go index cdab2f88c6..6c25bd21da 100644 --- a/stateproof/builder.go +++ b/stateproof/builder.go @@ -477,7 +477,6 @@ func (spw *Worker) deleteOldKeys(currentHdr *bookkeeping.BlockHeader) { spw.log.Warnf("deleteOldKeys: could not remove key for round %s %v %v", key.ParticipationID, oldestRoundToRemove, err) } } - } func (spw *Worker) deleteOldBuilders(currentHdr *bookkeeping.BlockHeader) { @@ -496,8 +495,8 @@ func (spw *Worker) deleteOldBuilders(currentHdr *bookkeeping.BlockHeader) { return deleteBuilders(tx, oldestRoundToRemove) }) if err != nil { + spw.log.Warnf("deleteOldBuilders: failed to delete builders from database: %v", err) } - spw.log.Warnf("deleteOldBuilders: failed to delete builders from database: %v", err) } func (spw *Worker) tryBroadcast() { diff --git a/stateproof/worker_test.go b/stateproof/worker_test.go index 1fccf157d9..d2afabeac6 100644 --- a/stateproof/worker_test.go +++ b/stateproof/worker_test.go @@ -257,13 +257,13 @@ func (s *testWorkerStubs) advanceRoundsAndStateProofs(delta uint64) { for r := uint64(0); r < delta; r++ { interval := basics.Round(config.Consensus[s.blocks[s.latest].CurrentProtocol].StateProofInterval) - s.addBlock(s.blocks[s.latest].StateProofTracking[protocol.StateProofBasic].StateProofNextRound) - tmp := s.blocks[s.latest].StateProofTracking[protocol.StateProofBasic] blk := s.blocks[s.latest] - if blk.Round%interval == 0 && tmp.StateProofNextRound-interval < blk.Round { - tmp.StateProofNextRound += interval - s.blocks[s.latest].StateProofTracking[protocol.StateProofBasic] = tmp + stateProofNextRound := s.blocks[s.latest].StateProofTracking[protocol.StateProofBasic].StateProofNextRound + if blk.Round%interval == 0 && stateProofNextRound-interval < blk.Round { + stateProofNextRound += interval } + + s.addBlock(stateProofNextRound) } } @@ -617,7 +617,7 @@ func TestSignerDeletesUnneededStateProofKeys(t *testing.T) { } } -func TestSignerDoesntDeleteKeysWhenDBDoesntStoreSigs(t *testing.T) { +func TestKeysRemoveOnlyAfterStateProofAccepted(t *testing.T) { partitiontest.PartitionTest(t) var keys []account.Participation @@ -635,14 +635,19 @@ func TestSignerDoesntDeleteKeysWhenDBDoesntStoreSigs(t *testing.T) { logger := logging.NewLogger() logger.SetOutput(io.Discard) - w := NewWorker(dbs.Wdb, logger, s, s, s, s) + const expectedNumberOfStateProofs = 3 + const firstExpectedStateproof = 512 + w := NewWorker(dbs.Wdb, logger, s, s, s, s) w.Start() defer w.Shutdown() proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceRoundsWithoutStateProof(3 * proto.StateProofInterval) + s.advanceRoundsWithoutStateProof(firstExpectedStateproof + expectedNumberOfStateProofs*proto.StateProofInterval) // Expect all signatures to be broadcast. + err := waitForBuilderAndSignerToWaitOnRound(s) + require.NoError(t, err) + require.NoError(t, w.db.Atomic( func(ctx context.Context, tx *sql.Tx) error { _, err := tx.Exec("DROP TABLE sigs") @@ -650,8 +655,30 @@ func TestSignerDoesntDeleteKeysWhenDBDoesntStoreSigs(t *testing.T) { }), ) - w.signStateProof(s.blocks[3*basics.Round(proto.StateProofInterval)]) - require.Zero(t, s.GetNumDeletedKeys()) + // since no state proofs was confirmed (i.e the next state proof round == firstExpectedStateproof), and the key for + // (firstExpectedStateproof - proto.StateProofInterval) is never used, we expect the builder to keep all keys + // up to firstExpectedStateproof + s.mu.Lock() + for i := 0; i < len(keys); i++ { + rnd, exists := s.deletedStateProofKeys[keys[i].ID()] + require.True(t, exists) + require.Equal(t, basics.Round(firstExpectedStateproof-proto.StateProofInterval), rnd) + } + s.mu.Unlock() + + // confirm stateproof for firstExpectedStateproof -> worker should remove keys from firstExpectedStateproof + s.advanceRoundsAndStateProofs(1) + + err = waitForBuilderAndSignerToWaitOnRound(s) + require.NoError(t, err) + + s.mu.Lock() + for i := 0; i < len(keys); i++ { + rnd, exists := s.deletedStateProofKeys[keys[i].ID()] + require.True(t, exists) + require.Equal(t, basics.Round(firstExpectedStateproof), rnd) + } + s.mu.Unlock() } func TestWorkerRemoveBuildersAndSignatures(t *testing.T) { @@ -1362,7 +1389,7 @@ func TestWorkerHandleSigCorrupt(t *testing.T) { require.Equal(t, network.OutgoingMessage{Action: network.Disconnect}, reply) } -func TestWorker_BuildersPersistenceAfterRestart(t *testing.T) { +func TestBuildersPersistenceAfterRestart(t *testing.T) { partitiontest.PartitionTest(t) a := require.New(t) @@ -1404,7 +1431,7 @@ func TestWorker_BuildersPersistenceAfterRestart(t *testing.T) { compareBuilders(a, expectedStateproofs, w, firstExpectedStateproof, proto) } -func TestWorker_OnlySignaturesInDatabase(t *testing.T) { +func TestWorkerInitOnlySignaturesInDatabase(t *testing.T) { partitiontest.PartitionTest(t) a := require.New(t) @@ -1478,7 +1505,7 @@ func compareBuilders(a *require.Assertions, expectedStateproofs int, w *Worker, } } -func TestWorker_LoadsBuilderAndSignatureUponMsgRecv(t *testing.T) { +func TestWorkerLoadsBuilderAndSignatureUponMsgRecv(t *testing.T) { partitiontest.PartitionTest(t) proto := config.Consensus[protocol.ConsensusCurrentVersion] From 9e5ec6188cedcd225f03732a323412afd8c56d98 Mon Sep 17 00:00:00 2001 From: algoidan Date: Mon, 24 Oct 2022 16:00:06 +0300 Subject: [PATCH 12/25] fix deletion bug --- data/account/participationRegistry.go | 4 ++-- data/accountManager.go | 3 ++- stateproof/builder.go | 9 +++++++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/data/account/participationRegistry.go b/data/account/participationRegistry.go index e1c82a8924..c4bdd49452 100644 --- a/data/account/participationRegistry.go +++ b/data/account/participationRegistry.go @@ -232,7 +232,7 @@ type ParticipationRegistry interface { // once, an error will occur when the data is flushed when inserting a duplicate key. AppendKeys(id ParticipationID, keys StateProofKeys) error - // DeleteStateProofKeys removes all stateproof keys preceding a given round (including) + // DeleteStateProofKeys removes all stateproof keys preceding a given round (excluded) DeleteStateProofKeys(id ParticipationID, round basics.Round) error // Delete removes a record from storage. @@ -347,7 +347,7 @@ const ( insertKeysetQuery = `INSERT INTO Keysets (participationID, account, firstValidRound, lastValidRound, keyDilution, vrf, stateProof) VALUES (?, ?, ?, ?, ?, ?, ?)` insertRollingQuery = `INSERT INTO Rolling (pk, voting) VALUES (?, ?)` appendStateProofKeysQuery = `INSERT INTO StateProofKeys (pk, round, key) VALUES(?, ?, ?)` - deleteStateProofKeysQuery = `DELETE FROM StateProofKeys WHERE pk=? AND round<=?` + deleteStateProofKeysQuery = `DELETE FROM StateProofKeys WHERE pk=? AND round Date: Mon, 24 Oct 2022 19:25:45 +0300 Subject: [PATCH 13/25] fix tests --- stateproof/worker_test.go | 131 +++++++++++++++++++++++++++++++------- 1 file changed, 107 insertions(+), 24 deletions(-) diff --git a/stateproof/worker_test.go b/stateproof/worker_test.go index d2afabeac6..fe0054662e 100644 --- a/stateproof/worker_test.go +++ b/stateproof/worker_test.go @@ -132,6 +132,15 @@ func (s *testWorkerStubs) StateProofKeys(rnd basics.Round) (out []account.StateP Voting: part.Voting, } signerInRound := part.StateProofSecrets.GetSigner(uint64(rnd)) + if signerInRound == nil { + continue + } + KeyInLifeTime, _ := signerInRound.FirstRoundInKeyLifetime() + + // simulate that the key was removed + if basics.Round(KeyInLifeTime) < s.deletedStateProofKeys[part.ID()] { + continue + } partRecordForRound := account.StateProofSecretsForRound{ ParticipationRecord: partRecord, StateProofSecrets: signerInRound, @@ -641,44 +650,118 @@ func TestKeysRemoveOnlyAfterStateProofAccepted(t *testing.T) { w := NewWorker(dbs.Wdb, logger, s, s, s, s) w.Start() defer w.Shutdown() + proto := config.Consensus[protocol.ConsensusCurrentVersion] s.advanceRoundsWithoutStateProof(firstExpectedStateproof + expectedNumberOfStateProofs*proto.StateProofInterval) - // Expect all signatures to be broadcast. - err := waitForBuilderAndSignerToWaitOnRound(s) require.NoError(t, err) - require.NoError(t, w.db.Atomic( - func(ctx context.Context, tx *sql.Tx) error { - _, err := tx.Exec("DROP TABLE sigs") - return err - }), - ) + // since no state proof was confirmed (i.e the next state proof round == firstExpectedStateproof), we expect a node + // to keep its keys to sign the state proof firstExpectedStateproof. every part should have keys for that round + s.mu.Lock() + checkedKeys := s.StateProofKeys(firstExpectedStateproof) + require.Equal(t, len(keys), len(checkedKeys)) + s.mu.Unlock() - // since no state proofs was confirmed (i.e the next state proof round == firstExpectedStateproof), and the key for - // (firstExpectedStateproof - proto.StateProofInterval) is never used, we expect the builder to keep all keys - // up to firstExpectedStateproof + advanceRoundsAndStateProofsSlowly(t, s, 2*proto.StateProofInterval) + + // second state proof is confirmed, no need for the keys of the first state proof s.mu.Lock() - for i := 0; i < len(keys); i++ { - rnd, exists := s.deletedStateProofKeys[keys[i].ID()] - require.True(t, exists) - require.Equal(t, basics.Round(firstExpectedStateproof-proto.StateProofInterval), rnd) - } + fmt.Println(s.blocks[s.latest].StateProofTracking[protocol.StateProofBasic].StateProofNextRound) + checkedKeys = s.StateProofKeys(firstExpectedStateproof) + require.Equal(t, 0, len(checkedKeys)) + checkedKeys = s.StateProofKeys(basics.Round(3 * proto.StateProofInterval)) + require.Equal(t, len(keys), len(checkedKeys)) + checkedKeys = s.StateProofKeys(basics.Round(4 * proto.StateProofInterval)) + require.Equal(t, len(keys), len(checkedKeys)) s.mu.Unlock() +} - // confirm stateproof for firstExpectedStateproof -> worker should remove keys from firstExpectedStateproof - s.advanceRoundsAndStateProofs(1) +func TestKeysRemoveOnlyAfterStateProofAccepted2(t *testing.T) { + partitiontest.PartitionTest(t) - err = waitForBuilderAndSignerToWaitOnRound(s) + const stateProofIntervalForTest = 64 + tmp := config.Consensus[protocol.ConsensusCurrentVersion] + tmp.StateProofInterval = stateProofIntervalForTest + config.Consensus[protocol.ConsensusCurrentVersion] = tmp + defer func() { + tmp = config.Consensus[protocol.ConsensusCurrentVersion] + tmp.StateProofInterval = 256 + config.Consensus[protocol.ConsensusCurrentVersion] = tmp + }() + + var keys []account.Participation + for i := 0; i < 2; i++ { + var parent basics.Address + crypto.RandBytes(parent[:]) + p := newPartKey(t, parent) + defer p.Close() + keys = append(keys, p.Participation) + } + + s := newWorkerStubs(t, keys, 10) + dbs, _ := dbOpenTest(t, true) + + logger := logging.NewLogger() + logger.SetOutput(io.Discard) + + const expectedNumberOfStateProofs = 3 + const firstExpectedStateproof = stateProofIntervalForTest * 2 + + w := NewWorker(dbs.Wdb, logger, s, s, s, s) + w.Start() + defer w.Shutdown() + + proto := config.Consensus[protocol.ConsensusCurrentVersion] + s.advanceRoundsWithoutStateProof(firstExpectedStateproof + expectedNumberOfStateProofs*proto.StateProofInterval) + err := waitForBuilderAndSignerToWaitOnRound(s) require.NoError(t, err) + // since no state proof was confirmed (i.e the next state proof round == firstExpectedStateproof), we expect a node + // to keep its keys to sign the state proof firstExpectedStateproof. every part should have keys for that round s.mu.Lock() - for i := 0; i < len(keys); i++ { - rnd, exists := s.deletedStateProofKeys[keys[i].ID()] - require.True(t, exists) - require.Equal(t, basics.Round(firstExpectedStateproof), rnd) - } + checkedKeys := s.StateProofKeys(firstExpectedStateproof) + require.Equal(t, len(keys), len(checkedKeys)) s.mu.Unlock() + + // confirm stateproof for firstExpectedStateproof + advanceRoundsAndStateProofsSlowly(t, s, proto.StateProofInterval) + + // the first state proof was confirmed. Nevertheless, node should keep the keys for the firstExpectedStateproof + // since both are within the same keylifetime + s.mu.Lock() + checkedKeys = s.StateProofKeys(firstExpectedStateproof) + require.Equal(t, len(keys), len(checkedKeys)) + checkedKeys = s.StateProofKeys(3 * stateProofIntervalForTest) + require.Equal(t, len(keys), len(checkedKeys)) + s.mu.Unlock() + + advanceRoundsAndStateProofsSlowly(t, s, 2*proto.StateProofInterval) + + // when the 3rd state proof is confirmed the first key is not longer needed + s.mu.Lock() + checkedKeys = s.StateProofKeys(firstExpectedStateproof) + require.Equal(t, 0, len(checkedKeys)) + checkedKeys = s.StateProofKeys(3 * stateProofIntervalForTest) + require.Equal(t, 0, len(checkedKeys)) + checkedKeys = s.StateProofKeys(4 * stateProofIntervalForTest) + require.Equal(t, len(keys), len(checkedKeys)) + + // make sure that we have key for the last state proof + checkedKeys = s.StateProofKeys(s.blocks[s.latest].StateProofTracking[protocol.StateProofBasic].StateProofNextRound) + require.Equal(t, len(keys), len(checkedKeys)) + s.mu.Unlock() +} + +func advanceRoundsAndStateProofsSlowly(t *testing.T, s *testWorkerStubs, numberOfRounds uint64) { + // since adding blocks to our mock ledger happens very fast, the worker might + // not iterate over the latest block. Hence, we add some blocks -> wait -> add one more -> wait + s.advanceRoundsAndStateProofs(numberOfRounds - 1) + err := waitForBuilderAndSignerToWaitOnRound(s) + require.NoError(t, err) + s.advanceRoundsAndStateProofs(1) + err = waitForBuilderAndSignerToWaitOnRound(s) + require.NoError(t, err) } func TestWorkerRemoveBuildersAndSignatures(t *testing.T) { From 65d8b2197865ab83223e54ea038c21941a9104b3 Mon Sep 17 00:00:00 2001 From: algoidan Date: Mon, 24 Oct 2022 20:23:04 +0300 Subject: [PATCH 14/25] refactoring tests --- stateproof/worker_test.go | 54 +++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/stateproof/worker_test.go b/stateproof/worker_test.go index fe0054662e..d73b3d7e77 100644 --- a/stateproof/worker_test.go +++ b/stateproof/worker_test.go @@ -62,9 +62,15 @@ type testWorkerStubs struct { txmsg chan transactions.SignedTxn totalWeight int deletedStateProofKeys map[account.ParticipationID]basics.Round + version protocol.ConsensusVersion } func newWorkerStubs(t testing.TB, keys []account.Participation, totalWeight int) *testWorkerStubs { + version := config.Consensus[protocol.ConsensusCurrentVersion] + return newWorkerStubsWithVersion(t, keys, protocol.ConsensusCurrentVersion, version, totalWeight) +} + +func newWorkerStubsWithVersion(t testing.TB, keys []account.Participation, version protocol.ConsensusVersion, params config.ConsensusParams, totalWeight int) *testWorkerStubs { s := &testWorkerStubs{ t: nil, mu: deadlock.Mutex{}, @@ -78,9 +84,10 @@ func newWorkerStubs(t testing.TB, keys []account.Participation, totalWeight int) txmsg: make(chan transactions.SignedTxn, 1024), totalWeight: totalWeight, deletedStateProofKeys: map[account.ParticipationID]basics.Round{}, + version: version, } s.latest-- - s.addBlock(2 * basics.Round(config.Consensus[protocol.ConsensusCurrentVersion].StateProofInterval)) + s.addBlock(2 * basics.Round(params.StateProofInterval)) return s } @@ -89,7 +96,7 @@ func (s *testWorkerStubs) addBlock(spNextRound basics.Round) { hdr := bookkeeping.BlockHeader{} hdr.Round = s.latest - hdr.CurrentProtocol = protocol.ConsensusCurrentVersion + hdr.CurrentProtocol = s.version var stateProofBasic = bookkeeping.StateProofTrackingData{ StateProofVotersCommitment: make([]byte, stateproof.HashSize), @@ -114,8 +121,9 @@ func (s *testWorkerStubs) addBlock(spNextRound basics.Round) { close(s.waiters[s.latest]) } } - func (s *testWorkerStubs) StateProofKeys(rnd basics.Round) (out []account.StateProofSecretsForRound) { + s.mu.Lock() + defer s.mu.Unlock() for _, part := range s.keys { partRecord := account.ParticipationRecord{ ParticipationID: part.ID(), @@ -303,15 +311,19 @@ func newTestWorker(t testing.TB, s *testWorkerStubs) *Worker { return newTestWorkerDB(t, s, dbs.Wdb) } +func newPartKey(t testing.TB, parent basics.Address) account.PersistedParticipation { + version := config.Consensus[protocol.ConsensusCurrentVersion] + return newPartKeyWithVersion(t, version, parent) +} + // You must call defer part.Close() after calling this function, // since it creates a DB accessor but the caller must close it (required for mss) -func newPartKey(t testing.TB, parent basics.Address) account.PersistedParticipation { +func newPartKeyWithVersion(t testing.TB, protoParam config.ConsensusParams, parent basics.Address) account.PersistedParticipation { fn := fmt.Sprintf("%s.%d", strings.ReplaceAll(t.Name(), "/", "."), crypto.RandUint64()) partDB, err := db.MakeAccessor(fn, false, true) require.NoError(t, err) - proto := config.Consensus[protocol.ConsensusCurrentVersion] - part, err := account.FillDBWithParticipationKeys(partDB, parent, 0, basics.Round(15*proto.StateProofInterval), proto.DefaultKeyDilution) + part, err := account.FillDBWithParticipationKeys(partDB, parent, 0, basics.Round(15*protoParam.StateProofInterval), protoParam.DefaultKeyDilution) require.NoError(t, err) return part @@ -658,48 +670,42 @@ func TestKeysRemoveOnlyAfterStateProofAccepted(t *testing.T) { // since no state proof was confirmed (i.e the next state proof round == firstExpectedStateproof), we expect a node // to keep its keys to sign the state proof firstExpectedStateproof. every part should have keys for that round - s.mu.Lock() checkedKeys := s.StateProofKeys(firstExpectedStateproof) require.Equal(t, len(keys), len(checkedKeys)) - s.mu.Unlock() advanceRoundsAndStateProofsSlowly(t, s, 2*proto.StateProofInterval) // second state proof is confirmed, no need for the keys of the first state proof - s.mu.Lock() - fmt.Println(s.blocks[s.latest].StateProofTracking[protocol.StateProofBasic].StateProofNextRound) checkedKeys = s.StateProofKeys(firstExpectedStateproof) require.Equal(t, 0, len(checkedKeys)) checkedKeys = s.StateProofKeys(basics.Round(3 * proto.StateProofInterval)) require.Equal(t, len(keys), len(checkedKeys)) checkedKeys = s.StateProofKeys(basics.Round(4 * proto.StateProofInterval)) require.Equal(t, len(keys), len(checkedKeys)) - s.mu.Unlock() } -func TestKeysRemoveOnlyAfterStateProofAccepted2(t *testing.T) { +func TestKeysRemoveOnlyAfterStateProofAcceptedSmallIntervals(t *testing.T) { partitiontest.PartitionTest(t) const stateProofIntervalForTest = 64 - tmp := config.Consensus[protocol.ConsensusCurrentVersion] - tmp.StateProofInterval = stateProofIntervalForTest - config.Consensus[protocol.ConsensusCurrentVersion] = tmp + const smallIntervalVersionName = "TestKeysRemoveOnlyAfterStateProofAcceptedSmallIntervals" + proto := config.Consensus[protocol.ConsensusCurrentVersion] + proto.StateProofInterval = stateProofIntervalForTest + config.Consensus[smallIntervalVersionName] = proto defer func() { - tmp = config.Consensus[protocol.ConsensusCurrentVersion] - tmp.StateProofInterval = 256 - config.Consensus[protocol.ConsensusCurrentVersion] = tmp + delete(config.Consensus, smallIntervalVersionName) }() var keys []account.Participation for i := 0; i < 2; i++ { var parent basics.Address crypto.RandBytes(parent[:]) - p := newPartKey(t, parent) + p := newPartKeyWithVersion(t, proto, parent) defer p.Close() keys = append(keys, p.Participation) } - s := newWorkerStubs(t, keys, 10) + s := newWorkerStubsWithVersion(t, keys, smallIntervalVersionName, proto, 10) dbs, _ := dbOpenTest(t, true) logger := logging.NewLogger() @@ -712,34 +718,29 @@ func TestKeysRemoveOnlyAfterStateProofAccepted2(t *testing.T) { w.Start() defer w.Shutdown() - proto := config.Consensus[protocol.ConsensusCurrentVersion] s.advanceRoundsWithoutStateProof(firstExpectedStateproof + expectedNumberOfStateProofs*proto.StateProofInterval) err := waitForBuilderAndSignerToWaitOnRound(s) require.NoError(t, err) // since no state proof was confirmed (i.e the next state proof round == firstExpectedStateproof), we expect a node // to keep its keys to sign the state proof firstExpectedStateproof. every part should have keys for that round - s.mu.Lock() checkedKeys := s.StateProofKeys(firstExpectedStateproof) require.Equal(t, len(keys), len(checkedKeys)) - s.mu.Unlock() // confirm stateproof for firstExpectedStateproof advanceRoundsAndStateProofsSlowly(t, s, proto.StateProofInterval) // the first state proof was confirmed. Nevertheless, node should keep the keys for the firstExpectedStateproof // since both are within the same keylifetime - s.mu.Lock() checkedKeys = s.StateProofKeys(firstExpectedStateproof) require.Equal(t, len(keys), len(checkedKeys)) checkedKeys = s.StateProofKeys(3 * stateProofIntervalForTest) require.Equal(t, len(keys), len(checkedKeys)) - s.mu.Unlock() advanceRoundsAndStateProofsSlowly(t, s, 2*proto.StateProofInterval) // when the 3rd state proof is confirmed the first key is not longer needed - s.mu.Lock() + checkedKeys = s.StateProofKeys(firstExpectedStateproof) require.Equal(t, 0, len(checkedKeys)) checkedKeys = s.StateProofKeys(3 * stateProofIntervalForTest) @@ -750,7 +751,6 @@ func TestKeysRemoveOnlyAfterStateProofAccepted2(t *testing.T) { // make sure that we have key for the last state proof checkedKeys = s.StateProofKeys(s.blocks[s.latest].StateProofTracking[protocol.StateProofBasic].StateProofNextRound) require.Equal(t, len(keys), len(checkedKeys)) - s.mu.Unlock() } func advanceRoundsAndStateProofsSlowly(t *testing.T, s *testWorkerStubs, numberOfRounds uint64) { From 21cc86eae8726b0c72aba88fb590131eef71eca9 Mon Sep 17 00:00:00 2001 From: algoidan Date: Tue, 25 Oct 2022 12:10:46 +0300 Subject: [PATCH 15/25] reuse code on tests --- stateproof/worker_test.go | 111 ++++++++------------------------------ 1 file changed, 21 insertions(+), 90 deletions(-) diff --git a/stateproof/worker_test.go b/stateproof/worker_test.go index d73b3d7e77..29ea84aba1 100644 --- a/stateproof/worker_test.go +++ b/stateproof/worker_test.go @@ -592,103 +592,31 @@ func TestWorkerHandleSig(t *testing.T) { func TestSignerDeletesUnneededStateProofKeys(t *testing.T) { partitiontest.PartitionTest(t) - var keys []account.Participation - nParticipants := 2 - for i := 0; i < nParticipants; i++ { - var parent basics.Address - crypto.RandBytes(parent[:]) - p := newPartKey(t, parent) - defer p.Close() - keys = append(keys, p.Participation) - } - - s := newWorkerStubs(t, keys, 10) - w := newTestWorker(t, s) - w.Start() - defer w.Shutdown() - proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceRoundsAndStateProofs(proto.StateProofInterval) // going to rnd 256 // the stproof is in 512 - require.Zero(t, s.GetNumDeletedKeys()) - - s.advanceRoundsAndStateProofs(2 * proto.StateProofInterval) // advancing rounds up to 768. - - ctx, cncl := context.WithTimeout(context.Background(), time.Second*5) - defer cncl() - - // this busy wait shouldn't take long, the signer starts from round 512... - for w.lastSignedBlock() < 513 { - select { - case <-ctx.Done(): - require.Fail(t, "test ran out of time.") - t.Fail() - return - default: - time.Sleep(time.Second) - } - } - require.Equal(t, nParticipants, s.GetNumDeletedKeys()) - - s.mu.Lock() - defer s.mu.Unlock() - require.NotZero(t, len(s.deletedStateProofKeys)) - for _, round := range s.deletedStateProofKeys { - require.LessOrEqual(t, int(round), 512) - } + verifyKeyDeletion(t, proto, protocol.ConsensusCurrentVersion, basics.Round(proto.StateProofInterval)) } -func TestKeysRemoveOnlyAfterStateProofAccepted(t *testing.T) { +func TestKeysRemoveOnlyAfterStateProofAcceptedSmallIntervals(t *testing.T) { partitiontest.PartitionTest(t) - var keys []account.Participation - for i := 0; i < 2; i++ { - var parent basics.Address - crypto.RandBytes(parent[:]) - p := newPartKey(t, parent) - defer p.Close() - keys = append(keys, p.Participation) - } - - s := newWorkerStubs(t, keys, 10) - dbs, _ := dbOpenTest(t, true) - - logger := logging.NewLogger() - logger.SetOutput(io.Discard) - - const expectedNumberOfStateProofs = 3 - const firstExpectedStateproof = 512 - - w := NewWorker(dbs.Wdb, logger, s, s, s, s) - w.Start() - defer w.Shutdown() - + const stateProofIntervalForTest = 64 + const smallIntervalVersionName = "TestKeysRemoveOnlyAfterStateProofAcceptedSmallIntervals" proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceRoundsWithoutStateProof(firstExpectedStateproof + expectedNumberOfStateProofs*proto.StateProofInterval) - err := waitForBuilderAndSignerToWaitOnRound(s) - require.NoError(t, err) - - // since no state proof was confirmed (i.e the next state proof round == firstExpectedStateproof), we expect a node - // to keep its keys to sign the state proof firstExpectedStateproof. every part should have keys for that round - checkedKeys := s.StateProofKeys(firstExpectedStateproof) - require.Equal(t, len(keys), len(checkedKeys)) - - advanceRoundsAndStateProofsSlowly(t, s, 2*proto.StateProofInterval) + proto.StateProofInterval = stateProofIntervalForTest + config.Consensus[smallIntervalVersionName] = proto + defer func() { + delete(config.Consensus, smallIntervalVersionName) + }() - // second state proof is confirmed, no need for the keys of the first state proof - checkedKeys = s.StateProofKeys(firstExpectedStateproof) - require.Equal(t, 0, len(checkedKeys)) - checkedKeys = s.StateProofKeys(basics.Round(3 * proto.StateProofInterval)) - require.Equal(t, len(keys), len(checkedKeys)) - checkedKeys = s.StateProofKeys(basics.Round(4 * proto.StateProofInterval)) - require.Equal(t, len(keys), len(checkedKeys)) + verifyKeyDeletion(t, proto, smallIntervalVersionName, stateProofIntervalForTest) } -func TestKeysRemoveOnlyAfterStateProofAcceptedSmallIntervals(t *testing.T) { +func TestKeysRemoveOnlyAfterStateProofAcceptedLargeIntervals(t *testing.T) { partitiontest.PartitionTest(t) - const stateProofIntervalForTest = 64 - const smallIntervalVersionName = "TestKeysRemoveOnlyAfterStateProofAcceptedSmallIntervals" + const stateProofIntervalForTest = 260 + const smallIntervalVersionName = "TestKeysRemoveOnlyAfterStateProofAcceptedLargeIntervals" proto := config.Consensus[protocol.ConsensusCurrentVersion] proto.StateProofInterval = stateProofIntervalForTest config.Consensus[smallIntervalVersionName] = proto @@ -696,6 +624,10 @@ func TestKeysRemoveOnlyAfterStateProofAcceptedSmallIntervals(t *testing.T) { delete(config.Consensus, smallIntervalVersionName) }() + verifyKeyDeletion(t, proto, smallIntervalVersionName, stateProofIntervalForTest) +} + +func verifyKeyDeletion(t *testing.T, proto config.ConsensusParams, smallIntervalVersionName protocol.ConsensusVersion, stateProofIntervalForTest basics.Round) { var keys []account.Participation for i := 0; i < 2; i++ { var parent basics.Address @@ -711,14 +643,14 @@ func TestKeysRemoveOnlyAfterStateProofAcceptedSmallIntervals(t *testing.T) { logger := logging.NewLogger() logger.SetOutput(io.Discard) - const expectedNumberOfStateProofs = 3 - const firstExpectedStateproof = stateProofIntervalForTest * 2 + const expectedNumberOfStateProofs = uint64(3) + firstExpectedStateproof := stateProofIntervalForTest * 2 w := NewWorker(dbs.Wdb, logger, s, s, s, s) w.Start() defer w.Shutdown() - s.advanceRoundsWithoutStateProof(firstExpectedStateproof + expectedNumberOfStateProofs*proto.StateProofInterval) + s.advanceRoundsWithoutStateProof(uint64(firstExpectedStateproof) + expectedNumberOfStateProofs*proto.StateProofInterval) err := waitForBuilderAndSignerToWaitOnRound(s) require.NoError(t, err) @@ -739,8 +671,7 @@ func TestKeysRemoveOnlyAfterStateProofAcceptedSmallIntervals(t *testing.T) { advanceRoundsAndStateProofsSlowly(t, s, 2*proto.StateProofInterval) - // when the 3rd state proof is confirmed the first key is not longer needed - + // when the 3rd state proof is confirmed the first key is no longer needed checkedKeys = s.StateProofKeys(firstExpectedStateproof) require.Equal(t, 0, len(checkedKeys)) checkedKeys = s.StateProofKeys(3 * stateProofIntervalForTest) From 48ef64d1069cc721e1d8534bafecefe63564f3ae Mon Sep 17 00:00:00 2001 From: algoidan Date: Tue, 25 Oct 2022 14:32:52 +0300 Subject: [PATCH 16/25] fix keys removal to use the next state proof --- stateproof/builder.go | 4 +- stateproof/worker_test.go | 165 ++++++++++++++++++++++++++++---------- 2 files changed, 124 insertions(+), 45 deletions(-) diff --git a/stateproof/builder.go b/stateproof/builder.go index e454ec1ac8..19b100ffd4 100644 --- a/stateproof/builder.go +++ b/stateproof/builder.go @@ -469,9 +469,7 @@ func (spw *Worker) deleteOldKeys(currentHdr *bookkeeping.BlockHeader) { return } - oldestRoundToRemove := stateProofNextRound.SubSaturate(basics.Round(proto.StateProofInterval)) - - keys := spw.accts.StateProofKeys(oldestRoundToRemove) + keys := spw.accts.StateProofKeys(stateProofNextRound) for _, key := range keys { roundToRemove, err := key.StateProofSecrets.FirstRoundInKeyLifetime() if err != nil { diff --git a/stateproof/worker_test.go b/stateproof/worker_test.go index 29ea84aba1..7751a6d679 100644 --- a/stateproof/worker_test.go +++ b/stateproof/worker_test.go @@ -592,9 +592,100 @@ func TestWorkerHandleSig(t *testing.T) { func TestSignerDeletesUnneededStateProofKeys(t *testing.T) { partitiontest.PartitionTest(t) + var keys []account.Participation + nParticipants := 2 + for i := 0; i < nParticipants; i++ { + var parent basics.Address + crypto.RandBytes(parent[:]) + p := newPartKey(t, parent) + defer p.Close() + keys = append(keys, p.Participation) + } + + s := newWorkerStubs(t, keys, 10) + w := newTestWorker(t, s) + w.Start() + defer w.Shutdown() + proto := config.Consensus[protocol.ConsensusCurrentVersion] + s.advanceRoundsAndStateProofs(proto.StateProofInterval) // going to rnd 256 // the stproof is in 512 + require.Zero(t, s.GetNumDeletedKeys()) + + s.advanceRoundsAndStateProofs(2 * proto.StateProofInterval) // advancing rounds up to 768. + + ctx, cncl := context.WithTimeout(context.Background(), time.Second*5) + defer cncl() + + // this busy wait shouldn't take long, the signer starts from round 512... + for w.lastSignedBlock() < 513 { + select { + case <-ctx.Done(): + require.Fail(t, "test ran out of time.") + t.Fail() + return + default: + time.Sleep(time.Second) + } + } + require.Equal(t, nParticipants, s.GetNumDeletedKeys()) + + s.mu.Lock() + defer s.mu.Unlock() + + require.NotZero(t, len(s.deletedStateProofKeys)) + for _, round := range s.deletedStateProofKeys { + require.LessOrEqual(t, int(round), 512) + } +} + +func createWorkerAndParticipants(t *testing.T, version protocol.ConsensusVersion, proto config.ConsensusParams) ([]account.Participation, *testWorkerStubs, *Worker) { + var keys []account.Participation + for i := 0; i < 2; i++ { + var parent basics.Address + crypto.RandBytes(parent[:]) + p := newPartKeyWithVersion(t, proto, parent) + defer p.Close() + keys = append(keys, p.Participation) + } - verifyKeyDeletion(t, proto, protocol.ConsensusCurrentVersion, basics.Round(proto.StateProofInterval)) + s := newWorkerStubsWithVersion(t, keys, version, proto, 10) + dbs, _ := dbOpenTest(t, true) + + logger := logging.NewLogger() + logger.SetOutput(io.Discard) + + w := NewWorker(dbs.Wdb, logger, s, s, s, s) + w.Start() + return keys, s, w +} + +func TestKeysRemoveOnlyAfterStateProofAccepted(t *testing.T) { + partitiontest.PartitionTest(t) + + proto := config.Consensus[protocol.ConsensusCurrentVersion] + const expectedNumberOfStateProofs = uint64(3) + firstExpectedStateproof := basics.Round(proto.StateProofInterval * 2) + + keys, s, w := createWorkerAndParticipants(t, protocol.ConsensusCurrentVersion, proto) + defer w.Shutdown() + + s.advanceRoundsWithoutStateProof(uint64(firstExpectedStateproof) + expectedNumberOfStateProofs*proto.StateProofInterval) + err := waitForBuilderAndSignerToWaitOnRound(s) + require.NoError(t, err) + + // since no state proof was confirmed (i.e the next state proof round == firstExpectedStateproof), we expect a node + // to keep its keys to sign the state proof firstExpectedStateproof. every participant should have keys for that round + checkedKeys := s.StateProofKeys(firstExpectedStateproof) + require.Equal(t, len(keys), len(checkedKeys)) + + // confirm stateproof for firstExpectedStateproof + advanceRoundsAndStateProofsSlowly(t, s, proto.StateProofInterval) + + // the first state proof was confirmed keys for that state proof can be removed + checkedKeys = s.StateProofKeys(firstExpectedStateproof) + require.Equal(t, 0, len(checkedKeys)) + checkedKeys = s.StateProofKeys(basics.Round(3 * proto.StateProofInterval)) + require.Equal(t, len(keys), len(checkedKeys)) } func TestKeysRemoveOnlyAfterStateProofAcceptedSmallIntervals(t *testing.T) { @@ -609,7 +700,32 @@ func TestKeysRemoveOnlyAfterStateProofAcceptedSmallIntervals(t *testing.T) { delete(config.Consensus, smallIntervalVersionName) }() - verifyKeyDeletion(t, proto, smallIntervalVersionName, stateProofIntervalForTest) + partitiontest.PartitionTest(t) + + const expectedNumberOfStateProofs = uint64(3) + firstExpectedStateproof := basics.Round(proto.StateProofInterval * 2) + + keys, s, w := createWorkerAndParticipants(t, smallIntervalVersionName, proto) + defer w.Shutdown() + + s.advanceRoundsWithoutStateProof(uint64(firstExpectedStateproof) + expectedNumberOfStateProofs*proto.StateProofInterval) + err := waitForBuilderAndSignerToWaitOnRound(s) + require.NoError(t, err) + + // since no state proof was confirmed (i.e the next state proof round == firstExpectedStateproof), we expect a node + // to keep its keys to sign the state proof firstExpectedStateproof. every participant should have keys for that round + checkedKeys := s.StateProofKeys(firstExpectedStateproof) + require.Equal(t, len(keys), len(checkedKeys)) + + // confirm stateproof for firstExpectedStateproof + advanceRoundsAndStateProofsSlowly(t, s, proto.StateProofInterval) + + // the first state proof was confirmed. However, since keylifetime is greater than the state proof interval + // the key for firstExpectedStateproof should be kept (since it is being reused on 3 * proto.StateProofInterval) + checkedKeys = s.StateProofKeys(firstExpectedStateproof) + require.Equal(t, len(keys), len(checkedKeys)) + checkedKeys = s.StateProofKeys(basics.Round(3 * proto.StateProofInterval)) + require.Equal(t, len(keys), len(checkedKeys)) } func TestKeysRemoveOnlyAfterStateProofAcceptedLargeIntervals(t *testing.T) { @@ -624,30 +740,10 @@ func TestKeysRemoveOnlyAfterStateProofAcceptedLargeIntervals(t *testing.T) { delete(config.Consensus, smallIntervalVersionName) }() - verifyKeyDeletion(t, proto, smallIntervalVersionName, stateProofIntervalForTest) -} - -func verifyKeyDeletion(t *testing.T, proto config.ConsensusParams, smallIntervalVersionName protocol.ConsensusVersion, stateProofIntervalForTest basics.Round) { - var keys []account.Participation - for i := 0; i < 2; i++ { - var parent basics.Address - crypto.RandBytes(parent[:]) - p := newPartKeyWithVersion(t, proto, parent) - defer p.Close() - keys = append(keys, p.Participation) - } - - s := newWorkerStubsWithVersion(t, keys, smallIntervalVersionName, proto, 10) - dbs, _ := dbOpenTest(t, true) - - logger := logging.NewLogger() - logger.SetOutput(io.Discard) - const expectedNumberOfStateProofs = uint64(3) - firstExpectedStateproof := stateProofIntervalForTest * 2 + firstExpectedStateproof := basics.Round(proto.StateProofInterval * 2) - w := NewWorker(dbs.Wdb, logger, s, s, s, s) - w.Start() + keys, s, w := createWorkerAndParticipants(t, protocol.ConsensusCurrentVersion, proto) defer w.Shutdown() s.advanceRoundsWithoutStateProof(uint64(firstExpectedStateproof) + expectedNumberOfStateProofs*proto.StateProofInterval) @@ -655,32 +751,17 @@ func verifyKeyDeletion(t *testing.T, proto config.ConsensusParams, smallInterval require.NoError(t, err) // since no state proof was confirmed (i.e the next state proof round == firstExpectedStateproof), we expect a node - // to keep its keys to sign the state proof firstExpectedStateproof. every part should have keys for that round + // to keep its keys to sign the state proof firstExpectedStateproof. every participant should have keys for that round checkedKeys := s.StateProofKeys(firstExpectedStateproof) require.Equal(t, len(keys), len(checkedKeys)) // confirm stateproof for firstExpectedStateproof advanceRoundsAndStateProofsSlowly(t, s, proto.StateProofInterval) - // the first state proof was confirmed. Nevertheless, node should keep the keys for the firstExpectedStateproof - // since both are within the same keylifetime - checkedKeys = s.StateProofKeys(firstExpectedStateproof) - require.Equal(t, len(keys), len(checkedKeys)) - checkedKeys = s.StateProofKeys(3 * stateProofIntervalForTest) - require.Equal(t, len(keys), len(checkedKeys)) - - advanceRoundsAndStateProofsSlowly(t, s, 2*proto.StateProofInterval) - - // when the 3rd state proof is confirmed the first key is no longer needed + // the first state proof was confirmed keys for that state proof can be removed checkedKeys = s.StateProofKeys(firstExpectedStateproof) require.Equal(t, 0, len(checkedKeys)) - checkedKeys = s.StateProofKeys(3 * stateProofIntervalForTest) - require.Equal(t, 0, len(checkedKeys)) - checkedKeys = s.StateProofKeys(4 * stateProofIntervalForTest) - require.Equal(t, len(keys), len(checkedKeys)) - - // make sure that we have key for the last state proof - checkedKeys = s.StateProofKeys(s.blocks[s.latest].StateProofTracking[protocol.StateProofBasic].StateProofNextRound) + checkedKeys = s.StateProofKeys(basics.Round(3 * proto.StateProofInterval)) require.Equal(t, len(keys), len(checkedKeys)) } From 6dd5a9e7af5a2a95d77c677f96cb1708f7663191 Mon Sep 17 00:00:00 2001 From: algoidan Date: Tue, 25 Oct 2022 15:28:12 +0300 Subject: [PATCH 17/25] builder will not remove signatures and builder to support recoverability --- stateproof/builder.go | 23 ++++++++++++++++------- stateproof/worker_test.go | 11 ++++++----- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/stateproof/builder.go b/stateproof/builder.go index 19b100ffd4..1e13c6667b 100644 --- a/stateproof/builder.go +++ b/stateproof/builder.go @@ -452,13 +452,17 @@ func (spw *Worker) broadcastSigs(brnd basics.Round, proto config.ConsensusParams } func (spw *Worker) deleteOldSigs(currentHdr *bookkeeping.BlockHeader) { - oldestRoundToRemove := GetOldestExpectedStateProof(currentHdr) + proto := config.Consensus[currentHdr.CurrentProtocol] + stateProofNextRound := currentHdr.StateProofTracking[protocol.StateProofBasic].StateProofNextRound + if proto.StateProofInterval == 0 || stateProofNextRound == 0 { + return + } err := spw.db.Atomic(func(ctx context.Context, tx *sql.Tx) error { - return deletePendingSigsBeforeRound(tx, oldestRoundToRemove) + return deletePendingSigsBeforeRound(tx, stateProofNextRound) }) if err != nil { - spw.log.Warnf("deletePendingSigsBeforeRound(%d): %v", oldestRoundToRemove, err) + spw.log.Warnf("deletePendingSigsBeforeRound(%d): %v", stateProofNextRound, err) } } @@ -473,7 +477,8 @@ func (spw *Worker) deleteOldKeys(currentHdr *bookkeeping.BlockHeader) { for _, key := range keys { roundToRemove, err := key.StateProofSecrets.FirstRoundInKeyLifetime() if err != nil { - spw.log.Warnf("deleteOldKeys: could not calculate keylifetime for account %v on round %s: %v", key.ParticipationID, roundToRemove, err) + spw.log.Errorf("deleteOldKeys: could not calculate keylifetime for account %v on round %s: %v", key.ParticipationID, roundToRemove, err) + continue } err = spw.accts.DeleteStateProofKey(key.ParticipationID, basics.Round(roundToRemove)) if err != nil { @@ -483,19 +488,23 @@ func (spw *Worker) deleteOldKeys(currentHdr *bookkeeping.BlockHeader) { } func (spw *Worker) deleteOldBuilders(currentHdr *bookkeeping.BlockHeader) { - oldestRoundToRemove := GetOldestExpectedStateProof(currentHdr) + proto := config.Consensus[currentHdr.CurrentProtocol] + stateProofNextRound := currentHdr.StateProofTracking[protocol.StateProofBasic].StateProofNextRound + if proto.StateProofInterval == 0 || stateProofNextRound == 0 { + return + } spw.mu.Lock() defer spw.mu.Unlock() for rnd := range spw.builders { - if rnd < oldestRoundToRemove { + if rnd < stateProofNextRound { delete(spw.builders, rnd) } } err := spw.db.Atomic(func(ctx context.Context, tx *sql.Tx) error { - return deleteBuilders(tx, oldestRoundToRemove) + return deleteBuilders(tx, stateProofNextRound) }) if err != nil { spw.log.Warnf("deleteOldBuilders: failed to delete builders from database: %v", err) diff --git a/stateproof/worker_test.go b/stateproof/worker_test.go index 7751a6d679..150eda44d6 100644 --- a/stateproof/worker_test.go +++ b/stateproof/worker_test.go @@ -839,7 +839,7 @@ func TestWorkerRemoveBuildersAndSignatures(t *testing.T) { a.Equal(3, len(roundSigs)) } -func TestWorkerBuildersRecoveryLimit(t *testing.T) { +func TestWorkerBuildersRecoveryIsNotLimited(t *testing.T) { partitiontest.PartitionTest(t) a := require.New(t) @@ -903,18 +903,19 @@ func TestWorkerBuildersRecoveryLimit(t *testing.T) { err = waitForBuilderAndSignerToWaitOnRound(s) a.NoError(err) - // should not give up on rounds - a.Equal(proto.StateProofMaxRecoveryIntervals+1, uint64(len(w.builders))) + // Although the max recovery has passed the worker will not delete + // builder and signatures + a.Equal(proto.StateProofMaxRecoveryIntervals+2, uint64(len(w.builders))) countDB, err = countBuildersInDB(w.db) a.NoError(err) - a.Equal(proto.StateProofMaxRecoveryIntervals+1, uint64(countDB)) + a.Equal(proto.StateProofMaxRecoveryIntervals+2, uint64(countDB)) roundSigs = make(map[basics.Round][]pendingSig) err = w.db.Atomic(func(ctx context.Context, tx *sql.Tx) (err error) { roundSigs, err = getPendingSigs(tx) return }) - a.Equal(proto.StateProofMaxRecoveryIntervals+1, uint64(len(roundSigs))) + a.Equal(proto.StateProofMaxRecoveryIntervals+2, uint64(len(roundSigs))) } func waitForBuilderAndSignerToWaitOnRound(s *testWorkerStubs) error { From 38b4f3d4cec2ab9a2da910eb971cf368ea6dd782 Mon Sep 17 00:00:00 2001 From: algoidan Date: Tue, 25 Oct 2022 16:04:03 +0300 Subject: [PATCH 18/25] refactoring + avoid remove if stateproof was not changed --- stateproof/builder.go | 43 ++++++++++++++++++--------------------- stateproof/worker.go | 5 +++-- stateproof/worker_test.go | 19 +++++++++++------ 3 files changed, 36 insertions(+), 31 deletions(-) diff --git a/stateproof/builder.go b/stateproof/builder.go index 1e13c6667b..bb072dc9c1 100644 --- a/stateproof/builder.go +++ b/stateproof/builder.go @@ -366,9 +366,7 @@ func (spw *Worker) builder(latest basics.Round) { continue } - spw.deleteOldSigs(&hdr) - spw.deleteOldKeys(&hdr) - spw.deleteOldBuilders(&hdr) + spw.deleteStaleStateProofBuildData(&hdr) // Broadcast signatures based on the previous block(s) that // were agreed upon. This ensures that, if we send a signature @@ -451,29 +449,34 @@ func (spw *Worker) broadcastSigs(brnd basics.Round, proto config.ConsensusParams } } -func (spw *Worker) deleteOldSigs(currentHdr *bookkeeping.BlockHeader) { +func (spw *Worker) deleteStaleStateProofBuildData(currentHdr *bookkeeping.BlockHeader) { proto := config.Consensus[currentHdr.CurrentProtocol] stateProofNextRound := currentHdr.StateProofTracking[protocol.StateProofBasic].StateProofNextRound if proto.StateProofInterval == 0 || stateProofNextRound == 0 { return } + if spw.LastCleanupRound == stateProofNextRound { + return + } + + spw.deleteStaleSigs(stateProofNextRound) + spw.deleteStaleKeys(stateProofNextRound) + spw.deleteStaleBuilders(stateProofNextRound) + spw.LastCleanupRound = stateProofNextRound +} + +func (spw *Worker) deleteStaleSigs(latestRoundToKeep basics.Round) { err := spw.db.Atomic(func(ctx context.Context, tx *sql.Tx) error { - return deletePendingSigsBeforeRound(tx, stateProofNextRound) + return deletePendingSigsBeforeRound(tx, latestRoundToKeep) }) if err != nil { - spw.log.Warnf("deletePendingSigsBeforeRound(%d): %v", stateProofNextRound, err) + spw.log.Warnf("deletePendingSigsBeforeRound(%d): %v", latestRoundToKeep, err) } } -func (spw *Worker) deleteOldKeys(currentHdr *bookkeeping.BlockHeader) { - proto := config.Consensus[currentHdr.CurrentProtocol] - stateProofNextRound := currentHdr.StateProofTracking[protocol.StateProofBasic].StateProofNextRound - if proto.StateProofInterval == 0 || stateProofNextRound == 0 { - return - } - - keys := spw.accts.StateProofKeys(stateProofNextRound) +func (spw *Worker) deleteStaleKeys(latestRoundToKeep basics.Round) { + keys := spw.accts.StateProofKeys(latestRoundToKeep) for _, key := range keys { roundToRemove, err := key.StateProofSecrets.FirstRoundInKeyLifetime() if err != nil { @@ -487,24 +490,18 @@ func (spw *Worker) deleteOldKeys(currentHdr *bookkeeping.BlockHeader) { } } -func (spw *Worker) deleteOldBuilders(currentHdr *bookkeeping.BlockHeader) { - proto := config.Consensus[currentHdr.CurrentProtocol] - stateProofNextRound := currentHdr.StateProofTracking[protocol.StateProofBasic].StateProofNextRound - if proto.StateProofInterval == 0 || stateProofNextRound == 0 { - return - } - +func (spw *Worker) deleteStaleBuilders(latestRoundToKeep basics.Round) { spw.mu.Lock() defer spw.mu.Unlock() for rnd := range spw.builders { - if rnd < stateProofNextRound { + if rnd < latestRoundToKeep { delete(spw.builders, rnd) } } err := spw.db.Atomic(func(ctx context.Context, tx *sql.Tx) error { - return deleteBuilders(tx, stateProofNextRound) + return deleteBuilders(tx, latestRoundToKeep) }) if err != nil { spw.log.Warnf("deleteOldBuilders: failed to delete builders from database: %v", err) diff --git a/stateproof/worker.go b/stateproof/worker.go index 7b2cd04e2d..6f61ada46f 100644 --- a/stateproof/worker.go +++ b/stateproof/worker.go @@ -65,8 +65,9 @@ type Worker struct { shutdown context.CancelFunc wg sync.WaitGroup - signed basics.Round - signedCh chan struct{} + signed basics.Round + signedCh chan struct{} + LastCleanupRound basics.Round } // NewWorker constructs a new Worker, as used by the node. diff --git a/stateproof/worker_test.go b/stateproof/worker_test.go index 150eda44d6..dd5527dd9a 100644 --- a/stateproof/worker_test.go +++ b/stateproof/worker_test.go @@ -66,11 +66,11 @@ type testWorkerStubs struct { } func newWorkerStubs(t testing.TB, keys []account.Participation, totalWeight int) *testWorkerStubs { - version := config.Consensus[protocol.ConsensusCurrentVersion] - return newWorkerStubsWithVersion(t, keys, protocol.ConsensusCurrentVersion, version, totalWeight) + return newWorkerStubsWithVersion(t, keys, protocol.ConsensusCurrentVersion, totalWeight) } -func newWorkerStubsWithVersion(t testing.TB, keys []account.Participation, version protocol.ConsensusVersion, params config.ConsensusParams, totalWeight int) *testWorkerStubs { +func newWorkerStubsWithVersion(t testing.TB, keys []account.Participation, version protocol.ConsensusVersion, totalWeight int) *testWorkerStubs { + proto := config.Consensus[version] s := &testWorkerStubs{ t: nil, mu: deadlock.Mutex{}, @@ -87,7 +87,7 @@ func newWorkerStubsWithVersion(t testing.TB, keys []account.Participation, versi version: version, } s.latest-- - s.addBlock(2 * basics.Round(params.StateProofInterval)) + s.addBlock(2 * basics.Round(proto.StateProofInterval)) return s } @@ -589,7 +589,14 @@ func TestWorkerHandleSig(t *testing.T) { } } -func TestSignerDeletesUnneededStateProofKeys(t *testing.T) { +//func TestSignerDeletesUnneededStateProofKeys(t *testing.T) { +// for i := 0; i < 300; i++ { +// TestSignerDeletesUnneededStateProofKeys1(t) +// } +// +//} + +func TestSignerDeletesUnneededStateProofKeys1(t *testing.T) { partitiontest.PartitionTest(t) var keys []account.Participation @@ -648,7 +655,7 @@ func createWorkerAndParticipants(t *testing.T, version protocol.ConsensusVersion keys = append(keys, p.Participation) } - s := newWorkerStubsWithVersion(t, keys, version, proto, 10) + s := newWorkerStubsWithVersion(t, keys, version, 10) dbs, _ := dbOpenTest(t, true) logger := logging.NewLogger() From d1dbb90dedb495f79b9df824c55b1c78e89fba01 Mon Sep 17 00:00:00 2001 From: algoidan Date: Tue, 25 Oct 2022 16:25:18 +0300 Subject: [PATCH 19/25] remove double test --- stateproof/worker_test.go | 56 --------------------------------------- 1 file changed, 56 deletions(-) diff --git a/stateproof/worker_test.go b/stateproof/worker_test.go index dd5527dd9a..f006904d02 100644 --- a/stateproof/worker_test.go +++ b/stateproof/worker_test.go @@ -589,62 +589,6 @@ func TestWorkerHandleSig(t *testing.T) { } } -//func TestSignerDeletesUnneededStateProofKeys(t *testing.T) { -// for i := 0; i < 300; i++ { -// TestSignerDeletesUnneededStateProofKeys1(t) -// } -// -//} - -func TestSignerDeletesUnneededStateProofKeys1(t *testing.T) { - partitiontest.PartitionTest(t) - - var keys []account.Participation - nParticipants := 2 - for i := 0; i < nParticipants; i++ { - var parent basics.Address - crypto.RandBytes(parent[:]) - p := newPartKey(t, parent) - defer p.Close() - keys = append(keys, p.Participation) - } - - s := newWorkerStubs(t, keys, 10) - w := newTestWorker(t, s) - w.Start() - defer w.Shutdown() - - proto := config.Consensus[protocol.ConsensusCurrentVersion] - s.advanceRoundsAndStateProofs(proto.StateProofInterval) // going to rnd 256 // the stproof is in 512 - require.Zero(t, s.GetNumDeletedKeys()) - - s.advanceRoundsAndStateProofs(2 * proto.StateProofInterval) // advancing rounds up to 768. - - ctx, cncl := context.WithTimeout(context.Background(), time.Second*5) - defer cncl() - - // this busy wait shouldn't take long, the signer starts from round 512... - for w.lastSignedBlock() < 513 { - select { - case <-ctx.Done(): - require.Fail(t, "test ran out of time.") - t.Fail() - return - default: - time.Sleep(time.Second) - } - } - require.Equal(t, nParticipants, s.GetNumDeletedKeys()) - - s.mu.Lock() - defer s.mu.Unlock() - - require.NotZero(t, len(s.deletedStateProofKeys)) - for _, round := range s.deletedStateProofKeys { - require.LessOrEqual(t, int(round), 512) - } -} - func createWorkerAndParticipants(t *testing.T, version protocol.ConsensusVersion, proto config.ConsensusParams) ([]account.Participation, *testWorkerStubs, *Worker) { var keys []account.Participation for i := 0; i < 2; i++ { From b9fb7a7ea3049b18be2f12ebbe59edf98d16a7df Mon Sep 17 00:00:00 2001 From: Jonathan Weiss Date: Tue, 25 Oct 2022 17:37:08 +0300 Subject: [PATCH 20/25] checking that the deletion round is not higher than expected --- stateproof/stateproofMessageGenerator_test.go | 24 +++--- stateproof/worker_test.go | 82 ++++++++++++------- 2 files changed, 65 insertions(+), 41 deletions(-) diff --git a/stateproof/stateproofMessageGenerator_test.go b/stateproof/stateproofMessageGenerator_test.go index 1b909d775f..4bdded817c 100644 --- a/stateproof/stateproofMessageGenerator_test.go +++ b/stateproof/stateproofMessageGenerator_test.go @@ -149,18 +149,18 @@ func (s *workerForStateProofMessageTests) addBlockWithStateProofHeaders(ccNextRo func newWorkerForStateProofMessageStubs(keys []account.Participation, totalWeight int) *workerForStateProofMessageTests { s := &testWorkerStubs{ - t: nil, - mu: deadlock.Mutex{}, - latest: 0, - waiters: make(map[basics.Round]chan struct{}), - waitersCount: make(map[basics.Round]int), - blocks: make(map[basics.Round]bookkeeping.BlockHeader), - keys: keys, - keysForVoters: keys, - sigmsg: make(chan []byte, 1024), - txmsg: make(chan transactions.SignedTxn, 1024), - totalWeight: totalWeight, - deletedStateProofKeys: map[account.ParticipationID]basics.Round{}, + t: nil, + mu: deadlock.Mutex{}, + latest: 0, + waiters: make(map[basics.Round]chan struct{}), + waitersCount: make(map[basics.Round]int), + blocks: make(map[basics.Round]bookkeeping.BlockHeader), + keys: keys, + keysForVoters: keys, + sigmsg: make(chan []byte, 1024), + txmsg: make(chan transactions.SignedTxn, 1024), + totalWeight: totalWeight, + deletedKeysBeforeRoundMap: map[account.ParticipationID]basics.Round{}, } sm := workerForStateProofMessageTests{w: s} return &sm diff --git a/stateproof/worker_test.go b/stateproof/worker_test.go index f006904d02..57062b9a64 100644 --- a/stateproof/worker_test.go +++ b/stateproof/worker_test.go @@ -50,19 +50,19 @@ import ( ) type testWorkerStubs struct { - t testing.TB - mu deadlock.Mutex - latest basics.Round - waiters map[basics.Round]chan struct{} - waitersCount map[basics.Round]int - blocks map[basics.Round]bookkeeping.BlockHeader - keys []account.Participation - keysForVoters []account.Participation - sigmsg chan []byte - txmsg chan transactions.SignedTxn - totalWeight int - deletedStateProofKeys map[account.ParticipationID]basics.Round - version protocol.ConsensusVersion + t testing.TB + mu deadlock.Mutex + latest basics.Round + waiters map[basics.Round]chan struct{} + waitersCount map[basics.Round]int + blocks map[basics.Round]bookkeeping.BlockHeader + keys []account.Participation + keysForVoters []account.Participation + sigmsg chan []byte + txmsg chan transactions.SignedTxn + totalWeight int + deletedKeysBeforeRoundMap map[account.ParticipationID]basics.Round + version protocol.ConsensusVersion } func newWorkerStubs(t testing.TB, keys []account.Participation, totalWeight int) *testWorkerStubs { @@ -72,19 +72,19 @@ func newWorkerStubs(t testing.TB, keys []account.Participation, totalWeight int) func newWorkerStubsWithVersion(t testing.TB, keys []account.Participation, version protocol.ConsensusVersion, totalWeight int) *testWorkerStubs { proto := config.Consensus[version] s := &testWorkerStubs{ - t: nil, - mu: deadlock.Mutex{}, - latest: 0, - waiters: make(map[basics.Round]chan struct{}), - waitersCount: make(map[basics.Round]int), - blocks: make(map[basics.Round]bookkeeping.BlockHeader), - keys: keys, - keysForVoters: keys, - sigmsg: make(chan []byte, 1024*1024), - txmsg: make(chan transactions.SignedTxn, 1024), - totalWeight: totalWeight, - deletedStateProofKeys: map[account.ParticipationID]basics.Round{}, - version: version, + t: nil, + mu: deadlock.Mutex{}, + latest: 0, + waiters: make(map[basics.Round]chan struct{}), + waitersCount: make(map[basics.Round]int), + blocks: make(map[basics.Round]bookkeeping.BlockHeader), + keys: keys, + keysForVoters: keys, + sigmsg: make(chan []byte, 1024*1024), + txmsg: make(chan transactions.SignedTxn, 1024), + totalWeight: totalWeight, + deletedKeysBeforeRoundMap: map[account.ParticipationID]basics.Round{}, + version: version, } s.latest-- s.addBlock(2 * basics.Round(proto.StateProofInterval)) @@ -146,7 +146,7 @@ func (s *testWorkerStubs) StateProofKeys(rnd basics.Round) (out []account.StateP KeyInLifeTime, _ := signerInRound.FirstRoundInKeyLifetime() // simulate that the key was removed - if basics.Round(KeyInLifeTime) < s.deletedStateProofKeys[part.ID()] { + if basics.Round(KeyInLifeTime) < s.deletedKeysBeforeRoundMap[part.ID()] { continue } partRecordForRound := account.StateProofSecretsForRound{ @@ -160,14 +160,14 @@ func (s *testWorkerStubs) StateProofKeys(rnd basics.Round) (out []account.StateP func (s *testWorkerStubs) DeleteStateProofKey(id account.ParticipationID, round basics.Round) error { s.mu.Lock() - s.deletedStateProofKeys[id] = round + s.deletedKeysBeforeRoundMap[id] = round s.mu.Unlock() return nil } func (s *testWorkerStubs) GetNumDeletedKeys() int { s.mu.Lock() - numDeltedKeys := len(s.deletedStateProofKeys) + numDeltedKeys := len(s.deletedKeysBeforeRoundMap) s.mu.Unlock() return numDeltedKeys @@ -610,6 +610,21 @@ func createWorkerAndParticipants(t *testing.T, version protocol.ConsensusVersion return keys, s, w } +// threshold == 0 meaning nothing was deleted. +func requireDeletedKeysToBeDeletedBefore(t *testing.T, s *testWorkerStubs, threshold basics.Round) { + s.mu.Lock() + defer s.mu.Unlock() + + for _, prt := range s.keys { + if threshold == 0 { + require.Equal(t, threshold, s.deletedKeysBeforeRoundMap[prt.ID()]) + continue + } + // minus one because we delete keys up to the round stated in the map but not including! + require.Greater(t, threshold, s.deletedKeysBeforeRoundMap[prt.ID()]-1) + } +} + func TestKeysRemoveOnlyAfterStateProofAccepted(t *testing.T) { partitiontest.PartitionTest(t) @@ -628,11 +643,14 @@ func TestKeysRemoveOnlyAfterStateProofAccepted(t *testing.T) { // to keep its keys to sign the state proof firstExpectedStateproof. every participant should have keys for that round checkedKeys := s.StateProofKeys(firstExpectedStateproof) require.Equal(t, len(keys), len(checkedKeys)) + requireDeletedKeysToBeDeletedBefore(t, s, firstExpectedStateproof) // i should at this point have the keys to sign on round 512.... how come they were deleted? // confirm stateproof for firstExpectedStateproof advanceRoundsAndStateProofsSlowly(t, s, proto.StateProofInterval) // the first state proof was confirmed keys for that state proof can be removed + // So we should have the not deleted keys for proto.StateProofInterval + firstExpectedStateproof + requireDeletedKeysToBeDeletedBefore(t, s, firstExpectedStateproof+basics.Round(proto.StateProofInterval)) checkedKeys = s.StateProofKeys(firstExpectedStateproof) require.Equal(t, 0, len(checkedKeys)) checkedKeys = s.StateProofKeys(basics.Round(3 * proto.StateProofInterval)) @@ -665,6 +683,7 @@ func TestKeysRemoveOnlyAfterStateProofAcceptedSmallIntervals(t *testing.T) { // since no state proof was confirmed (i.e the next state proof round == firstExpectedStateproof), we expect a node // to keep its keys to sign the state proof firstExpectedStateproof. every participant should have keys for that round + requireDeletedKeysToBeDeletedBefore(t, s, 0) checkedKeys := s.StateProofKeys(firstExpectedStateproof) require.Equal(t, len(keys), len(checkedKeys)) @@ -673,6 +692,8 @@ func TestKeysRemoveOnlyAfterStateProofAcceptedSmallIntervals(t *testing.T) { // the first state proof was confirmed. However, since keylifetime is greater than the state proof interval // the key for firstExpectedStateproof should be kept (since it is being reused on 3 * proto.StateProofInterval) + requireDeletedKeysToBeDeletedBefore(t, s, 0) + checkedKeys = s.StateProofKeys(firstExpectedStateproof) require.Equal(t, len(keys), len(checkedKeys)) checkedKeys = s.StateProofKeys(basics.Round(3 * proto.StateProofInterval)) @@ -703,6 +724,7 @@ func TestKeysRemoveOnlyAfterStateProofAcceptedLargeIntervals(t *testing.T) { // since no state proof was confirmed (i.e the next state proof round == firstExpectedStateproof), we expect a node // to keep its keys to sign the state proof firstExpectedStateproof. every participant should have keys for that round + requireDeletedKeysToBeDeletedBefore(t, s, firstExpectedStateproof) checkedKeys := s.StateProofKeys(firstExpectedStateproof) require.Equal(t, len(keys), len(checkedKeys)) @@ -710,6 +732,8 @@ func TestKeysRemoveOnlyAfterStateProofAcceptedLargeIntervals(t *testing.T) { advanceRoundsAndStateProofsSlowly(t, s, proto.StateProofInterval) // the first state proof was confirmed keys for that state proof can be removed + requireDeletedKeysToBeDeletedBefore(t, s, basics.Round(proto.StateProofInterval)+firstExpectedStateproof) + checkedKeys = s.StateProofKeys(firstExpectedStateproof) require.Equal(t, 0, len(checkedKeys)) checkedKeys = s.StateProofKeys(basics.Round(3 * proto.StateProofInterval)) From cc22234efdfe972b65cac221f2fc9d045dcfd526 Mon Sep 17 00:00:00 2001 From: algoidan Date: Tue, 25 Oct 2022 17:49:57 +0300 Subject: [PATCH 21/25] minor refactor --- stateproof/worker_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/stateproof/worker_test.go b/stateproof/worker_test.go index 57062b9a64..c9808220b6 100644 --- a/stateproof/worker_test.go +++ b/stateproof/worker_test.go @@ -653,7 +653,7 @@ func TestKeysRemoveOnlyAfterStateProofAccepted(t *testing.T) { requireDeletedKeysToBeDeletedBefore(t, s, firstExpectedStateproof+basics.Round(proto.StateProofInterval)) checkedKeys = s.StateProofKeys(firstExpectedStateproof) require.Equal(t, 0, len(checkedKeys)) - checkedKeys = s.StateProofKeys(basics.Round(3 * proto.StateProofInterval)) + checkedKeys = s.StateProofKeys(firstExpectedStateproof + basics.Round(proto.StateProofInterval)) require.Equal(t, len(keys), len(checkedKeys)) } @@ -696,7 +696,7 @@ func TestKeysRemoveOnlyAfterStateProofAcceptedSmallIntervals(t *testing.T) { checkedKeys = s.StateProofKeys(firstExpectedStateproof) require.Equal(t, len(keys), len(checkedKeys)) - checkedKeys = s.StateProofKeys(basics.Round(3 * proto.StateProofInterval)) + checkedKeys = s.StateProofKeys(firstExpectedStateproof + basics.Round(proto.StateProofInterval)) require.Equal(t, len(keys), len(checkedKeys)) } @@ -736,7 +736,7 @@ func TestKeysRemoveOnlyAfterStateProofAcceptedLargeIntervals(t *testing.T) { checkedKeys = s.StateProofKeys(firstExpectedStateproof) require.Equal(t, 0, len(checkedKeys)) - checkedKeys = s.StateProofKeys(basics.Round(3 * proto.StateProofInterval)) + checkedKeys = s.StateProofKeys(firstExpectedStateproof + basics.Round(proto.StateProofInterval)) require.Equal(t, len(keys), len(checkedKeys)) } From 100c282625d336244adf7c5b49b021ad4e1d4160 Mon Sep 17 00:00:00 2001 From: algoidan Date: Wed, 26 Oct 2022 13:32:42 +0300 Subject: [PATCH 22/25] we now remove the last ephemeral key --- data/accountManager.go | 13 ++++ stateproof/abstractions.go | 1 + stateproof/builder.go | 1 + stateproof/stateproofMessageGenerator_test.go | 4 ++ stateproof/worker_test.go | 63 +++++++++++++++++++ 5 files changed, 82 insertions(+) diff --git a/data/accountManager.go b/data/accountManager.go index 09f9ff4c9a..63e923a618 100644 --- a/data/accountManager.go +++ b/data/accountManager.go @@ -62,6 +62,19 @@ func MakeAccountManager(log logging.Logger, registry account.ParticipationRegist return manager } +// RemoveStateProofKeysForExpiredAccounts removes ephemeral keys for every expired account +func (manager *AccountManager) RemoveStateProofKeysForExpiredAccounts(currentRound basics.Round) { + for _, part := range manager.registry.GetAll() { + if currentRound <= part.LastValid { + continue + } + // since DeleteStateProofKeys doesn't remove the last round, we add one to make sure all secrets are being removed + if err := manager.registry.DeleteStateProofKeys(part.ParticipationID, part.LastValid+1); err != nil { + manager.log.Warnf("error while removing state proof keys for participant %v on round %v: %v", part.ParticipationID, part.LastValid+1, err) + } + } +} + // Keys returns a list of Participation accounts, and their keys/secrets for requested round. func (manager *AccountManager) Keys(rnd basics.Round) (out []account.ParticipationRecordForRound) { for _, part := range manager.registry.GetAll() { diff --git a/stateproof/abstractions.go b/stateproof/abstractions.go index 825b5090ea..5c802d92b6 100644 --- a/stateproof/abstractions.go +++ b/stateproof/abstractions.go @@ -55,6 +55,7 @@ type Network interface { type Accounts interface { StateProofKeys(basics.Round) []account.StateProofSecretsForRound DeleteStateProofKey(id account.ParticipationID, round basics.Round) error + RemoveStateProofKeysForExpiredAccounts(currentRound basics.Round) } // BlockHeaderFetcher captures the aspects of the Ledger that is used to fetch block headers diff --git a/stateproof/builder.go b/stateproof/builder.go index bb072dc9c1..7d954f4663 100644 --- a/stateproof/builder.go +++ b/stateproof/builder.go @@ -488,6 +488,7 @@ func (spw *Worker) deleteStaleKeys(latestRoundToKeep basics.Round) { spw.log.Warnf("deleteOldKeys: could not remove key for account %v on round %s: %v", key.ParticipationID, roundToRemove, err) } } + spw.accts.RemoveStateProofKeysForExpiredAccounts(latestRoundToKeep) } func (spw *Worker) deleteStaleBuilders(latestRoundToKeep basics.Round) { diff --git a/stateproof/stateproofMessageGenerator_test.go b/stateproof/stateproofMessageGenerator_test.go index 4bdded817c..630e901e9c 100644 --- a/stateproof/stateproofMessageGenerator_test.go +++ b/stateproof/stateproofMessageGenerator_test.go @@ -53,6 +53,10 @@ func (s *workerForStateProofMessageTests) DeleteStateProofKey(id account.Partici return s.w.DeleteStateProofKey(id, round) } +func (s *workerForStateProofMessageTests) RemoveStateProofKeysForExpiredAccounts(currentRound basics.Round) { + s.w.RemoveStateProofKeysForExpiredAccounts(currentRound) +} + func (s *workerForStateProofMessageTests) Latest() basics.Round { return s.w.Latest() } diff --git a/stateproof/worker_test.go b/stateproof/worker_test.go index c9808220b6..985d70d758 100644 --- a/stateproof/worker_test.go +++ b/stateproof/worker_test.go @@ -149,6 +149,9 @@ func (s *testWorkerStubs) StateProofKeys(rnd basics.Round) (out []account.StateP if basics.Round(KeyInLifeTime) < s.deletedKeysBeforeRoundMap[part.ID()] { continue } + if part.LastValid < rnd { + continue + } partRecordForRound := account.StateProofSecretsForRound{ ParticipationRecord: partRecord, StateProofSecrets: signerInRound, @@ -158,6 +161,17 @@ func (s *testWorkerStubs) StateProofKeys(rnd basics.Round) (out []account.StateP return } +func (s *testWorkerStubs) RemoveStateProofKeysForExpiredAccounts(currentRound basics.Round) { + s.mu.Lock() + defer s.mu.Unlock() + for _, part := range s.keys { + if currentRound <= part.LastValid { + continue + } + s.deletedKeysBeforeRoundMap[part.ID()] = currentRound + } +} + func (s *testWorkerStubs) DeleteStateProofKey(id account.ParticipationID, round basics.Round) error { s.mu.Lock() s.deletedKeysBeforeRoundMap[id] = round @@ -625,6 +639,55 @@ func requireDeletedKeysToBeDeletedBefore(t *testing.T, s *testWorkerStubs, thres } } +func TestAllKeysRemovedAfterExpiration(t *testing.T) { + partitiontest.PartitionTest(t) + + proto := config.Consensus[protocol.ConsensusCurrentVersion] + firstExpectedStateproof := basics.Round(proto.StateProofInterval * 2) + + var keys []account.Participation + for i := 0; i < 2; i++ { + var parent basics.Address + crypto.RandBytes(parent[:]) + p := newPartKeyWithVersion(t, proto, parent) + defer p.Close() + keys = append(keys, p.Participation) + } + + s := newWorkerStubs(t, keys, 10) + dbs, _ := dbOpenTest(t, true) + + logger := logging.NewLogger() + logger.SetOutput(io.Discard) + + w := NewWorker(dbs.Wdb, logger, s, s, s, s) + w.Start() + defer w.Shutdown() + + advanceRoundsAndStateProofsSlowly(t, s, uint64(firstExpectedStateproof)+(12*proto.StateProofInterval)) + s.mu.Lock() + for _, prt := range s.keys { + require.Equal(t, uint64(prt.LastValid)-proto.StateProofInterval, uint64(s.deletedKeysBeforeRoundMap[prt.ID()])) + } + s.mu.Unlock() + + advanceRoundsAndStateProofsSlowly(t, s, proto.StateProofInterval) + + s.mu.Lock() + for _, prt := range s.keys { + require.Equal(t, prt.LastValid, s.deletedKeysBeforeRoundMap[prt.ID()]) + } + s.mu.Unlock() + + advanceRoundsAndStateProofsSlowly(t, s, proto.StateProofInterval) + + s.mu.Lock() + for _, prt := range s.keys { + require.Less(t, prt.LastValid, s.deletedKeysBeforeRoundMap[prt.ID()]) + } + s.mu.Unlock() +} + func TestKeysRemoveOnlyAfterStateProofAccepted(t *testing.T) { partitiontest.PartitionTest(t) From c5f89f137fc40a57b396be4c4a9f598fe5c3deeb Mon Sep 17 00:00:00 2001 From: algoidan Date: Wed, 26 Oct 2022 13:44:47 +0300 Subject: [PATCH 23/25] nits --- data/account/participationRegistry.go | 2 +- data/accountManager.go | 10 +++---- stateproof/abstractions.go | 2 +- stateproof/builder.go | 26 +++++++++---------- stateproof/stateproofMessageGenerator_test.go | 4 +-- stateproof/worker_test.go | 2 +- 6 files changed, 23 insertions(+), 23 deletions(-) diff --git a/data/account/participationRegistry.go b/data/account/participationRegistry.go index c4bdd49452..483acde5d9 100644 --- a/data/account/participationRegistry.go +++ b/data/account/participationRegistry.go @@ -232,7 +232,7 @@ type ParticipationRegistry interface { // once, an error will occur when the data is flushed when inserting a duplicate key. AppendKeys(id ParticipationID, keys StateProofKeys) error - // DeleteStateProofKeys removes all stateproof keys preceding a given round (excluded) + // DeleteStateProofKeys removes all stateproof keys up to, and not including, a given round DeleteStateProofKeys(id ParticipationID, round basics.Round) error // Delete removes a record from storage. diff --git a/data/accountManager.go b/data/accountManager.go index 63e923a618..19fbb91f3b 100644 --- a/data/accountManager.go +++ b/data/accountManager.go @@ -45,8 +45,8 @@ type AccountManager struct { log logging.Logger } -// DeleteStateProofKey deletes all keys connected to ParticipationID that came before the given round. -// The round is excluded. +// DeleteStateProofKey deletes keys related to a ParticipationID. The function removes +// all keys up to, and not including, a given round the given round. func (manager *AccountManager) DeleteStateProofKey(id account.ParticipationID, round basics.Round) error { return manager.registry.DeleteStateProofKeys(id, round) } @@ -62,14 +62,14 @@ func MakeAccountManager(log logging.Logger, registry account.ParticipationRegist return manager } -// RemoveStateProofKeysForExpiredAccounts removes ephemeral keys for every expired account -func (manager *AccountManager) RemoveStateProofKeysForExpiredAccounts(currentRound basics.Round) { +// DeleteStateProofKeysForExpiredAccounts removes ephemeral keys for every expired account +func (manager *AccountManager) DeleteStateProofKeysForExpiredAccounts(currentRound basics.Round) { for _, part := range manager.registry.GetAll() { if currentRound <= part.LastValid { continue } // since DeleteStateProofKeys doesn't remove the last round, we add one to make sure all secrets are being removed - if err := manager.registry.DeleteStateProofKeys(part.ParticipationID, part.LastValid+1); err != nil { + if err := manager.DeleteStateProofKey(part.ParticipationID, part.LastValid+1); err != nil { manager.log.Warnf("error while removing state proof keys for participant %v on round %v: %v", part.ParticipationID, part.LastValid+1, err) } } diff --git a/stateproof/abstractions.go b/stateproof/abstractions.go index 5c802d92b6..951bb76587 100644 --- a/stateproof/abstractions.go +++ b/stateproof/abstractions.go @@ -55,7 +55,7 @@ type Network interface { type Accounts interface { StateProofKeys(basics.Round) []account.StateProofSecretsForRound DeleteStateProofKey(id account.ParticipationID, round basics.Round) error - RemoveStateProofKeysForExpiredAccounts(currentRound basics.Round) + DeleteStateProofKeysForExpiredAccounts(currentRound basics.Round) } // BlockHeaderFetcher captures the aspects of the Ledger that is used to fetch block headers diff --git a/stateproof/builder.go b/stateproof/builder.go index 7d954f4663..fa955af227 100644 --- a/stateproof/builder.go +++ b/stateproof/builder.go @@ -466,43 +466,43 @@ func (spw *Worker) deleteStaleStateProofBuildData(currentHdr *bookkeeping.BlockH spw.LastCleanupRound = stateProofNextRound } -func (spw *Worker) deleteStaleSigs(latestRoundToKeep basics.Round) { +func (spw *Worker) deleteStaleSigs(retainRound basics.Round) { err := spw.db.Atomic(func(ctx context.Context, tx *sql.Tx) error { - return deletePendingSigsBeforeRound(tx, latestRoundToKeep) + return deletePendingSigsBeforeRound(tx, retainRound) }) if err != nil { - spw.log.Warnf("deletePendingSigsBeforeRound(%d): %v", latestRoundToKeep, err) + spw.log.Warnf("deleteStaleSigs(%d): %v", retainRound, err) } } -func (spw *Worker) deleteStaleKeys(latestRoundToKeep basics.Round) { - keys := spw.accts.StateProofKeys(latestRoundToKeep) +func (spw *Worker) deleteStaleKeys(retainRound basics.Round) { + keys := spw.accts.StateProofKeys(retainRound) for _, key := range keys { - roundToRemove, err := key.StateProofSecrets.FirstRoundInKeyLifetime() + firstRoundAtKeyLifeTime, err := key.StateProofSecrets.FirstRoundInKeyLifetime() if err != nil { - spw.log.Errorf("deleteOldKeys: could not calculate keylifetime for account %v on round %s: %v", key.ParticipationID, roundToRemove, err) + spw.log.Errorf("deleteStaleKeys: could not calculate keylifetime for account %v on round %s: %v", key.ParticipationID, firstRoundAtKeyLifeTime, err) continue } - err = spw.accts.DeleteStateProofKey(key.ParticipationID, basics.Round(roundToRemove)) + err = spw.accts.DeleteStateProofKey(key.ParticipationID, basics.Round(firstRoundAtKeyLifeTime)) if err != nil { - spw.log.Warnf("deleteOldKeys: could not remove key for account %v on round %s: %v", key.ParticipationID, roundToRemove, err) + spw.log.Warnf("deleteStaleKeys: could not remove key for account %v on round %s: %v", key.ParticipationID, firstRoundAtKeyLifeTime, err) } } - spw.accts.RemoveStateProofKeysForExpiredAccounts(latestRoundToKeep) + spw.accts.DeleteStateProofKeysForExpiredAccounts(retainRound) } -func (spw *Worker) deleteStaleBuilders(latestRoundToKeep basics.Round) { +func (spw *Worker) deleteStaleBuilders(retainRound basics.Round) { spw.mu.Lock() defer spw.mu.Unlock() for rnd := range spw.builders { - if rnd < latestRoundToKeep { + if rnd < retainRound { delete(spw.builders, rnd) } } err := spw.db.Atomic(func(ctx context.Context, tx *sql.Tx) error { - return deleteBuilders(tx, latestRoundToKeep) + return deleteBuilders(tx, retainRound) }) if err != nil { spw.log.Warnf("deleteOldBuilders: failed to delete builders from database: %v", err) diff --git a/stateproof/stateproofMessageGenerator_test.go b/stateproof/stateproofMessageGenerator_test.go index 630e901e9c..bb68b89816 100644 --- a/stateproof/stateproofMessageGenerator_test.go +++ b/stateproof/stateproofMessageGenerator_test.go @@ -53,8 +53,8 @@ func (s *workerForStateProofMessageTests) DeleteStateProofKey(id account.Partici return s.w.DeleteStateProofKey(id, round) } -func (s *workerForStateProofMessageTests) RemoveStateProofKeysForExpiredAccounts(currentRound basics.Round) { - s.w.RemoveStateProofKeysForExpiredAccounts(currentRound) +func (s *workerForStateProofMessageTests) DeleteStateProofKeysForExpiredAccounts(currentRound basics.Round) { + s.w.DeleteStateProofKeysForExpiredAccounts(currentRound) } func (s *workerForStateProofMessageTests) Latest() basics.Round { diff --git a/stateproof/worker_test.go b/stateproof/worker_test.go index 985d70d758..f9465d4e40 100644 --- a/stateproof/worker_test.go +++ b/stateproof/worker_test.go @@ -161,7 +161,7 @@ func (s *testWorkerStubs) StateProofKeys(rnd basics.Round) (out []account.StateP return } -func (s *testWorkerStubs) RemoveStateProofKeysForExpiredAccounts(currentRound basics.Round) { +func (s *testWorkerStubs) DeleteStateProofKeysForExpiredAccounts(currentRound basics.Round) { s.mu.Lock() defer s.mu.Unlock() for _, part := range s.keys { From abb58c14075b0b2eda83a33ea8d4fc0f97ebfdce Mon Sep 17 00:00:00 2001 From: algoidan Date: Wed, 26 Oct 2022 14:20:49 +0300 Subject: [PATCH 24/25] add test for deleting all keys in account manger --- data/accountManager_test.go | 57 +++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/data/accountManager_test.go b/data/accountManager_test.go index 1fcfe56bf7..e570c825ee 100644 --- a/data/accountManager_test.go +++ b/data/accountManager_test.go @@ -250,6 +250,63 @@ func TestAccountManagerOverlappingStateProofKeys(t *testing.T) { a.Equal(1, len(res)) } +func TestAccountManagerRemoveStateProofKeysForExpiredAccounts(t *testing.T) { + partitiontest.PartitionTest(t) + a := assert.New(t) + + registry, dbName := getRegistryImpl(t, false, true) + defer registryCloseTest(t, registry, dbName) + + log := logging.TestingLog(t) + log.SetLevel(logging.Error) + + acctManager := MakeAccountManager(log, registry) + + databaseFiles := make([]string, 0) + defer func() { + for _, fileName := range databaseFiles { + os.Remove(fileName) + os.Remove(fileName + "-shm") + os.Remove(fileName + "-wal") + os.Remove(fileName + "-journal") + } + }() + + store, err := db.MakeAccessor("stateprooftest", false, true) + a.NoError(err) + root, err := account.GenerateRoot(store) + a.NoError(err) + part1, err := account.FillDBWithParticipationKeys(store, root.Address(), 0, basics.Round(merklesignature.KeyLifetimeDefault*2), 3) + a.NoError(err) + store.Close() + + keys1 := part1.StateProofSecrets.GetAllKeys() + + // Add participations to the registry and append StateProof keys as well + part1ID, err := acctManager.registry.Insert(part1.Participation) + a.NoError(err) + err = registry.AppendKeys(part1ID, keys1) + a.NoError(err) + + err = acctManager.registry.Flush(10 * time.Second) + a.NoError(err) + + for i := 1; i <= 2; i++ { + res := acctManager.StateProofKeys(basics.Round(i * merklesignature.KeyLifetimeDefault)) + a.Equal(1, len(res)) + } + + acctManager.DeleteStateProofKeysForExpiredAccounts(part1.LastValid + 1) + err = acctManager.registry.Flush(10 * time.Second) + a.NoError(err) + + for i := 1; i <= 2; i++ { + res := acctManager.StateProofKeys(basics.Round(i * merklesignature.KeyLifetimeDefault)) + a.Equal(0, len(res)) + } + +} + func TestGetStateProofKeysDontLogErrorOnNilStateProof(t *testing.T) { partitiontest.PartitionTest(t) a := assert.New(t) From ae5aa9bc62b52863a29f8664a85be5586abb4ae5 Mon Sep 17 00:00:00 2001 From: algoidan Date: Wed, 26 Oct 2022 14:33:53 +0300 Subject: [PATCH 25/25] move warning message to error --- data/accountManager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data/accountManager.go b/data/accountManager.go index 19fbb91f3b..6ca7d4cfa4 100644 --- a/data/accountManager.go +++ b/data/accountManager.go @@ -96,7 +96,7 @@ func (manager *AccountManager) StateProofKeys(rnd basics.Round) (out []account.S if part.StateProof != nil && part.OverlapsInterval(rnd, rnd) { partRndSecrets, err := manager.registry.GetStateProofSecretsForRound(part.ParticipationID, rnd) if err != nil { - manager.log.Errorf("error while loading round secrets from participation registry: %v", err) + manager.log.Warnf("could not load state proof keys from participation registry: %v", err) continue } out = append(out, partRndSecrets)