We are going to delete MtpHeight in a couple commits, but to let us do
the transition with smaller diffs, we will first change it to be easily
convertible to a BlockHeight/BlockMtp pair.
See the previous commit message for justification; for sensible
arithmetic on block timestamps we need the ability to do MTP
calculations on arbitrary MTPs and arbitrary intervals between them.
However, the absolute::Mtp and relative::MtpInterval types are severely
limited in both range and precision.
Also adds a bunch of arithmetic ops to match the existing ops for
BlockHeight and BlockInterval. These panic on overflow, just like the
underlying std arithmetic, which I think is reasonable behavior for
types which are documented as being thin wrappers around u32.
We may want to add checked_add, checked_sub and maybe checked_sum
methods, but that's out of scope for this PR.
For our relative locktime API, we are going to want to take differences
of arbitrary MTPs in order to check whether they meet some relative
timelock threshold.
However, the `locktime::absolute::Mtp` type can only represent MTPs that
exceed 500 million. In practice this is a non-issue; by consensus MTPs
must be monotonic and every real chain (even test chains) have initial
real MTPs well above 500 million, which as a UNIX timestamp corresponds
to November 5, 1985.
But in theory this is a big problem: if we were to treat relative MTPs
as "differences of absolute-timelock MTPs" then we will be unable to
construct relative timelocks on chains with weird timestamps (and on
legitimate chains, we'd have .unwrap()s everywhere that would be hard to
justify). But we need to treat them as a "difference of MTPs" in *some*
sense, because otherwise they'd be very hard to construct.
There is a lot of duplicated code between BlockHeight and BlockInterval.
It obfuscates the differences between them: which timelock types they
can be converted to/from and what their arithmetic properties are.
a5d96ceb39 hashes: remove private `internal_new` method (Andrew Poelstra)
0aeff359f5 hashes: reformat (Andrew Poelstra)
aadd7df266 hashes: move `from_engine` fn impl out of macro body (Andrew Poelstra)
Pull request description:
This is a followup to https://github.com/rust-bitcoin/rust-bitcoin/pull/4010 which does some simple cleanups to the hash macros and some function signatures.
ACKs for top commit:
tcharding:
ACK a5d96ceb39
Tree-SHA512: e8c3d8770fe3a49da4eb2d548af86cbe6e0e17efcac419815f4953b7dafffbd3e5c0be65574e08c86d09fe594d95512dfd7837534be429b1490fea973ec4d5e6
Refactor Taproot functions to accept any type implementing `Into<XOnlyPublicKey>`,
instead of requiring `XOnlyPublicKey` directly. This improves ergonomics when working
with compatible types, avoiding unnecessary `.into()` conversions at call sites.
The function name in the example is the std function not this crates.
There is also an unused variable.
Correct the name of the function and prefix the unused vairable with an
underscore.
There is a new lint error on nightly-2025-04-25 "variables can be used
directly in the `format!` string".
Use the variables in the `format!` string for all cases in
`bitcoin/examples/`.
There is a new lint error on nightly-2025-04-25 "variables can be used
directly in the `format!` string".
Exclude the lint to allow the existing syntax in `format!` strings.
53d32c9e4f bitcoin: remove torv2 support (Bruno Garcia)
Pull request description:
This PR removes support to TorV2 since it's deprecated and no longer useful to have it.
ACKs for top commit:
apoelstra:
ACK 53d32c9e4f0ef0f3b2c7d4dcba42e3ac5344f78a; successfully ran local tests
tcharding:
ACK 53d32c9e4f
Tree-SHA512: 69a2ba399d5eac7f132519ab83362fbd8739d9e975795e441cefa75896ddbf4041db2125ffde51316f9ad69aa0b62c8b226ccff042b0dae6d3c615826bc339f4
In #4373 we added a couple new conversion methods and deprecated the
`to_inner` ones. During that the deprecation date was set to `0.33.0`.
We have backported the changes and will deprecate in `0.32.6` so set the
version number now so we don't forget later.
44d01d0ce4 Fix broken changelog links (GarmashAlex)
Pull request description:
Fixed malformed markdown links in bitcoin/CHANGELOG.md that were causing errors. Specifically, removed double parentheses from links to primitives and units CHANGELOG.md files, ensuring proper rendering and accessibility of the documentation.
ACKs for top commit:
apoelstra:
ACK 44d01d0ce468549c64f8362c552c434eb1c3503b; successfully ran local tests
Tree-SHA512: e2d2598070c3df8e4dfa59c019d8ab9a1f86731a70624ceef80b68849454bd3828c558ef8eb30bb9d56c648fcacccea459d2a7a536021d65c2fbcbe5595916ce
055ab7e684 Remove impl Display from mutants exclusion list (Jamil Lambert, PhD)
Pull request description:
The output of Display should not change in stable crates for types that have well defined formatting and ones that implement FromStr. Crates that are included in the mutation testing have been updated to have regression tests for all relevant Display implementations in #4420 and #4422.
Remove the exclusion so that the Display implementations are included in mutation testing.
Together with #4420 and #4422Closes#4415
ACKs for top commit:
tcharding:
ACK 055ab7e684
apoelstra:
ACK 055ab7e684e53783b56139d14779fb39268e8f3c; successfully ran local tests
brunoerg:
code review ACK 055ab7e684
Tree-SHA512: 703aeee57ec9459b8cef38adbbf04d7bcb2bc425db22512e6936a5a9f6a51c740f62b03ab070aed11bd17740ae6b9f510ec743fd683f98cb03e0a1d086a4784e
This method is used to implement `from_byte_array`. But there is no need
for the method to exist. We can just inline it in `from_byte_array` and
reduce the amount of indirection in our macros.
Also make the same change in sha256t.
This macro is pretty weird -- it requires that a freestanding
`from_engine` method exists, which it uses to implement a `from_engine`
method within an impl block, by just calling through to the freestanding
method.
To reduce indirection, at a very small cost in increased repeated code
(we now need to add a `impl Hash {` and `}` and doccomment around each
freestanding function, we just remove this from the macro entirely.
This will allow us to implement `from_engine` for `sha256t::Hash` in a
different way than for the non-generic hash types.
To minimize the diff, we do not re-indent the freestanding
`from_engine`. We will do that in the next commit. However, the diff is
still a bit noisy because I had to replace `fn from_engine` with `pub fn
from_engine` in every case. I took the opportunity to also change the
return type from `Hash` to `Self` to make it clearer that these are
constructors.
Prefer the more descriptive NumOpResult return type over Option where
return types are fallible.
The returned type NumOpResult is already annotated as must_use per the
lint, so must_use can be removed after changing the method return types
to NumOpResult.
Fixed malformed markdown links in bitcoin/CHANGELOG.md that were causing errors.
Specifically, removed double parentheses from links to primitives and units
CHANGELOG.md files, ensuring proper rendering and accessibility of the documentation.
826acb8273 units: rename relative::HeightInterval constructors (Andrew Poelstra)
39b4f7670d units: rename relative::Height to HeightInterval (Andrew Poelstra)
d3619cc1bc units: deprecate relative::MtpInterval::to_consensus_u32 (Andrew Poelstra)
1a6b8b4c7a units: rename relative::Time to MtpInterval (Andrew Poelstra)
39b057fade units: rename absolute::Height consensus functions (Andrew Poelstra)
5a8f33f380 units: rename absolute::Mtp consensus functions (Andrew Poelstra)
8ffcd2cf30 units: rename absolute::Time to absolute::Mtp (Andrew Poelstra)
Pull request description:
This PR does a whole bunch of renames to the `units` locktime modules. These modules contain the underlying units for locktime types and hopefully aren't used directly too often, which should minimize the disruption from these renames.
This fixes a number of issues:
* Confusion between blocktimes and MTPs (which in fairness, *are* blocktimes, but they're not the one that users are likely to reach for)
* Constructor and conversion names that imply there is a "consensus encoding" for these bare units, which corresponded to the consensus encoding of the corresponding locktimes/sequence numbers
* `from_*` methods without `to_*` methods and vice-versa (overlaps with the above)
* the horribly named `value` method on relative heights and times (but not absolute ones)
This PR does **not** remove the `MtpAndHeight` type, nor does it add constructors for `Mtp` from lists of blocktimes. This is because the PR was too big already and I felt I should split it up.
Alternate to #4427
ACKs for top commit:
tcharding:
ACK 826acb8273
Tree-SHA512: 6e0491e17927625cde85c2cf92ff152a10613e632474122a626ee31b662d21c09fcb9fa3014c44708c97536535a33845cbbcd81e73dcdf98e9ee9fd6143c698f
3bb6c73f2d Add methods to retrieve inner types (Shing Him Ng)
Pull request description:
Resolves#4345
ACKs for top commit:
tcharding:
ACK 3bb6c73f2d
apoelstra:
ACK 3bb6c73f2d3edd1165b7b7f3a833fa471786e166; successfully ran local tests; should backport to 0.32.x
Tree-SHA512: c89017bbc2126ec62c756c4ee9b49dcc8b94a3063a8155aadcf7c69a6f0bc9337baedffe7f52a4ab6f0b738302bea683391d394483c4c7eefbb622b97d34d26c
Rename `value` to `to_height` to be symmetric with `from_height`;
deprecate `to_consensus_u32` which had no symmetric `from_consensus_u32`
and was only used to implement the corresponding methods in primitives
and bitcoin.
This is disruptive, but makes the type name consistent with
`MtpInterval` and also greatly improves clarity, helping to distinguish
between absolute and relative locktimes and reminding the author (and
reviewer) of locktime code that this needs to be a diff.
This method is weird. It's basically just used internally to implement
the locktime methods in `primitives` and `bitcoin`. It has no symmetric
from_consensus_u32.
Conversely the constructors from 512-second intervals have no symmetric
to_* method -- the inverse of these functions is called `value`, which
is a meaningless and undiscoverable name.
As with absolute::Mtp, there is no "consensus encoding" of a block
height, except that obtained by converting it to a locktime. For
symmetry with `Mtp`, rename the methods.
16f6fc6b5b Improve docs on absolute::LockTime::is_implied_by (Tobin C. Harding)
Pull request description:
If one wishes to verify a script that contains CLTV is valid in a transaction then one must compare the argument to CLTV (the locktime) to the transaction locktime. And to be valid the CLTV locktime must be less than or equal to the transaction locktime. This usage kind of lends itself to the term 'implied by' and we have a function already `is_implied_by` that does exactly this.
Improve the docs by adding a section mentioning this usecase.
ACKs for top commit:
apoelstra:
ACK 16f6fc6b5b469741c076bc3a4d92f7d4e619ffc7; successfully ran local tests
Tree-SHA512: f7de177927150ae27b3db187465e0f699da9e8e00a490b0354176b2dedc34d731fda40bf4d55324aa28b0d55f2f036653ab14d99b82d56705c929986db8fb30e
f646afdb81 Fix typo in githooks warning message (strmfos)
Pull request description:
Replaced “re-reun” with “re-run” in the post-merge hook notice to provide clear instructions for reinstalling project githooks.
ACKs for top commit:
apoelstra:
ACK f646afdb8189f738d7c31a0eda7c2abc537c1239; successfully ran local tests
Tree-SHA512: 7943dd801b1548846d54f08260090a4f5b2264d7d7527483af207bd03a0a90079642d0775fd8409ed0f7b89df6d994e2028a877105da6b594135cbdf37ff6356
There is no "consensus encoding" for a MTP. The intention for these
methods was that a user could interpret the MTP as a locktime and then
consensus-encode that locktime. However, it was instead interpreted as
the MTP representing a *blocktime* as it is consensus-encoded in a block
header.
Evidence of this misinterpretation is in several doccomments, which
casually refer to the Mtp (which used to be just called Time) as a
"block time", which is simply incorrect.
This is not a generic UNIX timestamp, but rather a MTP restricted to
have values between 500 million and u32::MAX. Most importantly, it is
*not* a blocktime, which is what is implied by its name and
constructors.
7bcdd5ff66 Add regression tests for Display impl (Jamil Lambert, PhD)
9c90bf49c9 Add lock by time and denomination display regression tests (Jamil Lambert, PhD)
eb7dee831e Rename locktime tests (Jamil Lambert, PhD)
Pull request description:
The output of Display should not change in stable crates for types that have well defined formatting and ones that implement FromStr. Error types do not need to be tested. Discussed in #4415.
Patch 1 is a simple rename to allow for clearer naming when both height and time locktimes are tested.
Patch 2 adds the missing time locktime and denomination `Display` and round trip tests to the existing macro
Patch 3 adds all the other missing `Display` tests in `units`
ACKs for top commit:
tcharding:
ACK 7bcdd5ff66
apoelstra:
ACK 7bcdd5ff6655576dac79e14b9c1a50c74db310a2; successfully ran local tests
Tree-SHA512: af8810af1282cfa94ddf8d2badfa0984c190d1d20812c45b725d4b5467e2ac971eadbd0d821c24e81be8708a167782f9c1274b1ad228d0863d6560c2174537c0
82da8a599e Add regression tests for Display impl (Jamil Lambert, PhD)
Pull request description:
The output of `Display` should not change in stable crates for types that have well defined formatting and ones that implement `FromStr`. Error types do not need to be tested. Discussed in #4415.
Add missing tests for all implementations in `primitives` and round trips for types that implement `FromStr`.
ACKs for top commit:
tcharding:
ACK 82da8a599e
apoelstra:
ACK 82da8a599e4a046b8da7763e8aaeff28becc49a6; successfully ran local tests
Tree-SHA512: ca70ee81e8a76bee27bc0314f2bc718893190c7cc37e5a357244f0925bc3fb675cb2e096aaf764a35216ea7c544f491bd071034f23f97cfa55e3bf50f47b2d84
For TweakedKeypair, `to_inner` is also renamed to `to_keypair` to maintain
consistency. Similarly, `to_inner` is renamed to `to_x_only_pubkey` for
TweakedPublicKey
If one wishes to verify a script that contains CLTV is valid in a
transaction then one must compare the argument to CLTV (the locktime) to
the transaction locktime. And to be valid the CLTV locktime must be less
than or equal to the transaction locktime. This usage kind of lends
itself to the term 'implied by' and we have a function already
`is_implied_by` that does exactly this.
Improve the docs by adding a section mentioning this usecase.
The output of `Display` should not change in stable crates for types
that have well defined formatting and ones that implement `FromStr`.
Error types do not need to be tested.
Add missing tests for all implementations in `units`.
The output of Display should not change in stable crates for types that
have well defined formatting and ones that implement FromStr.
Crates that are included in the mutation testing have been updated to
test all relevant Display implementations.
Remove the exclusion so that the Display implementations are included in
mutation testing.
The existing display regression tests only tested height locktimes.
Denomination display regression and round trip were not tested.
Add tests for time locktimes and denomination.
Andrew Poelstra
Erick Cestari
Jamil Lambert, PhD
Daniel Roberts
Tobin C. Harding
Bruno Garcia
yancy
GarmashAlex
Fmt Bot
strmfos
Shing Him Ng