Skip to content

Commit

Permalink
ledger refactoring: fix catchpoint tracker (#3214)
Browse files Browse the repository at this point in the history
## Summary

When implementing the catchpoint tracker, I missed the re-initilization location for some of the local variables.
This would generate incorrect catchpoint labels after a node completes a fast-catchup.

#3085

## Test Plan

- [x] Add unit tests
- [x] Perform manual testing
  • Loading branch information
tsachiherman authored Nov 16, 2021
1 parent 8441d91 commit eea0a75
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 6 deletions.
12 changes: 6 additions & 6 deletions ledger/catchpointtracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,12 @@ func (ct *catchpointTracker) loadFromDisk(l ledgerForTracker, lastBalancesRound
ct.log = l.trackerLog()
ct.dbs = l.trackerDB()

ct.roundDigest = nil
ct.catchpointWriting = 0
// keep these channel closed if we're not generating catchpoint
ct.catchpointSlowWriting = make(chan struct{}, 1)
close(ct.catchpointSlowWriting)

err = ct.dbs.Wdb.Atomic(func(ctx context.Context, tx *sql.Tx) error {
err0 := ct.accountsInitializeHashes(ctx, tx, lastBalancesRound)
if err0 != nil {
Expand Down Expand Up @@ -207,12 +213,6 @@ func (ct *catchpointTracker) loadFromDisk(l ledgerForTracker, lastBalancesRound
}
blockHeaderDigest := blk.Digest()

ct.catchpointWriting = 0
// keep these channel closed if we're not generating catchpoint
ct.catchpointSlowWriting = make(chan struct{}, 1)
close(ct.catchpointSlowWriting)
ct.roundDigest = nil

ct.generateCatchpoint(context.Background(), basics.Round(writingCatchpointRound), ct.lastCatchpointLabel, blockHeaderDigest, time.Duration(0))
return nil
}
Expand Down
40 changes: 40 additions & 0 deletions ledger/catchpointtracker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,4 +372,44 @@ func TestReproducibleCatchpointLabels(t *testing.T) {
}
}
}

// test to see that after loadFromDisk, all the tracker content is lost ( as expected )
require.NotZero(t, len(ct.roundDigest))
require.NoError(t, ct.loadFromDisk(ml, ml.Latest()))
require.Zero(t, len(ct.roundDigest))
require.Zero(t, ct.catchpointWriting)
select {
case _, closed := <-ct.catchpointSlowWriting:
require.False(t, closed)
default:
require.FailNow(t, "The catchpointSlowWriting should have been a closed channel; it seems to be a nil ?!")
}
}

func TestCatchpointTrackerPrepareCommit(t *testing.T) {
partitiontest.PartitionTest(t)

ct := &catchpointTracker{}
const maxOffset = 40
const maxLookback = 320
ct.roundDigest = make([]crypto.Digest, maxOffset+maxLookback)
for i := 0; i < len(ct.roundDigest); i++ {
ct.roundDigest[i] = crypto.Hash([]byte{byte(i), byte(i / 256)})
}
dcc := &deferredCommitContext{}
for offset := uint64(1); offset < maxOffset; offset++ {
dcc.offset = offset
for lookback := basics.Round(0); lookback < maxLookback; lookback += 20 {
dcc.lookback = lookback
for _, isCatchpointRound := range []bool{false, true} {
dcc.isCatchpointRound = isCatchpointRound
require.NoError(t, ct.prepareCommit(dcc))
if isCatchpointRound {
expectedRound := offset + uint64(lookback) - 1
expectedHash := crypto.Hash([]byte{byte(expectedRound), byte(expectedRound / 256)})
require.Equal(t, expectedHash[:], dcc.committedRoundDigest[:])
}
}
}
}
}

0 comments on commit eea0a75

Please sign in to comment.