Use SelectionID to detect when to zero out StateProofID

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

Hide file tree

Showing 3 changed files with 14 additions and 14 deletions.
diff --git a/ledger/eval_simple_test.go b/ledger/eval_simple_test.go
@@ -456,7 +456,7 @@ func TestAbsentTracking(t *testing.T) {
 		require.Zero(t, regger.LastProposed)
 		require.True(t, regger.Status == basics.Online)
 
-		if ver >= 39 {
+		if ver >= checkingBegins {
 			require.NotZero(t, regger.LastHeartbeat) // online keyreg caused update
 		} else {
 			require.Zero(t, regger.LastHeartbeat)
@@ -492,9 +492,9 @@ func TestAbsentTracking(t *testing.T) {
 			Receiver: addrs[0],
 			Amount:   0,
 		})
-		require.Equal(t, ver < 39, lookup(t, dl.generator, addrs[0]).Status == basics.Online)
+		require.Equal(t, ver < checkingBegins, lookup(t, dl.generator, addrs[0]).Status == basics.Online)
 		require.True(t, lookup(t, dl.generator, addrs[1]).Status == basics.Online)
-		require.Equal(t, ver < 39, lookup(t, dl.generator, addrs[2]).Status == basics.Online)
+		require.Equal(t, ver < checkingBegins, lookup(t, dl.generator, addrs[2]).Status == basics.Online)
 
 	})
 }

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)
 		}
 	}
 }