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

DL multiple batch updates. #17414

Merged
merged 31 commits into from Feb 23, 2024
Merged

DL multiple batch updates. #17414

merged 31 commits into from Feb 23, 2024

Conversation

fchirica
Copy link
Contributor

@fchirica fchirica commented Jan 29, 2024

Allows to split a batch updates into multiple commands, by setting publish_on_chain=False inside the batch_update. If using the option, a new batch update will be continued on top of the previous one, which is saved only locally. It's possible to discard the unpublished batch by doing clear_pending_roots, or to publish it by doing publish_pending_root. Internally, this implements the new root status PENDING_BATCH, which works similarly to the previous PENDING root, except it allows batch_update to build on top of it and it's never published on chain - when it gets published, it'll change to the PENDING status.

@fchirica fchirica added DataLayer Added Required label for PR that categorizes merge commit message as "Added" for changelog labels Jan 29, 2024
chia/cmds/data.py Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Feb 5, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Feb 5, 2024
Copy link
Contributor

github-actions bot commented Feb 6, 2024

Conflicts have been resolved. A maintainer will review the pull request shortly.

@github-actions github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Feb 6, 2024
@fchirica fchirica closed this Feb 6, 2024
@fchirica fchirica reopened this Feb 6, 2024
@fchirica fchirica closed this Feb 6, 2024
@fchirica fchirica reopened this Feb 6, 2024
Co-authored-by: Kyle Altendorf <sda@fstab.net>
@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Feb 14, 2024
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Feb 14, 2024
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@altendky
Copy link
Collaborator

I misunderstood and thought the pending batch -> committed status was a permanent thing rather than an intermediate change that would get reverted. My above review has several comments that I think end up related to this so I will go back through and review them now.

altendky
altendky previously approved these changes Feb 15, 2024
@fchirica fchirica marked this pull request as draft February 15, 2024 18:47
Copy link

coveralls-official bot commented Feb 16, 2024

Pull Request Test Coverage Report for Build 7998480313

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • -6 of 237 (97.47%) changed or added relevant lines in 9 files are covered.
  • 984 unchanged lines in 40 files lost coverage.
  • Overall coverage increased (+0.05%) to 90.979%

Changes Missing Coverage Covered Lines Changed/Added Lines %
chia/data_layer/data_layer.py 18 19 94.74%
chia/data_layer/data_store.py 16 18 88.89%
chia/rpc/data_layer_rpc_api.py 12 15 80.0%
Files with Coverage Reduction New Missed Lines %
chia/consensus/block_record.py 1 95.24%
chia/daemon/server.py 1 88.56%
chia/data_layer/dl_wallet_store.py 1 95.74%
chia/data_layer/download_data.py 1 71.04%
chia/introducer/introducer.py 1 78.26%
chia/rpc/data_layer_rpc_api.py 1 83.05%
chia/util/json_util.py 1 88.46%
tests/core/full_node/stores/test_block_store.py 1 99.19%
tests/core/util/test_file_keyring_synchronization.py 1 97.1%
tests/fee_estimation/test_fee_estimation_unit_tests.py 1 99.06%
Totals Coverage Status
Change from base Build 7916358366: 0.05%
Covered Lines: 97240
Relevant Lines: 106853

💛 - Coveralls

Copy link
Contributor

File Coverage Missing Lines
chia/data_layer/data_layer.py 94.7% lines 320
chia/data_layer/data_store.py 88.9% lines 1363, 1368
chia/rpc/data_layer_rpc_api.py 80.0% lines 251, 255, 294
Total Missing Coverage
237 lines 6 lines 97%

@fchirica fchirica marked this pull request as ready for review February 22, 2024 16:36
@fchirica
Copy link
Contributor Author

Removing coverage diff: only exceptions thrown in case of bugs are failing now

@fchirica fchirica added the ready_to_merge Submitter and reviewers think this is ready label Feb 22, 2024
@Starttoaster Starttoaster merged commit 368fecc into main Feb 23, 2024
277 of 279 checks passed
@Starttoaster Starttoaster deleted the fc.multiple_batch_updates branch February 23, 2024 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Added Required label for PR that categorizes merge commit message as "Added" for changelog DataLayer ready_to_merge Submitter and reviewers think this is ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants