When checking a locktime against block height we add 1 because when the
next block is being validated that is what the height will be and
`is_satisfied_by` is defined to return true if a transaction with this
locktime can be included in the next block.
As we have in `relative`, and for the same reasons, add an API to the
absolute `Height` and `MedianTimePast`: `is_satisfied_by`. Also add
`is_satisfied_by_{height, time}` variants to `absolute::LockTime`.
Call through to the new functions in `units`.
Define 'is satisfied by' - this is a classic off-by-one problem, if a
relative lock is satisfied does that mean it can go in this block or the
next? Its most useful if it means 'it can go in the next' and this is
how relative height and MTP are used in Core.
Ramifications:
- When checking a time based lock we check against the chain tip MTP,
then when Core verifies a block with the output in it it uses the
previous block (and this is still the chain tip).
- When checking a height base lock we check against chain tip height + 1
because Core checks against height of the block being verified.
Additionally we currently have a false negative in the satisfaction
functions when the `crate` type (height or MTP) is to big to fit in a
u16 - in this case we should return true not false because a value too
big definitely is > the lock value.
One final API paper cut - currently if the caller puts the args in the
wrong order they get a false negative instead of an error.
Fix all this by making the satisfaction functions return errors, update
the docs to explicitly define 'satisfaction'.
For now remove the examples in rustdocs, we can circle back to these
once the dust settles.
API test of Errors:
Some of the errors are being 'API tested' tested in `primitives` but
they should be being done in `units/tests/api.rs` - put all the new
errors in the correct places.
We recently improved the relative locktime function `is_satisfied_by` by
adding mined at and chain tip. We can now do the same for the
height/time satisfaction functions.
Note I believe these functions should still be provided because a user
may for some reason have either blocktime data or height data and not
have the other.
Requires some work to the errors, elect to just remove the original
field that held the function argument.
For now remove the examples in rustdocs, we can circle back to these
once the dust settles.
06096d2a66 Automated update to Github CI to rustc nightly-2025-05-09 (Update Nightly Rustc Bot)
Pull request description:
Automated update to Github CI workflow `rust.yml` by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action
ACKs for top commit:
tcharding:
ACK 06096d2a66
Tree-SHA512: 987f6ae0f4a54cf94c58a1e90d7e99b7687a1d0c7fecab3cde9bc0acd75b25a5336c2eb6445200fe8288e3e0af038123a717541b823a215301db7412bb555bf4
a7d059151e Assert error type (yancy)
2f7e74da45 Add MathOp helper methods (yancy)
Pull request description:
Follow up from https://github.com/rust-bitcoin/rust-bitcoin/pull/4428 to assert the error type which I agree improves the test.
Added some helper functions since it can be nice to see what type of overflow happened.
ACKs for top commit:
tcharding:
ACK a7d059151e
apoelstra:
ACK a7d059151eb47bf4202302604309c95c0d66371d; successfully ran local tests
Tree-SHA512: e1b3eba640de2e4f98e075270fd797582601c541e1eebef2959a4e609bf51129e8ad38baab1253b40474c39f82ee4802658ec545cc5c3590637f2ddb13f873f7
e2d9a8a0d8 primitives: Add an API test module (Tobin C. Harding)
8ec2d353c9 primitives: Derive Clone on witness::Iter (Tobin C. Harding)
Pull request description:
In preparation for 1.0-ing `primitives` add an `api` test module that makes an effort to verify the API surface.
This is similar to what is in `units` and what is in development for `hashes` (in #4017).
Note, there is a WIP attempt at this in #3992.
Close: #3928
ACKs for top commit:
apoelstra:
ACK e2d9a8a0d86f0e836a2a42b1803f8f9d96fde0ca; successfully ran local tests
Tree-SHA512: 5ec5c87c9aa5e86e579283a5485dcb2b3b5ae59359ae5ab96f8e6634285072bef0d0f111b6780852fd88fe29677f1a84c791a3343a0cb2b09093e77125f3962b
dca4266205 units: Fix rustdoc column width (Tobin C. Harding)
d557caf552 Run the formatter (Tobin C. Harding)
7c2115b68f Rename MtpInterval to NumberOf512Seconds (Tobin C. Harding)
3a97ea2259 Rename HeightInterval to NumberOfBlocks (Tobin C. Harding)
c3b7457f6c Rename Mtp to MedianTimePast (Tobin C. Harding)
b38d2256fd Run the formatter (Tobin C. Harding)
Pull request description:
Naming things is hard, naming lock time things is _really_ hard. This is another attempt at improving the very low level types in the `units::locktime` modules.
Formatting done separately so that the diffs are easier to tab through.
ACKs for top commit:
apoelstra:
ACK dca4266205361eb0aee802a46164b163b797d0ee; successfully ran local tests
Tree-SHA512: 24d1b0cae3d7f926622177f00aedc50c53164e04f9d400f9c2c009157f36fd1f55ac2d1b70ceb10b0a165130ffe7eef25d64dc6ea5e6f3caec812c39ab0bbd66
In preparation for 1.0-ing `primitives` add an `api` test module that
makes an effort to verify the API surface.
This is similar to what is in `units` and what is in development for
`hashes` (in #4017).
f5b716b099 Change rustdoc tag from compile_fail to ignore (Jamil Lambert, PhD)
Pull request description:
The example code is supposed to fail to compile, but if it does run it panics. This somehow confuses the compiler so that either the `compile_fail` tag compiles but causes a lint warning about the panic or using `should_panic` fails to compile.
Change the tag to `ignore`
Closes#4259
ACKs for top commit:
apoelstra:
ACK f5b716b09952957dd1b57181fdfa98173e7b5981; successfully ran local tests
Tree-SHA512: 41d14d45f12f989f5875490311a135372325a3bd626cfcea0767de32c14873348c639d14c4774219fca60de712772bb1922ad1b572435f4b5b1e318e1049620c
The example code is supposed to fail to compile, but if it does run it
panics. This somehow confuses the compiler so that either the
compile_fail tag compiles but causes a lint warning about the panic or
using should_panic fails to compile.
Change the tag to ignore
In an effort to bring developer attention to the myriad of APIs for
parsing and formatting scripts as hex add a section to the rustodcs of
`Script` and `ScriptBuf` (same text for both).
Recently we made an attempt at making the hex APIs for scripts easier to
use, better documented, and shown via an example.
After that work we decided it would be better if `LowerHex`/`UpperHex`
did not have the prefix. We also wanted to further clarify the inherent
function names to make the all explicit.
See GitHub issue #4316 for the thread of discussion.
Note that this PR does not require changes to the serde regression test
which were non changed in the original work either.
Name the type exactly what it is. This used to be `Time`, then we tried
`MtpInterval`.
Note that this makes some of the original function names overly verbose
e.g., `NumberOf512seconds::from_512_second_intervals()` but given the
curlyness of locktimes too verbose is better than too terse. Also this
type, along with `NumberOfBlocks` is not going to be in very wide use so
the ergonomic hit is worth the additional clarity.
Name this type exactly what it is. Note for the error we just use
'height' even though this is a bit stale but the general concept is ok
in the error type because the name is long already.
47c77afaac units: delete MtpAndHeight type (Andrew Poelstra)
d82b8c0bcb primitives: stop using MtpAndHeight (Andrew Poelstra)
72d5fbad73 units: stop using MtpAndHeight in locktime::relative is_satisfied_by methods (Andrew Poelstra)
d933c754f5 units: change type of MtpHeight::to_mtp to BlockMtp (Andrew Poelstra)
dcbdb7ca8a units: add checked arithmetic to Block{Height,Mtp}{Interval,} (Andrew Poelstra)
4300271f0c units: add constructor for absolute::Mtp from timestamps (Andrew Poelstra)
4e4601b3d5 units: rename BlockInterval to BlockHeightInterval (Andrew Poelstra)
cb882c5ce1 units: add global `BlockMtpInterval` type (Andrew Poelstra)
4e3af5162f units: add global `BlockMtp` type (Andrew Poelstra)
a3228d4636 units: pull u32 conversions for BlockHeight/BlockInterval into macro (Andrew Poelstra)
Pull request description:
This is a more involved PR than I'd expected but hopefully the individual commits make sense and are well-motivated. Essentially, my goal was to replace `MtpAndHeight` as used by relative locktimes with a pair of `Mtp` and `Height`.
However, relative locktimes, when given a MTP/Height for the UTXO creation and the chain tip, are roughly modeled as "take a diff of MTPs to get a `relative::MtpInterval`, a diff of heights to get a `relative::HeightInterval`, and compare to the locktimes". *However*, we have no standalone MTP type to "take a diff of", and also there are failure modes when creating the diffs (e.g. if the diff would exceed the range of `MtpInterval` or `HeightInterval`).
So I backed up and decided to use the existing `BlockHeight`/`BlockInterval` as the type to "take a diff of". I needed to introduce a `BlockMtp`/`BlockMtpInterval` to work with MTPs. These types have full-u32 range, unlike the similarly-named types in `units::locktimes::absolute`. I then needed to add some conversion methods. Along the way, I cleaned up the APIs and documentation, added checked arithmetic, etc., as needed.
See the individual commit messages for more detail.
I believe the resulting API is much more consistent and discoverable, even though it has more surface than the old API.
I considered splitting this into 2 PRs but I think the first half of the changes aren't well-motivated with out the second half. Let me know.
ACKs for top commit:
tcharding:
ACK 47c77afaac
Tree-SHA512: ebe19a5b1684db8c2d913274347c994026aaa0dcdd79349c237920a82fe55560777278efdbbc7f1b1424c9391d9bbd891ae844db885deea75288000437a8a287
3fa7889ff7 just: Add update-lock-files alias (Tobin C. Harding)
Pull request description:
More terse more betterer - `just ulf` already.
ACKs for top commit:
apoelstra:
ACK 3fa7889ff7920b2458df40266fb3d4f32ca72c77; successfully ran local tests; sure
Tree-SHA512: 9445fbf4dd54a95a55907b8f1a6099876251de57a29395f563628ce0e0b55cf04041f0a616370fd655f01ba8ec5a7e2c1db12e0570dcf4ab342f65104c2c2718
13cbead947 Use NumOpResult instead of Option (yancy)
002a0382aa Mark function constant (yancy)
Pull request description:
Prefer the more descriptive NumOpResult return type over Option where return types are fallible.
Closes https://github.com/rust-bitcoin/rust-bitcoin/issues/4419
ACKs for top commit:
apoelstra:
ACK 13cbead94766987f59482b1fbc1d0ebd0799737c; successfully ran local tests
tcharding:
ACK 13cbead947
Kixunil:
ACK 13cbead947
Tree-SHA512: 1a870962dcafe901a07abd93bd8075e41696341c1a4b3efef615493c73d5e5728bbc2326f8c2c95b9034ab001d0b3c668c9d64793ab03486d3a19f31df907c96
52940d4e12 Prefix unused variables with _ in rustdocs (Jamil Lambert, PhD)
a852aef4b8 Remove unused imports in rustdocs (Jamil Lambert, PhD)
Pull request description:
There is a lint warning about unused variables and imports in the rustdoc examples.
Remove the unused imports and prefix the unused variables with an underscore.
ACKs for top commit:
apoelstra:
ACK 52940d4e1216ad5118f7980cb2e6b8b425c61589; successfully ran local tests
tcharding:
ACK 52940d4e12
Tree-SHA512: 953862d546dc6e0bcd64172e8b383f0fc2a1a851971a1bcad0c1e30cbaeeaea993a0de7dd8b424c4ac1410053e179c52d0b5c90cd1b6560c27123b6b7fa49732
c11772a768 Accept flexible input types for Taproot-related functions (Erick Cestari)
2a518d62e6 Wrap secp256k1::XOnlyPublicKey to improve error handling (Erick Cestari)
Pull request description:
This PR addresses issue #4361 by creating a wrapper type for XOnlyPublicKey instead of directly re-exporting it from the secp256k1 library.
### Key Changes
1. Created a new `XOnlyPublicKey` struct that wraps `secp256k1::XOnlyPublicKey`
2. Implemented custom error types:
- `ParseXOnlyPublicKeyError` for handling parsing errors
- `TweakXOnlyPublicKeyError` for tweaking an `XOnlyPublicKey`
3. Updated all imports and usage throughout the codebase
4. Implemented necessary traits and methods for compatibility
Closes#4361
ACKs for top commit:
apoelstra:
ACK c11772a768eefd89dcc0e3b1a369d535c191f94a; successfully ran local tests
tcharding:
ACK c11772a768
Tree-SHA512: c8da3486e7ffcab6c24cc08f9b2f964dd9158449ef2bd720e54d56176bc7027052314ea23cac3f673d217fa785238ea8a9b5323ba57f02199f20e56df5893965
7ecef176f9 Fix documentation error for `TweakedPublicKey::serialize` (Daniel Roberts)
Pull request description:
Fixes an ancient copy/paste error in documentation ( `secp256k1::schnorrsig::PublicKey::serialize()` docs copied from ECDSA docs, which was copied into rust-bitcoin)
Is there a threshold beneath which a PR is too trivial?
ACKs for top commit:
apoelstra:
ACK 7ecef176f9523cd8fece7e8e71040507f46fb9c2; successfully ran local tests; thanks!
tcharding:
ACK 7ecef176f9
Tree-SHA512: 9b7469d34eadfcabc93264c114f292c415d2dbb09b41ec05de4ac399677d5c80f1d09ecd0c382680996450824f1fd60503d3e3d3ec8bdd8135cebdf7ef82fe0d
5ba763f1a2 Update Github CI to rustc nightly-2025-05-02 (Jamil Lambert, PhD)
09132b80e1 Fix rustdoc compile_fail example (Jamil Lambert, PhD)
282434d4bd Use variable directly in format! string (Jamil Lambert, PhD)
2fbbc825c9 Allow uninlined format args (Jamil Lambert, PhD)
Pull request description:
There is a new lint error on nightly-2025-04-25 "variables can be used directly in the `format!` string".
The existing syntax `format!("{}", x)` is more commonly used than `format!("{x}")` therefore allow it in existing code.
Also the rustdoc example in #4259 now causes the new nightly to fail CI because of the unused variable.
Patches in the PR:
- Exclude the lint to allow the existing syntax in `format!` strings in all crate `lib.rs`, `build.rs.` and test files.
- Use the variables in the `format!` string for all cases in `bitcoin/examples/` since there are no other allowed lints in examples.
- Correct the function names in the rustdoc example and prefix the unused variable with an underscore.
- Update rustc to nightly-2025-05-02 (2025-04-25 had a bug which is fixed in 2025-05-02).
ACKs for top commit:
apoelstra:
ACK 5ba763f1a2ebea2cb80ee50a80228e6bda11936f; successfully ran local tests
Kixunil:
ACK 5ba763f1a2
Tree-SHA512: 20b97d2bedc631715c2b541285559a6ab84bbdb8f2f11d7282bdfecadba0cc8781a1973f0c01c25432aaceaad09e3ddbf59afe54c0bba54768e93ed9d5e50d5a
51fe619fe0 Set deprecation to released date of to_inner (Tobin C. Harding)
Pull request description:
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.
ACKs for top commit:
apoelstra:
ACK 51fe619fe0c9e154a7979a3a71c56707bfa73e9c; successfully ran local tests
shinghim:
ACK 51fe619fe0
Tree-SHA512: 9f8badee92684204966107013b272ff51b88bee632bd6779ec6ecf2c78221c2228428a3c180200db5ae3205d042e58a6919dc9e621e153171e200e9b81c628d6
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.
Tobin C. Harding
Fmt Bot
Daniel Roberts
Fallengirl
yancy
merge-script
Jamil Lambert, PhD
futreall