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.
08e0d4f0e5 fuzz: cover minimal_non_dust_custom for Script (Bruno Garcia)
66fee1ef87 fuzz: add consume_u64 (Bruno Garcia)
35e7027a08 fuzz: move consume_random_bytes to a util file (Bruno Garcia)
eb8ecd5e3c fuzz: cover minimal_non_dust for Script (Bruno Garcia)
cab8a6134f fuzz: cover count_sigops{_legacy} for Script (Bruno Garcia)
Pull request description:
This PR adds fuzz coverage for `count_sigops`, `count_sigops_legacy`, `minimal_non_dust` and `minimal_non_dust_custom`. In order to not duplicate `consume_random_bytes`, it moves it to a util file and adds a `consume_u64` function to be used in the `deserialize_script` target to fuzz `minimal_non_dust_custom`.
ACKs for top commit:
apoelstra:
ACK 08e0d4f0e53d97edd655ec1103d60a2b4715fb91; successfully ran local tests
Tree-SHA512: d9b0b94af3e2c1b06b790e0b7d13095493d372f8c22babb9139f25f45ff580020a04c00bdba023bf4f89e2db1e13ffdff8e43f073516e7699c2ccc0233b0eb4b
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.)
If folk really want to index into a hash they can us `as_byte_array`
then index that.
Includes a bump to the version number of `hashes` to `v0.15.0` - this
is because otherwise `secp` won't build since we are breaking an API
that is used in the current release of secp.
Fix: #3115
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 version 1.63 satisfies our requirements for MSRV and provides
significant benefits so this commit bumps it. This commit also starts
using some advantages of the new MSRV, namely namespaced features, weak
dependencies and the ability to use trait bounds in `const` context.
This however does not yet migrade the `rand-std` feature because that
requires a release of `secp256k1` with the same kind of change - bumping
MSRV to 1.63 and removing `rand-std` in favor of weak dependency.
Currently the `fuzz.sh` script fails with "unbound variable" if called
without any arguments, this has gone undetected since we added `set
-euox pipefail` because in CI we always call it with an argument.
Use chatGPT to fix the bug.
Fix: #2924
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.
Currently we have a trait `Hash` that is required for `Hmac`, `Hkdf`,
and other use cases. However, it is unegonomic for users who just want
to do a simple hash to have to import the trait.
Add inherent functions to all hash types including those created with
the new wrapper type macros.
This patch introduces some duplicate code but we are trying to make
progress in the hashes API re-write. We can come back and de-dublicate
later.
Includes making `to_byte_array`,`from_byte_array`, `as_byte_array`, and
`all_zeros` const where easily possible.
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
We were using an outdated CBOR crate for MSRV reasons. But this old
crate is causing suprious test failures. So delete it. (Sadly, updating
the crate doesn't fix the issue, replacing it with ciborium breaks our
MSRV tests because it needs a more recent `half` dependency, and
replacing it with `minicbor` doesn't work because minicbor is not based
on serde. So we don't really have any options.)
In general, I am suspicious of this decode-then-reencode test. CBOR has
some ambiguity in integer encoding. Empirically it has seemed to
work for a long time, but this seems more like an indictment of our test
than a positive result.
Also, round-trip testing serde encoding of a byte vector is probably not
a great use of our fuzz resources. I don't believe we have ever had a
problem with this.
Fixes#2801
Recently we modified the fuzz job manually and forgot about the
`generate-files.sh` file.
Update the script to match the current CI job, running it now produces
the same file `cron-daily-fuzz.sh`.
3fa3d37c21 Add set -euo pipefail (Tobin C. Harding)
Pull request description:
Add `euo pipefail` to all non-trial shell scripts, note if `x` is already set we maintain it.
ACKs for top commit:
sanket1729:
utACK 3fa3d37c21
apoelstra:
ACK 3fa3d37c21
Tree-SHA512: 24c0da96bea44ea4f550847c8f73bbe1d9ccd1bfee1714ef8a0c23075e3a6d438b6006e351cce0df7a1cb0b9d1b50096270f65a62d0def05c84c9573ffefacba
Add `euo pipefail` to all non-trial shell scripts, note if `x` is
already set we maintain it.
Note we have a pipe in `run_task.sh` that relies on grep not finding
anything i.e., failing, so we cannot use pipefail there. Disable it and
re-enable it after the pipe.
Waiting for the fuzzer slows down the dev feedback loop because it makes
CI slow. We still want fuzz coverage but not in a way that slows down
devs.
Instead of running the fuzzer on every PR just run it once a nightly.
As we do for other daily jobs, rename the yaml file to make explicit
what it does.
Open questions:
- This should run for 30 minutes but I can't work out why the current
set up only runs for a shorter time than that?
- Is this less fuzzing i.e., is 30 minutes once a day better or worse that 1
minute 30 times?
These are not run in CI since #2353 and are likely to go out of date. If
we want a script that users can run locally then we should create a new
script that wraps our current CI.
Fmt Bot
merge-script
rustaceanrob
Tobin C. Harding
Luis Schwab
Erick Cestari
Jamil Lambert, PhD
Bruno Garcia
Chris Hyunhum Cho
Martin Habovstiak
yancy
Shing Him Ng
Divyansh Gupta
Jose Storopoli