Skip to content

test: XORed blocks test follow up #30669

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
merged 6 commits into from
Aug 26, 2024
Merged

test: XORed blocks test follow up #30669

merged 6 commits into from
Aug 26, 2024
+22 −13

Conversation

tdb3
Copy link
Contributor

@tdb3 tdb3 commented Aug 17, 2024 •
edited
Loading

Builds on PR #30657.

Refactors read_xor_key() from util.py to test_node.py (comment #30657 (comment))

Adds a check that xor.dat is created when missing (comment #30657 (comment))

Help states:

-blocksxor
       Whether an XOR-key applies to blocksdir *.dat files. The created XOR-key
       will be zeros for an existing blocksdir or when `-blocksxor=0` is
       set, and random for a freshly initialized blocksdir. (default: 1)

@DrahtBot
Copy link
Contributor

DrahtBot commented Aug 17, 2024 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK maflcko, theStack, brunoerg, achow101
Stale ACK danielabrozzoni, glozow

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

@DrahtBot DrahtBot added the Tests label Aug 17, 2024
danielabrozzoni
danielabrozzoni approved these changes Aug 19, 2024
View reviewed changes
Copy link
Contributor

@danielabrozzoni danielabrozzoni left a comment

Choose a reason for hiding this comment

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

tACK 07365d0

brunoerg
brunoerg reviewed Aug 19, 2024
View reviewed changes
@tdb3 tdb3 force-pushed the 20240817_test_new_xor_dat_created branch from 07365d0 to 7b7c287 Compare August 19, 2024 21:11
@tdb3
Copy link
Contributor Author

tdb3 commented Aug 19, 2024

Thanks @danielabrozzoni and @brunoerg. Pushed an update to address comments, and clarified the log message.

theStack
theStack approved these changes Aug 20, 2024
View reviewed changes
Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

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

ACK 7b7c287

Thanks for following up! If you want, you could also tackle the suggestion to move the read_xor_key helper to the TestNode class, see #30657 (comment)

@theStack theStack mentioned this pull request Aug 20, 2024
Merged
glozow
glozow reviewed Aug 20, 2024
View reviewed changes
Copy link
Member

@glozow glozow left a comment

Choose a reason for hiding this comment

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

ACK 7b7c287

brunoerg
brunoerg approved these changes Aug 20, 2024
View reviewed changes
Copy link
Contributor

@brunoerg brunoerg left a comment

Choose a reason for hiding this comment

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

ACK 7b7c287

@tdb3
Copy link
Contributor Author

tdb3 commented Aug 21, 2024

If you want, you could also tackle the suggestion to move the read_xor_key helper to the TestNode class, see #30657 (comment)

Thanks for the suggestion! This PR seems to be a reasonable place to perform the move, so I updated it to include this. Happy to make tweaks if you or @maflcko have any further suggestions.

@tdb3 tdb3 force-pushed the 20240817_test_new_xor_dat_created branch from 7b7c287 to fee7e1c Compare August 21, 2024 00:34
@tdb3 tdb3 changed the title test: check xor.dat creation when missing test: XORed blocks test follow up Aug 21, 2024
theStack
theStack reviewed Aug 22, 2024
View reviewed changes
@tdb3
test: add blocks_key_path
c8176f7 
Adds a convenience function to TestNode
to provide the path to the blocks xor key.
Updates util and feature_blocksxor to use it.
@tdb3 tdb3 force-pushed the 20240817_test_new_xor_dat_created branch from fee7e1c to ab9d245 Compare August 23, 2024 17:16
theStack
theStack approved these changes Aug 24, 2024
View reviewed changes
Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

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

ACK ab9d245

@DrahtBot DrahtBot requested review from glozow and brunoerg August 24, 2024 23:22
@tdb3 tdb3 force-pushed the 20240817_test_new_xor_dat_created branch from ab9d245 to 1c403af Compare August 24, 2024 23:29
theStack
theStack approved these changes Aug 24, 2024
View reviewed changes
Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

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

re-ACK 1c403af

maflcko
maflcko reviewed Aug 25, 2024
View reviewed changes
@tdb3 tdb3 force-pushed the 20240817_test_new_xor_dat_created branch from 1c403af to e1d5dd7 Compare August 25, 2024 12:53
@maflcko
Copy link
Member

maflcko commented Aug 26, 2024

ACK e1d5dd7

@DrahtBot DrahtBot requested a review from theStack August 26, 2024 07:32
theStack
theStack approved these changes Aug 26, 2024
View reviewed changes
Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

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

re-ACK e1d5dd7

brunoerg
brunoerg approved these changes Aug 26, 2024
View reviewed changes
Copy link
Contributor

@brunoerg brunoerg left a comment

Choose a reason for hiding this comment

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

reACK e1d5dd7

@maflcko maflcko added this to the 28.0 milestone Aug 26, 2024
@achow101
Copy link
Member

ACK e1d5dd7

@achow101 achow101 merged commit d50f0ce into bitcoin:master Aug 26, 2024
16 checks passed
@bitcoin bitcoin locked and limited conversation to collaborators Aug 26, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@danielabrozzoni danielabrozzoni Awaiting requested review from danielabrozzoni

@glozow glozow Awaiting requested review from glozow

3 more reviewers

@maflcko maflcko maflcko left review comments

@theStack theStack theStack approved these changes

@brunoerg brunoerg brunoerg approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Tests
Projects
None yet
Milestone
28.0
Development

Successfully merging this pull request may close these issues.
8 participants
@tdb3 @DrahtBot @maflcko @achow101 @theStack @brunoerg @danielabrozzoni @glozow