This is a bit ugly and requires that we put our where-clauses in
parentheses because the macro_rules parser sucks, but it allows us to
move the blanket-impls on NumOpResult into the macro.
This commit moves one instance and updates the macro; the next commits
will change the rest.
Looks like a large diff but if you run
git show --color-moved-ws=allow-indentation-change
you will see that it's 100% moves (though moves of code into the
reference macro). May be easier to just look at src/amount/result.rs
after this; it's pretty short now.
The next commit changes a lot of code, but almost entirely by moving and
indenting it. We try to do the moves here ahead of time, so it the diff
for the next commit will be just deletions and indentations.
This macro can generally handle a lot of different cases where we
implement "the same trait but on references". We introduce it here and
use it in two places. We will use it in many more, but I wanted to make
the diff small on this commit, which introduces the actual macro code
and might take a bit of reading to understand.
You may want to use --color-moved-ws=allow-indentation-change to review
this, and the next commit.
The next set of changes will mechanically delete other macros that are
made redundant by this.
The `NumOpResult` type is way more ergonomic to use if it derives
`Copy`. This restricts the `NumOpResult` to being `Copy` as well.
This does restrict what we can include in the error type in the future.
Derive Copy for `NumOpResult` and `NumOpResult`.
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
82a8aefc53 sighash: Document witness_script in p2wsh (Jose Storopoli)
Pull request description:
A note suggestion for the `p2wsh_signature_hash` in the same vein as the one in the "sister" function `p2wpkh_signature_hash`: 33f5e5a6ac/bitcoin/src/crypto/sighash.rs (L863-L867)
Any better wordings are welcomed. Not married to my sentences.
ACKs for top commit:
luisschwab:
ACK 82a8aefc53
Kixunil:
ACK 82a8aefc53
Tree-SHA512: 1ba5f9b7c287b5c1494ba9ab03c146ff99fa8be791397b61ff4609a8dbab5be5c1376d9f401b56ec4a94196eec5a96ba140965a5a8178938a2c293994e71dbfe
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)
4eb3177fab Abstract out "debug-print hex fields" using WrapDebug (Erick Cestari)
Pull request description:
This pr 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.
Closes#4074
ACKs for top commit:
Kixunil:
ACK 4eb3177fab
apoelstra:
ACK 4eb3177fab0655457a8a6d1556fd7eba54459250; successfully ran local tests
Tree-SHA512: 533ab10a8ca2be891c0f9e374bdb990cff941e332414a345ba6573d03cca19595a840d818eb24947c8b8bf88512532eed395adbc0dd3a7bb66ffb4cc641cbf21
948647a9dd update documentation for changing lockfiles; change script to support mac nanaive cp function (jeremiah)
Pull request description:
1. Updates `CONTRIBUTING.md` with latest instructions on using `just` to create new lock files. See discussion [here](https://github.com/rust-bitcoin/rust-bitcoin/pull/4045/files).
2. For the command `cp`, `--force` is not an option on the Mac. Changed to `-f` which as far as I can tell is equivalent.
```
➜ rust-bitcoin git:(jr_update_contrib) ✗ cp --force
cp: illegal option -- -
```
ACKs for top commit:
tcharding:
ACK 948647a9dd
Kixunil:
ACK 948647a9dd
apoelstra:
ACK 948647a9dd95bedbb8c34f0b9548e859bfc723b1; successfully ran local tests
Tree-SHA512: a9486ec323b15a9cfa78cd70332e153881537ec8fd26e6e23f8b61ed2272251271299723e9aa390542044588357ec072eb1559cd5e32cb707757f58361a6fb05
2349e5be73 Automated update to Github CI to rustc stable-1.85.0 (Update Stable Rustc Bot)
Pull request description:
Automated update to Github CI workflow `semver-checks.yml` by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action
ACKs for top commit:
tcharding:
ACK 2349e5be73
Tree-SHA512: 49d78997ea252b1a98844fe6ab7a98b798e464c6a99f7346e13611c567c7bb8abea618de00dd92fb945e84dc01d8a44d10d659ca3ef175460a534288e35e6b0e
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.
e41653d188 add chacha20 benchmarks (jeremiah)
Pull request description:
Add benchmarks for chacha20. These could be useful to understand future improvements.
I copied the same 10, 1k and 64k sizes as used in the sha256 benchmark. Some of the lines changed here, such as in `Config.toml`, are a little mysterious to me as I am new to rust, so please check.
```
➜ chacha20_poly1305 git:(jr_chachabench) RUSTFLAGS='--cfg=bench' cargo +nightly bench
...
test benches::chacha20_10 ... bench: 188.67 ns/iter (+/- 2.70) = 53 MB/s
test benches::chacha20_1k ... bench: 2,885.83 ns/iter (+/- 90.06) = 354 MB/s
test benches::chacha20_64k ... bench: 186,493.88 ns/iter (+/- 5,074.53) = 351 MB/s
```
ACKs for top commit:
apoelstra:
ACK e41653d18838e0d6e157db475b7b034b2456c924; successfully ran local tests; nice!
Kixunil:
ACK e41653d188
Tree-SHA512: fa37a4b6842ab78edc32726b22410d9769c1f03e1bc3488517e1e59e17388cabb63e58eb6ea6f112cb27ff1015296f356275ddee12d261c848050e98e3c6eed6
f61e93ccf1 Properly deprecate Hash::from_slice (Tobin C. Harding)
50c0af7138 Stop using Hash::from_slice (Tobin C. Harding)
Pull request description:
The `hashes::error::FromSliceError` error is only returned from `from_slice`. We attempted to deprecate this function but it seems we only did half a job at it.
- deprecate _all_ instances of the method/function
- deprecate the error type
- stop using the deprecated functions in `bitcoin`
Close: #4053
ACKs for top commit:
apoelstra:
ACK f61e93ccf1db7e7e3c9604fdb09b4e25195d88b2; successfully ran local tests
Tree-SHA512: 61a0e5127019859776ffac66bd4d320c86b8462bb1e908127d0bf42896aaa8df85fd2b06850342b694ca1cd68ed50355c81cad6ae3e9a5fd6e3933efe85498ad
cdece244d5 Update to latest maintainer-tools (Tobin C. Harding)
Pull request description:
We have made a couple of improvements to the `run_task` script lately (*cough* actually test code with `--all-features`).
Lets update to use it.
This PR explicitly tests https://github.com/rust-bitcoin/rust-bitcoin-maintainer-tools/pull/20 because it uses the commit hash created in that PR.
ACKs for top commit:
apoelstra:
ACK cdece244d5b660c658461e58093090d982e4835e; successfully ran local tests; will one-ack merge this since it is trivial
Tree-SHA512: 97597d01beb3b0a87fb0c279ba0e57093f4898a38e7373040cfa18b16c62f9cbceb41ac53429a2e05f70f2ea9fdd7d409e9925131575b3f66d00c5395ba6bfc1
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
da8b85ed7c Implement Debug for Hkdf (Tobin C. Harding)
85652359e8 hashes: Derive Copy and Clone for Hkdf (Tobin C. Harding)
Pull request description:
Currently the `Hkdf` type does not derive any traits.
Derive `Copy` and `Clone` and implement `Debug` based on secret obfuscation algo in `rust-secp` (in the `secret` module).
ACKs for top commit:
apoelstra:
ACK da8b85ed7cf34c0510c0b64c67477d3819bee369; successfully ran local tests
Kixunil:
ACK da8b85ed7c
Tree-SHA512: 8ae0e8857ea0e32ad5ef8f544979eeb9d530beb1b6f046ce28a286ca2231f8f696a9f4f8d9ea219d3389c4216d6b69766dbd96edbb27e7489803ac583bf3b200
The `hashes::error::FromSliceError` error is only returned from
`from_slice`. We attempted to deprecate this function but it seems we
only did half a job at it.
- deprecate _all_ instances of the method/function
- deprecate the error type
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
914730d7f1 Add inline to small functions in result module (Jamil Lambert, PhD)
Pull request description:
Close#4064
ACKs for top commit:
Kixunil:
ACK 914730d7f1
apoelstra:
ACK 914730d7f1cb5e032cce155440bbe4f064a09533; successfully ran local tests
Tree-SHA512: eb6d743df462462a048686f803d687756b230fdbd6b20c3bccd210f839a37dae1a390ff732efd0686f1b1df36d771b8147c5da87010025af3b1964b5b2c65751
8c2439550a hashes: Test macros in function scope (Tobin C. Harding)
Pull request description:
The two main public macros can be used in function scope - prove it.
While we are at it prove that additional attributes are supported by them both as well as visability keywords.
ACKs for top commit:
apoelstra:
ACK 8c2439550acdf3cb52c78d69eb55160f9d03a139; successfully ran local tests
Kixunil:
ACK 8c2439550a
Tree-SHA512: 66d6a9fd142966f6e68e9a5cb849345a77357dc6e415a10507418425a4cf6a1440deaf47515db43b9ec5b5d337d53164c617db165cb2ff782cf6a6e4ff195c77
Andrew Poelstra
Tobin C. Harding
Liu-Cheng Xu
Jose Storopoli
jrakibi
Fmt Bot
Martin Habovstiak
Erick Cestari
jeremiah
Jamil Lambert, PhD