catchpoints: Add onlineaccounts and onlineroundparamstail tables to snapshot files #6177
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6177 +/- ##
==========================================
- Coverage 51.86% 51.79% -0.08%
==========================================
Files 639 640 +1
Lines 85492 85818 +326
==========================================
+ Hits 44344 44447 +103
- Misses 38331 38525 +194
- Partials 2817 2846 +29 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
| TotalOnlineAccounts uint64 `codec:"onlineAccountsCount"` | ||
| TotalOnlineRoundParams uint64 `codec:"onlineRoundParamsCount"` |
There was a problem hiding this comment.
shouldn't these added at the end of the struct? If I remember correctly msgp can encode structs into arrays or maps
| Balances []encoded.BalanceRecordV6 `codec:"bl,allocbound=BalancesPerCatchpointFileChunk"` | ||
| numAccounts uint64 | ||
| KVs []encoded.KVRecordV6 `codec:"kv,allocbound=BalancesPerCatchpointFileChunk"` | ||
| OnlineAccounts []encoded.OnlineAccountRecordV6 `codec:"oa,allocbound=BalancesPerCatchpointFileChunk"` |
There was a problem hiding this comment.
why adding into v6 struct and not defining v7 struct?
| @@ -1165,7 +1201,7 @@ func (ct *catchpointTracker) isWritingCatchpointDataFile() bool { | |||
| // - Balance and KV chunk (named balances.x.msgpack). | |||
| // ... | |||
| // - Balance and KV chunk (named balances.x.msgpack). | |||
| func (ct *catchpointTracker) generateCatchpointData(ctx context.Context, accountsRound basics.Round, catchpointGenerationStats *telemetryspec.CatchpointGenerationEventDetails, encodedSPData []byte) (totalKVs, totalAccounts, totalChunks, biggestChunkLen uint64, err error) { | |||
| func (ct *catchpointTracker) generateCatchpointData(ctx context.Context, accountsRound basics.Round, catchpointGenerationStats *telemetryspec.CatchpointGenerationEventDetails, encodedSPData []byte) (totalAccounts, totalKVs, totalOnlineAccounts, totalOnlineRoundParams, totalChunks, biggestChunkLen uint64, err error) { | |||
There was a problem hiding this comment.
looks like it is time to switch from 7 return params to struct?
| } | ||
| } | ||
|
|
||
| func (l *CatchpointLabelMakerCurrent) buffer() []byte { |
There was a problem hiding this comment.
I got lost, we added online account into v8 label, didn't we?
| @@ -854,7 +854,7 @@ func TestExactAccountChunk(t *testing.T) { | |||
| catchpointFilePath := filepath.Join(tempDir, t.Name()+".catchpoint.tar.gz") | |||
|
|
|||
| cph := testWriteCatchpoint(t, dl.validator.trackerDB(), catchpointDataFilePath, catchpointFilePath, 0) | |||
| require.EqualValues(t, cph.TotalChunks, 1) | |||
| require.EqualValues(t, cph.TotalChunks, 2) | |||
There was a problem hiding this comment.
why is there one more chunk? Added online accounts overgrew the max chunk size?
Summary
The new opcodes in #5984 require access to historical lookups of online account details and total circulation amounts. During catchpoint restore, replaying and evaluating old blocks bootstraps the tracker DBs. However transactions that use these opcodes will fail, since they need access to historical ledger amounts earlier than the horizon of the catchpoint snapshot. This PR adds the
onlineaccountsandonlineroundparamstailtables to the snapshot files, which provide online account lookup and total circulation amounts, respectively.Test Plan
Existing tests should pass — working on extending an existing catchpoint test to use the
voter_params_getandonline_stakeopcodes (similar to some catchpoint tests written for testing use of box opcodes).