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`.
We release `v0.32.4` and `v0.32.5` already but forgot to merge the
changelog entries back into master.
Grab the missing changelog entries from the `0.32.x` release branch.
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
We already have `tapscript` method on `Witness` which is broken because
it doesn't check that the leaf script is a tapscript, however that
behavior might have been intended by some consumers who want to inspect
the script independent of the version. To resolve the confusion, we're
going to add a new method that returns both the leaf script and, to
avoid forgetting version check, also the leaf version.
This doesn't touch the `tapscript` method yet to make backporting of
this commit easier. It's also worth noting that leaf script is often
used together with version. To make passing them around easier it'd be
helpful to use a separate type. Thus this also adds a public POD type
containing the script and the version. In anticipation of if being
usable in different APIs it's also generic over the script type.
Similarly to the `tapscript` method, this also only adds the type and
doesn't change other functions to use it yet. Only the newly added
`taproot_leaf_script` method uses it now.
This is a part of #4073
The previous commit fixed a bug when `taproot_control_block` returned
`Some` on key-spends. This adds a test case for it which succeeds when
applied after the previous commit and fails if applied before it.
The `taproot_control_block` did not properly detect whether it deals
with script spend or key spend. As a result, if key spend with annex was
used it'd return the first element (the signature) as if it was a
control block.
Further, the conditions identifying which kind of spend it was were
repeated multiple times but behaved subtly differently making only
`taproot_control_block` buggy but the other places confusing.
To resolve these issues this change adds a `P2TrSpend` enum that
represents a parsed witness and has a single method doing all the
parsing. The other methods can then be trivially implemented by matching
on that type. This way only one place needs to be verified and the
parsing code is more readable since it uses one big `match` to handle
all possibilities.
The downside of this is a potential perf impact if the parsing code
doesn't get inlined since the common parsing code has to shuffle around
data that the caller is not intersted in. I don't think this will be a
problem but if it will I suppose it will be solvable (e.g. by using
`#[inline(always)]`).
The enum also looks somewhat nice and perhaps downstream consumers could
make use of it. This change does not expose it yet but is written such
that after exposing it the API would be (mostly) idiomatic.
Closes#4097
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
Andrew Poelstra
Tobin C. Harding
costcould
leopardracer
Liu-Cheng Xu
Martin Habovstiak
Jose Storopoli
jrakibi
Fmt Bot
Erick Cestari
jeremiah