Use SelectionID to detect when to zero out StateProofID

algorand · Jan 10, 2024 · c8dd389 · c8dd389
1 parent 1afb148
commit c8dd389
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 11 deletions.
diff --git 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
@@ -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)
 		}
 	}
 }