Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ledger: fix txtail restoring from catchpoints #4460

Merged

Conversation

algorandskiy
Copy link
Contributor

Summary

  • Catchpoints only have minimal data, and online accounts/txtail derive it
    when applying catchpoint
  • Deriving code is reused from migration the migration only assumed
    MaxTxnLife worth of block history
  • In fact, it need to restore MaxTxnLife + DeeperBlockHeaderHistory
    when used as part of catchpoint apply code path

Test Plan

Added new test: fails before the fix, works with the fix.

* Catchpoints only have minimal data, and online accounts/txtail derive it
  when applying catchpoint
* Deriving code is reused from migration the migration only assumed
  MaxTxnLife worth of block history
* In fact, it need to restore MaxTxnLife + DeeperBlockHeaderHistory
  when used as part of catchpoint apply code path
@codecov
Copy link

codecov bot commented Aug 24, 2022

Codecov Report

Merging #4460 (a49996c) into master (87867c9) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4460      +/-   ##
==========================================
- Coverage   55.26%   55.23%   -0.04%     
==========================================
  Files         398      398              
  Lines       50263    50264       +1     
==========================================
- Hits        27780    27761      -19     
- Misses      20167    20186      +19     
- Partials     2316     2317       +1     
Impacted Files Coverage Δ
ledger/accountdb.go 72.87% <100.00%> (+0.01%) ⬆️
network/wsPeer.go 65.47% <0.00%> (-2.20%) ⬇️
agreement/cryptoVerifier.go 67.60% <0.00%> (-2.12%) ⬇️
agreement/proposalManager.go 96.07% <0.00%> (-1.97%) ⬇️
catchup/service.go 68.14% <0.00%> (-1.24%) ⬇️
catchup/peerSelector.go 98.95% <0.00%> (-1.05%) ⬇️
ledger/acctupdates.go 69.29% <0.00%> (-0.60%) ⬇️
network/wsNetwork.go 64.82% <0.00%> (+0.19%) ⬆️
ledger/tracker.go 74.78% <0.00%> (+0.85%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@algorandskiy algorandskiy merged commit dba1310 into algorand:master Aug 25, 2022
onetechnical pushed a commit to onetechnical/go-algorand that referenced this pull request Aug 25, 2022
* Catchpoints only have minimal data, and online accounts/txtail derive it
  by applying catchpoint
* Deriving code is reused from DB schema migration but the migration only assumed
  MaxTxnLife worth of block history
* In fact, it needs to restore MaxTxnLife + DeeperBlockHeaderHistory
  when used as part of catchpoint apply code path
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants