-
Notifications
You must be signed in to change notification settings - Fork 38.8k
refactor: Signet fixups #19993
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
refactor: Signet fixups #19993
Conversation
ajtowns
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept ACK
src/signet.h
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is much better!
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
@ajtowns suggestions are good, ACK otherwise. |
m_valid implies the block solution has been checked, which is not the case. It only means the txs could be parsed. C++17 comes with std::optional, so just use that instead.
fadee17 to
facaf9e
Compare
|
Code review ACK fadee171c8b0b591daf8f1d65640e4f288b3c3bf |
dr-orlovsky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed & ACK facaf9e
|
ACK facaf9e -- code review only |
kallewoof
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review ACK facaf9e
facaf9e doc: Document signet BIP (MarcoFalke) faf0a26 doc: Update comments for new chain settings (-signet and -chain=signet) (MarcoFalke) fae0548 fuzz: Remove needless guard (MarcoFalke) 77771a0 refactor: Remove SignetTxs::m_valid and use optional instead (MarcoFalke) fa2ad5d test: Run signet test even when wallet was not compiled (MarcoFalke) Pull request description: Some doc and test fixups for bitcoin#18267 ACKs for top commit: ajtowns: ACK facaf9e -- code review only dr-orlovsky: Reviewed & ACK bitcoin@facaf9e kallewoof: Code review ACK facaf9e Tree-SHA512: 8085027c488d84bb4bddccba78bd2d4c5af0d8e2644ee72265f1f30fa8c83f61a961d9da2c796f2940e69682291cbee7b1028b6a6ce123ad9134c0ebbf4723b0
Some doc and test fixups for #18267