3355400d67 docs: document IWP function return limit and panic case (yancy)
8559a49e03 Do not bound Arbitrary parameters passed to InputWeightPrediction (yancy)
e4c3d1e7a6 Use saturating add in IWP constructors (yancy)
8552534b61 Use u32 for struct and member variables in IWP, saturating to u32::MAX (yancy)
Pull request description:
Use u32 for struct and member variables in InputWeightPrediction (saturating to u32::MAX). To avoid panics during construction and while using auxiliary methods such as `total_size()`, support saturating operations.
closes: https://github.com/rust-bitcoin/rust-bitcoin/issues/4547
ACKs for top commit:
apoelstra:
ACK 3355400d6706ce8fee3daa258e9dbbd648a87dca; successfully ran local tests; looks great!
tcharding:
ACK 3355400d67
Tree-SHA512: 8e4af86914152b4c159749ba71f1a2a45682b7c16ba7b35a0c4fd4e8c7c162d3999f4280dffd71b231c7e24f0b4a18907465bd99d8ef958fb7bc81f519059f63
53b93321c8 psbt: add test vector where non-witness UTXO TXID does not match input txid (Andrew Poelstra)
74f22a0a67 psbt: validate that non_witness_utxo txids match the input txids (Andrew Poelstra)
3337b7a030 psbt: introduce IncorrectNonWitnessUtxo error variant (Andrew Poelstra)
Pull request description:
Fixes#4617
ACKs for top commit:
tcharding:
ACK 53b93321c8
Tree-SHA512: bf841708493fe38a37c7448cc02a26061ea66fd3c7acdbf9df4bbdfe07d8611075dceed20813de2b7cd63864ad80a9f66eaf8b869249c3a2028e1be25b48c5ae
This requires updating a couple serde test vectors -- we are tightening
validation so that some existing serde-serialized PSBTs no longer
deserialize. I believe this is the correct thing to do.
To prevent panics during addition if `usize` is `u64`, use `u32` member
variables internally. TK use `u32::saturating_add` instead of basic
addition. However, to use `u32::saturating_add()`, the variables need
to be of type `u32`. Therefore, this commit transforms the internal
types to `u32`.
In so doing, add a `const` function `saturate_to_u32()` which saturates
a usize to `u32`. Also, replace `compact_size::encoded_size()` with a new
function `encoded_size()` which returns a `u32`, avoiding needless casts.
a5db001fc0 Update Github CI to rustc nightly-2025-06-20 (Jamil Lambert, PhD)
0624d96415 Use the anonymous lifetime for path (Jamil Lambert, PhD)
f22f10b5ca Simplify PartialOrd implementation (Jamil Lambert, PhD)
Pull request description:
Automated daily update to rustc (to nightly-2025-06-20) failed CI due to new lint errors (#4598).
Fix the lint errors.
Update to nightly-2025-06-20.
ACKs for top commit:
apoelstra:
ACK a5db001fc0be214fd7027997cfac991fa0c1e0c4; successfully ran local tests
Tree-SHA512: a396b7b696c2a6a904a832e3ffd14bc2a6f27936aa076641d3f03c6000be4962606b9398c1f82d9aa12ddf919023d87302b338a9779084ef521411f6f0151220
Lint error from new nightly "lifetime flowing from input to output with
different syntax can be confusing".
Add the anonymous lifetime to make it explicit.
Lint error with new nightly version "non-canonical implementation of
`partial_cmp` on an `Ord` type".
Update to directly call instead of dereferencing first.
d9cf7270eb Move `bitcoin/p2p` into `p2p` (rustaceanrob)
Pull request description:
Could use a rebase on #4568
Somehow lost the branch of my original draft so some of the review context is removed, but all comments were addressed in separate PRs.
First commit moves everything required to implement encoding within `p2p`, so we have a reference for functions to add to future moves to `internals`. Second commit does the relocation and third commit removes the `bitcoin/p2p` module.
ACKs for top commit:
apoelstra:
ACK d9cf7270eb457fd660fa505701895ab4756e394d; successfully ran local tests
tcharding:
ACK d9cf7270eb
Tree-SHA512: 31bf960788f45bb60b04dd73793a06828a5540e1e6118376776494bad9330dc8ebcb57749bc84b70a00e4d1d2a5686506e089269833ed99c4f9fa0c3e3b2e5b7
Moves all of the content from `bitcoin/p2p` into `p2p`.
`TryFrom<Network>` must be implemented for `Network -> Magic` now that
`Network` is a foreign, non-exhaustive type. Ser/de test is updated
accordingly, as well as the `Magic::from_network` constructor, which I
have opted to return an `Option<Self>`. The `TryFrom` implementation
uses a new `UnknownNetworkError(Network)` that mirrors the `Magic ->
Network` error.
The example handshake does not generate a random nonce for the version
message, as there is no `rand` dependency in this crate and the program
only makes a single, user-defined connection.
It appears we can do better than copying and pasting the consensus
encoding macros and functions from `bitcoin` without doing weird
cross-crate macros that require some special knowledge of the crate to
know when they will compile.
4284deed29 DerivationPath: support 'h' in Display output for hardened components (vicjuma)
Pull request description:
DerivationPath now supports a display format using both 'h' a single quote (') to indicate hardened components.
This aligns it with its ChildNumber's output style.
Resolves: #4618
ACKs for top commit:
apoelstra:
ACK 4284deed29114c5e31ef7c29e28e352b860f74e7; successfully ran local tests
tcharding:
ACK 4284deed29
Tree-SHA512: 24e053c22ec94b851935debbab83d15ad9f41ccfca0a7c34a061450989b176295512e8ffb187b2a54c6c6926f82ec7e4a4186ccdba2ec2616a6e0603d90d2a9b
0ff8d82193 Make FeeRate from sat constructors infallible (Tobin C. Harding)
3b0286bd56 Return NumOpResult for FeeRate and Weight (Tobin C. Harding)
15065b78c7 Return NumOpResult when calculating fee on Amount (Tobin C. Harding)
75106e6d82 Remove checked_ prefix from fee functions (Tobin C. Harding)
Pull request description:
This was 14dc950b54 from #4610.
Remove the `checked_` prefix from `fee` functions then make them all return `NumOpResult`.
ACKs for top commit:
apoelstra:
ACK 0ff8d82193ef562c1aedabe98a6283d09f803c0c; successfully ran local tests; nice!
Tree-SHA512: 62f2af6701f2a0e17b9f0a0ee132ca4a9289fe00e22c94f746602ed3c1f3758bb22323b224362cb659432ff5297391f3106ee460e9a8f65a39f904d72bc98aeb
We now have constructors that take an arbitrary size fee
rate (`Amount`). The `from_sat_per_foo` constructors can be made
infallible by taking a `u32` instead of `u64`. This makes the API more
ergonomic but limits the fee rate to just under 42 BTC which is plenty.
Note we just delete the `from_sat_per_vb_u32` function because it is
unreleased, in the past we had `from_sat_per_vb_unchecked` so we could
put that back in if we wanted to be a bit more kind to downstream. Can
be done later, we likely want to go over the public API before release
and add a few things back in that we forgot to deprecate or could not
for some reason during dev.
Fuzz with a new function that consumes a `u32`.
Currently we call the `Amount` fee calculation functions `div_by_foo`
and return an `Option` to indicate error. We have the `NumOpResult`
type that better indicates the error case.
Includes a bunch of changes to the `psbt` tests because extracting the
transaction from a PSBT has a bunch of overflow paths that need testing
caused by fee calculation.
A bunch of changes have been implemented lately in the fee calculation
logic. As a result of this a at once time `unreachable` statement is now
reachable - bad rust-bitcoin devs, no biscuit.
Saturate to `FeeRate::MAX` when calculating the fee rate, check against
the limit arg, and win.
Co-developed-by: Andrew Poelstra <apoelstra@wpsoftware.net>
aa76108315 bitcoin: secp256k1 global-context feature re-export (Jose Storopoli)
Pull request description:
Often we want to have the `global-context` feature in `secp256k1` without having to add manually the `secp256k1` dependency and enabling the `global-context` feature.
Having the ability to do that directly from `bitcoin` without having to add `secp256k1` and do the whole tango of tightly coupling the two dependecies versions together, e.g. `bitcoin` `0.32.x` and `secp256k1` `0.29.x` would be really nice and would also simplify a lot code maintainability for anyone who depends on bitcoin.
This needs to be backported to `0.32.x`, which I'll gladly do as well.
ACKs for top commit:
luisschwab:
ACK aa76108315
tcharding:
ACK aa76108315
Tree-SHA512: ff0e0016a6bf7cadf42cb40857393acace538fc3fb5214b89d4222a2fe570e9ae79617ae276273eefc34c3b68a64100bb22a9c01f9075cc3ed0705d7a264b9b3
New clippy lint in rustc nightly "lifetime flowing from input to output
with different syntax can be confusing".
Apply the suggested fix and use the anonymous lifetime for paths.
ebb9861c58 Add backticks around OPCodes (yancy)
Pull request description:
Item in documentation is missing backticks
ACKs for top commit:
tcharding:
ACK ebb9861c58
apoelstra:
ACK ebb9861c58419c1f2e3cce2ef4ecdfa1dee144e0; successfully ran local tests
Tree-SHA512: 1a06db90c026544efda2b248d2e1fd2ca49af9e9828c52b427077edf65e8a6fa789b691e9411d704459a55aae7f28b4bd784a493e89506b83c872b4ecbb9fe1e
dd3f3e44bc Split into_derivation_path tests out (Tobin C. Harding)
0c9dd31f53 Test with m prefix (Tobin C. Harding)
c5073f4c51 Refactor simple valid path tests into a loop (Tobin C. Harding)
3e7fdad5fd Split empty master test out (Tobin C. Harding)
ed36a980f8 Refactor invalid derivation path tests (jamillambert)
015fb1be3b Split invalid derivation path test (jamillambert)
76dd6100a2 Split derivation path test into valid and invalid (jamillambert)
Pull request description:
Refactor tests like its our job.
Co-developed-by: Jamil Lambert <jamil.lambert@proton.me>
ACKs for top commit:
apoelstra:
ACK dd3f3e44bc3329830aacacb252162c57858b0029; successfully ran local tests
Tree-SHA512: 8907ff127c11767a600cf856fb34645eb78392c755998565675a026dc4ddd9db00083963414f7e45fc7a2194ad3c5f251f7249bc7d7a2150a5840d1ec8b5d4da
a1e8fabbbe Remove unnecessary closure (Tobin C. Harding)
Pull request description:
We can just pass the function directly. Found by clippy bizarrely after running `rustfmt` (in bot-created PR #4586).
Internal change only, no logic change.
ACKs for top commit:
yancyribbens:
cr ACK a1e8fabbbe
apoelstra:
ACK a1e8fabbbe26cbe62e1a966cc0c760407bfb063f; successfully ran local tests
Tree-SHA512: 19d5ee65da7cfdae82a9a6609afac6d6b03d1c44c0c5bed21916ec31665e76adfc836ac6a1980251db2b9d5704baa577783dc967b1f0035cf662389c87fc6a0a
028a0d6558 Remove conversion impl macro for `Magic`/`Network` (rustaceanrob)
Pull request description:
Closes#4560 (or at least one way to close it)
Handle the coversion of new networks directly in the `From` and `TryFrom` implementations, as new networks are added infrequently.
ACKs for top commit:
tcharding:
ACK 028a0d6558
apoelstra:
ACK 028a0d6558129f554de8c4247481bc0ad80f1c27; successfully ran local tests
Tree-SHA512: 07b768e229305878849f23e54d3fb4940a736ce44122950e4f4bf68ddeb4f82f2d35020840e8176bd7b562726e46055650ba6de8559bae7559c881b64a437169
4621d2bde1 Modify locktime serde implemenations (Tobin C. Harding)
200c276315 bitcoin: Make test code spacing uniform (Tobin C. Harding)
Pull request description:
Patch 1 is preparatory clean up. Patch 2 is the meat and potatoes. See commit log there for full explanation.
Briefly:
- Remove `serde` stuff from `units::locktime`
- Manually implement `serde` traits on `relative::LockTime`
- Fix the regression test to use the new format
ACKs for top commit:
apoelstra:
ACK 4621d2bde14d71b3d6ef0b14258a7479c049ba3b; successfully ran local tests
Tree-SHA512: bc96d79dd4d9f5a124c22f0d3be4750cb7b6e86ba448b31e233982567578337d331b2582c6e1f953c00e8393c4a4552c4b574fe8a55323c911115b269b24090e
56516757ad Add code comment to amount calculation (Tobin C. Harding)
8cf1dc39b5 Fix incorrect code comment (Tobin C. Harding)
7c186e6081 Refactor fee functions (Tobin C. Harding)
bf0776e3dd Remove panic using checked arithmetic (Tobin C. Harding)
b65860067f Make fee functions const (Tobin C. Harding)
9b2fc021b2 Improve rustdocs on FeeRate (Tobin C. Harding)
1bd1e89458 Re-introduce FeeRate encapsulate module (Tobin C. Harding)
b27d8e5819 Change the internal representation of FeeRate (Tobin C. Harding)
2e0b88ba76 bitcoin: Fix dust 'fee' identifiers (Tobin C. Harding)
399bca531c Reduce the FeeRate::MAX value (Tobin C. Harding)
d174c06a4a Saturate to_fee to Amount::MAX (Tobin C. Harding)
64ac33754f Add missing argument docs (Tobin C. Harding)
fe0a448e78 Temporarily remove const from fee calc function (Tobin C. Harding)
b929022d56 Add floor/ceil versions of to_sat_per_kwu (Tobin C. Harding)
64098e4578 Remove encapsulate module from fee rate (Tobin C. Harding)
Pull request description:
The `FeeRate` is a bit entangled with amount and weight. Also we have an off-by-one bug caused by rounding errors and the fact that we use kwu internally.
We can get more precision in the fee rate by internally using per million virtual bytes.
- Fix: #4516
- Fix: #4497
- Fix: #3806
ACKs for top commit:
apoelstra:
ACK 56516757ad8874d8121dba468946546bb8fd7d4e; successfully ran local tests
Tree-SHA512: 0c6e7e2c9420886d5332e32519838d6ea415d269abda916e51d5847aa2475c87fa4abfc29b5f75e8b10c44df67ae29d823aa93f7cfabbc89eb27f2173b103242
We can just pass the function directly. Found by clippy bizarrely after
running `rustfmt` (in bot-created PR #4586).
Internal change only, no logic change.
We have a single test case that tests for the m prefix while all the
others do not.
Move one test case into the loop and then test on each iteration the
path with m prefix added.
cbe04b00c6 Remove all `p2p` dependency from `network` (rustaceanrob)
Pull request description:
Motivated by moving the `p2p` module to its own crate. `TryFrom` and `From` are already implement for converting to and from `Network`/`Magic`. The methods related to `Magic` are removed from `Network`, as well as any reference to `p2p` in the documentation, as `bitcoin` would no longer depend on `p2p`.
The deser roundtrip test are relocated to `p2p/mod.rs`
ACKs for top commit:
tcharding:
ACK cbe04b00c6
apoelstra:
ACK cbe04b00c67eab896b7ec0535194771ec36cb68f; successfully ran local tests
Tree-SHA512: ac3aa2eef4d78efd719ccc032a4266827faff8c87997111fa0050765b732462e5e5811c8aa923aedba335cbaad9a374fd54dbbe0f4978a1111d0839a5942af7d
The `units::locktime` types are used for two things:
- They are the inner types of `primitives` `LockTime`s
- They are used ephemerally for checking satisfaction
Neither of these use cases requires `serde` impls for the `units` types.
Since we are trying to release 1.0 with minimal amounts of code we
should remove them.
For `LockTime`s that need to be stored on disk or go over the wire we
can manually implement the `serde` traits. For `absolute::LockTime` this
is done already and there is no reason the `relative::LockTime` impl
cannot be the same [0]. This differs from the current `serde` trait impls
but we have already decided that in 0.33 we are going to accept breakage
and direct users to use 0.32 to handle it.
- Remove `serde` stuff from `units::locktime`
- Manually implement `serde` traits on `relative::LockTime`
- Fix the regression test to use the new format
While we are at it use a uniform terse call in `serialize`.
[0] This is because there is an unambiguous encoding for the whole set
of locktimes - consensus encoding.
Fmt Bot
Andrew Poelstra
yancy
Jamil Lambert, PhD
Daniel Roberts
rustaceanrob
vicjuma
Tobin C. Harding