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
This type was defined in the `transaction` module because it was
originally used in a function that had been deprecated in favour of
moving the logic to the `sighash` module.
We just removed the deprecated code so we can now move this type to the
`sighash` module where it is used.
We only keep deprecated code around for one release so we can now remove
code deprecated in v0.30.0
Done in preparation as we gear up for v0.31.0 release.
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.
There is not need to return the general `script::Error` from the
transaction verify functions. We can better describe the error path by
returning a custom error.
There is no need no nest the `bitcoinconsensus::Error` type within the
`script::Error`, it is the only error type returned by the verify
functions so just return it directly.
Not totally necessary but since I went to the trouble of working out the
last working version add it to the docs so the next guy can grep for
`cargo update` to find them.
To help folk work out how to run the `hashes/embedded` test crate copy
over the `script` directory and an updated version of the `README` from
`embedded/bitcoin`.
In order to keep the embedded and schemacs test crates building when we
update their local transient dependencies we need to use a `patch`
section.
- For `bitcoin/embedded` add `patch` section for `internals`, `hashes`
already has an entry.
- For `hashes/embedded` add `patch` section for `internals`.
- For `hashes/extendend_tests/schemars` add `patch` section for
`internals`.
FTR for direct local dependencies we use a `path` field when specifying
the dependency.
We use two different methods for specifying local dependencies, `patch`
and also `path`. There does not seem to be a reason why we use both,
lets be uniform. Elect to use `patch` for all local crates.
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 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.
92749d29e4 Rename PartiallySignedTransaction to Psbt (Tobin C. Harding)
Pull request description:
Last release we added a type alias for `Psbt`, now lets just rename the type and be done with it.
Includes re-export at the crate root because `bitcoin::Psbt` is clear and obvious.
ACKs for top commit:
sanket1729:
ACK 92749d29e4.
apoelstra:
ACK 92749d29e4
Tree-SHA512: 2ded728409829709a46acd2a83ce9a91839bce222264b2fca122b346ec4f45a52c3f970eb05001794e2f355ce9391df1a184b57baf24589e8a5c3f77f72f6ec7
8813a63ec9 internals: Bump version to 0.2.0 (Tobin C. Harding)
Pull request description:
In preparation for release bump the version and add a changelog entry. Includes updating the dependency in `bitcoin` and `hashes`.
ACKs for top commit:
apoelstra:
ACK 8813a63ec9
sanket1729:
utACK 8813a63ec9
Tree-SHA512: a9bd9d4d69cba21329f3f63a9948afe566bb97c8c65f5d46c329a696a814e9eb31372d378de1ecf0f43f0cb42f11d53dc51bc467223b34629e61315d48b39a29
Last release we added a type alias for `Psbt`, now lets just rename the
type and be done with it.
Includes re-export at the crate root because `bitcoin::Psbt` is clear
and obvious.
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.
71c0043127 Remove docsrs attributes (Tobin C. Harding)
Pull request description:
Somehow when we started using `doc_auto_cfg` we forgot to remove a bunch of docsrs attributes.
ACKs for top commit:
apoelstra:
ACK 71c0043127
sanket1729:
utACK 71c0043127
Tree-SHA512: 16ff8eec0f6cd392d496f8f07cc0773bbda28f7c71022ae6b5e2c47a98d40c94a9169c60c0d8fa5a819f07910593d65a47b69bdc748d64cda0aac3323e9599a6
81a42536f9 Use hex_lit::hex in benches (Tobin C. Harding)
Pull request description:
Currently the test `hex` macro is only available when the `test` compiler configuration option is set but we are using it in benches code, this works for use because `cargo bench` sets `test` for the current crate, however it breaks downstream crates.
Fix: #1830
ACKs for top commit:
RCasatta:
ACK 81a42536f9
apoelstra:
ACK 81a42536f9
Tree-SHA512: 429d38093cf42c50464ce5389313fde7c7d2644423ef11ed8f0a3eed1d55f2d2e4b66b7c2dc6e59e4c2cb96128b09d45a1b48369b404ac5eaecf845d2098f467
Currently the test `hex` macro is only available when the `test`
compiler configuration option is set but we are using it in benches
code, this works for use because `cargo bench` sets `test` for the
current crate, however it breaks downstream crates.
Fix: #1830
In preparation for release bump the version and add a changelog entry.
Includes updating the dependency in `bitcoin` and `hashes` as well as
the minimal/recent lock files.
d45dbef3e7 Manually implement Debug on Witness (Tobin C. Harding)
Pull request description:
The current derived debug implementation on `Witness` prints the content field as an array of integers. We can do better than this by manually implementing `Debug`.
With this applied `Witness` is printed as follows: (first line is `{:?}` and the next is `{:#?}`):
Using `{:?}`:
```
Witness: { indices: 3, indices_start: 8, witnesses: [[0x00], [0x02, 0x03], [0x04, 0x05]] }
```
Using `{:#?}`:
```
Witness: {
indices: 3,
indices_start: 8,
witnesses: [
[0x00],
[0x02, 0x03],
[0x04, 0x05],
],
}
```
ACKs for top commit:
sanket1729:
tested ACK d45dbef3e7. This would be helpful for debugging downstream.
apoelstra:
ACK d45dbef3e7
Tree-SHA512: eacf4fa8e3f38c4e9ddc45de78afb8eab5b5b196b77a6612f61860e0e4e7ba96de2e7f434b92816e0b00532e73c05378cafc046ec9c34108e9d9216fb36c524a
552f19abe3 Add more rustdocs to WitnessProgram (Tobin C. Harding)
89303c1464 Move witness types to the script module (Tobin C. Harding)
Pull request description:
This is done as part of an ongoing effort to improve the `address` module and `Address` type.
- Patch 1: Move `WitnessVersion` and `WitnessProgram` to their own modules within `script` - this is code move only except for the variant changes to the `address::Error` enum (see note below on `rustfmt` acting up).
- Patch 2: Improves documentation on the `WitnessProgram`
-
### Note on `rustfmt`
There are a bunch of formatting changes that shouldn't be in here, I stashed and re-ran the formatter a bunch of times but for some reason `rustfmt` wouldn't change `address` as it is but after I patched it `rustfmt` made a bunch of changes.
ACKs for top commit:
sanket1729:
ACK 552f19abe3
apoelstra:
ACK 552f19abe3
Tree-SHA512: 6c33124e1fd4fd7dcc51af4df0584579da8ed8451f4243dc3392babe3e3385d68a7beab9d052cd3f08342032dce7add4c892c6d0187133d64622115bef9fa872
Add rustdocs to `WitnessProgram` commenting on why we carry the witness
version number around with the witness program. This is mainly a dev
comment but it helps document the invariants so make it a rustdoc
comment.
From BIP 141:
> A scriptPubKey (or redeemScript as defined in BIP16/P2SH) that
> consists of a 1-byte push opcode (for 0 to 16) followed by a data push
> between 2 and 40 bytes gets a new special meaning. The value of the
> first push is called the "version byte". The following byte vector
> pushed is called the "witness program".
`WitnessVersion` and `WitnessProgram` are scriptPubkey concerns and
scriptPubkey is basically synonymous with address so in one way it makes
sense that these types are in `address` however we are in the process of
overhauling the `Address` (and `AddressInner`) types so lets move the
witness stuff to `script` and put it in individual sub-modules.
This move helps simplify the address error type also.
Note please, there are a bunch of formatting changes in here in the
error type that I cannot explain and could not remove.
c3a99c62ad CI: Pin serde_json for MSRV build (Tobin C. Harding)
Pull request description:
Recent release of `serde_json` depends on `serde` 1.0.66 but we pin to 1.0.56
Pin `serde_json` for MSRV build to v1.0.99
ACKs for top commit:
apoelstra:
ACK c3a99c62ad
sanket1729:
ACK c3a99c62ad
Tree-SHA512: f9c4e679c9b7f827132f4172056d48fd7428330d8acdb390b022825cfcf20d96610dd7a5cd77c2e833efb8ad52f0d0a5895a7657758a0af01da47db7a881a797
Riccardo Casatta
Tobin C. Harding
Steven Roose
Andrew Poelstra
Marko Bencun
sanket1729