03b8e08752 Fix argument check in labeler script (Fallengirl)
Pull request description:
### Description
Removed unnecessary quotes around the `!=` operator in the Bash test expression, so the `--force` flag is now evaluated correctly instead of always passing.
ACKs for top commit:
apoelstra:
ACK 03b8e08752e77742374b367a3947d987b81797fa; successfully ran local tests
Tree-SHA512: 479763d384a2ee06dc2a2fdd4cf3d35d1731ffccdee8888104bf0fd4bc778ee493d2e4e56f33c544c66e987a637c994036b237661e21ec02d4d86e0586ba8cb4
b038520c4d Change the return type of effective_value (yancy)
Pull request description:
Prefer the more informative return type NumOpResult over Option. Also a returns section was added which describes the different possible returns.
The api of NumOpResult could probably be extended to cleanup the match statement. Also consider api addition for unchecked calculations natively.
ACKs for top commit:
tcharding:
ACK b038520c4d
apoelstra:
ACK b038520c4dc8c2f92cb40a9fd272608ae2e9b799; successfully ran local tests
Tree-SHA512: 2e66a3ca83514f0ad011660178f8e5ea9d9b1de03dc030e7c57f558e08a42261724251364bb7a746b6970b4288a45539a33d3b36b114c855b6246a56fee3c61c
fd90c8782a Mark method as constant (yancy)
Pull request description:
Allow external const calls to access this method
Followup from https://github.com/rust-bitcoin/rust-bitcoin/pull/4428
ACKs for top commit:
tcharding:
ACK fd90c8782a
apoelstra:
ACK fd90c8782ab602861a715c72ca1481ed963e5c39; successfully ran local tests
Tree-SHA512: 2d04da9bddd58040972942e70096b2714405740236889f0909c00cb6993e38f3ae51ff05bbda13792a31243d3598f5976649590eecb0572c4f00c166f717399d
f732b1d3cc units: Use functional style (Tobin C. Harding)
Pull request description:
This is Rust not C, use functional style.
Close: #4464
ACKs for top commit:
apoelstra:
ACK f732b1d3cc2b3f9d012b3eeecc70d007666d4f1d; successfully ran local tests
Tree-SHA512: 65f523d4632c78bb166b9e4fb49e715e92da28fc2d07d54a3df8e0db7097277958dfbde0989cf1e443e6f56635001298b47fef3eb5e76b8350041b1ee7be46a6
3b8164139f primitives: Add docs section for script hex API (Tobin C. Harding)
6b90e42e78 Finalize the script hex APIs (Tobin C. Harding)
Pull request description:
In #4316 we made some 'improvements' to what script functions and trait implementations do and do not include the length prefix. Iterate again on it as described here: https://github.com/rust-bitcoin/rust-bitcoin/pull/4316#issuecomment-2847710436
- Patch 1 does the changes
- Patch 2 adds some more docs, requires a grammarian to check my Aussie lingua
ACKs for top commit:
apoelstra:
ACK 3b8164139f6ecebc97b66a299b4a87c2288d8a76; successfully ran local tests
Tree-SHA512: 481db88ae1b6f5751e81e4cd126f545cfc34bef6dcfcf857f1c7464aeb41f5de95fc4582c015abde04372fe025efa13cdf2906e75517f62cff3ddec05c4d9711
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
Shing Him Ng
merge-script
ChrisCho-H
yancy
Tobin C. Harding
Fmt Bot
Daniel Roberts
Fallengirl
Jamil Lambert, PhD