diff --git a/ledger/catchpointfilewriter.go b/ledger/catchpointfilewriter.go index 8152030734..7daf0f86bc 100644 --- a/ledger/catchpointfilewriter.go +++ b/ledger/catchpointfilewriter.go @@ -77,8 +77,6 @@ type catchpointFileWriter struct { kvDone bool onlineAccountRows trackerdb.TableIterator[*encoded.OnlineAccountRecordV6] onlineAccountsDone bool - onlineAccountPrev basics.Address - onlineAccountPrevRound basics.Round onlineRoundParamsRows trackerdb.TableIterator[*encoded.OnlineRoundParamsRecordV6] onlineRoundParamsDone bool } @@ -390,7 +388,7 @@ func (cw *catchpointFileWriter) readDatabaseStep(ctx context.Context) error { // Create the OnlineAccounts iterator JIT if cw.onlineAccountRows == nil { // MakeOrderedOnlineAccountsIter orders by (address, updateRound). - rows, err := cw.tx.MakeOrderedOnlineAccountsIter(ctx, false, cw.onlineExcludeBefore) + rows, err := makeCatchpointOrderedOnlineAccountsIterFactory(cw.tx.MakeOrderedOnlineAccountsIter, cw.accountsRound, cw.params)(ctx, false, cw.onlineExcludeBefore) if err != nil { return err } @@ -403,70 +401,6 @@ 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 @@ -540,3 +474,108 @@ func hasContextDeadlineExceeded(ctx context.Context) (contextExceeded bool, cont func catchpointLookbackHorizonForNextRound(rnd basics.Round, params config.ConsensusParams) basics.Round { return (rnd + 1).SubSaturate(basics.Round(params.MaxBalLookback)) } + +type catchpointOnlineAccountsIterWrapper struct { + iter trackerdb.TableIterator[*encoded.OnlineAccountRecordV6] + onlineAccountPrev basics.Address + onlineAccountPrevRound basics.Round + accountsRound basics.Round + params config.ConsensusParams +} + +// makeCatchpointOrderedOnlineAccountsIter wraps the MakeOrderedOnlineAccountsIter iterator to deterministically set +// the UpdateRound number to zero for online accounts beyond the "horizon" of online history of 320 rounds (defined by +// MaxBalLookback). +func makeCatchpointOrderedOnlineAccountsIterFactory( + iterFactory func(context.Context, bool, basics.Round) (trackerdb.TableIterator[*encoded.OnlineAccountRecordV6], error), + accountsRound basics.Round, + params config.ConsensusParams, +) ( + wrappedIterFactory func(context.Context, bool, basics.Round) (trackerdb.TableIterator[*encoded.OnlineAccountRecordV6], error), +) { + // return an iterFactory that wraps the provided iterFactory + return func(ctx context.Context, useStaging bool, excludeBefore basics.Round) (trackerdb.TableIterator[*encoded.OnlineAccountRecordV6], error) { + iter, err := iterFactory(ctx, useStaging, excludeBefore) + if err != nil { + return nil, err + } + return &catchpointOnlineAccountsIterWrapper{ + iter: iter, + accountsRound: accountsRound, + params: params, + }, nil + } +} + +func (i *catchpointOnlineAccountsIterWrapper) Next() bool { return i.iter.Next() } +func (i *catchpointOnlineAccountsIterWrapper) Close() { i.iter.Close() } +func (i *catchpointOnlineAccountsIterWrapper) GetItem() (*encoded.OnlineAccountRecordV6, error) { + oa, err := i.iter.GetItem() + if err != nil { + return nil, 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(i.accountsRound, i.params) { + // Is this the first (and thus oldest) row for this address? + if i.onlineAccountPrev.IsZero() || i.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 !i.onlineAccountPrev.IsZero() && i.onlineAccountPrev == oa.Address { + return nil, fmt.Errorf("bad online account data: multiple horizon rows for %s, prev updround %d cur updround %d", oa.Address, i.onlineAccountPrevRound, oa.UpdateRound) + } + } + + i.onlineAccountPrev = oa.Address + i.onlineAccountPrevRound = oa.UpdateRound + return oa, nil +} diff --git a/ledger/catchpointfilewriter_test.go b/ledger/catchpointfilewriter_test.go index 6413c741c4..ae2280c4e6 100644 --- a/ledger/catchpointfilewriter_test.go +++ b/ledger/catchpointfilewriter_test.go @@ -1086,47 +1086,6 @@ func TestCatchpointAfterTxns(t *testing.T) { } } -type catchpointOnlineAccountsIterWrapper struct { - ts trackerdb.TransactionScope - iter trackerdb.TableIterator[*encoded.OnlineAccountRecordV6] - onlineAccountCurrent basics.Address - accountsRound basics.Round - params config.ConsensusParams -} - -// makeCatchpointOrderedOnlineAccountsIter wraps normal MakeOrderedOnlineAccountsIter iterator -// in order to manipulate the update round number to simulate the catchpoint generation process. -func (i *catchpointOnlineAccountsIterWrapper) makeCatchpointOrderedOnlineAccountsIter( - ctx context.Context, useStaging bool, excludeBefore basics.Round, -) (trackerdb.TableIterator[*encoded.OnlineAccountRecordV6], error) { - var err error - i.iter, err = i.ts.MakeOrderedOnlineAccountsIter(ctx, useStaging, excludeBefore) - if err != nil { - return nil, err - } - - return i, nil -} - -func (i *catchpointOnlineAccountsIterWrapper) Next() bool { return i.iter.Next() } -func (i *catchpointOnlineAccountsIterWrapper) Close() { i.iter.Close() } -func (i *catchpointOnlineAccountsIterWrapper) GetItem() (*encoded.OnlineAccountRecordV6, error) { - item, err := i.iter.GetItem() - if err != nil { - return nil, err - } - - // this is the same condition as in catchpointFileWriter.readDatabaseStep - if i.onlineAccountCurrent.IsZero() || i.onlineAccountCurrent != item.Address { - i.onlineAccountCurrent = item.Address - // If so, is the updateRound for this row beyond the lookback horizon (R-320)? Then set it to 0. - if item.UpdateRound < (i.accountsRound + 1).SubSaturate(basics.Round(i.params.MaxBalLookback)) { - item.UpdateRound = 0 - } - } - return item, nil -} - func TestCatchpointAfterStakeLookupTxns(t *testing.T) { partitiontest.PartitionTest(t) // t.Parallel() No: config.Consensus is modified @@ -1284,21 +1243,11 @@ assert var genOAHash, valOAHash crypto.Digest var genOARows, valOARows uint64 require.NoError(t, dl.generator.trackerDB().Transaction(func(ctx context.Context, tx trackerdb.TransactionScope) (err error) { - oaGenIterWrapper := catchpointOnlineAccountsIterWrapper{ - ts: tx, - accountsRound: genDBRound, - params: config.Consensus[proto], - } - genOAHash, genOARows, err = calculateVerificationHash(context.Background(), oaGenIterWrapper.makeCatchpointOrderedOnlineAccountsIter, onlineExcludeBefore, false) + genOAHash, genOARows, err = calculateVerificationHash(context.Background(), makeCatchpointOrderedOnlineAccountsIterFactory(tx.MakeOrderedOnlineAccountsIter, genDBRound, config.Consensus[proto]), onlineExcludeBefore, false) return err })) require.NoError(t, dl.validator.trackerDB().Transaction(func(ctx context.Context, tx trackerdb.TransactionScope) (err error) { - oaValIterWrapper := catchpointOnlineAccountsIterWrapper{ - ts: tx, - accountsRound: valDBRound, - params: config.Consensus[proto], - } - valOAHash, valOARows, err = calculateVerificationHash(context.Background(), oaValIterWrapper.makeCatchpointOrderedOnlineAccountsIter, onlineExcludeBefore, false) + valOAHash, valOARows, err = calculateVerificationHash(context.Background(), makeCatchpointOrderedOnlineAccountsIterFactory(tx.MakeOrderedOnlineAccountsIter, valDBRound, config.Consensus[proto]), onlineExcludeBefore, false) return err })) require.Equal(t, genOAHash, valOAHash) diff --git a/ledger/catchpointtracker.go b/ledger/catchpointtracker.go index 9b1a7e62f0..79ed834faa 100644 --- a/ledger/catchpointtracker.go +++ b/ledger/catchpointtracker.go @@ -256,7 +256,7 @@ func (ct *catchpointTracker) finishFirstStage(ctx context.Context, dbRound basic // Generate hashes of the onlineaccounts and onlineroundparams tables. err := ct.dbs.Snapshot(func(ctx context.Context, tx trackerdb.SnapshotScope) error { var dbErr error - onlineAccountsHash, _, dbErr = calculateVerificationHash(ctx, tx.MakeOrderedOnlineAccountsIter, onlineExcludeBefore, false) + onlineAccountsHash, _, dbErr = calculateVerificationHash(ctx, makeCatchpointOrderedOnlineAccountsIterFactory(tx.MakeOrderedOnlineAccountsIter, dbRound, params), onlineExcludeBefore, false) if dbErr != nil { return dbErr