9eaaf114b0 fix(witness): remove explicit type (aagbotemi)
5b572c5976 docs(witness): use constructs instead of creates (aagbotemi)
Pull request description:
This PR fixes#4379
- Removed explicit type
- updated documentation
- PR is in two seperate patches
ACKs for top commit:
apoelstra:
ACK 9eaaf114b0d6afb810cec8e9dac603ecadf64ff4; successfully ran local tests
Tree-SHA512: 65a6ca880c64aa81d83dffbfcd21a0427c874237a606d82ab70f226839adb164c9ee7fd1ecd5d8cee662660efcd1c458c523a27964cd1d00704d40a3bb6a1deb
a5f904559d primitives: Make hex optional (Tobin C. Harding)
Pull request description:
Make the `hex` dependency optional. This means not implementing
`Display` for some types if `hex` is not enabled and only implementing
`Debug`.
Also without `hex` enabled:
- We loose the ability to parse an `OutPoint` from string because we
can no longer parse `Txid`.
- We loose the hex formatting of witness elements.
Note also that `primitives` builds with the `serde` feature even if
`hex?/serde` is excluded from the `serde` feature. I found this
surprising.
Close: #4183
ACKs for top commit:
apoelstra:
ACK a5f904559d3b5d2dfbcbed8b1746305e32103fee; successfully ran local tests
Tree-SHA512: c105e089508036af8251cb923f3eda163b8f2d6151ea043aa1489edb6ce396975d1f598a240f4ca6e48f6ef780774f7d86cb70ec9399f3d2ff87c0ac53ceee91
Make the `hex` dependency optional. This means not implementing
`Display` for some types if `hex` is not enabled and only implementing
`Debug`.
Also without `hex` enabled:
- We loose the ability to parse an `OutPoint` from string because we
can no longer parse `Txid`.
- We loose the hex formatting of witness elements.
Note also that `primitives` builds with the `serde` feature even if
`hex?/serde` is excluded from the `serde` feature. I found this
surprising.
Accessing the internals of tested object is problematic because it makes
changes to layout harder, it makes tests harder to read and it checks
implementation details rather than semantics of the API (behvaior).
We had such tests in `primitives::witness` so this changes them to use
the newly added APIs instead. However, it still leaves
`from_parts__unstable` which needs to be dealt with separately.
Since `Witness` is semantically equivalent to `&[&[u8]]` they should
also be comparable. However we only had the impl to compare `Witness`
with itself. Being able to compare `Witness` with other containers is
particularly needed in tests.
`Witness` was missing conversions from arrays (and variations) which was
annoying when creating known-sized witnesses. These come up when
spending statically-known inputs and in tests.
Previously we've used `try_into().expect()` because const generics were
unavailable. Then they became available but we didn't realize we could
already convert a bunch of code to not use panicking conversions. But we
can (and could for a while).
This adds an extension trait for arrays to provide basic non-panicking
operations returning arrays, so they can be composed with other
functions accepting arrays without any conversions. It also refactors a
bunch of code to use the non-panicking constructs but it's certainly not
all of it. That could be done later. This just aims at removing the
ugliest offenders and demonstrate the usefulness of this approach.
Aside from this, to avoid a bunch of duplicated work, this refactors
BIP32 key parsing to use a common method where xpub and xpriv are
encoded the same. Not doing this already led to a mistake where xpriv
implemented some additional checks that were missing in xpub. Thus this
change also indirectly fixes that bug.
df500e9b71 primitives: Enable pedantic lints (Tobin C. Harding)
Pull request description:
Draft to check the subjective ones please, then I'll squash.
ACKs for top commit:
apoelstra:
ACK df500e9b71187fe658da76adafdb3300a51de2ef; successfully ran local tests
Tree-SHA512: 8cc8c9b369a63c1b2b26461e288a818e3b74e0f9b7359c964c1650028d3161db1d79369c74f18e79958873bf4d223ee72fa481708600f0297d79377d97a84dda
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
Enhance Witness struct element access methods:
- Rename `nth()` to `get()` for clearer slice-like element retrieval
- Introduce `get_back()` method for flexible reverse indexing
- Remove redundant `second_to_last()` and `third_to_last()` methods
- Add `#[track_caller]` to index implementation for better error tracking
- Update all references to use new method names
- Improve documentation with usage examples
The changes provide a more intuitive and consistent approach to
accessing witness elements.
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.
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
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.
Enable lint `clippy::return_self_not_must_use` and add attribute
`must_use` as required.
Also run the linter with `clippy::must_use_candidate` enabled and
manually check every warning site.
While we are at it change the current `must_use` usages to have no
message. We can always add a message later if needed.
In #2646 we introduced a bug in the taproot witness stack getter
functions, of which we have three:
- `tapscript`
- `taproot_control_block`
- `taproot_annex`
Each returns `Some` if a possible bytes slice is found (with no other
guarantees).
Use `taproot_annex` combined with getters from `primitives` to implement
the other two getters. This simplifies the code and fixes the bug.
Add an additional getter to `primitives` `Witness::third_from_last`.
Fix: #3598
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.
This has been fixed and we use nightly to lint so we have access to the
merged fix.
Removing the attribute uncovers a bunch of real lint warnings, fix
them while we are at it.
During this release cycle we deprecated `to_vec` in favour of
`to_bytes`, we have since reversed our position on the name.
Remove the deprecation of `to_bytes` from the three types that had it
and use `to_vec`.
Move the `Witness` over to `primitives` leaving behind any method that
takes or returns a `Script` or a signature.
Includes addition of a feature gate to unit test.
Fmt Bot
merge-script
aagbotemi
Tobin C. Harding
Jamil Lambert, PhD
Martin Habovstiak
Erick Cestari