From c8dd38907e4c3e2094cf35f57a2d6a3dcae1e1ad Mon Sep 17 00:00:00 2001 From: John Jannotti Date: Wed, 10 Jan 2024 15:04:54 -0500 Subject: [PATCH] Use SelectionID to detect when to zero out StateProofID --- ledger/store/trackerdb/sqlitedriver/schema.go | 10 +++++----- ledger/store/trackerdb/sqlitedriver/schema_test.go | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/ledger/store/trackerdb/sqlitedriver/schema.go b/ledger/store/trackerdb/sqlitedriver/schema.go index 0450784ae3..a36dd2cd90 100644 --- a/ledger/store/trackerdb/sqlitedriver/schema.go +++ b/ledger/store/trackerdb/sqlitedriver/schema.go @@ -735,11 +735,11 @@ func performOnlineAccountsTableMigration(ctx context.Context, e db.Executable, p } // We had a bug that didn't remove StateProofIDs when going offline. - // Tidy up such accounts. This would be wrong to run after the - // incentives work is complete, since voting material is retained for - // suspended accounts, which are offline. But we won't be running this - // schema upgrade on future databases. - if ba.Status != basics.Online && !ba.StateProofID.IsEmpty() { + // Tidy up such accounts. We don't zero it out based on + // `!basics.Online` because accounts can be suspended, in which case + // they are Offline, but retain their voting material. But it remains + // illegal to have a StateProofID without a SelectionID. + if ba.SelectionID.IsEmpty() && !ba.StateProofID.IsEmpty() { // store old data for account hash update state := acctState{old: ba, oldEnc: encodedAcctData} ba.StateProofID = merklesignature.Commitment{} diff --git a/ledger/store/trackerdb/sqlitedriver/schema_test.go b/ledger/store/trackerdb/sqlitedriver/schema_test.go index 31a3d76756..9143eba9c1 100644 --- a/ledger/store/trackerdb/sqlitedriver/schema_test.go +++ b/ledger/store/trackerdb/sqlitedriver/schema_test.go @@ -166,7 +166,7 @@ func TestAccountDBTxTailLoad(t *testing.T) { } } -func TestRemoveOfflineStateProofID(t *testing.T) { +func TestRemoveStrayStateProofID(t *testing.T) { partitiontest.PartitionTest(t) accts := ledgertesting.RandomAccounts(20, true) @@ -176,7 +176,7 @@ func TestRemoveOfflineStateProofID(t *testing.T) { accts[addr] = acct expectedAcct := acct - if acct.Status != basics.Online { + if acct.SelectionID.IsEmpty() { expectedAcct.StateProofID = merklesignature.Commitment{} } expectedAccts[addr] = expectedAcct @@ -236,8 +236,8 @@ func TestRemoveOfflineStateProofID(t *testing.T) { var ba trackerdb.BaseAccountData err = protocol.Decode(encodedAcctData, &ba) require.NoError(t, err) - if expected && (ba.Status != basics.Online) { - require.Equal(t, merklesignature.Commitment{}, ba.StateProofID) + if expected && ba.SelectionID.IsEmpty() { + require.Zero(t, ba.StateProofID) } addHash := trackerdb.AccountHashBuilderV6(addr, &ba, encodedAcctData) added, err := trie.Add(addHash) @@ -286,8 +286,8 @@ func TestRemoveOfflineStateProofID(t *testing.T) { var ba trackerdb.BaseAccountData err = protocol.Decode(encodedAcctData, &ba) require.NoError(t, err) - if ba.Status != basics.Online { - require.True(t, ba.StateProofID.IsEmpty()) + if ba.SelectionID.IsEmpty() { + require.Zero(t, ba.StateProofID) } } }