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.
7b52b9c3a3 Fix typo (yancy)
Pull request description:
Seems the comment has a stutter
ACKs for top commit:
tcharding:
ACK 7b52b9c3a3
apoelstra:
ACK 7b52b9c3a3c28afad41d9a246302ce95e4997ee5; successfully ran local tests
Tree-SHA512: 1cb11c03bb3043c19936dbe4c2ccfa1e8fa43e90c118c969da536d010d95762966cc86e257e3365d18d96cc6dfe4a63d94c4d16b1b766dde3882286ce9bb4925
e47836fb1e Do not derive Default on CompactTarget (Tobin C. Harding)
Pull request description:
It is not immediately obvious what a zero value compact target is. Because of this and the fact that we are trying to release minimal code in `primitives 1.0` its best to remove the `Default` derive.
This is an API breaking change.
ACKs for top commit:
apoelstra:
ACK e47836fb1e0c96936096efb2dfca4e2cbc9b9e5f; successfully ran local tests
Tree-SHA512: 7edb067a7ed66af8155b86b2391ee839b0a21b3d8d36b555123b14e93f1822b89c080328464b194e4ff47643440c56fa5afee929fe58b52d5f1cf84c9b96ca83
2a3e606d89 units: Move some things out of impl_u32_macro (Tobin C. Harding)
Pull request description:
Recently we added a private `impl_u32_macro`. It included a bunch of associated consts and a pair of u32 constructor/getter functions.
We overlooked the fact that the macro produces incorrect docs.
Move the offending code out of the macro and into the already existent impl block for each type.
Docs only, no other logic change.
ACKs for top commit:
apoelstra:
ACK 2a3e606d89d25ce54ff91b38b0aa29f7f733b9d9; successfully ran local tests
Tree-SHA512: ad4bdbb35bc674e9664e293841e14dc2374c8baddf3e795edb666c75860f02728e939ef5a93ede6f4c951e92c5dd5368d6a6b9662cf6d5b268f73ab5ac97e2cc
6335c623f6 fix(p2p): remove `AddrV2` <> `SocketAddr` conversions (Luis Schwab)
Pull request description:
This PR reverts #4521 due to loss/creation of port information during the conversion. Users should use the `AddrV2` <> `IpAddr` conversion instead.
Closes#4566
ACKs for top commit:
Kixunil:
ACK 6335c623f6
tcharding:
ACK 6335c623f6
Tree-SHA512: f183756467ca17bb7e7078023a769ec33d3deb4146ab7ec63ef961107ab93ec975c9adffd9eeddc79250abfa1cee9eccbbcaef7466cf4c2c21205b69d8e9eb4f
9d956e8643 test: remove redundant `ServiceFlags` test (rustaceanrob)
Pull request description:
The `ServiceFlags` type is already tested within p2p/mod.rs with a nearly identical test. This type also has nothing to do with `network`
ref: https://github.com/rust-bitcoin/rust-bitcoin/blob/master/bitcoin/src/p2p/mod.rs#L400
ACKs for top commit:
tcharding:
ACK 9d956e8643
apoelstra:
ACK 9d956e8643214b3c1ad0e42cc630e2f35f7b7994; successfully ran local tests
Tree-SHA512: d113609070e3df6d44b1ed57c40c1bce251d59fa22eb1abbf6c576fd84e21464553bee7fa97eba000ad9c1b8ef98e2ba04aec7077486273de9b7e54341ba894b
9dac4d69e0 Implement CheckedSum for Weight iterator (yancy)
0dbcd09bbc Move CheckedSum trait to crate root (yancy)
Pull request description:
* Move CheckedSum to the crate root so that other types can be added
* Add Weight to CheckedSum
ACKs for top commit:
tcharding:
ACK 9dac4d69e0
apoelstra:
ACK 9dac4d69e0f142e9a2dc4b61ea49365a8cae3f4b; successfully ran local tests
Tree-SHA512: 3e4b7f79074e23493ccd17a026542081f0d7a811f4f35edb7994ada95bf414a166531f142ad4986d27fcec448209b2ffa56813b495b5df6b6e8fcd589392e0c1
Recently we added a private `impl_u32_macro`. It included a bunch of
associated consts and a pair of u32 constructor/getter functions.
We overlooked the fact that the macro produces incorrect docs.
Move the offending code out of the macro and into the already existent
impl block for each type.
Docs only, no other logic change.
It is not immediately obvious what a zero value compact target is.
Because of this and the fact that we are trying to release minimal code
in `primitives 1.0` its best to remove the `Default` derive.
This is an API breaking change.
In order to add other types to CheckedSum, remove from the Amount
module. In so doing, other types added to CheeckSum do not need to be
imported into Amount.
2481695b45 Add tests for BIP-373 PSBT_{IN,OUT}_MUSIG2_PARTICIPANT_PUBKEYS serialization and deserialization (Daniel Roberts)
3e8e6d9aa1 Add BIP-373 PSBT_{IN,OUT}_MUSIG2_PARTICIPANT_PUBKEYS serialization and deserialization (Daniel Roberts)
Pull request description:
This change adds support for serializing and deserializing two PSBT keys from BIP-373: `PSBT_IN_MUSIG2_PARTICIPANT_PUBKEYS` and `PSBT_OUT_MUSIG2_PARTICIPANT_PUBKEYS`
This is a part of #4207 that can be implemented independently of the rest (which depends on https://github.com/rust-bitcoin/rust-secp256k1/pull/716). I believe this satisfactorily avoids changing things multiple times on end users, *however* it's not *completely* transparent to end users, since any code that currently accesses these fields through `unknown` will need to be updated. Later, when `PSBT_IN_MUSIG2_PUB_NONCE` and `PSBT_IN_MUSIG2_PARTIAL_SIG` are supported, code will need to be updated a second time to retrieve them from the correct place instead of `unknown`. I'm of the opinion that this imposes a very minor maintenance burden, only consisting of *removing* deserialization code.
### Notes/Requests for feedback
- For the most part I used my judgement rather than `cargo fmt` since `cargo fmt` already had a lot of other complaints, but of course I'll update if I need to.
- To satisfy the requirement that every commit pass tests, the commit updating the psbt serde regression test should probably be squashed into the first commit, but I just wanted to confirm that before I did it. I suppose similarly, the test commit could be squashed as well?
- I waffled between `musig2_participants` and `musig2_participant_pubkeys`, but I've decided to go with `musig2_participant_pubkeys` because that is consistent with Bitcoin Core
ACKs for top commit:
tcharding:
ACK 2481695b45
apoelstra:
ACK 2481695b456bcccbb25c247c1fd39bbda24dbb30; successfully ran local tests
Tree-SHA512: af884923593c9cbb24ff3f1f08219458538592fabde85d5d65bc2d9bc7bf0b1a73dac38d2c56303b4f3162088db129ea7e879c3d4b324e965933c121ef939a07
8dd24cb67b Add Arbitrary type for InputWeightPrediction (yancy)
Pull request description:
This type creates sane Arbitrary InputWeightPrediction types that do not panic. To this end, when a custom type is created by calling new() or from_slice() constructor, only use values that would no greater than block size 4 MB of witness data or 1 MB of non-witness data. This idea is from the discussion here https://github.com/rust-bitcoin/rust-bitcoin/issues/4547
I'd be up for a future PR that limits the constructors to no greater than 1MB or 4MB witness data, although I feel like this Arbitrary type will be less controversial.
Furthermore, I did test this locally against my application and no panics where produced either through the constructor nor auxiliary methods `total_weight` or `witness_weight`.
ACKs for top commit:
tcharding:
ACK 8dd24cb67b
apoelstra:
ACK 8dd24cb67b948e619038b387d6c3bd79252a5de1; successfully ran local tests
Tree-SHA512: ff33f1a6ced4f68c4d236e362b1d03da405a8b9701cda0405405421ca21a563fa288c8065a9309e542fec0c8bc850119bcf93a2dc5e27677c42b35b1e5e52722
This type creates sane Arbitrary InputWeightPrediction types that do
not panic. To this end, when a custom type is created by calling new()
or from_slice() constructor, only use values that would no greater than
block size 4 Mwu.
9fb48bc0ef fix error grater to greater array_vec.rs (Alex Pikme)
Pull request description:
Error fixed:
**One `grater` than 15 - One `greater` than 15**
ACKs for top commit:
yancyribbens:
cr ACK 9fb48bc0ef
apoelstra:
ACK 9fb48bc0efec96e332beb8e0d4c6239eb23b6da5; successfully ran local tests
Tree-SHA512: 5f2d88ed33ea30971a20b58a383f9c407feeab3e8d65b44356477b9aff0743e36f7a901099df2cd64c6168b2cf4ce92ed9c3a93c4ea596eff315522e8bf2a1dd
07c4f76052 Add comparison traits to InputWeightPrediction (yancy)
Pull request description:
* Partial Eq is added to Enable symmetric and transitive comparison.
* Eq is added to enable reflexive comparison.
ACKs for top commit:
apoelstra:
ACK 07c4f760523b7a196bf160f585c2b437dea5b532; successfully ran local tests
tcharding:
ACK 07c4f76052
Tree-SHA512: baeb957f000ac0f3be89166243b9cc7126daad06ad6688b811037ca5f5713cad1184c7135b2f4f32235457c0f53eb41304846bdd8a84e57b10a6eff0905224e8
9aa235c24d BREAKING: Change Psbt serde implementations (Daniel Roberts)
d7e9a84339 Fix Psbt preimage keys in serde test (Daniel Roberts)
62026c1e2d Don't use PSBT_GLOBAL_XPUB as an unknown key in Psbt serde test (Daniel Roberts)
Pull request description:
Implements the conclusion of #3454 by serializing Psbts using the BIP-174 binary encoding, optionally using base64 where appropriate.
The core of the problem is the old derived serde implementation by its nature can't be backwards compatible when serialized in binary formats like bincode. Fields will be added to the Psbt (my motivating case in #4440 ) and this will always break formats like bincode.
ACKs for top commit:
apoelstra:
ACK 9aa235c24d65d23de2afc21fcbd019892bf4ad2a; successfully ran local tests
tcharding:
ACK 9aa235c24d
Tree-SHA512: 4dc9dbf1a71f06769d74fada7e3d5557a3df3ee78769c66c2d8480c434baa0abd2efba555137563af58da2cc1d545813eb43b6c696b363a5777a9836bc1b7382
dc2cbc21f9 Make fee_rate test identifiers uniform (Tobin C. Harding)
2e16dbb7e5 fee_rate: Put test assertions in correct order (Tobin C. Harding)
Pull request description:
By convention we put assertions in the order `got` then `want`. This makes debugging easier.
ACKs for top commit:
apoelstra:
ACK dc2cbc21f973251e22901dbf3b4fe80e2fa25b58; successfully ran local tests
Tree-SHA512: 8844a6180c170b391dc1ea6fe225022fcd0858ad8a692af807c7668e0e055e514a0886d0dcc8a63fe23740f46ea0094291c1912035e08077974717cde976f289
4ed0b8918b Automated update to Github CI to rustc nightly-2025-05-23 (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 4ed0b8918b
Tree-SHA512: 4edf4f8e4953f34c5e5c55817b9c5c39fff49e591c6dde11571f35071014f0846baad70204a3036dd9244a12ec66aa05a1a8d45be0ea5fef101dda2c205ae4ee
We have a bunch of unit tests, some use `f` and some use `fee_rate`.
Uniform would be better.
Elect to use the longer form just because there are only 4 instances of
the shorter one (although I personally prefer the shorter form).
a1ce2d1ac8 Use _u32 in FeeRate constructor instead of _unchecked (Tobin C. Harding)
Pull request description:
When constructing an `Amount` it was observed recently that a `u32` is often big enough. For the same reason we can use a `u32` to construct a fee rate instead of overflowing.
Use `_u32`in the constructor and remove the panic.
ACKs for top commit:
apoelstra:
ACK a1ce2d1ac8d646b63532ed9ac459ffea39ec8c20; successfully ran local tests
Tree-SHA512: 339974c954ad613b0be684f7b2fa1402f59fe401969f3785a702ffbb14ac913ecf4c8228240d068ba4b482e38263590154167a071d823ccc9b4d0691036ca484
0a0e23fedf style: use Self:: instead of type aliases in error impls (frankomosh)
Pull request description:
Replace `use FooError as E` with `Self::` , during pattern matching
on the same type in Display/Error trait implementations.
close#4536
ACKs for top commit:
tcharding:
ACK 0a0e23fedf
apoelstra:
ACK 0a0e23fedf3552df677b379d86a1e0ac6b063152; successfully ran local tests
Tree-SHA512: 25c45a890635f990afc3bc09096929f8793007852ac435f061348bff2bd79e3faabf034d1e1e277596f4f7365477f477798f1b1e0c4b918c5b0fa08f8c49e732
395252c6d9 Fix FeeRate::checked_add/sub (Tobin C. Harding)
Pull request description:
Currently the `checked_add` and `checked_sub` functions use a `u64` for the right hand side. This leaks the inner unit because the RHS value is implicitly in the same unit.
Make the functions have `FeeRate` on the RHS.
ACKs for top commit:
apoelstra:
ACK 395252c6d95fdd581ce5e8ad5d7978515aafea23; successfully ran local tests; will one-ACK merge as this is units-only
Tree-SHA512: 1f0893f75c47f720ac741ace0274171ed24efcb6d2724d0fed941899d43b213e165b97aa050d6e40eea78c527af45d090a81b9d6cbd95835ef7105585786fca6
c63f80baec Remove impl From<u64> for FeeRate (Tobin C. Harding)
Pull request description:
This function leaks the inner unit of `FeeRate`. We want to change the unit, best to break downstream so they notice.
ACKs for top commit:
apoelstra:
ACK c63f80baec0780622d70e4c8699369b0a972cb62; successfully ran local tests; will one-ACK merge as this is units-only
Tree-SHA512: 5748f2a4cb29d6554226fe20c5479cb53081da8c2788ac31abfe1a2edb4d17f13a3b3037a840fbdc29e842af3e1accc27835fd5429c7355c1351eb8883943fdc
1fef5a3dc5 units: re-order ceil/floor functions (Tobin C. Harding)
Pull request description:
We have 4 functions, 2 ceil, 2 floor but they are ordered ceil, floor, floor, ceil - this causes my head to twitch when I read it.
The logic in the floor versions is easier so put them first, this is uniform with `fee_rate/mod.rs`.
Code move only.
ACKs for top commit:
apoelstra:
ACK 1fef5a3dc55bfc1858c32f81f18840ec1d01c807; successfully ran local tests; will one-ack merge on the basis that this is trivial
Tree-SHA512: 8f15a34ee637cb6aa4013385e1e7f7c70c2e760908bec01d43a96b9c85a3d309c7b9528f13ec5f072f1166511eb7560d1b5aefdccc1a44922c92e68d527fe1a5
7e67737393 Use / to divide fee by weight (Tobin C. Harding)
Pull request description:
Looks like this code was written before we added support for dividing `Amount` by `Weight`.
Refactor, no logic change.
ACKs for top commit:
apoelstra:
ACK 7e67737393bc1a966bf2ce544d291ea30dc4f0f7; successfully ran local tests; will one-ack merge on the basis that this is trivial
Tree-SHA512: e1c97bea4eaa07ef24e82844c07c899a9baca65d0d3d2dfe32371e7b3c81363ac5844b4fdce9dbe12d8c0131d03dfd4cc13fcc5cc8a0b595ade1c1b06c727c10
Replace derived Psbt serde implementation with one that conforms to
BIP-174. In human readable serde contexts, serialize to the base64
encoded format, and in binary serde contexts, serialize to the raw
binary format.
The previous derived serde implementation cannot be used in a backward or
forward compatible way in binary formats like bincode, which means that
every field added to the Psbt struct would break serde de/serialization
into binary formats. Instead, this one-time breaking change will fix the
issue going forward.
Downstream users with persisted data in the old serde format should continue
using 0.32.x to create migrations to the new format.
The preimage values in the serde Psbt don't actually correspond to the
hash keys they should in the serde test. This doesn't cause an error
currently because the derived serde implementation doesn't enforce their
validity during deserialization, but it will when the serde
implementation is modified to use the BIP-174 format.
The previous test used global key id 1 which is not unknown, it's PSBT_GLOBAL_XPUB.
That's an inconsistent state for a Psbt to be in, and will cause a
deserialization error when the Psbt serde implementation is modified to
use the BIP-174 format.
Currently the `checked_add` and `checked_sub` functions use a `u64` for
the right hand side. This leaks the inner unit because the RHS value is
implicitly in the same unit.
Make the functions have `FeeRate` on the RHS.
We have 4 functions, 2 ceil, 2 floor but they are ordered ceil, floor,
floor, ceil - this causes my head to twitch when I read it.
The logic in the floor versions is easier so put them first, this is
uniform with `fee_rate/mod.rs`.
Code move only.
When constructing an `Amount` it was observed recently that a `u32` is
often big enough. For the same reason we can use a `u32` to construct a
fee rate instead of overflowing.
Use `_u32`in the constructor and remove the panic.
yancy
merge-script
Luis Schwab
rustaceanrob
Tobin C. Harding
Daniel Roberts
Alex Pikme
Fmt Bot
Fallengirl
frankomosh