Skip to content

catchpoint: actualize limit constants and fix txtail table init #4703

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.

Sign up for GitHub

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

Jump to bottom
Merged
algorandskiy merged 7 commits into from
Nov 1, 2022
Merged

catchpoint: actualize limit constants and fix txtail table init #4703

algorandskiy merged 7 commits into from
Nov 1, 2022

Conversation

@algorandskiy
Copy link
Contributor

@algorandskiy algorandskiy commented Oct 27, 2022 •
edited
Loading

Summary

  1. Actualize account/resource sizing constants
  2. Fixed account db reader for rebuilding catchpoint root
  3. Handle txtail creation for 1321 blocks

Test Plan

  1. Extended a unit tests to check MT root after applying a catchpoint.
  2. Checked betanet from 21630000
  3. Checked mainnet from 24390000

@algorandskiy algorandskiy self-assigned this Oct 27, 2022
@codecov
Copy link

codecov bot commented Oct 27, 2022 •
edited
Loading

Codecov Report

Merging #4703 (97c232d) into master (97c232d) will not change coverage.
The diff coverage is n/a.

❗ Current head 97c232d differs from pull request most recent head 5b04db7. Consider uploading reports for the commit 5b04db7 to get more accurate results

@@           Coverage Diff           @@
##           master    #4703   +/-   ##
=======================================
  Coverage   54.37%   54.37%           
=======================================
  Files         414      414           
  Lines       53504    53504           
=======================================
  Hits        29095    29095           
  Misses      22002    22002           
  Partials     2407     2407

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

@algorandskiy algorandskiy requested a review from cce October 28, 2022 00:29
algorandskiy
algorandskiy commented Oct 28, 2022
View reviewed changes
ledger/catchpointwriter.go Outdated
Copy link
Contributor Author

@algorandskiy algorandskiy Oct 28, 2022

Choose a reason for hiding this comment

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

removed MaxEncodedAccountDataSize as not used in msgp

@algorandskiy algorandskiy force-pushed the pavel/catchpoint-apply-fix branch from cc66f01 to 34e8346 Compare October 28, 2022 15:46
@algorandskiy algorandskiy marked this pull request as ready for review October 28, 2022 16:36
@algorandskiy algorandskiy changed the title catchpoint: actualize limit constants catchpoint: actualize limit constants and fix txtail table init Oct 28, 2022
algorandskiy
algorandskiy commented Oct 28, 2022
View reviewed changes
data/basics/userBalance_test.go Outdated
Copy link
Contributor Author

@algorandskiy algorandskiy Oct 28, 2022

Choose a reason for hiding this comment

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

partially re-used in a similar test in accountdb_test for base account data and resource data max size checks

algorandskiy
algorandskiy commented Oct 28, 2022
View reviewed changes
ledger/accountdb.go Outdated
Copy link
Contributor Author

@algorandskiy algorandskiy Oct 28, 2022

Choose a reason for hiding this comment

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

this is a fix for the MT recreation after catchpoint apply

algorandskiy
algorandskiy commented Oct 28, 2022
View reviewed changes
ledger/accountdb.go Outdated
Copy link
Contributor Author

@algorandskiy algorandskiy Oct 28, 2022

Choose a reason for hiding this comment

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

this is a fix for block seed opcode in the replay range after applying a catchpoint

iansuvak
iansuvak reviewed Oct 31, 2022
View reviewed changes
@algorandskiy algorandskiy force-pushed the pavel/catchpoint-apply-fix branch from f748190 to 78ca730 Compare October 31, 2022 19:45
@algorandskiy
Copy link
Contributor Author

algorandskiy commented Oct 31, 2022

rebased

cce
cce reviewed Oct 31, 2022
View reviewed changes
cce
cce reviewed Oct 31, 2022
View reviewed changes
cce
cce previously approved these changes Oct 31, 2022
View reviewed changes
cce
cce reviewed Oct 31, 2022
View reviewed changes
@algorandskiy algorandskiy force-pushed the pavel/catchpoint-apply-fix branch from 98baaa2 to 8f03faf Compare October 31, 2022 20:45
@algorandskiy algorandskiy requested a review from cce October 31, 2022 21:16
cce
cce reviewed Nov 1, 2022
View reviewed changes
@algorandskiy algorandskiy force-pushed the pavel/catchpoint-apply-fix branch from 344ed12 to 4e0fb23 Compare November 1, 2022 02:10
@algorandskiy algorandskiy force-pushed the pavel/catchpoint-apply-fix branch from 4e0fb23 to 5b04db7 Compare November 1, 2022 11:30
@algorandskiy algorandskiy requested a review from cce November 1, 2022 12:05
cce
cce approved these changes Nov 1, 2022
View reviewed changes
Copy link
Contributor

@cce cce left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM — it's funny, the changes in this PR only impact catchpoint readers, but the listing of files being changed would make you think the catchpoint writers were changing I forgot you are changing ResourcesPerCatchpointFileChunk to 100k for writers now

@algorandskiy algorandskiy merged commit 15efc02 into algorand:master Nov 1, 2022
@Algo-devops-service Algo-devops-service mentioned this pull request Nov 1, 2022
Merged
@Algo-devops-service Algo-devops-service mentioned this pull request Nov 10, 2022
Closed
@Algo-devops-service Algo-devops-service mentioned this pull request Nov 28, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cce cce cce approved these changes

@algonautshant algonautshant Awaiting requested review from algonautshant

+1 more reviewer

@iansuvak iansuvak iansuvak left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@algorandskiy algorandskiy

Labels

Bug-Fix Team Carbon-11

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@algorandskiy @cce @iansuvak