d9cf7270eb Move `bitcoin/p2p` into `p2p` (rustaceanrob)
Pull request description:
Could use a rebase on #4568
Somehow lost the branch of my original draft so some of the review context is removed, but all comments were addressed in separate PRs.
First commit moves everything required to implement encoding within `p2p`, so we have a reference for functions to add to future moves to `internals`. Second commit does the relocation and third commit removes the `bitcoin/p2p` module.
ACKs for top commit:
apoelstra:
ACK d9cf7270eb457fd660fa505701895ab4756e394d; successfully ran local tests
tcharding:
ACK d9cf7270eb
Tree-SHA512: 31bf960788f45bb60b04dd73793a06828a5540e1e6118376776494bad9330dc8ebcb57749bc84b70a00e4d1d2a5686506e089269833ed99c4f9fa0c3e3b2e5b7
Moves all of the content from `bitcoin/p2p` into `p2p`.
`TryFrom<Network>` must be implemented for `Network -> Magic` now that
`Network` is a foreign, non-exhaustive type. Ser/de test is updated
accordingly, as well as the `Magic::from_network` constructor, which I
have opted to return an `Option<Self>`. The `TryFrom` implementation
uses a new `UnknownNetworkError(Network)` that mirrors the `Magic ->
Network` error.
The example handshake does not generate a random nonce for the version
message, as there is no `rand` dependency in this crate and the program
only makes a single, user-defined connection.
It appears we can do better than copying and pasting the consensus
encoding macros and functions from `bitcoin` without doing weird
cross-crate macros that require some special knowledge of the crate to
know when they will compile.
We now have constructors that take an arbitrary size fee
rate (`Amount`). The `from_sat_per_foo` constructors can be made
infallible by taking a `u32` instead of `u64`. This makes the API more
ergonomic but limits the fee rate to just under 42 BTC which is plenty.
Note we just delete the `from_sat_per_vb_u32` function because it is
unreleased, in the past we had `from_sat_per_vb_unchecked` so we could
put that back in if we wanted to be a bit more kind to downstream. Can
be done later, we likely want to go over the public API before release
and add a few things back in that we forgot to deprecate or could not
for some reason during dev.
Fuzz with a new function that consumes a `u32`.
To get more precision use sats per million virtual bytes.
To make review easier keep most calls in tests using
`FeeRate::from_sats_per_kwu` and just unwrap. These can likely be
cleaned up later on if we want to.
For `serde` just change the module to `_floor` and leave it at that. The
serde stuff likely needs re-visiting before release anyways.
The new module `fuzz_utils` in the `fuzz_targets/` directory causes
`verify-execution` to fail.
Move the module to the `src/` directory. Create a `lib.rs` file.
On the way re-design the API by doing:
- Introduce `Checked` and `Unchecked` tags
- Rename the `txdata` field to `transactions`
- Make the `Block` fields private
- Add getters for `header` and `transactions` fields
- Move the various `compute_` methods to be free standing functions
- Make the `check_` functions private
- Introduce extension traits
WARNING: This is not like all the other extension traits.
Because of the use of generics on various `Transaction` methods it is
not easily possible to use the `define_extension_trait` macro.
Manually create the extension traits (public and private) for the
`Transaction` type. This is quite ugly but c'est la vie
(Includes two in the `transaction` module and one in the
`consensus_validation` module.)
In preparation to move script types to `primitives` we replace impl
block with extension traits by replacing the temporary modules with
`define_extension_trait`.
the `blockdata` directory is code organisation thing, all the
types/modules are re-exported from other places. In preparation for, and
to make easier, the `primitives` crate smashing work - remove all
explicit usage of `blockdata`.
Note that the few instances remain as they seem required e.g.,
`pub(in crate::blockdata::script)`
Refactor only, no logic changes.
ee30eaa81b fuzz: add more coverage for `deserialize_block` (Bruno Garcia)
Pull request description:
This PR adds more coverage for the `deserialize_block` target. First, it serializes the block and compares with the data provided and then call some block functions to ensure they run without any issue.
ACKs for top commit:
apoelstra:
ACK ee30eaa81b Thanks!
Tree-SHA512: db6c7befa7b429267e9b28fb7910d2b9e8d462e22c791b544b1aa528c7ba4f8e1cd32e1276c67d3da1f97ce484e7ed19ec20b10d2030f2977fd8f855cf510ffe
Recently we introduced a bug in the weight/size code, while
investigating I found that our `Transaction`/`Block` weight/size APIs
were in a total mess because:
- The docs were stale
- The concept of weight (weight units) and size (bytes) were mixed up
I audited all the API functions, read some bips (141, 144) and re-wrote
the API with the following goals:
- Use terminology from the bips
- Use abstractions that mirror the bips where possible
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.
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.
AFAICT we literally never used this; it was available only on the
bitcoin targets and not the honggfuzz ones; AFL has a broken dep
tree (or at least, requires some more MSRV pins that I did not care
to investigate).
Just remove it entirely.
Fmt Bot
merge-script
rustaceanrob
Tobin C. Harding
Luis Schwab
Erick Cestari
Jamil Lambert, PhD
Bruno Garcia
Chris Hyunhum Cho
Martin Habovstiak
Shing Him Ng
Divyansh Gupta