We just added a `Timestamp` type without knowing that there was a push
by OpenTimestamps to also create a timestamp and that our new type may
lead to confusion. Our timestamp is explicitly for the `time` field in a
block so we can call it `BlockTime`. This name change makes the module
name stale but we will change that in a following patch to ease review.
Bitcoin block headers have a timestamp. Currently we are using a
`u32`. while this functions correctly it gives the compiler no chance
to enforce type safety.
Add a `Timestamp` newtype that is a thin wrapper around a `u32`.
Document it and test the API surface in `api.rs`.
Back in 2022 we elected to use `mutagen` for mutation testing. Since
then `cargo mutants` has progressed to a point where we would now like
to use it instead.
Remove all the `mutagen` stuff and update the lock files.
Close: #2829
Recent clippy nightly update introduces warnings about precedence. While
ours are, IMO, clear the lint docs have some cases that are not so I
don't think we should ignore this lint. Specifically I could easily miss
this one
1 << 2 + 3 equals 32, while (1 << 2) + 3 equals 7
ref: https://rust-lang.github.io/rust-clippy/master/#/precede
Add parenthesis to explicitly show precedence. Refactor only no logic
changes.
Bitcoin Core's consensus rules allow this timespan interval to be
negative. This commit also updates Params::pow_target_timespan to be a
u32. This field is almost compared to u64 and i64s, so changing this to
a u32 will allow users to use `.into()` for converstions
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
1649b68589 Standardize wording to `constructs a new` (Jamil Lambert, PhD)
27f94d5540 Replace `creates` with `constructs` (Jamil Lambert, PhD)
Pull request description:
As discussed in issue #3575 there are various ways of saying a new object is created.
These have all be standardized to the agreed version.
Close#3575
ACKs for top commit:
apoelstra:
ACK 1649b68589834dfe9d5b63812da3e9f0e5930107; successfully ran local tests
tcharding:
ACK 1649b68589
Tree-SHA512: 0ed9b56819c95f1fc14da1e0fdbbe03c4af2d97a95ea6b56125f72913e8d832db5d2882d713ae139d00614e651f3834a4d72528bdf776231cceb6772bf2f9963
There is a range of different wordings used in the docs of constructor
type functions.
Change all to start with `Constructs a new` or `Constructs an empty`.
In functions that act like constructors there is a mixture of the usage
of `creates` and `constructs`.
Replace all occurrences of `creates` with `constructs` in the first line
of docs of constructor like functions.
The extension traits are temporary just while we try to stabalize
`primitives`, they are not intended to be implemented by downstream.
Seal the extension traits so that downstream crates cannot implement
them.
Fix: #3231
ff64fa3c46 Move block::Header to primitives (Tobin C. Harding)
65925117a0 Manually implement block::Header::block_hash (Tobin C. Harding)
b02ab25342 Add regression test for block_hash function (Tobin C. Harding)
5614694b69 block: Remove wildcard re-export (Tobin C. Harding)
Pull request description:
- Patch 1: Trivial preparation.
- Patch 2 and 3: Reduce the API surface in a disappointing way due to the orphan rule.
- Patch 4: Do the move.
ACKs for top commit:
apoelstra:
ACK ff64fa3c46 successfully ran local tests; nice!
Tree-SHA512: 8d93de4a12c9f71f9dfbdc023fd8defb78f7d3da995490af8f83a927c2ca6338dd236bc08691311dc7345183fb6d0be61eedce3e4424d4060be2de934facd60a
Recently we removed the `all_zeros` function from `OutPoint` in favour
of a more meaningfully named associated const. We can do the same for
`BlockHash`, the all zeros has is used for the previous blockhash of the
genesis block, add a const named as such.
In test code where we use the `all_zeros` function, just use the more
explicit form `from_byte_array([0; 32])`.
dae42bef9d do not enable bitcoin-io by default (Antoni Spaanderman)
a14cdaf859 don't enable std by default when testing (Antoni Spaanderman)
e83830dcfc use slice instead of array to not have to hardcode the length (Antoni Spaanderman)
55749d6f61 use `hash.to_byte_array` to check equality with `test.output` (Antoni Spaanderman)
969864e3b0 use fixed size array if possible, otherwise `&'static [u8]` (Antoni Spaanderman)
28ccf70fa6 remove unnecesarry borrow operator (`&`) (Antoni Spaanderman)
fa3a3afd02 remove unnecessary slicing (Antoni Spaanderman)
22e42ab86c fix test code being unnecessarily feature gated (Antoni Spaanderman)
Pull request description:
- remove 2 unnecessary cfg attributes from tests left over from #3167 (it made them not dependent on `alloc` anymore)
- simplify assertion logic by removing unnecessary conversions before comparing
- make tests `no_std` compatible by adding imports to alloc or std
- feature gate tests behind the `alloc` feature if they use anything from the alloc crate (like the `format!` macro)
- `schemars` feature enables `alloc` because (for example) its trait wants implementations to return `String`
- fix `bitcoin-io` always enabling when `std` is enabled (only useful if people depend on `hashes` only, `bitcoin` depends on `bitcoin-io` already)
ACKs for top commit:
tcharding:
ACK dae42bef9d
Kixunil:
ACK dae42bef9d
apoelstra:
ACK dae42bef9d successfully ran local tests
Tree-SHA512: 622fd4963ef21530a98af89bcfc71abe8723aac12d363ab88d9bd30dcf2f75392711bec10e2901fab5f1a30e11897d1aae36e22892738aa1e5670166f91fddd4
Potentially the whole `pow` module will move to `primitives` but this
is not possible easily right now. However, we would like to be able to
move the `BlockHash` and `block::Header` types over to `primitives`
and doing so requires the `CompactTarget` to be there.
Move the `CompactTarget` type to `primitives` and re-export it from the
`primitives` crate root.
Note also, we re-export the type publicly from `bitcoin::pow`.
In preparation for moving the `CompactTarget` type to `primitives` stop
using the inner field in code that will stay behind in the
`bitcoin::pow` module.
In preparation for moving the `CompactTarget` to `primitives` remove the
generic `Into` impl and explicitly implement for just the `From` impls
that the `pow` unit tests use.
Test code only.
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.
54c30556a2 Move params to network module (Tobin C. Harding)
045a661ebe Create network directory (Tobin C. Harding)
Pull request description:
Discussed in #2779. Patch one moves `network.rs` to `network/mod.rs`, and patch 2 moves the `params` module over there.
ACKs for top commit:
apoelstra:
ACK 54c30556a2 Yeah, this seems like a good place for it
Kixunil:
ACK 54c30556a2
Tree-SHA512: 134813419db21323d303d465b12fcbf37fd61dc1baf3305e156d324eafd822379e63dede02877ee99dce41540193a29e6e13acd13f9121f3e2fe11096524aa5e
The `Params` struct is currently defined in the `consensus` module which
has become a collection of orthogonal consensus-ish things. We would
like to put things in more descriptive places.
The `Params` struct defines constants that are network specific so it
makes sense to put it in the `network` module. As soft proof of this
argument note in this patch how often the `Params` type is imported
along with the `Network` type.
API break:
The type is no longer available at `bitcoin::consensus::Params` but
rather is re-exported at `bitcoin::network::Params`.
8ee1744b9b Make 'use core::fmt' calls consistent (Shing Him Ng)
Pull request description:
I started taking a look at #2869 and looked for everything that was implementing the `Display` trait:
```rust
impl fmt::Display for _
```
but found some places where the imports weren't consistent:
```rust
impl Display for _
```
There were only a few instances of the latter, so I went ahead and cleaned those up before starting #2869
I started pulling this thread when I saw the same thing was happening for `fmt::Debug` and `fmt::Formatter` so I updated the rest of the `use core::fmt::*` statements with a few exceptions:
- No updates to `use core::fmt::*` if it was being called from within a function since I felt like the function scope was small enough to not cause confusion
- No updates to `use core::fmt::{self, Write as _};`
ACKs for top commit:
Kixunil:
ACK 8ee1744b9b
tcharding:
ACK 8ee1744b9b
Tree-SHA512: 33eb6ea0c4e808ef78bc87de6547144b756bde206c50d80488f740e97cd8d11f1abcb8936c487d7bfd29be5e21c7f40ff88f82acdaaec9aacb4b6362ffc4c680
Manually implement it for Wtxid, Txid and BlockHash, where the all-zero
"hash" has a consensus meaning. But in general we should not be
implementing this method unless we have a good reason to do so. It can
be emulated or implemeted in terms of from_byte_array.
The use of Wtxid::all_zeros is obscure and specific enough that I am
tempted to drop it. But for txid and blockhash, the 0 hash appears in
actual blockdata and we should keep it.
All other uses of all_zeros were either in test code or in places where
the specific hash was not important and [u8; 32] was a more appropriate
type.
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.
18b2788a5a api: Run just check-api (Tobin C. Harding)
6b7d02e5ae Add inherent functions to hashes (Tobin C. Harding)
Pull request description:
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.
ACKs for top commit:
apoelstra:
ACK 18b2788a5a
Tree-SHA512: 6b7a8d8a8501e981416d767040e5bd9fa8d1134be2ca133b5c53aa55f65c8456dccb63b642e30d0d571ca838c6f9eaeff6527d92a9b4212819a49ce619c4e093
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.
Add a unit test that fails if put before the "pow: Fix off-by-one error"
patch. Tests that we can correctly parse a 32 character long hex string
into a `U256`.
The `error` module is empty except for public re-exports. We are still
in the "break everything and get the API right" stage so this module
adds no value - remove it.
Add to `units::parse` the complete suit of hex unit parsing functions:
- remove prefix
- assert without prefix
- parse with or without prefix
- parse with prefix
- parse without prefix
- parse prefix unchecked
Refactor `bitcoin` to use the exact function we need, removing code
duplication.
This is a breaking change to `units`, it does however keep the current
re-exports from the public, now empty, `bitcoin::error` module.
Alex Pikme
healthyyyoung
Tobin C. Harding
Shing Him Ng
Fmt Bot
merge-script
Jamil Lambert, PhD
Antoni Spaanderman
leichak
Martin Habovstiak
Divyansh Gupta