16a813734c Implement consensus deserialize_hex (Tobin C. Harding)
Pull request description:
We have `serialize_hex` and `deserialize` but no `deserialize_hex`, add it.
Move the `IterReader` out of `consensus::serde` to the `consensus` module.
Add some additional logic to the `DecodeError`, I'm not sure why this wasn't there before?
Use the `HexSliceToBytesIter` by way of the `IterReader` to deserialize an arbitrary hex string. Add unit tests to check that we consume all bytes when deserializing a fixed size object (a transaction).
ACKs for top commit:
apoelstra:
ACK 16a813734c
sanket1729:
ACK 16a813734c
Tree-SHA512: 121285cb328ca01bf9fd2a716e6d625fa93113a11613d44c576e3e49a9d06dc181165d2d9bfb9beea7c3d2aff264f64ade4965acd594b05ce0d1660e7493d2e4
cbee9781e8 Move unit types to units (Tobin C. Harding)
5bd0d7194b Remove unused absolute::Error (Tobin C. Harding)
Pull request description:
Move the following unit types to the new `units` crate:
- `locktime::absolute::{Height, Time}`
- `locktime::relative::{Height, Time}`
- `FeeRate`
- `Weight`
Also move the `parse` module as well as constants as required.
Do minimal changes to get things building:
- Feature gate on "alloc" as needed.
- Remove rustdocs that use `bitcoin` types.
- Re-export units types so this is a non-breaking change.
- Fix import paths.
Patch 1 was originally #2526, putting it in via this PR to try and speed up the process.
Close: #2282
ACKs for top commit:
sanket1729:
ACK cbee9781e8
apoelstra:
ACK cbee9781e8 lgtm. this is a good start. I think the LockTime types should follow Height and Time
Tree-SHA512: 6b0d63c7b054008598d7fa81be7d8c112f2778883b5529d79d446617b94b3c196c9ac735f840d1dfb488700894d3161c6976d44ab0e12ac3af4008068eac5f87
0d64ae6eb4 Added tests for PublicKey::from_str (Sh0g0-1758)
Pull request description:
Fixes: #2550
Added some new tests and refactored some older tests.
ACKs for top commit:
sanket1729:
ACK 0d64ae6eb4
apoelstra:
ACK 0d64ae6eb4 thanks for bearing with me!
tcharding:
ACK 0d64ae6eb4
Tree-SHA512: b6792590c56ccac8e8cf6f182e74cb77c4652c537c0357456ff21a7814ebcc8cf48e0fad4c8d47e6e786a50e2cbb48134cb64406bcc900b4fcad9304d9cf4167
41e8fb0863 Support signing taproot in psbt (yu)
Pull request description:
Hi team, I'm from Keystone Wallet team. currently rust-bitcoin does not support signing taproot transactions in psbt.
We think this founction should be included in the psbt module, we submit this PR. Some context and discussion about this PR can be found here: #2418.
For this PR, mostly two new functions are introduced:
- `bip32_sign_schnorr`: sign a taproot input.
- `sighash_taproot`: calculate the sighash message to sign a taproot input along with the sighash type.
Looking forward to your feedback.
ACKs for top commit:
tcharding:
ACK 41e8fb0863
sanket1729:
ACK 41e8fb0863.
Tree-SHA512: 2eb14a3204e6ed848515483778dd7986662aacb332783d187da72d29e207b78a2d427939f2b958135a32de5459221385e6f1f5bae89f491b58d8bc79f202b724
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
We have `serialize_hex` and `deserialize` but no `deserialize_hex`, add it.
Move the `IterReader` out of `consensus::serde` to the `consensus`
module.
Add some additional logic to the `DecodeError`, I'm not sure why this
wasn't there before?
Use the `HexSliceToBytesIter` by way of the `IterReader` to deserialize
an arbitrary hex string. Add unit tests to check that we consume all
bytes when deserializing a fixed size object (a transaction).
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
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?
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
Move the following unit types to the new `units` crate:
- `locktime::absolute::{Height, Time}`
- `locktime::relative::{Height, Time}`
- `FeeRate`
- `Weight`
Also move the `parse` module as well as constants as required.
Do minimal changes to get things building:
- Feature gate on "alloc" as needed.
- Remove rustdocs that use `bitcoin` types.
- Re-export units types so this is a non-breaking change.
- Fix import paths.
The `absolute::Error` is not used, we originally intended it as possibly
useful for users of the library. We have not made effort in other
modules to provide such errors - lets remove it.
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
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
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
This can be checked against the 0.29.x branch, and against the commit
prior to #1659 (40c246743b^) and you will see that it is consistent
EXCEPT:
* In rust-bitcoin 0.29.x we did not have multiple sighash types, only
`Sighash`; we now have `LegacySighash`, `SegwitV0Sighash`, and
`TapSighash`.
* In #1565 we deliberately changed the display direction of the
sighashes, to match BIP 143.
Fixes#2495.
The `address` module is currently publicly re-exporting all error types
that appear as return values for any pubic function, except for the
`P2shError` - we should be uniform.
This re-export of error thing has not been discussed/agreed upon as a
policy but I have been doing it for the last few months anytime I
introduced an `error` module - there has been no push back so I assumed
it was acceptable. Before 1.0 we should probably have a policy on this.
7e2a81d03b Remove unused address::Error type (Tobin C. Harding)
a92dc9c35c Add NetworkValidationError (Tobin C. Harding)
Pull request description:
Replaces #2502 because there is going to be way too much arguing on this to bother a newer contributor with.
This PR takes into consideration #2507 but does not improve the issue, it also does not make it worse. I propose to do this and then consider #2507 since this is a step forwards IMO.
Remove the `address::Error` because its not good. Add a `NetworkValidationError` and return it from `require_network` - leaving the door open for resolving Kix's issue in 2507.
ACKs for top commit:
Kixunil:
ACK 7e2a81d03b
apoelstra:
ACK 7e2a81d03b
Tree-SHA512: 0a220dbec1457f35e3d95f32399f258e65c0477e8b4d14dbe2dfad0a44966ab0339d4c2d9828da318bf131db45cecb2447c0e32d5669a5f4c4739b90c83d3c0d
9d688396c9 base58: Use pub extern crate instead of module (Tobin C. Harding)
Pull request description:
We don't add any implementations to the `base58` types so we can just `pub extern` the crate instead of using a module and re-exporting.
ACKs for top commit:
Kixunil:
ACK 9d688396c9
apoelstra:
ACK 9d688396c9
Tree-SHA512: 521af0fd1ae365a6d060dd3c38fc18c1fb3a6c46adb7cba64e53128c7a898c766bcc603078b4cb8a3672df96559633495b23203a33147b5b4959b0c690ab7451
We have three integer wrapping types that can be created from hex
strings where the conversion from an integer is infallible:
- `absolute::LockTime`
- `Sequence`
- `CompactTarget`
We would like to improve our handling of the two prefix characters (eg
0x) by making it explicit.
- Modify the inherent `from_hex` method on each type to error if the
input string does not contain a prefix.
- Add an additional inherent method on each type `from_unprefixed_hex`
that errors if the input string does contain a prefix.
This patch does not touch the wrapper types that cannot be infallibly
constructed from an integer (i.e. absolute `Height` and `Time`).
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.
c2d658ac05 Add `P2shError` for handling errors related to P2sh (harshit933)
5182a8d7a8 Remove unused variants from `Address::Error` (harshit933)
05b24946eb Add the `FromScriptError` for handling errors in `address` (harshit933)
Pull request description:
This commit adds the `FromScriptError` struct to handle the errors while generating address from any script. It includes :
- Unrecognized script error.
- Witness Program error.
- Witness Version error.
ACKs for top commit:
tcharding:
ACK c2d658ac05
apoelstra:
ACK c2d658ac05
Tree-SHA512: 891eed787129aaf1b664cc16d325178d5d2f77cc41a0543a3d9d1a5af1b58188daece1f6a653bdc6b76b82db0490a39e9bba7fc090e3727d15ee9b8977733698
ac88bc03fd Make constructors const (Tobin C. Harding)
Pull request description:
Audit the codebase for any function that starts with `/// Creates` and see if we can make it const. Inline them at the same time.
ACKs for top commit:
Kixunil:
ACK ac88bc03fd
apoelstra:
ACK ac88bc03fd
Tree-SHA512: 0c71e38018e74b3ce1aae871fc6208b87655a0970ae6cca7a538b9f896c95542c6905eb4d7e02539847e88d8a22a873039a5734130c2a46efb4d1b2b9ffd9f4a
Add a new `base58` crate to the workspace and move the `bitcoin::base58`
module to it.
Done as part of crate smashing, specifically so that we can make `bip32`
into a separate crate.
This commit adds the `FromScriptError` struct to handle the errors
while generating address from any script. It includes :
- Unrecognized script error.
- Witness Program error.
- Witness Version error.
Fixes a gap in the API of the taproot module. Callers can now use
TapTree::root_hash or NodeInfo::node_hash to extract the taproot
tree merkle root hash for fast validation without any ECC overhead.
7e1ba7895f Remove broken kani test (Tobin C. Harding)
Pull request description:
This test is failing. I do not want to dive back into kani right now, just remove it.
This is what I originally did in #2454 but changed directions and tried to fix it. Running kani test takes ages and I'd need to dig back to refresh my memory to work with kani. I don't have the motivation to do that at the moment. Just remove the test.
FTR I added the test recently without fulling thinking it through and it has never passed so we are not loosing any coverage. Doing this was the original mistake I should not have made.
ACKs for top commit:
Kixunil:
ACK 7e1ba7895f
apoelstra:
ACK 7e1ba7895f
Tree-SHA512: cb76807173b637be9d5ce790b015e711ca76add95ce0f0acfdc56947c075f57ea89774c09c4314dbc89086dcf7a8e21053552bfae805fd5dc9c91051cd53c468
10cf51c4c5 Inline private ScriptBuf::p2wpkh function (Tobin C. Harding)
Pull request description:
This function is a bit unclear and is only called once, just inline it.
Refactor only, no logic changes.
ACKs for top commit:
apoelstra:
ACK 10cf51c4c5
Kixunil:
ACK 10cf51c4c5
Tree-SHA512: 3907923f2258089a5fc1cc1e1d0b34e99457d69a5822cefa7bf90405d7ac05d570fb2855f62e2b5b4b871485e349e8dc09eb8f14c0676a8bdd70593e345b9b41
d3d5ee1047 Improve error handling in errors emmited by `keys` (harshit933)
Pull request description:
For now I have tried to group those functions which can produce more than one error and changed the functions which were generating single error from `Key::Error` to the respective error. Let me know if this needs to be changed.
Also in `psbt/error.rs` I have changed the `InvalidPublicKey(crate::crypto:🔑:Error)` to `InvalidPublicKey(crate::crypto:🔑:FromSliceError)`. What should be done here?
Changes -
- in `from_slice` changed the `error` to `FromSliceError`.
- in `verify` changed to `secp256k1::Error` as it can return only one error.
- in `from_str` changed to `FromSliceError`.
- in `CompressedPublicKey` changed `verify` from `Error` to `secp236k1::Error` as it only returns one error.
- introduces CompressedPublicKeyError
- Removes impl from `bip32.rs`
Potential fix#2291
ACKs for top commit:
Kixunil:
ACK d3d5ee1047
tcharding:
ACK d3d5ee1047
Tree-SHA512: 21681bbf87c37eb0caaefe4b356a8a5e1d9b17de3207a0c9294de66b367ab348a7dda1916eb866fe4382e852af14ccab7b9f25a279291cd5beb56bb60b2523c2
ccbd09d5fb Remove unnecessary m/ prefix requirement (josibake)
Pull request description:
`m` in BIP0032 is a variable, not a constant. Requiring it as a constant here is confusing and can lead to erroneous conclusions if using this library as a means of understanding BIP0032.
Fixes#2449
ACKs for top commit:
Kixunil:
ACK ccbd09d5fb
apoelstra:
ACK ccbd09d5fb
Tree-SHA512: b641679f958f20a51c1890b23bbaa0153716802d6180dfd1f649e104f291c5a99143e02b75d292b22254201b28e5c53a04ecd7b6a88ff6f964073106419c5ec1
47569302fc Fix broken kani test (Tobin C. Harding)
Pull request description:
Recently we added a kani test that doesn't work because of `debug_assert` calls in ops traits.
Instead of opening the can of worms that is correct panic behaviour in ops lets just remove the test.
ACKs for top commit:
Kixunil:
ACK 47569302fc
apoelstra:
ACK 47569302fc
Tree-SHA512: f4a862d99173c1502e70fe4c2b9085a1f23dd4501f2ae25dc8a92e3edda7804b42b0580ef32fef2a3d5ea0d98e16b6f0fdba456cf4f0926c5b051ec8a6e54c78
In BIP0032, m is used as a variable for the root extended key. It is not
meant to be used as a constant prefix when serializing paths.
Update the DerivationPath parser to no longer require the m prefix.
Remove the m prefix from the unit tests and the bip32, ecdsa-psbt,
and taproot-psbt examples.
close#2449
Changes -
- in `from_slice` changed the `error` to `FromSliceError`.
- in `verify` changed to `secp256k1::Error` as it can return only one error.
- in `from_str` changed to `FromSliceError`.
- in `CompressedPublicKey` changed `verify` from `Error` to `secp236k1::Error` as it only returns one error.
- introduces CompressedPublicKeyError
- Removes impl from `bip32.rs`
- introduces `ParsePubKeyError` to return errors while generating publickey from string
3c62f74684 Add public functions p2wpkh_script_code (Tobin C. Harding)
a246dc98a4 Run sighash example in CI (Tobin C. Harding)
Pull request description:
This was done to fix#1920, it may be of questionable value though.
- Patch 1 is definitely useful, its a CI fix.
- Patch 2 adds two new API functions.
Fix: #1920
ACKs for top commit:
Kixunil:
ACK 3c62f74684
apoelstra:
ACK 3c62f74684
Tree-SHA512: 58743612c48e392f9ac0a94477588aee959c5fe9191dd04405bbb71aed7b0730b5927ad98f9da34dc93caaaac939617348c3f71318cc7e65c2c154b0f3897b89
c084afa8b2 Print hex in Debug for Sequence (Tobin C. Harding)
Pull request description:
Printing the `Sequence` as a decimal is not super useful when debugging, print it in hex instead.
Using code:
let seq = Sequence::from_consensus(0xFFFFFFFF);
println!("sequence: {:?}", seq);
Before applying this patch we get:
sequence: Sequence(4294967295)
And after applying we get:
sequence: Sequence(0xffffffff)
ACKs for top commit:
Kixunil:
ACK c084afa8b2
apoelstra:
ACK c084afa8b2
Tree-SHA512: d60cd8896ca56a30fc8bd030cf3dd1bc1fd3a1609e99bfc2f26b9bd665b11c34c9df93b3f3ad731506d916513ca4a192dde476e16d99f2d4c4b2697f70a7bc98
Add two public API functions on the two public keys, both called
`p2wpkh_script_code` to do exactly as the name suggests.
Of note, I was not able to find anywhere to use these in example code,
this is because of we always use the new `p2wpkh_signature_hash`
function. The new functions may be useful for a user calling
`segwit_v0_encode_signing_data_to`. The may help document the library as
well.
Printing the `Sequence` as a decimal is not super useful when debugging,
print it in hex instead.
Using code:
let seq = Sequence::from_consensus(0xFFFFFFFF);
println!("sequence: {:?}", seq);
Before applying this patch we get:
sequence: Sequence(4294967295)
And after applying we get:
sequence: Sequence(0xffffffff)
Using `non_exhaustive` as well as a public inner field is incorrect, it
prohibits users from creating or matching on the error and does not
achieve forward comparability.
This was never right, we shouldn't have done it.
c69caafefc Remove attribute comments (Tobin C. Harding)
3e83ef9276 Remove consensus error wrapper TODO (Tobin C. Harding)
bfabea94e9 Remove unwrap comment (Tobin C. Harding)
8bdaf4a34d Remove carrying_mul TODO (Tobin C. Harding)
Pull request description:
Add issues and remove the TODOs from the code.
Resolves: #2368
ACKs for top commit:
apoelstra:
ACK c69caafefc
Kixunil:
ACK c69caafefc
Tree-SHA512: b10a3de8da7ace890735023f8441605dd11b0227c27a2357556b8aaa8276a7f34ed220e3bcbc93aad4b35357319318ff7de27210e8f60dd90f6c55af23e21470
0997382772 io: Enable alloc from std (Tobin C. Harding)
ba1166a63b Make crate level attributes uniform (Tobin C. Harding)
Pull request description:
Make the trait level attributes uniform across all released crates in the repo. Excludes things that are obviously not needed, eg, bench stuff if there is not bench code.
- Remove `uninhabited_references` - this is allow by default now.
- Remove `unconditional_recursion` and mark the single false positive we have with an `allow`.
Note, this does not add `missing_docs` to the `io` crate. There is an open PR at the moment to add that along with the required docs.
ACKs for top commit:
apoelstra:
ACK 0997382772
Kixunil:
ACK 0997382772
Tree-SHA512: ef1f638aca171536287cce369be98998e871d26468ad2d8c39d9004db610b406471809c283540a4a19bcede78b12b8976a1bb37e5d431fbff8c8a3e53a64d4e3
The errors returned from various lock time functions had several issues.
Among the obvious - `Error` being returned from all operations even when
some of its variants were unreachable, there were subtle issues around
error messages:
* `ParseIntError` didn't contain information whether the parsed object
is `Height` or `Time`.
* Logically overflow and out-of-bounds should be the same thing but
produced different error messages.
* Mentioning integers is too technical for a user, talking about upper
and lower bound is easier to understand.
* When minus sign is present `std` reports it as invalid digit which is
less helpful than saying negative numbers are not allowed.
It is also possible that `ParseIntError` will need to be removed from
public API during crate smashing or stabilization, so avoiding it may be
better.
This commit significantly refactors the errors. It adds separate types
for parsing `Height` and `Time`. Notice that we don't compose them from
`ParseIntError` and `ConversionError` - that's not helpful because they
carry information that wouldn't be used when displaying which is
wasteful. Keeping errors small can be important.
It's also worth noting that exposing the inner representation could
cause confusion since the same thing: out of bounds can be represented
as an overflow or as a conversion error. So for now we conservatively
hide the details and even pretend there's no `source` in case of
overflow. This can be expanded in the future if needed.
The returned errors are now minimal. `LockTime` parsing errors are
currentlly unchanged.
Make the trait level attributes uniform across all released crates in
the repo. Excludes things that are obviously not needed, eg, bench stuff
if there is not bench code.
- Remove `uninhabited_references` - this is allow by default now.
- Remove `unconditional_recursion` and mark the single false positive we
have with an `allow`.
Note, this does not add `missing_docs` to the `io` crate. There is an
open PR at the moment to add that along with the required docs.
The code originally used `if` and incorrectly casted the value into
`usize` rather than `u64`. This change replaces the whole thing with
`unsigned_abs`.
Closes#1247
6ddb5cce37 Use Magic::BITCOIN in unit tests (Tobin C. Harding)
Pull request description:
We are currently calling `From` to create the magic bytes, this is unnecessary since `Magic` provides consts.
Refactor only, no logic changes.
ACKs for top commit:
Kixunil:
ACK 6ddb5cce37
apoelstra:
ACK 6ddb5cce37
Tree-SHA512: 20e2e017683f123309e3c0876bba42d86a9411bb225f07c486716184fc79837e04a832338ec8b18874ac76791260f6a4620b932ede92c8b222dac08d468cef8a
5eb2de1660 Remove TODO about rand trait (Tobin C. Harding)
66cc007c2b p2p: Remove TODO comments (Tobin C. Harding)
0b5fb45ea0 consensus: Remove HEX_BUF_SIZE todo (Tobin C. Harding)
579668892a consensus: Remove TODO (Tobin C. Harding)
53beb9db30 Remove ancient todos in test code (Tobin C. Harding)
abe2241828 units: Remove "alloc" TODO (Tobin C. Harding)
5386ef0fd2 psbt: Delete TODO comments (Tobin C. Harding)
14c8a2232b examples: Remove TODO (Tobin C. Harding)
Pull request description:
Done while working on #2368. There are 5 left. Do we want to leave the MSRV ones in there?
```bash
bitcoin/src/blockdata/weight.rs:66: // TODO replace with panic!() when MSRV = 1.57+
bitcoin/src/consensus/serde.rs:101: // TODO: statically prove impossible cases
bitcoin/src/pow.rs:445: // TODO: Use `carrying_mul` when stabilized: https://github.com/rust-lang/rust/issues/85532
units/src/amount.rs:595: // TODO replace whith unwrap() when available in const context.
units/src/amount.rs:599: // TODO replace with panic!() when MSRV = 1.57+
```
ACKs for top commit:
Kixunil:
ACK 5eb2de1660
apoelstra:
ACK 5eb2de1660
Tree-SHA512: 285b1711a6e6fba126e2c4159b25454c7f894122b76fde1d3d29e57b2ec0a6e90230e46ac79d70aa133da177c75d267fc5a13489b69881862649de771027ec8e
6715e93e89 Add Witness::p2tr_key_spend function (Tobin C. Harding)
Pull request description:
Add a function for creating the witness when doing a key path spend for a P2TR output.
This mirrors what we did for P2WPKH when adding `Witness::p2wpkh`.
Includes update to the taproot signing example to use the new constructor.
ACKs for top commit:
Kixunil:
ACK 6715e93e89
apoelstra:
ACK 6715e93e89
Tree-SHA512: aab51329e8fda471442bb9cebd6327636548dd157bb9842fe66993fcdd211bb04b2b829aa9d5962dd619f5c0b73d19644a44529c1a5958df1a6bc892147b44f5
Development for `psbt` has move to another repo, these TODO comments are
over there alread, lets just remove them from `rust-bitcoin` as part of
an effort to remove TODOs from the codebase.
fb81bff61f Add a from impl for ParseIntError (Tobin C. Harding)
2130150df6 absolute: Use Self in error type (Tobin C. Harding)
Pull request description:
While reviewing #2335 I noticed a few places that error code needed some love.
ACKs for top commit:
Kixunil:
ACK fb81bff61f
Harshit933:
ACK [`fb81bff`](fb81bff61f)
apoelstra:
ACK fb81bff61f
Tree-SHA512: 17f4e448862be47534d4c1c2962d5db8f90a471e53226022d7c2e153fc501705cd65b070eb17f3239fb8ad242223340f78fe828ea7df3d43707d3e42fcbef557
3cfd746bbc Add functionality to serialize signatures to a writer (Tobin C. Harding)
Pull request description:
Serializing the ecdsa and taproot `Signature` straight to a writer is a useful thing to be able to do.
Add `to_writer` to both `SerializedSignature`s and also to the `Signature`s (calling through to `SerializedSignature`).
Remove TODO comments from code.
ACKs for top commit:
Kixunil:
ACK 3cfd746bbc
Tree-SHA512: 82eb6d42c7b327cdfe5e89348890e45ea39c664420f7ea17d7826a5c388c7aaae917b1334e3f3df645fc4a81a11b59d97c7d6958e99077fbd67193e2a588f2eb
faa45cf10f Remove stale comment (Tobin C. Harding)
c82f26e960 Use hex-conservative to display pubkey (Tobin C. Harding)
Pull request description:
We introduced `hex-conservative` ages ago, use it to display the `PublicKey`.
ACKs for top commit:
Kixunil:
ACK faa45cf10f
apoelstra:
ACK faa45cf10f
Tree-SHA512: 8ad14c7697314f8393ecb9a287215c505924d0655f7bf3536d4be83af983b142e06a96f802beb4548e2de051f1783549d8d1d1a8ebfb678f372a54010717752e
3c4f6850f4 Flatten trivial errors. (Martin Habovstiak)
a4d01d0b6c Factor out `io::Error` from sighash errors (Martin Habovstiak)
Pull request description:
The hadnling of `io::Error` in sighash had a few problems:
* It used `io::ErrorKind` instead of `io::Error` losing inforation
* Changing `io::ErrorKind` to `io::Error` would disable `PartialEq`&co
* The `Io` error wariants were duplicated
It turns out all of these can be solved by moving the `Io` variant into a separate error.
ACKs for top commit:
apoelstra:
ACK 3c4f6850f4
tcharding:
ACK 3c4f6850f4
Tree-SHA512: b7ad6b692062d636ce29e4ebb448a8ac8ea3090feee1d349472e13f905f1f3785decc86e037d2d9658c1331a271e730076139a8d8f6c9b7dadda8b3221f6d434
Add a function for creating the witness when doing a key path spend for
a P2TR output.
This mirrors what we did for P2WPKH when adding `Witness::p2wpkh`.
Includes update to the taproot signing example to use the new constructor.
As is customary add a `From` impl for the `ParseIntError` and use `?`.
While this does not make much difference it saves devs wondering why
there is a `From` impl for one of the variants and not the other.
Serializing the ecdsa and taproot `Signature` straight to a writer is a
useful thing to be able to do.
To both ECDSA and Taproot types:
- Add `SerializedSignature::to_writer`
- Add `Signature::serialize_to_writer`
Remove TODO comments from code.
Andrew Poelstra
Tobin C. Harding
448 OG
Sh0g0-1758
Ava Chow
Liam Aharon
geekvest
harshit933
yu
conduition
yancy
josibake
Martin Habovstiak