Enable all the pedantic lints and fix warnings.
Notable items:
- `enum_glob_used` import types with a single character alias
- `doc_markdown`: add a whitelist that includes SegWit and OpenSSL
a74393324b Move opcodes back to bitcoin (Tobin C. Harding)
Pull request description:
Duplicate `opcodes` in `bitcoin` and hide it in `primitives` so we do not have to commit to the API.
We use opcodes in `impl fmt::Display for Script`.
Close: #4144
ACKs for top commit:
apoelstra:
ACK a74393324bd47f89fd47281d567ab15ab6bcb2ba; successfully ran local tests; sure
Kixunil:
ACK a74393324b
Tree-SHA512: 738685b9cd2288a581daa6219e3b21bd48bb4845ea627bf6b8085e0e48f5649ac5ec616a3421d10cd37543f76b66d31f94fd55bf94effc2fb8f91d1ecf5c8611
b656d7a16c Inline small functions (Jamil Lambert, PhD)
Pull request description:
Functions that fit the below criteria should be inline:
> Basically, if a function jut delegates into another function and passing through arguments, it should be inline. Also when doing assignment of `u32` or similarly trivial operation. Also if it checks the validity of argument(s) and then performs something simple (like a call to `_unchecked`).
_Originally posted by Kixunil in https://github.com/rust-bitcoin/rust-bitcoin/pull/4099#discussion_r1966156399_
Add `#[inline]` to all functions in `primitives` that fit the criteria.
ACKs for top commit:
tcharding:
ACK b656d7a16c
apoelstra:
ACK b656d7a16c2d0cdd6d8a94fffeac842ffc0fabf4; successfully ran local tests
Tree-SHA512: 0059aa25252e634e55482b201a9414d19a01435fea801a20dc38b4b701a52f4a14565a1cfb0ff4ed7a771a25629eb192fd69cb4ee81e78f6a0ef79d56db0ef5b
I took a look at the rendered HMTL of `bitcoin`, `primitives`, `units`,
`serde`, and `tokio` and picked a header style that I thought looked
good.
Use it for `primitives` and `units`.
Duplicate `opcodes` in `bitcoin` and hide it in `primitives` so we do
not have to commit to the API.
We use opcodes in `impl fmt::Display for Script`.
Close: #4144
43ae9d7516 primitives: Hide script error internals (Tobin C. Harding)
2d8227f091 Hide relative locktime error internals (Tobin C. Harding)
Pull request description:
Make the struct fields private and add getters.
ACKs for top commit:
apoelstra:
ACK 43ae9d751622c7bef548a469466d74cf01284129; successfully ran local tests; nice! Way easier to understand these types with the new incompatible / expected names
Tree-SHA512: cfe67d60ea61a2a4c27b09071a6b11739ca281bf0b4a655121f90215ce38c3a637acf53a6e01aa2ef26fa80004cd919bf3b3334dbd9566ee2f594cab7750b563
I don't know what I was thinking when I move the taproot hash types to
`primitives`. As correctly pointed out by Kix we agreed to only have
blockdata in `primitives`.
Move the taproot hash types back to `bitcoin::taproot` and remove the
extension traits.
As part of the 1.0 effort and forward maintainability hide the internals
of the two error types in the `script` module. Add getters to get at the
invalid size.
As part of the 1.0 effort and forward maintainability hide the internals
of the two error types in the `relative` locktime module. Doing so
allows us to remove the `non_exhaustive` attribute. Add getters to get
at the error innards.
5680b4e870 Refer to `Script{Buf}` as `Self` where relevant (Martin Habovstiak)
ce55dd5b70 Make `ScriptHash` & `WScriptHash` obey sanity rule (Martin Habovstiak)
9ec9adc71d Add a note about Electrum's script hashes (Martin Habovstiak)
82f553aada Expose `ScriptBuf`'s `capacity` (Martin Habovstiak)
6b9d439dc1 Remove stale FIXME comments (Martin Habovstiak)
0567e6fe1d Put `#[inline]` on most `Script{Buf}` methods (Martin Habovstiak)
b7e2af1b6b Implement `Arbitrary` for `&'a Script` (Martin Habovstiak)
bca2864084 Move `Deref{Mut}` from common module to `owned` (Martin Habovstiak)
3b15e900f0 Add `const` to some `Script` methods (Martin Habovstiak)
277223da6a Make `Script` and `ScriptBuf` obey sanity rules (Martin Habovstiak)
Pull request description:
This implements various improvements related to `Script`. Please refer to the individual commits for details.
This is a part of #4059
ACKs for top commit:
tcharding:
ACK 5680b4e870
apoelstra:
ACK 5680b4e870ba3b7340432256c24d37d2b6ead15a; successfully ran local tests
Tree-SHA512: 5daa8bf6c0b439a579d31d23944077e4a7fa89e14052003d2b81c745f225147f8f6f693d068e0567830027cefea7dda2516596da632bc817199352fa29af0a9b
cb270eae8e Make transaction::Version field private (jrakibi)
6c69b66b0d Use Version constant (jrakibi)
Pull request description:
This commit addresses #4041 by making the `transaction::Version` field private.
This forces people to either use the associated constants (`Version::ONE/TWO/THREE`) or the `non_standard`/`from_consensus` methods for any other transaction version.
This aligns with our approach to `block::Version`
ACKs for top commit:
tcharding:
ACK cb270eae8e
Kixunil:
ACK cb270eae8e
Tree-SHA512: dcf5b50dfeda04e56fec350acd735dcb7099989b552afce4261d559a8a846c0eb3705369ad635ef9bbbfb2373d203a2c3641178925de6685426aa91245db9a8c
This commit addresses #4041 by making the transaction::Version field private
Changes:
- Make the `Version` field private with `pub(crate)`
- Rename `non_standard` to `maybe_non_standard` for clarity since it accepts both standard and non-standard versions
- Add `#[inline]` attributes to small, frequently used methods:
- `as_u32`
- `maybe_non_standard`
Users now must use either:
- Constants (`Version::ONE/TWO/THREE`) for standard versions
- `maybe_non_standard` method for any version (standard or non-standard)
These were re-implementing hashing after the check rather than calling
the `_unchecked` method, so this replaces the manual implementation with
the method.
The Electrum protocol uses hashes of `script_pubkey` that might look
similar to the ones we have in the crate and could be confused. This
change notes that to hopefully avoid the confusion.
Resolves https://github.com/rust-bitcoin/rust-bitcoin/discussions/3997
There are already several methods referring to capacity but none to
retrieve it. Those methods also promise certain behavior that mandates
having *a* capacity field inside the struct, so no changes in layout
will ever remove it. Thus it's OK to expose the field.
Aside from exposing the field, this also fixes up the tests to obey the
sanity rules.
These methods are either newtype casts that should compile to no-ops or
directly calling into some other function with at most some pointer
adjustments. As such making them `#[inline]` is definitely benefitial.
There are also methods that check length and then call some other
function. These are also worth inlining since the length could be known
at compile time and the check could be eliminated.
We have several trait implementations for `Script` and `ScriptBuf` in a
common module so that it's easy to verify that they are same but `Deref`
and `DerefMut` should *not* be implemented for `Script` so having them
in the common module is not helpful. This moves them to the appropriate
`Owned` module.
The newtype sanity rules (a name I came up with):
* Newtypes should have at most one constructor that directly references
the inner field.
* Newtypes should have at most three accessor methods that directly
reference the ineer field: one for owned access, the second for
borrowed and the third for mutably borrowed.
* All other methods should use the methods above to perform operations
on the newtype and not directly access the fields.
This commit makes `Script` and `ScriptBuf` obey these except for
`reserve` and `reserve_exact` since we don't have `as_mut_vec` method.
As a side effect it also adds `const` to `ScriptBuf::from_bytes`.
This commit introduces `WrapDebug` in `internals` and updates `Witness`
debug implementation to use it. The previous `DebugElements` struct has
been removed in favor of an ad-hoc closure inside `WrapDebug`, which
formats witness elements as a debug list of hex-encoded values.
By abstracting out the "debug-print hex fields" pattern, we reduce
code duplication and improve maintainability.
4259dab93a Remove rust-ordered dependency (Tobin C. Harding)
d392cdbd7d Remove PartialOrd from locktimes (Tobin C. Harding)
Pull request description:
These two things are related so we remove them both in the same PR. Please see commit logs for full explanation.
For more context see discussion below and also:
- #2500
- #4002
- #3881Close: #4029
ACKs for top commit:
Kixunil:
ACK 4259dab93a
apoelstra:
ACK 4259dab93ab52795305db4b889b9151595bcee51; successfully ran local tests
Tree-SHA512: 7526d4faaa9edf8017d2af412c41a33f33d851ad5130c9a745bba86d9c71dc1db7f20d07377aaf3a25fec2c0de79f3ffabc2c538a5a366e415c7a6eaa730153c
It has turned out that the `rust-ordered` crate and it's
`ArbitraryOrd` trait are only useful for locktimes and only marginally
useful for them at best.
Remove the `ArbitraryOrd` impls and the `rust-ordered` dependency.
This topic was discussed in various places including:
- #2500
- #4002
- #3881Close: #4029
Currently it is possible to write
```rust
if this_locktime < that_locktime {
do_this();
}
```
with the hope that this code means if a locktime is satisfied by the
value in the other locktime. This is a footgun because locktimes are
incommensurate.
We provide the `is_satisfied_by` API to help users do locktime
comparisons.
Remove the `PartialOrd` implementation from both locktime types.
8d8edd2c77 make Debug representation of Witness to be slice of hex-encoded bytes strings (Erick Cestari)
Pull request description:
This PR updates the Debug implementation for the Witness type to improve its readability by displaying the witness data as a slice of hex-encoded strings rather than a concatenated blob or list of raw u8 values. The changes include:
- Improved Output:
The debug output now shows pseudo-fields such as the number of elements and the total length of all elements, making it easier to understand the underlying data without exposing internal indices like indices_start.
- Hex-Encoding:
Each witness element is displayed as a hex-encoded string, similar to Bitcoin Core's output style, which enhances clarity during debugging sessions.
These changes should provide a more developer-friendly view of the witness data and align with similar patterns used elsewhere in the ecosystem.
Closes#4023.
Example display:
```
Witness {
num_elements: 3,
total_bytes: 5,
elements: [
0b,
1516,
1f20,
],
}
```
```
Witness { num_elements: 3, total_bytes: 5, elements: [0b, 1516, 1f20] }
```
ACKs for top commit:
tcharding:
ACK 8d8edd2c77
Kixunil:
ACK 8d8edd2c77
apoelstra:
ACK 8d8edd2c77de9b0423533fc70802171803761fcd; successfully ran local tests
Tree-SHA512: ffcdf67542049f405317eecd74876b51972d27ec552eec8e9c7b6324f18f31f4721fc4d2be1e596232c39af90a8d169c082f9b0636e5aa1a80fe1b063d645456
ab2f709181 Implement Default for Script (jrakibi)
Pull request description:
This PR implements Default for `Script` to return an empty script.
*Note: ScriptBuf already has `#[derive(Default)]` in the code so it's already handled*
Resolves#3735
ACKs for top commit:
tcharding:
ACK ab2f709181
Kixunil:
ACK ab2f709181
apoelstra:
ACK ab2f7091814333b20669d41f1f78e0e52795df08; successfully ran local tests; neat!
Tree-SHA512: c06ba98d9bf8568e323ef9082a7f06756586360d6bef2b93721db7f6e28a777852e494c86319c97b0fd5444a0010d6c679625753534c0e1c8116e452ce8fa9cc
6cecc40ae4 Test LockTime PartialOrd (Jamil Lambert, PhD)
Pull request description:
Add tests to kill the mutants in both relative and absolute PartialOrd.
ACKs for top commit:
tcharding:
ACK 6cecc40ae4
apoelstra:
ACK 6cecc40ae4d52b711f58998315155bc8c6b19d7b; successfully ran local tests; thanks!
Tree-SHA512: dba7d90e3f6e62f0d3417bacc09d38145dd29bf654f84c2d3bc68af30c0e65b105146466a384bd35ef4326913ca414fd31f92daa3d7ffe3ff409c49bd1c05d96
7e66091e1e Add from impl tests (Jamil Lambert, PhD)
2f95064cfd Add from_parts test (Jamil Lambert, PhD)
3ee66c5bb8 Modify push test (Jamil Lambert, PhD)
Pull request description:
Add tests to kill the mutants in `primitives/src/witness.rs`
ACKs for top commit:
tcharding:
ACK 7e66091e1e
apoelstra:
ACK 7e66091e1e8b6cdd3e40d001ea1824125f7175e7; successfully ran local tests
Tree-SHA512: 57b2b0e4dbd93023d1a6a9709a02fa843e3ef9b25e7293ad641726b9c335e220a4ed87b717ec5dda999217677a916b86ac7daa9aaaec077afbfee4789836344e
435750f292 Add a parse_vout test (Jamil Lambert, PhD)
957be3c978 Add OutPoint test (Jamil Lambert, PhD)
a4ef027134 Add tests of transaction functions (Jamil Lambert, PhD)
Pull request description:
Add tests to kill the mutants in `primitives/src/transaction.rs`
ACKs for top commit:
tcharding:
ACK 435750f292
Tree-SHA512: 78baf40ad6ed1cd5b3a33346b4702c3a7efbb03ae6eec9a802d3dea99910373cf4e8053c73e9fdc02ce54852d3ee43253f2ff0c149c86870ecfed2fc909e5bcf
Cargo mutants found mutants in witness.
Add to the existing test `push` to kill the mutants from `is_empty` and
`third_to_last`.
Change the dummy values to make the progression of `elements` down the
test easier to follow.
Currently in order to release `hashes v1.0` we need to 1.0 `io` as well.
For multiple reasons, many out of our control, the `io` crate may not
stabalise any time soon.
Instead we can invert the dependency between the two crates.
This is an ingenious idea, props to Kixunil for coming up with it.
Notes
- `io` does not currently re-export the `hashes` crate.
- This work highlights that we cannot call `hash_reader` on a siphash.
- The `Hmac::hash_reader` uses the default key which may not be obvious.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
Currently the `primitives` crate does not use the `io` dependency. I
don't know if this is just a mistake of if it used to and the manifest
is just stale.
963dba2bdf Test sequence formatting (Jamil Lambert, PhD)
6c3d7f6443 Test Sequence properties (Jamil Lambert, PhD)
Pull request description:
Add tests to kill the mutants found in Sequence.
ACKs for top commit:
tcharding:
ACK 963dba2bdf
apoelstra:
ACK 963dba2bdf102b2015393014d06cc56b8473520a; successfully ran local tests
Tree-SHA512: f03a02cd49df2b29232bf4f9baf663a7b51b0de504de88ae4d62de96d4e672800ac2387d0675aa48280dcbdfc0738f80484842f98b58324a09462c01956566cf
e5c74c38a2 Add missing re-exports to primitives crate root (Jamil Lambert, PhD)
Pull request description:
There are re-exports in the bitcoin root from primitives that were not re-exported in the primitives crate root.
Add the missing re-exports to primitives crate root.
First mentioned here https://github.com/rust-bitcoin/rust-bitcoin/pull/4012#discussion_r1943895283
ACKs for top commit:
tcharding:
ACK e5c74c38a2
apoelstra:
ACK e5c74c38a20e153f0919b43755d58dff8f2c8da2; successfully ran local tests
Tree-SHA512: c3355981ccd3a5db32423f6f44a6e169062c8cbce1f5e358ae2791e24801459a9db22065b83e54950dd3fd745ac4af4dcb1e1bd4625b5f25a9dd4f8fd4e6b860
b2d0737acc Add tests to CompactTarget (Jamil Lambert, PhD)
Pull request description:
Add tests to pow.rs to kill the mutants found in CompactTarget.
ACKs for top commit:
tcharding:
ACK b2d0737acc
apoelstra:
ACK b2d0737accb998071e0f19cda6f988956b6c097a; successfully ran local tests
Tree-SHA512: 8245b2342fabb7142cc0369cc53f4213f8f685dd0ed9d357214b2f692237b3484236462a96315d2c9409625fcf59484fc505674859542311893c23e50da6ffdf
486d55f042 Add test to opcodes (Jamil Lambert, PhD)
Pull request description:
Cargo mutant found a mutant in opcodes.
Add a test to kill it.
ACKs for top commit:
tcharding:
ACK 486d55f042
apoelstra:
ACK 486d55f0421a05c0aaa8f883bc15cacd212dcc19; successfully ran local tests
Tree-SHA512: 8bd28d56d6e5d79a3531f0a3391ac26021f8351a77e632dcceb8a57b2f1cce05aa02ec5e37d5b5771c3a42a2a7010a241d8f232521678604cc6ecb4bdb3327a5
Tobin C. Harding
merge-script
Jamil Lambert, PhD
Fmt Bot
jrakibi
Martin Habovstiak
Erick Cestari
Andrew Toth