Add Amount and SignedAmount #270
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
apoelstra
reviewed
May 30, 2019
apoelstra
reviewed
May 30, 2019
0d6776f to
0c9bc44
Compare
|
Looks great now, except that the fuzz tests don't compile :) |
0c9bc44 to
7af134c
Compare
|
Fixed in force-push of the last commit. |
apoelstra
approved these changes
Jun 4, 2019
sgeisler
approved these changes
Jun 6, 2019
This was referenced Jun 7, 2019
Closed
yancyribbens
pushed a commit
to yancyribbens/rust-bitcoin
that referenced
this pull request
Mar 23, 2024
422b83c interpreter: use `relative::LockTime` for constraints from Miniscript (Andrew Poelstra) fc42e48 miniscript: use RelLockTime in Terminal (Andrew Poelstra) c490f49 policy: use `RelLockTime` in semantic and concrete policies (Andrew Poelstra) a7a1b9a miniscript: use RelLockTime in `Satisfaction` (Andrew Poelstra) fc6bfa8 introduce RelLockTime type (Andrew Poelstra) 637720a plan/satisfy: use relative::LockTime instead of Sequence to indicate relative locktimes (Andrew Poelstra) bd6ef65 interpreter: rename "age" to "sequence" (Andrew Poelstra) 950b793 AbsLockTime: move to module, consolidate error checking (Andrew Poelstra) 60d7c98 types: drop unused "strongness" errors (Andrew Poelstra) 2f66778 types: replace closure with iterator (Andrew Poelstra) Pull request description: This PR: * Changes user-facing "check locktime" functions to use `bitcoin::relative::LockTime` rather than `Sequence` numbers. * Introduces a new `RelLockTime` type analogous to `AbsLockTime` which is used to provide a locktime type that implements `Ord`. * In both types, validate that the locktimes are legal for use in Miniscript during creation, rather than during typechecking or validity checks. * In many places, replace manual code with the new `is_implied_by` method from rust-bitcoin. This might be a little hard to review because it messes with locktime code which is so easy to get backward. But it appears that there are pretty extensive unit tests which caught several iterations of this where I did get them backward. Going forward things should be much easier to maintain because we have stronger types and an encapsulated `is_implied_by` method. The next PR will do a similar thing with thresholds to validate the `0 < k <= n` invariant when thresholds are created, avoiding the repeated and inconsistent post-hoc error checking logic. Fixes rust-bitcoin#270 ACKs for top commit: sanket1729: ACK 422b83c Tree-SHA512: 42a749a405c142c7ec468b2fd2e8c3c559c31e6eacf72a351c4dd24887385fa74d8128982a70cef69489c4ef72d6247860e1cd96e52ec83a00bdee0e3359d9b8
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Supersedes #252.
After long discussion about whether
Amountshould beu64ori64internally and after switching between them several times, we agreed on having two types.