Our decoding code reads bytes in very small chunks. Which is not
efficient when dealing with the OS where the cost of a context switch is
significant. People could already buffer the data but it's easy to
forget it by accident.
This change requires the new `io::BufRead` trait instead of `io::Read`
in all bounds.
Code such as `Transaction::consensus_decode(&mut File::open(foo))` will
break after this is applied, uncovering the inefficiency.
This was originally Kix's work, done before we had the `io` crate.
Changes to `bitcoin` were originally his, any new mistakes are my own.
Changes to `io` are mine.
Co-developed-by: Martin Habovstiak <martin.habovstiak@gmail.com>
There is no advantage in having `io::Read` as opposed to `Read` and
importing the trait. It is surprising that we do so.
Remove `io::` path from `io::Read` and `io::Write`. Some docs keep the
path, leave them as is. Add import `use io::{Read, Write}`.
Refactor only, no logic changes.
We would like all the various hash types to be defined where they
rightly live instead of in the `hash_types` module.
Move transaction hash types to the `transaction` module.
We would like all the various hash types to be defined where they
rightly live instead of in the `hash_types` module.
Move the block hash types to the `block` module. While moving, add full
stops to the rustdoc of each hash.
Re-export _all four_ types from lib.rs (previously `WitnessMerkleNode`
was not re-exported).
We have a convention in `rust-bitcoin` to use external crates directly
when importing them not via `crate::foo`.
Update all the import paths for `io` to use this form.
Improve the public exports in two ways:
1. Inline re-exports into the docs of the module that re-exports them.
2. Separate public and private use statements
Recently we discussed a way to separate the public and private import
statements to make the code more clear and prevent `rustfmt` joining
them all together.
Separate public exports using a code block and `#[rustfmt::skip]`. Has
the nice advantage of reducing the number of `#[doc(inline)]` attributes
also.
1. Modules first, as they are part of the project's structure.
2. Private imports
3. Public re-exports (using `rustfmt::skip` to prevent merge)
Use the format
```rust
mod xyz;
mod abc;
use ...;
pub use {
...,
};
```
This patch introduces changes to the rendered HTML docs.
8eff4d0385 Remove private hex test macro (Tobin C. Harding)
Pull request description:
We have this macro in `hex-conservative` now, remove the version here.
This patch does not change the public API and only touches test code.
ACKs for top commit:
apoelstra:
ACK 8eff4d0385
clarkmoody:
ACK 8eff4d0385
Tree-SHA512: 93a08fff778930071cd1a28c19202e4a94ca8881b2e873538de2e942b71c2cd6184ed6364c572538a8a699295a71761c6f836accaf251a15683138b71f148fab
10374af75c Make error types uniform (Tobin C. Harding)
43d3306822 Use explicit error::Error impl instead of the default (Tobin C. Harding)
2512dbafc2 Remove impl_std_error macro (Tobin C. Harding)
6933ca4fc2 Add suffix to HiddenNodes error type (Tobin C. Harding)
2b40ea24fb Add suffix to IncompleteBuilder error type (Tobin C. Harding)
f41416a0ea Add suffix to UnknownMagic error type (Tobin C. Harding)
5658dac024 Add suffix to UnknownChainHash error type (Tobin C. Harding)
2fb71dd943 Move p2p error types to bottom of file (Tobin C. Harding)
39314ad52f Move error code to match conventional layout (Tobin C. Harding)
Pull request description:
PR aims to achieve two things:
- Make error code brain dead easy to read
- Get error code closer to being ready for v1.0
The first 8 patches are pretty basic, and are broken up into really small changes. The last patch is much bigger, it has a long git log to explain it but reviewing should not take too much brain power.
This PR does not introduce anything new, it just applies what we have been doing recently with errors. Before v1.0.0 others will likely want to re go over all the error types. As such I believe this PR can be merged under the one ack carve-out.
### TODOs (future PRs)
We have a few errors that still need splitting up:
- Split up `merkle_tree::block::MerkleBlockError`
- Split up `psbt::error::Error`
- Split up `IncompleteBuilderError`
Also, all error From's should probably have `#[inline]`, I noticed late in the process and did not have the heart to visit every error again.
ACKs for top commit:
apoelstra:
ACK 10374af75c
clarkmoody:
ACK 10374af75c
Tree-SHA512: 4f4f3533f42dc11af8e7978f3272752bb56d12a68199752ed4af0c02a46a87892b55c695b7007bc3d0bdf389493068d068e2be1780e8c3008815efec3a02eedf
On our way to v1.0.0 we are defining a standard for our error types,
this includes:
- Uses the following derives (unless not possible, usually because of `io::Error`)
`#[derive(Debug, Clone, PartialEq, Eq)]`
- Has `non_exhaustive` unless we really know we can commit to not adding
anything.
Furthermore, we are trying to make the codebase easy to read. Error code
is write-once-read-many (well it should be) so if we make all the error
code super uniform the users can flick to an error and quickly see what
it includes. In an effort to achieve this I have made up a style and
over recent times have change much of the error code to that new style,
this PR audits _all_ error types in the code base and enforces the
style, specifically:
- Is layed out: definition, [impl block], Display impl, error::Error impl, From impls
- `error::Error` impl matches on enum even if it returns `None` for all variants
- Display/Error impls import enum variants locally
- match uses *self and `ref e`
- error::Error variants that return `Some` come first, `None` after
Re: non_exhaustive
To make dev and review easier I have added `non_exhaustive` to _every_
error type. We can then remove it error by error as we see fit. This is
because it takes a bit of thinking to do and review where as this patch
should not take much brain power to review.
752adff9d1 Add method calc_height (Tobin C. Harding)
46f5588646 Add unit test for calc_tree_width (Tobin C. Harding)
Pull request description:
Add a private `PartialMerkleTree::calc_tree_width` function and a unit test to test it.
ACKs for top commit:
apoelstra:
ACK 752adff9d1
clarkmoody:
ACK 752adff9d1
Tree-SHA512: 9c4ad9f6ff47d8faad1c7c1e977427f1528af2712ceffd05357d0c9117b5fdb7b2783afc00a75cb19b853bfbd7b3895baa3a3563bdc496593cc9b06ce80dbbf8
Throughout the codebase we cast values to `u64` when constructing a
`VarInt`. We can make the code marginally cleaner by adding `From<T>`
impls for all unsigned integer types less than or equal to 64 bits.
Also allows us to (possibly unnecessarily) comment the cast in a single
place.
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)]`.
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)
1c3bbd4bf2 internals: Remove attribution from all files (Tobin C. Harding)
99673ab5c4 hashes: Introduce SPDX license identifiers (Tobin C. Harding)
984fe69448 bitcoin: Remove attribution from all files (Tobin C. Harding)
Pull request description:
Please note, whether or not we need a per-file license comment is out of scope for this PR. This PR leaves us with the most simple per-file solution possible and leaves the merit of per-file license comment to be discussed on another day.
Simplify the per-file license stuff by doing:
- Remove the attribution line from each file.
Currently we have a mishmash of attribution lines accompanying the SPDX identifier. These lines are basically meaningless because:
- The date is often wrong
- The original author attributed is not the only contributor to a file
- The term "rust bitcoin developers" is basically just noise
- Introduce SPDX license identifiers into `hashes` and remove attribution line (ie, make `hashes` uniform with `bitcoin`)
Required before merge please:
- [x] ack from apoelstra because as the library original author many of the changes in this PR remove his name
- [x] ack from Kixunil because he had some concerns in the issue descussion
Fix: #1816
ACKs for top commit:
Kixunil:
ACK 1c3bbd4bf2
sanket1729:
ACK 1c3bbd4bf2
apoelstra:
ACK 1c3bbd4bf2
Tree-SHA512: c5ac05c5eb23b3b6a760f707c344b22f5871a4dedee4990b1840f57e4cee1d38560ff4507c354bbf29bc8ff05a179d95d7e100fcf19bd93c5362344a352c7b5a
Currently we have a mishmash of attribution lines accompanying the SPDX
identifier. These lines are basically meaningless because:
- The date is often wrong
- The original author attributed is not the only contributor to a file
- The term "rust bitcoin developers" is basically just noise
Just remove all the attribution lines and be done with it. While we are
at it add an SPDX line to the few files missing it, whether this license
nonsense is even needed is left as an argument for another day.
If we use `#![cfg_attr(docsrs, feature(doc_auto_cfg))]` instead of
`#![cfg_attr(docsrs, feature(doc_cfg))]` we no longer need to manually
mark types with `#[cfg_attr(docsrs, doc(cfg(feature = "std")))]`.
Sweeeeeet.
Various formatting issues have crept into the codebase because we do not
run the formatter in CI.
In preparation for enabling formatting checks in CI run `cargo +nightly
fmt` to fix current formatting issues. No changes other than those
create by the formatter.
Currently we have an associated type on hash types `Inner` with
accompanying methods `into_inner`, `from_inner`, `as_inner`. Also, we
provide a way to create new wrapped hash types. The use of 'inner'
becomes ambiguous with the addition of wrapped types because the inner
could be the inner hash type or the `Inner` byte array of the inner
wrapped hash type.
In an effort to make the API more clear and uniform do the following:
- Rename `Inner` -> `Bytes`
- Rename `*_inner` -> `*_byte_array`
- Rename the inner hash to/from methods to `*_raw_hash`
Correct method prefix `into_` -> `to_` because theses methods convert
owned `Copy` types.
Add the trait Bound `Copy` to the `Bytes` type because we rely on this
trait bound for the conversion methods to be correctly named according
to convention.
Because of the dependency hole created by `secp256k1` this patch changes
the secp dependency to a git tag dependency that includes changes to the
hashes calls required so that we can get green lights on CI in this
repo.
The `MerkleBlock` struct is the main type in this file, put it at the
top of the file. This leaves the next most important type,
`PartialMerkleTree` below that.
Refactor only, no logic changes.
Remove `FromHex` from hash and script types
- Remove the `FromHex` implementation from hash types and `ScriptBuf`
- Remove the `FromStr` implementation from `ScriptBuf` because it does not
roundtrip with `Display`.
- Implement a method `from_hex` on `ScriptBuf`.
- Implement `FromStr` on hash types using a fixed size array.
This leaves `FromHex` implementations only on `Vec` and fixed size arrays.
We use `internals::hex::display::DisplayHex` in many places, we can
improve ergonomics of the `internals` crate by re-exporting it from the
`prelude` module.
The `ToHex` trait was replaced by either simple `Display`/`LowerHex`
where appropriate or `DisplayHex` from `bitcoin_internals` which is
faster.
This change replaces the usages and removes the trait.
This makes the code less noisy and is a preparation for changing it to
`const`-based literal. Because of the preparation, places that used
variables to store the hex string were changed to constants.
There are still some instances of `Vec::from_hex` left - where they
won't be changeable to `const` and where `hex!` is unavailable
(integration tests). These may be dealt with later.
See also #1189
In order to get better test coverage we should not enable the secp26k1
feature "rand-std" in dev-dependencies but instead feature gate tests
that depend on this feature.
Move the `merkleblock` module into the `merkle_tree` module in a
submodule called `block`. In order to do the minimum amount of changes
in this patch DO NOT rename types to improved naming and reduce stutter.
Note:
- block module is private
- the three types are re-exported from `merkle_block`
This patch purposefully does the minimum amount of changes because there
a whole bunch of improvements to the old "merkleblock" module that are
coming next.
In preparation for moving `MerkleBlock` into the `merkle_tree` module;
create a new directory for the module and move `merkle_tree.rs` to
`merkle_tree/mod.rs`.
Tobin C. Harding
yancy
Andrew Poelstra
Steven Roose
Riccardo Casatta
hashmap
Martin Habovstiak