catchpoints: add accountsRound check against R-320 horizon values to catchpointFileWriter

cmd/catchpointdump/file.go

@@ -591,7 +591,7 @@ func printOnlineAccounts(databaseName string, stagingTables bool, outFile *os.Fi
 	}
 
 	return dbAccessor.Atomic(func(ctx context.Context, tx *sql.Tx) error {
-		rows, err := sqlitedriver.MakeOnlineAccountsIter(ctx, tx, stagingTables, 0)
+		rows, err := sqlitedriver.MakeOrderedOnlineAccountsIter(ctx, tx, stagingTables, 0)
 		if err != nil {
 			return err
 		}

ledger/acctdeltas_test.go

@@ -2936,7 +2936,7 @@ func testOnlineAccountsDeletion(t *testing.T, addrA, addrB basics.Address, tx *s
 func testOnlineAccountsExcludeBefore(t *testing.T, addrA, addrB basics.Address, tx *sql.Tx) {
 	// Use MakeOnlineAccountsIter to dump all data, starting from rnd
 	getAcctDataForRound := func(rnd basics.Round, expectedCount int64) map[basics.Address][]*encoded.OnlineAccountRecordV6 {
-		it, err := sqlitedriver.MakeOnlineAccountsIter(context.Background(), tx, false, rnd)
+		it, err := sqlitedriver.MakeOrderedOnlineAccountsIter(context.Background(), tx, false, rnd)
 		require.NoError(t, err)
 
 		var count int64

ledger/acctonline_test.go

@@ -1334,7 +1334,7 @@ func TestAcctOnlineVotersLongerHistory(t *testing.T) {
 	var foundCount int
 	err = oa.dbs.Snapshot(func(ctx context.Context, tx trackerdb.SnapshotScope) error {
 		// read staging = false, excludeBefore = excludeRound
-		it, err2 := tx.MakeOnlineAccountsIter(ctx, false, excludeRound)
+		it, err2 := tx.MakeOrderedOnlineAccountsIter(ctx, false, excludeRound)
 		require.NoError(t, err2)
 		defer it.Close()
 

ledger/catchpointfilewriter.go

@@ -70,12 +70,15 @@ type catchpointFileWriter struct {
 	biggestChunkLen        uint64
 	accountsIterator       trackerdb.EncodedAccountsBatchIter
 	maxResourcesPerChunk   int
+	accountsRound          basics.Round
 	onlineExcludeBefore    basics.Round
 	accountsDone           bool
 	kvRows                 trackerdb.KVsIter
 	kvDone                 bool
 	onlineAccountRows      trackerdb.TableIterator[*encoded.OnlineAccountRecordV6]
 	onlineAccountsDone     bool
+	onlineAccountPrev      basics.Address
+	onlineAccountPrevRound basics.Round
 	onlineRoundParamsRows  trackerdb.TableIterator[*encoded.OnlineRoundParamsRecordV6]
 	onlineRoundParamsDone  bool
 }
@@ -111,7 +114,7 @@ func (data catchpointStateProofVerificationContext) ToBeHashed() (protocol.HashI
 	return protocol.StateProofVerCtx, protocol.Encode(&data)
 }
 
-func makeCatchpointFileWriter(ctx context.Context, params config.ConsensusParams, filePath string, tx trackerdb.SnapshotScope, maxResourcesPerChunk int, onlineExcludeBefore basics.Round) (*catchpointFileWriter, error) {
+func makeCatchpointFileWriter(ctx context.Context, params config.ConsensusParams, filePath string, tx trackerdb.SnapshotScope, maxResourcesPerChunk int, accountsRound, onlineExcludeBefore basics.Round) (*catchpointFileWriter, error) {
 	aw, err := tx.MakeAccountsReader()
 	if err != nil {
 		return nil, err
@@ -167,6 +170,7 @@ func makeCatchpointFileWriter(ctx context.Context, params config.ConsensusParams
 		compressor:             compressor,
 		tar:                    tar,
 		accountsIterator:       tx.MakeEncodedAccountsBatchIter(),
+		accountsRound:          accountsRound,
 		maxResourcesPerChunk:   maxResourcesPerChunk,
 		onlineExcludeBefore:    onlineExcludeBefore,
 	}
@@ -385,7 +389,8 @@ func (cw *catchpointFileWriter) readDatabaseStep(ctx context.Context) error {
 	if cw.params.EnableCatchpointsWithOnlineAccounts && !cw.onlineAccountsDone {
 		// Create the OnlineAccounts iterator JIT
 		if cw.onlineAccountRows == nil {
-			rows, err := cw.tx.MakeOnlineAccountsIter(ctx, false, cw.onlineExcludeBefore)
+			// MakeOrderedOnlineAccountsIter orders by (address, updateRound).
+			rows, err := cw.tx.MakeOrderedOnlineAccountsIter(ctx, false, cw.onlineExcludeBefore)
 			if err != nil {
 				return err
 			}
@@ -398,6 +403,70 @@ func (cw *catchpointFileWriter) readDatabaseStep(ctx context.Context) error {
 			if err != nil {
 				return err
 			}
+			// We set UpdateRound to 0 here, so that all nodes generating catchpoints will have the
+			// verification hash for the onlineaccounts table data (which is used to calculate the
+			// catchpoint label). Depending on the history of an online account, nodes may not have
+			// the same updateRound column value for the oldest "horizon" row for that address,
+			// depending on whether the node caught up from genesis, or restored from a
+			// catchpoint. This does not have any impact on the correctness of online account
+			// lookups, but is due to changes in the database schema over time:
+			//
+			//   1. For nodes that have been online for a long time, the unlimited assets release
+			//   (v3.5.1, PR #3652) introduced a BaseAccountData type with an UpdateRound field,
+			//   consensus-flagged to be zero until EnableAccountDataResourceSeparation was enabled
+			//   in consensus v32. So accounts that have been inactive since before consensus v32
+			//   will continue to have a zero UpdateRound, until a transaction updates the
+			//   account. This behavior is consistent for all nodes and validated by the merkle trie
+			//   generated each catchpoint round.
+			//
+			//   2. The onlineaccounts table, introduced later in v3.9.2 (PR #4003), uses a
+			//   migration to populate the onlineaccounts table by selecting all online accounts
+			//   from the accounts table. This migration copies the BaseAccountData.UpdateRound
+			//   field, along with voting data, to set the initial values of the onlineaccounts
+			//   table for each address. After that, the onlineaccounts table's updateRound column
+			//   would only be updated if voting data changed -- so certain transactions like
+			//   receiving a pay txn of 0 algos, or receiving an asset transfer, etc, would not
+			//   result in a new onlineaccounts row with a new updateRound (unless it triggered a
+			//   balance or voting data change). This criteria is implemented in
+			//   onlineAccountsNewRound in acctdeltas.go, separate from accountsNewRound &
+			//   makeCompactAccountDeltas, which set the account table's UpdateRound value.
+			//
+			//   3. Node operators using fast catchup to restore from a catchpoint file version V6
+			//   or V7 (used before v4.0.1 and consensus v40, which added the
+			//   EnableCatchpointsWithOnlineAccounts flag) initialize the onlineaccounts table by
+			//   first restoring the accounts table from the snapshot, then running the same
+			//   migration introduced in (2), where updateRound (and account data) comes from
+			//   BaseAccountData. This means catchpoint file writers and fast catchup users could
+			//   see some addresses have a horizon row with an updateRound that was set to zero
+			//   (case 1), or the round of the last account data change (case 2). Since v4.0.1,
+			//   catchpoint file version V8 includes the onlineaccounts and onlineroundparams tables
+			//   in snapshots, to support the voter_params_get and online_stake opcodes (PR #6177).
+			//
+			//   4. However, a node catching up from scratch without using fast catchup, running
+			//   v3.9.2 or later, must track the online account history to verify block certificates
+			//   as it validates each block in turn.  It sets updateRound based on observing all
+			//   account voting data changes starting from round 0, whether or not
+			//   EnableAccountDataResourceSeparation is set. These nodes will have horizon rows for
+			//   addresses with updateRound set to the round of the last actual voting data change,
+			//   not zero (case 1) or the round of the last account data change (case 2).
+			//
+
+			// Is the updateRound for this row beyond the lookback horizon (R-320)?
+			if oa.UpdateRound < catchpointLookbackHorizonForNextRound(cw.accountsRound, cw.params) {
+				// Is this the first (and thus oldest) row for this address?
+				if cw.onlineAccountPrev.IsZero() || cw.onlineAccountPrev != oa.Address {
+					// Then set it to 0.
+					oa.UpdateRound = 0
+				}
+
+				// This case should never happen: there should only be one horizon row per account.
+				if !cw.onlineAccountPrev.IsZero() && cw.onlineAccountPrev == oa.Address {
+					return fmt.Errorf("bad online account data: multiple horizon rows for %s, prev updround %d cur updround %d", oa.Address, cw.onlineAccountPrevRound, oa.UpdateRound)
+				}
+			}
+
+			cw.onlineAccountPrev = oa.Address
+			cw.onlineAccountPrevRound = oa.UpdateRound
 			onlineAccts = append(onlineAccts, *oa)
 			if len(onlineAccts) == BalancesPerCatchpointFileChunk {
 				break
@@ -462,3 +531,12 @@ func hasContextDeadlineExceeded(ctx context.Context) (contextExceeded bool, cont
 	}
 	return
 }
+
+// catchpointLookbackHorizonForNextRound returns the lookback horizon used to evaluate the next
+// round after the provided `rnd`, according to consensus settings in `params`. That is, to evaluate
+// blocks starting from rnd+1, this function returns the oldest round that will be needed to evaluate
+// votes, certificates or other consensus data. Anything older than the returned round is beyond
+// the horizon and needed to evaluate blocks starting from rnd+1.
+func catchpointLookbackHorizonForNextRound(rnd basics.Round, params config.ConsensusParams) basics.Round {
+	return (rnd + 1).SubSaturate(basics.Round(params.MaxBalLookback))
+}