I believe there is currently a bug in the MemorySanitizer, when we pass
various types across the FFI boundry MSAN gives a unititialized variable
error:
- `usize` passed as `size_t` (cannot be uninitialized)
- byte slice passed as `const char *`
In order to let other work continue disable the MSAN job.
6ecc41d126 Return error when constructing pubkey from slice (Tobin C. Harding)
Pull request description:
This PR fixes a bug introduced by me in #2473, and uncovered by #2563 - amazing that it was found so quickly!
Constructing a pubkey using `PublicKey::from_slice` can fail for reasons other than just incorrect length - we should not be using `expect` but rather returning the error.
A purist might argue that we are now returning a nested error type with an unreachable variant:
`ParsePublicKeyError::Encoding(FromSliceError::InvalidLength)`
Is this acceptable or do we want to further improve this?
ACKs for top commit:
sanket1729:
ACK 6ecc41d126
apoelstra:
ACK 6ecc41d126
Tree-SHA512: ae8299b21c4787a104f98533105308e8e7678cd5a29b78c30012982d741c05ba5f2bb1edd1d61d3a5ce028235d18c1511e1f94207479bc19e88cfec7a7ca1737
56132f59d5 Remove the `:#` formatting for `hex_fmt_impl` macro (448 OG)
Pull request description:
This commit attempts to solve #2505 by ensuring that formatting is not forced using the `:#` in the hex macro code generating in macro rule `hex_fmt_impl` in the hashes/utils.rs file.
The write! macro forces all formatting to add the prefix `0x` by adding an alternate by (#) default
```rust
impl<$($gen: $gent),*> $crate::_export::_core::fmt::Debug for $ty<$($gen),*> {
#[inline]
fn fmt(&self, f: &mut $crate::_export::_core::fmt::Formatter) -> $crate::_export::_core::fmt::Result {
write!(f, "{:#}", self) // <-- This is where the formatting is being forced.
}
}
```
By removing this formatting, the `:#` must be specified by the user in order for a prefix to be added.
```rust
let outpoint = bitcoin::OutPoint::default();
println!("{:?}", &outpoint);
println!("{:#?}", &outpoint);
println!("{:#}", &outpoint);
println!("{:x}", &outpoint.txid);
// `{:#}` must be specified to pretty print with a prefix
println!("{:#}", &outpoint.txid);
dbg!(&outpoint);
dbg!(&outpoint.txid);
```
The PR also adds testcase for this when running `cargo test` .
ACKs for top commit:
tcharding:
ACK 56132f59d5
apoelstra:
ACK 56132f59d5
Tree-SHA512: 9e4fc9f30ab0b3cf2651d3c09f7f01d8245ac8ea7ae3a82bb4efd19f25c77662bf279020a31fa61b37587cc0c74284696c56045c59f1ba63b2dd42a210d98ebc
4e3bb7350a Add support for SHA-384 (Matt Corallo)
Pull request description:
Based on #2473 as we need support for 48-byte arrays <-> hex conversions.
Closes#2483
ACKs for top commit:
Kixunil:
ACK 4e3bb7350a
sanket1729:
ACK 4e3bb7350a
Tree-SHA512: e78d97f80ab8afda8a3ea240023338f17f7e95604a879b38fc9bde057fbb45b74b1f3fb3bd2b17af89682b79dda42bf114989e7c63066b3029451ef07894e82f
f8de7954b2 Remove unused pow::TryFromError type (Tobin C. Harding)
43c5eb765c Fix witness_version leaf error type (Tobin C. Harding)
2af764e859 hashes: Fix leaf error type (Tobin C. Harding)
Pull request description:
In light of recent discussion go over the codebase and look for some places that the leaf errors are wrong. Does not do the whole code base, excludes `p2p` and a couple of other places.
ACKs for top commit:
apoelstra:
ACK f8de7954b2
Kixunil:
ACK f8de7954b2
Tree-SHA512: 2905878363869ee205cce49c58c060c712c9b7b55965ee60bb856128842968a4be86c93a194ffffdb35e215b2bea8ad33b04ee47e8e17cc784b0641ea48518e5
Constructing a pubkey using `PublicKey::from_slice` can fail for reasons
other than just incorrect length - we should not be using `expect` but
rather returning the error.
A purist might argue that we are now returning a nested error type with
an unreachable variant:
`ParsePublicKeyError::Encoding(FromSliceError::InvalidLength)`
Is this acceptable or do we want to further improve this?
be329c2d7b Upgrade bitcoinconsenus (Tobin C. Harding)
Pull request description:
Upgrade to the most recent `bitcoinconsensus` version that excludes Taproot verification i.e., one version before latest.
ACKs for top commit:
sanket1729:
ACK be329c2d7b
apoelstra:
ACK be329c2d7b
Tree-SHA512: 428e8e5010719a46401be58c2d4fe88662991766afe97c879147599c1bd79c85ced09dcf5ef37f3cf3f49d79c53b390881969eac93c1475dad751cc760ffd64e
62f726d5a8 Remove useless convertion (Tobin C. Harding)
Pull request description:
No clue how this got into master, I thought pinning nightly stopped things like this; found with `just sane`.
Clippy emits:
useless conversion to the same type: `amount::ParseAmountError`
As suggested, remove the useless conversion.
ACKs for top commit:
sanket1729:
utACK 62f726d5a8
Tree-SHA512: c73a7ba79c17451f226ba004e2abea709e083aa71b9c117e39beb1623c16d9ca85c398f12da079de14bfdd21725057ba89e1515be034970bcc2ce40fc766cd14
This fixes the issue where pretty debug like `dbg` or `{:#}` introduce the use of
`0x` prefix to hex encoded transaction ID.
The transaction id is being forced to pretty print inside the `hex_fmt_impl` macro
using `{:#}` in the line `write!(f, "{:#}", self)` debug formatter.
Resolves: #2505
No clue how this got into master; found with `just sane`.
Clippy emits:
useless conversion to the same type: `amount::ParseAmountError`
As suggested, remove the useless conversion.
f337dec2b1 hashes: Remove unnecessary feature guard from test (Tobin C. Harding)
0cea90d505 Test hashes honour Formatter::precision (Tobin C. Harding)
4bfb466bb9 Upgrade hex dependency (Tobin C. Harding)
f0558e8eb9 Use fmt_hex_exact (Tobin C. Harding)
6820f51408 hashes: Add fmt roundtrip tests (Tobin C. Harding)
e302e30e7c Import with super::* in unit test (Tobin C. Harding)
Pull request description:
Upgrade to use the newly released `hex` code.
- Patch 1: Does trivial preparatory cleanup
- Patch 2: Adds some unit tests to check we roundtrip hashes correctly (added because in the test PR I had the `Midstate` iml wrong and it was not being caught).
- Patch 3: Uses macro in place of `forward_hex` and `backward_hex` - needs concept review, I hacked this without understanding why the functions existed in the first place.
- Patch 4: Does the upgrade, I've attempted to make minimal changes, so there is room for a bunch of cleanups if/when this merges.
- Patch 5: Adds a unit test to verify that we can close#2494
- Patch 6: Removes unnecessary feature gate from unit test.
ACKs for top commit:
Kixunil:
ACK f337dec2b1
apoelstra:
ACK f337dec2b1
Tree-SHA512: 7913d1b3079cf5ba1b0e70f5c33e091c5ef1258026c8f27bbe8a050100bbc7622b6555d560b15be3b3d90d47ce873f137a73cf2d772108d2915fb30ed129bded
3c8edae25b Split relative locktime error up (Tobin C. Harding)
Pull request description:
The `relative` module has a single general error type, we are moving away from this style to specific error types.
Split the `relative::Error` up into three error structs.
I forget the policy on public inner fields.
ACKs for top commit:
sanket1729:
utACK 3c8edae25b
apoelstra:
ACK 3c8edae25b
Tree-SHA512: f3079f81a825125f1efe54657fbba64618530b25aecaa3844902900517bf23bec26ff5399cf22f4e63e44316ebb603e8692cbaece2782ecafe09ffed3eab553c
Test that the new version of `hex` honours `Formatter::precision` for
new wrapped hash types (ie, types created with `hashes::hash_newtype`).
Fix: #2494
Currently we have two functions for displaying forwards and backwards and
we also have `fmt_hex_exact`. I do not know why we added the functions.
In the latest version of hex we do not have the ability to construct a
`DisplayArray` type so we have to use `fmt_hex_exact`.
Make the change now, separate from the `hex` upgrade, to assist review.
Different hashes output to hex strings differently depending on whether
they display backward or not but we are not currently testing that our
parsing and formatting impls both correctly handle backwards/forwards.
Add unit tests to roundtrip through a hex string, do so for one forwards
printing hash (sha256), on backwards printing hash (sha256d), and also
test that the `hash_newtype!` macro correctly passes on display backward.
08a9962035 Replaced Deprecated Function (Sh0g0-1758)
Pull request description:
Changed deprecated Function with a supported one.
ACKs for top commit:
apoelstra:
ACK 08a9962035 yep, this seems reasonable. Thanks!
tcharding:
ACK 08a9962035
Tree-SHA512: fd0a55ab25cd15a3254a6c353e4906b4f4c74175d648e1f532be8b8642490e5187b96aa0aa7365771016e10a481054d3377b9074b93cd001da515177315b0d92
The `relative` module has a single general error type, we are moving
away from this style to specific error types.
Split the `relative::Error` up into three error structs.
Note the change of parameter `h` to `height`, and using `h` as the
pattern matched variable - this makes sense because it gives the
variable with large scope the longer name.
3a56ecc677 Add consts to Params for individual networks (Tobin C. Harding)
Pull request description:
Add consts to the `Params` type for the individual networks.
ACKs for top commit:
apoelstra:
ACK 3a56ecc677
Kixunil:
ACK 3a56ecc677
sanket1729:
ACK 3a56ecc677
Tree-SHA512: 0d265a14dd6a591a267da5381d3dcfd0d313f950dec4922f96d25349047d0c8a366c41dcdc1fc523fe4b178ec6a00b717bda25286625e222194f345cee5e7a97
e49a3c0bfe Remove link to std from rustdoc (Tobin C. Harding)
Pull request description:
We can't link to `std::error::Error` in rustdoc because it breaks non-std builds. Just use backticks - this is not an optimal solution but I know no other.
I have no clue why this is showing up now.
ACKs for top commit:
apoelstra:
ACK e49a3c0bfe
sanket1729:
ACK e49a3c0bfe
Tree-SHA512: 96c540aec59b1db7ad9b185d4ebf4e431dc2a4c9599e2e241b1948ca47995ffe88bf753bb63b01a35c9f82783231744bf8a2bdb89bfb95fbd9324172c4f9c608
6b3c8b9f6b ci: use personal access token for daily nightly update (Andrew Poelstra)
775d0c74cd ci: change automatic CI bot committer name to something without special characters (Andrew Poelstra)
aa8655552b ci: change branch of nightly bot to not conflict with rustfmt bot (Andrew Poelstra)
Pull request description:
Rename the bot's committer name to not have special characters, which causes some problem in tcharding's locale; also change the branch name to not overlap with the "weekly CI" job, since it's causing the two jobs to overwrite each others' PRs.
Was part of #2531 but pulled out here to be in a branch that isn't getting clobbered every 24 hours :).
ACKs for top commit:
tcharding:
ACK 6b3c8b9f6b
Tree-SHA512: 2e605991132eb36543d8fb2636bc70c7cc834db6078d2cc6242c65c0f8c5fd270fc9881e6a5c28f5d87a03551fa4f1e0ad3e87c86c226db5e5b8a4c21270ef8c
The chatbot says this should cause CI to trigger on the nightly update
PRs. With the default token this does not happen because it has the
potential to cause recursive CI actions. (How this is avoided by using a
PAT, I don't know, but whatever.)
ec67456172 Fix CJDNS marker byte check (Ava Chow)
Pull request description:
Only the first byte of a CJDNS address is 0xfc, the second byte should be ignored.
See https://github.com/hyperboria/peers for examples of CJDNS addresses.
ACKs for top commit:
apoelstra:
ACK ec67456172
sanket1729:
urACK ec67456172.
Kixunil:
ACK ec67456172
Tree-SHA512: 0da1054a8e997b6bf6e0aaedc40943edb6a6c7b23f660c92b34dc9395e6153e7e10b0268335a77fbcb5bb635352e1ed92839b350188fd6d33dabe558e88c00bb
b873a3cd44 Do infallible int from hex conversions (Tobin C. Harding)
4d762cb08c Remove the FromHexStr trait (Tobin C. Harding)
026537807f Remove mention of packed (Tobin C. Harding)
Pull request description:
The `FromHexStr` trait is used to parse integer-like types, however we can achieve the same using inherent methods.
Move the hex parsing functionality to inherent methods, keeping the same behaviour in regard to the `0x` prefix.
Patch 1 is trivial preparatory cleanup.
ACKs for top commit:
apoelstra:
ACK b873a3cd44
sanket1729:
ACK b873a3cd44
Tree-SHA512: a280169b68304fcc1a531cc9ffb6914b70238efc4c2241a766105053911a373a0334b73e5ea3525c331ccb81ce98c43fea96dae77668804e608376a48d5ed8ac
de264c0c55 Automated update to Github CI to rustc nightly-2024-03-05 (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:
apoelstra:
ACK de264c0c55 just gonna merge this since we have no problem with the update, just with the CI config
Tree-SHA512: 87c7054680c7fc746223573ebb6d6812bf9a80bdd4880b66ee8ecb4bff24183bcb7ae5c3b48696e5d8e96364d71fdd06a5aa3db4af26f8f13ccadb1ce87e3094
41f0a802af Remove assigned docs (Tobin C. Harding)
7ba63fec82 Fix typo (Tobin C. Harding)
Pull request description:
Do two trivial improvements to the contributing doc. Yes, I am struggling to find stuff to work on because hex is clogging the pipeline :)
ACKs for top commit:
apoelstra:
ACK 41f0a802af
Tree-SHA512: f6a302d4b26b36b4d73aea38ae43230811ad266296706b31bbf220f8031ea4e95feaa770bc28f030495cb41762c7cb2ad6583393323c444389a503f4a2b58689
ee113aa91f ci: add daily job to update nightly rustc (Andrew Poelstra)
f82567fda4 ci: rename a couple .yml files to indicate that they're scheduled (Andrew Poelstra)
85ead84a99 ci: pin nightly in current CI (Andrew Poelstra)
c97f2ccc69 ci: require a nightly compiler rather than using +nightly (Andrew Poelstra)
e386cbfadf ci: delete *test.sh files (Andrew Poelstra)
Pull request description:
We should be pinning our nightly compiler in CI so that
* running CI on old release branches will continue to work (not quite -- we'd want to do the same thing for Cargo.lock, but ok, one PR at a time)
* when CI breaks due to nightly updates (usually new Clippy lints), we can address that in a dedicated "update nightly" PR rather than having every other PR suddenly break on us
Co-authored with ChatGPT which found several typos and suggested most of the error-handling logic.
ACKs for top commit:
tcharding:
ACK ee113aa91f
Kixunil:
ACK ee113aa91f
Tree-SHA512: f198349291a7654f4e6f03998d02c1f7d2c7f999e0b5a89a915beb3e7c741148c2c65367b107c54c34d6669e6f0972699401ef85e76e76e5900c1fb5c844db4f
e3db95226a Tidy description (yancy)
Pull request description:
I noticed `uncheced_add` looked really bad with two spaces (my mistake). Fixed some others as well.
ACKs for top commit:
apoelstra:
ACK e3db95226a oops, we should have caught this. Thanks for the fix!
tcharding:
ACK e3db95226a
Tree-SHA512: 8a2e7f85262f17063bea6ac22855ae45d99a1559a2d30f2627ffba1108f0fd8ebd0b541b50fe746b5af2ebb013cb3e9ea432987b90f37c046120388a808c5443
08f83898a3 Report the position of an invalid char in amount (Martin Habovstiak)
73b325aec5 Report position of the first "too precise" digit (Martin Habovstiak)
28d83551eb Improve `ParseAmountError::InputTooLarge` (Martin Habovstiak)
b7689a7d60 Split up `ParseAmountError::InvalidFormat` (Martin Habovstiak)
Pull request description:
This contains several improvements to `PareAmountError`& co - see the individual commit messages. The changes should be pretty easy to follow.
Note that my planned amount error work is not complete by this, so if this happens to get ACK by tomorrow, I'll wait for this to merge, if not I'll add more commits (without changing the existing ones).
I want to get this stuff into the first `units` release.
ACKs for top commit:
tcharding:
ACK 08f83898a3
apoelstra:
ACK 08f83898a3
Tree-SHA512: aee476c4b306b940978b61f8e7ea89ce401a5736971fb3d4bd6fbfc54916695631022199cd0247a3ecb678653facbe486c89e667de26dc9e67902037cb00b23a
Tobin C. Harding
Andrew Poelstra
Matt Corallo
448 OG
Sh0g0-1758
Ava Chow
Liam Aharon
Update Nightly Rustc Bot
geekvest
yancy