Skip to content

scripted-diff: Regenerate key_io data deterministically #21279

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
laanwj merged 2 commits into from
Apr 19, 2022
Merged

scripted-diff: Regenerate key_io data deterministically #21279

laanwj merged 2 commits into from
Apr 19, 2022

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Feb 23, 2021

No description provided.

@maflcko
Copy link
Member Author

maflcko commented Feb 23, 2021

Requested by @Sjors here: #20861 (review)

@Sjors
Copy link
Member

Sjors commented Feb 23, 2021

tACK fab2a4a, modulo the numbers in the README should be updated...

Also, I find it less tedious to run this from the root of the project:

PYTHONPATH=test/functional/test_framework contrib/testgen/gen_key_io_test_vectors.py valid 61 > src/test/data/key_io_valid.json
PYTHONPATH=test/functional/test_framework contrib/testgen/gen_key_io_test_vectors.py invalid 60 > src/test/data/key_io_invalid.json
git diff

Sjors
Sjors reviewed Feb 23, 2021
View reviewed changes
@sipa
Copy link
Member

sipa commented Feb 23, 2021

This is generally a good idea, and we should do it. But this does still mean that changes to what is generated based on the RNG output will result in an entirely different output. E.g. adding an option to a random.choice() may cause more entropy to be extracted from the RNG, and thus cause all future data from it to be shifted. In theory this could be addresses using consistent hashing, but that's probably overkill.

Concept ACK

@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 24, 2021 •
edited
Loading

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

Conflicts

No conflicts as of last run.

@DrahtBot DrahtBot mentioned this pull request Feb 24, 2021
Merged
@Sjors
Copy link
Member

Sjors commented Mar 2, 2021

re-utACK fa21feb

@DrahtBot DrahtBot mentioned this pull request Mar 4, 2021
Merged
maflcko
maflcko commented Mar 7, 2021
View reviewed changes
contrib/testgen/gen_key_io_test_vectors.py Outdated
PYTHONPATH=../../test/functional/test_framework ./gen_key_io_test_vectors.py valid 50 > ../../src/test/data/key_io_valid.json
PYTHONPATH=../../test/functional/test_framework ./gen_key_io_test_vectors.py invalid 50 > ../../src/test/data/key_io_invalid.json
'''
# 2012 Wladimir J. van der Laan
Copy link
Member Author

@maflcko maflcko Mar 7, 2021

Choose a reason for hiding this comment

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

@laanwj This says your name, which makes you qualified to review this? 😬

Copy link
Member

@laanwj laanwj Mar 29, 2021

Choose a reason for hiding this comment

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

I definitely don't remember writing this 😓

Copy link
Member Author

@maflcko maflcko Apr 7, 2022

Choose a reason for hiding this comment

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

Your name is removed in current master, but you are still welcome to review

@laanwj laanwj changed the title scripted-diff: Regenerate key_io data deterministically scripted-diff: Regenerate key_io data deterministically Mar 22, 2021
@DrahtBot DrahtBot mentioned this pull request Mar 16, 2022
Merged
contrib: make gen_key_io_test_vectors deterministic
fafb479 
Also, remove instructions which are redundant with the README
@maflcko maflcko force-pushed the 2102-testDetGen branch 2 times, most recently from fa54903 to 11ebc98 Compare April 6, 2022 15:05
scripted-diff: Regenerate key_io data deterministically
fa506ad 
-BEGIN VERIFY SCRIPT-
 ./contrib/testgen/gen_key_io_test_vectors.py valid 70 > ./src/test/data/key_io_valid.json
 ./contrib/testgen/gen_key_io_test_vectors.py invalid 70 > ./src/test/data/key_io_invalid.json
-END VERIFY SCRIPT-
@fanquake fanquake requested review from Sjors and laanwj April 6, 2022 15:17
@Sjors
Copy link
Member

Sjors commented Apr 19, 2022

ACK fa506ad

I get the same JSON files when running the script. There are no duplicate addresses and they all change if you modify the 42 seed value (as expected).

@laanwj
Copy link
Member

laanwj commented Apr 19, 2022

Tested ACK fa506ad
Good idea to put it in the scripted-diff. I also verified locally that the output is the same.

@laanwj laanwj merged commit b297b94 into bitcoin:master Apr 19, 2022
@maflcko maflcko deleted the 2102-testDetGen branch April 19, 2022 12:02
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 19, 2022
@laanwj
Merge bitcoin#21279: scripted-diff: Regenerate key_io data determinis…
9a2f0bc 
…tically
@bitcoin bitcoin locked and limited conversation to collaborators Apr 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@laanwj laanwj Awaiting requested review from laanwj

@Sjors Sjors Awaiting requested review from Sjors

Assignees

No one assigned

Labels

Scripts and tools

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@maflcko @Sjors @sipa @DrahtBot @laanwj