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

catchpoint: store certs with blocks during catchpoint restore #5798

Merged
merged 3 commits into from Oct 24, 2023

Conversation

ohill
Copy link
Contributor

@ohill ohill commented Oct 23, 2023

Summary

Each block certificate is downloaded but thrown away during catchpoint restore, leading to errors like #5352 when trying to retrieve blocks with their certificates later. Since we always store blocks and certs together this keeps the certificates from being thrown away so certdata ends up in the block db with the blkdata

Closes #5352.

Test Plan

Updated TestBasicCatchpointCatchup to request a msgpack block at the end.

algorandskiy
algorandskiy previously approved these changes Oct 23, 2023
@codecov
Copy link

codecov bot commented Oct 23, 2023

Codecov Report

Merging #5798 (8095d26) into master (40c16e5) will increase coverage by 1.09%.
Report is 2 commits behind head on master.
The diff coverage is 23.07%.

@@            Coverage Diff             @@
##           master    #5798      +/-   ##
==========================================
+ Coverage   54.45%   55.54%   +1.09%     
==========================================
  Files         475      475              
  Lines       66848    66852       +4     
==========================================
+ Hits        36399    37131     +732     
+ Misses      27903    27199     -704     
+ Partials     2546     2522      -24     
Files Coverage Δ
ledger/catchupaccessor.go 64.80% <100.00%> (+1.05%) ⬆️
ledger/store/blockdb/blockdb.go 30.97% <0.00%> (+0.75%) ⬆️
catchup/catchpointService.go 8.62% <12.50%> (+6.85%) ⬆️

... and 81 files with indirect coverage changes

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

@ohill ohill requested a review from winder October 23, 2023 19:02
Copy link
Contributor

@winder winder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as advertised:

mkdir localtest && cd localtest
cp /path/to/betanet/genesis.json .
algocfg profile set conduit -d .
goal node start -d .
goal node catchup -d .
# catchpoint 30660000 is used
# block in json format
curl -H "Authorization: Bearer <token>" "localhost:8080/v2/blocks/30660000?format=json"
<expected output>
# block in msgpack format
curl -H "Authorization: Bearer <token>" "localhost:8080/v2/blocks/30660000?format=msgpack"
<expected output>
# delta output
curl -H "Authorization: Bearer <token>" "localhost:8080/v2/deltas/30660000"
<expected output>

Please remember to change this from >= to > after the next consensus upgrade:
https://github.com/algorand/conduit/blob/3357b4078db592f3c2d9ddfb341f98d92b5fed40/conduit/plugins/importers/algod/algod_importer.go#L189

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.

Unable to fetch first block after catching up.
3 participants