Skip to content

Commit d94b696

Browse files
algojohnleealgojohnlee
authored
Merge pull request #6254 from Algo-devops-service/relbeta4.0.2
2 parents 04909c4 + 2a2d6d1 commit d94b696

5 files changed

+120
-126
lines changed

‎ledger/catchpointfilewriter.go

+106-67
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,6 @@ type catchpointFileWriter struct {
7777
kvDone bool
7878
onlineAccountRows trackerdb.TableIterator[*encoded.OnlineAccountRecordV6]
7979
onlineAccountsDone bool
80-
onlineAccountPrev basics.Address
81-
onlineAccountPrevRound basics.Round
8280
onlineRoundParamsRows trackerdb.TableIterator[*encoded.OnlineRoundParamsRecordV6]
8381
onlineRoundParamsDone bool
8482
}
@@ -390,7 +388,7 @@ func (cw *catchpointFileWriter) readDatabaseStep(ctx context.Context) error {
390388
// Create the OnlineAccounts iterator JIT
391389
if cw.onlineAccountRows == nil {
392390
// MakeOrderedOnlineAccountsIter orders by (address, updateRound).
393-
rows, err := cw.tx.MakeOrderedOnlineAccountsIter(ctx, false, cw.onlineExcludeBefore)
391+
rows, err := makeCatchpointOrderedOnlineAccountsIterFactory(cw.tx.MakeOrderedOnlineAccountsIter, cw.accountsRound, cw.params)(ctx, false, cw.onlineExcludeBefore)
394392
if err != nil {
395393
return err
396394
}
@@ -403,70 +401,6 @@ func (cw *catchpointFileWriter) readDatabaseStep(ctx context.Context) error {
403401
if err != nil {
404402
return err
405403
}
406-
// We set UpdateRound to 0 here, so that all nodes generating catchpoints will have the
407-
// verification hash for the onlineaccounts table data (which is used to calculate the
408-
// catchpoint label). Depending on the history of an online account, nodes may not have
409-
// the same updateRound column value for the oldest "horizon" row for that address,
410-
// depending on whether the node caught up from genesis, or restored from a
411-
// catchpoint. This does not have any impact on the correctness of online account
412-
// lookups, but is due to changes in the database schema over time:
413-
//
414-
// 1. For nodes that have been online for a long time, the unlimited assets release
415-
// (v3.5.1, PR #3652) introduced a BaseAccountData type with an UpdateRound field,
416-
// consensus-flagged to be zero until EnableAccountDataResourceSeparation was enabled
417-
// in consensus v32. So accounts that have been inactive since before consensus v32
418-
// will continue to have a zero UpdateRound, until a transaction updates the
419-
// account. This behavior is consistent for all nodes and validated by the merkle trie
420-
// generated each catchpoint round.
421-
//
422-
// 2. The onlineaccounts table, introduced later in v3.9.2 (PR #4003), uses a
423-
// migration to populate the onlineaccounts table by selecting all online accounts
424-
// from the accounts table. This migration copies the BaseAccountData.UpdateRound
425-
// field, along with voting data, to set the initial values of the onlineaccounts
426-
// table for each address. After that, the onlineaccounts table's updateRound column
427-
// would only be updated if voting data changed -- so certain transactions like
428-
// receiving a pay txn of 0 algos, or receiving an asset transfer, etc, would not
429-
// result in a new onlineaccounts row with a new updateRound (unless it triggered a
430-
// balance or voting data change). This criteria is implemented in
431-
// onlineAccountsNewRound in acctdeltas.go, separate from accountsNewRound &
432-
// makeCompactAccountDeltas, which set the account table's UpdateRound value.
433-
//
434-
// 3. Node operators using fast catchup to restore from a catchpoint file version V6
435-
// or V7 (used before v4.0.1 and consensus v40, which added the
436-
// EnableCatchpointsWithOnlineAccounts flag) initialize the onlineaccounts table by
437-
// first restoring the accounts table from the snapshot, then running the same
438-
// migration introduced in (2), where updateRound (and account data) comes from
439-
// BaseAccountData. This means catchpoint file writers and fast catchup users could
440-
// see some addresses have a horizon row with an updateRound that was set to zero
441-
// (case 1), or the round of the last account data change (case 2). Since v4.0.1,
442-
// catchpoint file version V8 includes the onlineaccounts and onlineroundparams tables
443-
// in snapshots, to support the voter_params_get and online_stake opcodes (PR #6177).
444-
//
445-
// 4. However, a node catching up from scratch without using fast catchup, running
446-
// v3.9.2 or later, must track the online account history to verify block certificates
447-
// as it validates each block in turn. It sets updateRound based on observing all
448-
// account voting data changes starting from round 0, whether or not
449-
// EnableAccountDataResourceSeparation is set. These nodes will have horizon rows for
450-
// addresses with updateRound set to the round of the last actual voting data change,
451-
// not zero (case 1) or the round of the last account data change (case 2).
452-
//
453-
454-
// Is the updateRound for this row beyond the lookback horizon (R-320)?
455-
if oa.UpdateRound < catchpointLookbackHorizonForNextRound(cw.accountsRound, cw.params) {
456-
// Is this the first (and thus oldest) row for this address?
457-
if cw.onlineAccountPrev.IsZero() || cw.onlineAccountPrev != oa.Address {
458-
// Then set it to 0.
459-
oa.UpdateRound = 0
460-
}
461-
462-
// This case should never happen: there should only be one horizon row per account.
463-
if !cw.onlineAccountPrev.IsZero() && cw.onlineAccountPrev == oa.Address {
464-
return fmt.Errorf("bad online account data: multiple horizon rows for %s, prev updround %d cur updround %d", oa.Address, cw.onlineAccountPrevRound, oa.UpdateRound)
465-
}
466-
}
467-
468-
cw.onlineAccountPrev = oa.Address
469-
cw.onlineAccountPrevRound = oa.UpdateRound
470404
onlineAccts = append(onlineAccts, *oa)
471405
if len(onlineAccts) == BalancesPerCatchpointFileChunk {
472406
break
@@ -540,3 +474,108 @@ func hasContextDeadlineExceeded(ctx context.Context) (contextExceeded bool, cont
540474
func catchpointLookbackHorizonForNextRound(rnd basics.Round, params config.ConsensusParams) basics.Round {
541475
return (rnd + 1).SubSaturate(basics.Round(params.MaxBalLookback))
542476
}
477+
478+
type catchpointOnlineAccountsIterWrapper struct {
479+
iter trackerdb.TableIterator[*encoded.OnlineAccountRecordV6]
480+
onlineAccountPrev basics.Address
481+
onlineAccountPrevRound basics.Round
482+
accountsRound basics.Round
483+
params config.ConsensusParams
484+
}
485+
486+
// makeCatchpointOrderedOnlineAccountsIter wraps the MakeOrderedOnlineAccountsIter iterator to deterministically set
487+
// the UpdateRound number to zero for online accounts beyond the "horizon" of online history of 320 rounds (defined by
488+
// MaxBalLookback).
489+
func makeCatchpointOrderedOnlineAccountsIterFactory(
490+
iterFactory func(context.Context, bool, basics.Round) (trackerdb.TableIterator[*encoded.OnlineAccountRecordV6], error),
491+
accountsRound basics.Round,
492+
params config.ConsensusParams,
493+
) (
494+
wrappedIterFactory func(context.Context, bool, basics.Round) (trackerdb.TableIterator[*encoded.OnlineAccountRecordV6], error),
495+
) {
496+
// return an iterFactory that wraps the provided iterFactory
497+
return func(ctx context.Context, useStaging bool, excludeBefore basics.Round) (trackerdb.TableIterator[*encoded.OnlineAccountRecordV6], error) {
498+
iter, err := iterFactory(ctx, useStaging, excludeBefore)
499+
if err != nil {
500+
return nil, err
501+
}
502+
return &catchpointOnlineAccountsIterWrapper{
503+
iter: iter,
504+
accountsRound: accountsRound,
505+
params: params,
506+
}, nil
507+
}
508+
}
509+
510+
func (i *catchpointOnlineAccountsIterWrapper) Next() bool { return i.iter.Next() }
511+
func (i *catchpointOnlineAccountsIterWrapper) Close() { i.iter.Close() }
512+
func (i *catchpointOnlineAccountsIterWrapper) GetItem() (*encoded.OnlineAccountRecordV6, error) {
513+
oa, err := i.iter.GetItem()
514+
if err != nil {
515+
return nil, err
516+
}
517+
// We set UpdateRound to 0 here, so that all nodes generating catchpoints will have the
518+
// verification hash for the onlineaccounts table data (which is used to calculate the
519+
// catchpoint label). Depending on the history of an online account, nodes may not have
520+
// the same updateRound column value for the oldest "horizon" row for that address,
521+
// depending on whether the node caught up from genesis, or restored from a
522+
// catchpoint. This does not have any impact on the correctness of online account
523+
// lookups, but is due to changes in the database schema over time:
524+
//
525+
// 1. For nodes that have been online for a long time, the unlimited assets release
526+
// (v3.5.1, PR #3652) introduced a BaseAccountData type with an UpdateRound field,
527+
// consensus-flagged to be zero until EnableAccountDataResourceSeparation was enabled
528+
// in consensus v32. So accounts that have been inactive since before consensus v32
529+
// will continue to have a zero UpdateRound, until a transaction updates the
530+
// account. This behavior is consistent for all nodes and validated by the merkle trie
531+
// generated each catchpoint round.
532+
//
533+
// 2. The onlineaccounts table, introduced later in v3.9.2 (PR #4003), uses a
534+
// migration to populate the onlineaccounts table by selecting all online accounts
535+
// from the accounts table. This migration copies the BaseAccountData.UpdateRound
536+
// field, along with voting data, to set the initial values of the onlineaccounts
537+
// table for each address. After that, the onlineaccounts table's updateRound column
538+
// would only be updated if voting data changed -- so certain transactions like
539+
// receiving a pay txn of 0 algos, or receiving an asset transfer, etc, would not
540+
// result in a new onlineaccounts row with a new updateRound (unless it triggered a
541+
// balance or voting data change). This criteria is implemented in
542+
// onlineAccountsNewRound in acctdeltas.go, separate from accountsNewRound &
543+
// makeCompactAccountDeltas, which set the account table's UpdateRound value.
544+
//
545+
// 3. Node operators using fast catchup to restore from a catchpoint file version V6
546+
// or V7 (used before v4.0.1 and consensus v40, which added the
547+
// EnableCatchpointsWithOnlineAccounts flag) initialize the onlineaccounts table by
548+
// first restoring the accounts table from the snapshot, then running the same
549+
// migration introduced in (2), where updateRound (and account data) comes from
550+
// BaseAccountData. This means catchpoint file writers and fast catchup users could
551+
// see some addresses have a horizon row with an updateRound that was set to zero
552+
// (case 1), or the round of the last account data change (case 2). Since v4.0.1,
553+
// catchpoint file version V8 includes the onlineaccounts and onlineroundparams tables
554+
// in snapshots, to support the voter_params_get and online_stake opcodes (PR #6177).
555+
//
556+
// 4. However, a node catching up from scratch without using fast catchup, running
557+
// v3.9.2 or later, must track the online account history to verify block certificates
558+
// as it validates each block in turn. It sets updateRound based on observing all
559+
// account voting data changes starting from round 0, whether or not
560+
// EnableAccountDataResourceSeparation is set. These nodes will have horizon rows for
561+
// addresses with updateRound set to the round of the last actual voting data change,
562+
// not zero (case 1) or the round of the last account data change (case 2).
563+
//
564+
// Is the updateRound for this row beyond the lookback horizon (R-320)?
565+
if oa.UpdateRound < catchpointLookbackHorizonForNextRound(i.accountsRound, i.params) {
566+
// Is this the first (and thus oldest) row for this address?
567+
if i.onlineAccountPrev.IsZero() || i.onlineAccountPrev != oa.Address {
568+
// Then set it to 0.
569+
oa.UpdateRound = 0
570+
}
571+
572+
// This case should never happen: there should only be one horizon row per account.
573+
if !i.onlineAccountPrev.IsZero() && i.onlineAccountPrev == oa.Address {
574+
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)
575+
}
576+
}
577+
578+
i.onlineAccountPrev = oa.Address
579+
i.onlineAccountPrevRound = oa.UpdateRound
580+
return oa, nil
581+
}

‎ledger/catchpointfilewriter_test.go

+6-55
Original file line numberDiff line numberDiff line change
@@ -1086,47 +1086,6 @@ func TestCatchpointAfterTxns(t *testing.T) {
10861086
}
10871087
}
10881088

1089-
type catchpointOnlineAccountsIterWrapper struct {
1090-
ts trackerdb.TransactionScope
1091-
iter trackerdb.TableIterator[*encoded.OnlineAccountRecordV6]
1092-
onlineAccountCurrent basics.Address
1093-
accountsRound basics.Round
1094-
params config.ConsensusParams
1095-
}
1096-
1097-
// makeCatchpointOrderedOnlineAccountsIter wraps normal MakeOrderedOnlineAccountsIter iterator
1098-
// in order to manipulate the update round number to simulate the catchpoint generation process.
1099-
func (i *catchpointOnlineAccountsIterWrapper) makeCatchpointOrderedOnlineAccountsIter(
1100-
ctx context.Context, useStaging bool, excludeBefore basics.Round,
1101-
) (trackerdb.TableIterator[*encoded.OnlineAccountRecordV6], error) {
1102-
var err error
1103-
i.iter, err = i.ts.MakeOrderedOnlineAccountsIter(ctx, useStaging, excludeBefore)
1104-
if err != nil {
1105-
return nil, err
1106-
}
1107-
1108-
return i, nil
1109-
}
1110-
1111-
func (i *catchpointOnlineAccountsIterWrapper) Next() bool { return i.iter.Next() }
1112-
func (i *catchpointOnlineAccountsIterWrapper) Close() { i.iter.Close() }
1113-
func (i *catchpointOnlineAccountsIterWrapper) GetItem() (*encoded.OnlineAccountRecordV6, error) {
1114-
item, err := i.iter.GetItem()
1115-
if err != nil {
1116-
return nil, err
1117-
}
1118-
1119-
// this is the same condition as in catchpointFileWriter.readDatabaseStep
1120-
if i.onlineAccountCurrent.IsZero() || i.onlineAccountCurrent != item.Address {
1121-
i.onlineAccountCurrent = item.Address
1122-
// If so, is the updateRound for this row beyond the lookback horizon (R-320)? Then set it to 0.
1123-
if item.UpdateRound < (i.accountsRound + 1).SubSaturate(basics.Round(i.params.MaxBalLookback)) {
1124-
item.UpdateRound = 0
1125-
}
1126-
}
1127-
return item, nil
1128-
}
1129-
11301089
func TestCatchpointAfterStakeLookupTxns(t *testing.T) {
11311090
partitiontest.PartitionTest(t)
11321091
// t.Parallel() No: config.Consensus is modified
@@ -1144,8 +1103,10 @@ func TestCatchpointAfterStakeLookupTxns(t *testing.T) {
11441103
shortMax := max(config.Consensus[protocol.ConsensusFuture].MaxBalLookback, config.Consensus[protocol.ConsensusFuture].CatchpointLookback)
11451104
shortRounds := 2*shortMax + 50
11461105
longRounds := uint64(1500)
1147-
t.Run("future", func(t *testing.T) { testCatchpointAfterStakeLookupTxns(t, protocol.ConsensusFuture, longRounds, true) })
1148-
t.Run("future_noSP", func(t *testing.T) { testCatchpointAfterStakeLookupTxns(t, futureNoSP, longRounds, false) })
1106+
if !testing.Short() {
1107+
t.Run("future", func(t *testing.T) { testCatchpointAfterStakeLookupTxns(t, protocol.ConsensusFuture, longRounds, true) })
1108+
t.Run("future_noSP", func(t *testing.T) { testCatchpointAfterStakeLookupTxns(t, futureNoSP, longRounds, false) })
1109+
}
11491110
t.Run("future_short", func(t *testing.T) { testCatchpointAfterStakeLookupTxns(t, protocol.ConsensusFuture, shortRounds, true) })
11501111
t.Run("future_noSP_short", func(t *testing.T) { testCatchpointAfterStakeLookupTxns(t, futureNoSP, shortRounds, false) })
11511112
}
@@ -1284,21 +1245,11 @@ assert
12841245
var genOAHash, valOAHash crypto.Digest
12851246
var genOARows, valOARows uint64
12861247
require.NoError(t, dl.generator.trackerDB().Transaction(func(ctx context.Context, tx trackerdb.TransactionScope) (err error) {
1287-
oaGenIterWrapper := catchpointOnlineAccountsIterWrapper{
1288-
ts: tx,
1289-
accountsRound: genDBRound,
1290-
params: config.Consensus[proto],
1291-
}
1292-
genOAHash, genOARows, err = calculateVerificationHash(context.Background(), oaGenIterWrapper.makeCatchpointOrderedOnlineAccountsIter, onlineExcludeBefore, false)
1248+
genOAHash, genOARows, err = calculateVerificationHash(context.Background(), makeCatchpointOrderedOnlineAccountsIterFactory(tx.MakeOrderedOnlineAccountsIter, genDBRound, config.Consensus[proto]), onlineExcludeBefore, false)
12931249
return err
12941250
}))
12951251
require.NoError(t, dl.validator.trackerDB().Transaction(func(ctx context.Context, tx trackerdb.TransactionScope) (err error) {
1296-
oaValIterWrapper := catchpointOnlineAccountsIterWrapper{
1297-
ts: tx,
1298-
accountsRound: valDBRound,
1299-
params: config.Consensus[proto],
1300-
}
1301-
valOAHash, valOARows, err = calculateVerificationHash(context.Background(), oaValIterWrapper.makeCatchpointOrderedOnlineAccountsIter, onlineExcludeBefore, false)
1252+
valOAHash, valOARows, err = calculateVerificationHash(context.Background(), makeCatchpointOrderedOnlineAccountsIterFactory(tx.MakeOrderedOnlineAccountsIter, valDBRound, config.Consensus[proto]), onlineExcludeBefore, false)
13021253
return err
13031254
}))
13041255
require.Equal(t, genOAHash, valOAHash)

‎ledger/catchpointtracker.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ func (ct *catchpointTracker) finishFirstStage(ctx context.Context, dbRound basic
256256
// Generate hashes of the onlineaccounts and onlineroundparams tables.
257257
err := ct.dbs.Snapshot(func(ctx context.Context, tx trackerdb.SnapshotScope) error {
258258
var dbErr error
259-
onlineAccountsHash, _, dbErr = calculateVerificationHash(ctx, tx.MakeOrderedOnlineAccountsIter, onlineExcludeBefore, false)
259+
onlineAccountsHash, _, dbErr = calculateVerificationHash(ctx, makeCatchpointOrderedOnlineAccountsIterFactory(tx.MakeOrderedOnlineAccountsIter, dbRound, params), onlineExcludeBefore, false)
260260
if dbErr != nil {
261261
return dbErr
262262

‎ledger/simple_test.go

+3
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package ledger
1919
import (
2020
"context"
2121
"fmt"
22+
"path/filepath"
2223
"strings"
2324
"testing"
2425

@@ -74,8 +75,10 @@ func newSimpleLedgerFull(t testing.TB, balances bookkeeping.GenesisBalances, cv
7475
require.NoError(t, err)
7576
require.False(t, genBlock.FeeSink.IsZero())
7677
require.False(t, genBlock.RewardsPool.IsZero())
78+
tempDir := t.TempDir()
7779
dbName := fmt.Sprintf("%s.%d", t.Name(), crypto.RandUint64())
7880
dbName = strings.Replace(dbName, "/", "_", -1)
81+
dbName = filepath.Join(tempDir, dbName)
7982
cfg.Archival = !slCfg.notArchival
8083
log := slCfg.logger
8184
if log == nil {

‎test/e2e-go/upgrades/application_support_test.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -344,9 +344,10 @@ func TestApplicationsUpgradeOverGossip(t *testing.T) {
344344
a.NoError(err)
345345

346346
// Fund the manager, so it can issue transactions later on
347-
_, err = client.SendPaymentFromUnencryptedWallet(creator, user, fee, 10000000000, nil)
347+
tx0, err := client.SendPaymentFromUnencryptedWallet(creator, user, fee, 10000000000, nil)
348348
a.NoError(err)
349-
client.WaitForRound(round + 2)
349+
isCommitted := fixture.WaitForTxnConfirmation(round+10, tx0.ID().String())
350+
a.True(isCommitted)
350351

351352
round, err = client.CurrentRound()
352353
a.NoError(err)
@@ -454,7 +455,7 @@ int 1
454455
// Try polling 10 rounds to ensure txn is committed.
455456
round, err = client.CurrentRound()
456457
a.NoError(err)
457-
isCommitted := fixture.WaitForTxnConfirmation(round+10, txid)
458+
isCommitted = fixture.WaitForTxnConfirmation(round+10, txid)
458459
a.True(isCommitted)
459460

460461
// check creator's balance record for the app entry and the state changes

0 commit comments

Comments
 (0)