3c0bb63423 Do trivial rustdoc improvements (Tobin C. Harding)
3225aa9556 Use defensive documentation (Tobin C. Harding)
80d5d6665a crypto: key: Move error code to the bottom of the file (Tobin C. Harding)
fe3b1e1140 Move From for Error impl (Tobin C. Harding)
5f8e0ad67e Fix docs on error type (Tobin C. Harding)
f23155aa16 Do not capitalize error messages (Tobin C. Harding)
ae07786c27 Add InvalidSighashTypeError (Tobin C. Harding)
baba0fde57 Put NonStandardSighashTypeError inside ecdsa::Error variant (Tobin C. Harding)
6c9d9d9c36 Improve error display imlps (Tobin C. Harding)
22c7aa8808 Rename non standard sighash error type (Tobin C. Harding)
Pull request description:
EDIT: The commit hashes below are stale but the text is valid still.
In an effort to "perfect" our error handling, overhaul the error handling in the `crypto` module.
The aim is to do a small chunk so we can bikeshed on it then I can apply the learnings to the rest of the codebase.
Its all pretty trivial except:
- commit `4c180277 Put NonStandardSighashTypeError inside ecdsa::Error variant`
- comimt `5a196535 Add InvalidSighashTypeError`
- commit `05772ade Use defensive documentation`
Particularly the last one might be incorrect/controversial.
Also, please take the time to check the overall state of error code in the `crypto` module on this branch in case there is anything else we want to do.
Thanks
ACKs for top commit:
apoelstra:
ACK 3c0bb63423
Tree-SHA512: 7e5f8590aec5826098d4d8d33351a41b10c42b6379ff86e5b889e73271b71921fc3ca9525baa5da53e07fa2e961e710393694e04658a8243799950b4604caf43
d4e8f49fc3 Move p2p::constants::Network to crate root (Tobin C. Harding)
0f78943ef0 Move p2p::constants::Magic to p2p module (Tobin C. Harding)
d9d5a4ed4f Move p2p::constants::ServiceFlags to p2p module (Tobin C. Harding)
99d8ae1173 Improve rustdocs on PROTOCOL_VERSION (Tobin C. Harding)
4330722d62 Move p2p::constants::PROTOCOL_VERSION to p2p module (Tobin C. Harding)
1bac1fd518 Rename the network module to p2p (Tobin C. Harding)
dac97072a5 Add BorrowMut to prelude (Tobin C. Harding)
Pull request description:
This PR has grown.
- Rename the `network` module to `p2p` (fix: #1855)
- add `BorrowMut` to prelude
- Move the `Network`, `ChainHash`, and `Magic` structs to a newly create `p2p::network` module
- Do a few cleanups
ACKs for top commit:
apoelstra:
ACK d4e8f49fc3
Tree-SHA512: 7cee93d36363b8acbece6c92c6afc34bb3abe6d5801373d291db1323c934e7c384f3b5881cefb8b0d102dea53da6ed8da1bf60c324bf60d5dab3af598e0310da
55be538dac policy: Add refactor carve out (Tobin C. Harding)
Pull request description:
I have managed to burn out or bore our reviewers/maintainers. Getting two acks is becoming increasingly difficult. I've pestered everyone to the limit that I feel socially comfortable doing so, and as such, am requesting a carve out to the 2-ACK before merge rule.
The primary justification is that I feel we should have a bit more of BDFL and a bit less total consensus if we are to push forwards.
### Example PRs where this change would apply
- https://github.com/rust-bitcoin/rust-bitcoin/pull/1925
- https://github.com/rust-bitcoin/rust-bitcoin/pull/1854
- https://github.com/rust-bitcoin/rust-bitcoin/pull/1862
ACKs for top commit:
elichai:
I agree this makes sense for refactors ACK 55be538dac
apoelstra:
ACK 55be538dac
sanket1729:
ACK 55be538dac. Same reasons as apoelstra. And this is only for re-factors that are not adding any new features.
RCasatta:
ACK 55be538dac
Tree-SHA512: a5e206252015f49245ed282a3be7a35760d16f94dc6e60f31edf589a41ef642eba52a3bd7d1375b6033f3cf0128f47beee4f03e59cad151c64eedd71ac98baac
The `ServiceFlags` type is used by the p2p layer. It can live in the
`mod.rs` file of the `p2p` module. Done in preparation for removing the
`p2p::constants` module.
This is a straight code move, the `ServiceFlags` replaces the
current re-export.
The `PROTOCOL_VERSION` const is a p2p layer constant. It can live in the
`mod.rs` file of the `p2p` module.
This is a straight code move, the `PROTOCOL_VERSION` replaces the
current re-export.
The `network` module deals with data types and logic related to
internetworking bitcoind nodes, this is commonly referred to as the p2p
layer.
Rename the `network` module to `p2p` and fix all the paths.
Only commit in the docs and error messages to what we _really_ know.
In an attempt to reduce the likelyhood of the code going stale only
commit to what is guaranteed - that we have an error from a module.
This does arguably reduce the amount of context around the error.
As we do for `NonStandardSighashErrorType` add an error struct for
invalid sighash type, used by the `taproot` module instead of returning
a generic error enum with loads of unused variants.
Error types conventionally include `Error` as a suffix.
Rename `NonStandardSighashType` to `NonStandardSighashTypeError`.
While we are at it make the inner type private to the crate, there is no
need to leak the inner values type.
07041d584d Apply rustfmt (The rustfmt Tyranny)
dada6d65b7 script: Move some inspector methods from ScriptBuf to Script (Steven Roose)
Pull request description:
Noticed that these methods belong in Script.
ACKs for top commit:
tcharding:
ACK 07041d584d
sanket1729:
ACK 07041d584d.
apoelstra:
ACK 07041d584d
Tree-SHA512: cdcbdf22f0457123205621ec2834164c4598be1e5b221cf859d60e88110b19f8c1e484e86f60653af237e9c2acbcdbe5d2b4c98ccf239924386639c4ba6222f7
a70b1b9c6c Use standard set of derives on all error types (Tobin C. Harding)
2867acfffa Add non_exhaustive to SignError (Tobin C. Harding)
Pull request description:
This is a redo of #1961 without changing `io::Error` to `io::ErrorKind` and just leaving errors with an IO error in them with only `#[derive(Debug)]`.
ACKs for top commit:
apoelstra:
ACK a70b1b9c6c
sanket1729:
utACK a70b1b9c6c.
Tree-SHA512: 4a716077ed7e852d40b649d03239dea80039b5f32d30b8c572a5c3cd8e0ffbfe2593f8e5b18855fb4e659d08eefb515a6508e138fd0b14e82ab225876653d056
As part of an ongoing effort to make our error types stable and useful
add a stand set of derives to all error types in the library.
`#[derive(Debug, Clone, PartialEq, Eq)]`
Add `Copy` if possible and the error type does not include
`#[non_exhaustive]`.
If an error type includes `io::Error` it only gets `#[derive(Debug)]`.
bb8bd16302 internals: Remove hex module (Tobin C. Harding)
2268b44911 Depend on hex-conservative (Tobin C. Harding)
db50509cd3 Add usage docs to the "core2" feature (Tobin C. Harding)
Pull request description:
Use the newly released `hex-conservative` crate, by doing the following:
- Depend on `hex-conservative` in `bitcoin` and `hashes`
- Re-export `hex-conservative` as `hex` from both crate roots.
- Remove all the old hex code from `hashes`
- Remove all the old hex code from `internals`
- Remove the now unused `internals::prelude`
- Fix all the import statements (makes up the bulk of the lines changes in this patch)
ACKs for top commit:
apoelstra:
ACK bb8bd16302
sanket1729:
utACK bb8bd16302
Tree-SHA512: ec83b3941cae6f32272471779f28461bb04959a3f6a126a68bbf2c748d83ff9518ff8932d9e937a6f389c10028bf3eb58c6b6d71ea066924dd7a34faaec7a087
1edc5f4098 Fill in deprecated since NEXT-RELEASE placeholder (Tobin C. Harding)
Pull request description:
As we gear up for the v0.31.0 release we can fill in the NEXT-RELEASE placeholders.
ACKs for top commit:
sanket1729:
utACK 1edc5f4098
apoelstra:
ACK 1edc5f4098
Tree-SHA512: 61a78e9df0d563a1ebefef3d6c3447b767c5d878f05b45e9a20aecdcef897cc9d8c40d499882a7b8d74d5e99b25a256946c39edf2f05abab370cc0223346f66b
5c8933001c Avoid serialize inner data in RawNetworkMessage (Riccardo Casatta)
bc66ed82b2 Impl Encodable for NetworkMessage (Riccardo Casatta)
8560baaca2 Make fields of RawNetworkMessage non public (Riccardo Casatta)
Pull request description:
This PR removes the need to serialize the inner NetworkMessage in the RawNetworkMessage encoding, thus saving memory and reducing allocations.
To achieve this payload_len and checksum are kept in the RawNetworkMessage and checksum kept in CheckedData, to preserve invariants fields of the struct are made non-public.
ACKs for top commit:
apoelstra:
ACK 5c8933001c
tcharding:
ACK 5c8933001c
Tree-SHA512: aca3c7ac13d2d71184288f7815449e72c4c04fc617a65effba592592ef4ec50f18b6f83dbff58e9c4237cb1fe8e7af52cd43db9036658bdaf7888c07011e46cc
RawNetworkMessage keep the payload_len and its checksum in the struct, thus
is not needed to serialize the inner network message
pub in fields of both RawNetworkMessage and CheckedData are removed so that
invariant are preserved.
provide accessor method and new for downstream libs.
This is done in order to more easily change the struct without impacting
downstream and also in order to add another field while preserving struct
invariant in future commit.
d3460daee7 Rename HashParse error variant to InvalidHash (Tobin C. Harding)
Pull request description:
Recently we changed the inner type of this variant and the name became stale because it is caused by a from slice constructor not from parsing.
ACKs for top commit:
apoelstra:
ACK d3460daee7
sanket1729:
ACK d3460daee7
Tree-SHA512: 0b4c405ba7f043f261bd1fba0c3ce943304fddcf33ef9d338e054bf048888008a173d24981c6ce90783dbfbb021d2430f19524f9dc13285c227a1e23ad3e1413
We have just released the `hex-conservative` crate, we can now use it.
Do the following:
- Depend on `hex-conservative` in `bitcoin` and `hashes`
- Re-export `hex-conservative` as `hex` from both crate roots.
- Remove all the old hex code from `hashes`
- Fix all the import statements (makes up the bulk of the lines changed
in this patch)
7b402e930c schemars: Add pinning docs (Tobin C. Harding)
0848ab7e25 Fix clippy warnings for embedded build (Tobin C. Harding)
5b1443a91c hashes/embedded: Add script dir and README (Tobin C. Harding)
94732aecbf Add patch section to test crates (Tobin C. Harding)
512d982275 Remove path field from internals dependency (Tobin C. Harding)
Pull request description:
Do a bunch of infrastructure fixes that either are needed for adding additional crate deps (hex) or updating deps (internals, hashes), or just make the tests more maintainable.
ACKs for top commit:
apoelstra:
ACK 7b402e930c
sanket1729:
ACK 7b402e930c
Tree-SHA512: 9349bb20225363914acc774cca672a23e6562fb02aea644777c558074d5eeb65289d68a93b5be59a93e9b32167e2494f6599caedc8a0d9cfbee2f94d406edbfc
6640074d34 bitcoin/bip32: add DerivationPath::to_u32_vec (Marko Bencun)
Pull request description:
This is useful to pass the keypath to other libraries which expect it to be represented with a list of u32 ints.
Fixes#1944
ACKs for top commit:
apoelstra:
ACK 6640074d34
RCasatta:
ACK 6640074d34
Tree-SHA512: c2327716370558dd9d7e0419f898707ba5e56555284ea7ca746c973080061aae53674b41d8fe7c68a00d7c4bec1e4bb53e8991141749a87dfa40febe9f456369
e30c492faf witness: clean up Debug implementation (Andrew Poelstra)
Pull request description:
The previous code seems to have been rebased/iterated on too many times, and had room for significant simplification. By inlining the indentation logic we can eliminate 40 LOC and also clean up the output by removing trailing spaces.
Fixes#1937
It is not good form to add unit tests for debug output but you can test this locally with the patch
```
diff --git a/bitcoin/src/blockdata/witness.rs b/bitcoin/src/blockdata/witness.rs
index d0b7408c..a2c38af0 100644
--- a/bitcoin/src/blockdata/witness.rs
+++ b/bitcoin/src/blockdata/witness.rs
@@ -619,6 +619,9 @@ mod test {
"304402207c800d698f4b0298c5aac830b822f011bb02df41eb114ade9a6702f364d5e39c0220366900d2a60cab903e77ef7dd415d46509b1f78ac78906e3296f495aa1b1b54101")
];
assert_eq!(witness.to_vec(), expected_witness);
+
+ println!("{:?}", witness);
+ panic!();
}
#[test]
```
And by sticking `{:#?}` in there to see the alternate output.
ACKs for top commit:
tcharding:
tACK e30c492faf
RCasatta:
ACK e30c492faf
Tree-SHA512: 0ec07885f5c75f3f34965852cf5b42b63290295d1f56e9fef7d5b3610b8ac8d318cbf8f184da5b8a9ed5b352bb2c0402797b41714cb9d5488e93c2e290340c2a
9787ba6c96 Rename Script::empty to Script::new (Tobin C. Harding)
Pull request description:
The `empty` constructor is mis-named for the following reasons:
- Non-uniform with `ScriptBuf::new`
- Non-standard with respect to stdlib which uses `Path::new` and `PathBuf::new` (on which we based the `Scritp`/`ScriptBuf`)
Rename the function to `new`, put it at the top of the impl block while we are at it.
ACKs for top commit:
apoelstra:
ACK 9787ba6c96
RCasatta:
ACK 9787ba6c96
Tree-SHA512: 2dee0f61fa9097a48369a0df802ebf238b00ad3e9ed520fbf31affa1cb2a1820cbb910b525be63513e4586acb2aa0b593cecddcad0b6cd894cdac0ba7fcf0871
Pull all the code that depends on `bitcoinconsensus` out into a separate
module `consensus::validation`.
Leave transaction testing of bitcoinconsensus code in the transaction
module.
Andrew Poelstra
Daniela Brozzoni
Tobin C. Harding
The rustfmt Tyranny
Steven Roose
Riccardo Casatta
Marko Bencun