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
Add standard constants to lock times #1574
Conversation
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.
ACK 72b60c3
I don't think it's that complex but I don't mind being a bit more careful. This is unlikely to conflict anyway. |
Perhaps "hash many edge cases" is better than "notoriously complex" :) |
Looks good to me! Will ACK shortly. I agree it's not terribly complex but I do think it's reasonable to demand 3 ACKs for anything touching locktimes. cc @sanket1729 can you review this one? |
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.
ACK 72b60c3
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.
ACK 72b60c3
Needs rebase after #1575. |
Some of the lock time structs (`Height`, `Time` ect.) are missing standard constants for min, max ect. Add standard constants taking into consideration the various locktime corner cases. Add `max_value` and `min_value` to be consistent with Rust 1.41.1 (incl. `Sequence`). Fix: rust-bitcoin#1451
3c0598b
Rebase only, no other changes. |
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.
ACK 3c0598b
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.
ACK 3c0598b
Some of the lock time structs (
Height
,Time
ect.) are missing standard constants for min, max ect.Add standard constants taking into consideration the various locktime corner cases.
Add
max_value
andmin_value
to be consistent with Rust 1.41.1 (incl.Sequence
).Fix: #1451
This PR is not complex in itself but locktimes are notoriously complex, please wait for 3 acks before merging - and ack'ing makes no guarantee that reviewer got all corner cases :)
There is no rush on this one, apoelstra, Kixunil, sanket1729 please just review when your brain is fresh.