548725c5fb test: reject message (de)serialization (0xb10c)
fc572aba86 fix: use var_str in 'reject' msgs (0xb10c)
Pull request description:
[BIP-61 defines `response-to-msg`][bip61] (`Reject::message` in rust-bitcoin; the message that triggered the reject) to be a `var_str`. However, by using the `CommandString` it was (de)serialized as 12 byte string. A test is added that de- and serializes two reject messages received from an older Bitcoin Core peer.
Reject message sending has been removed from Bitcoin Core, I'm still receiving them from older peers from time to time.
[bip61]: https://github.com/bitcoin/bips/blob/master/bip-0061.mediawiki#common-payload
gh-ref: https://github.com/rust-bitcoin/rust-bitcoin/pull/323
ACKs for top commit:
apoelstra:
ACK 548725c5fb
Tree-SHA512: e5cbf215a471f113b4dd7f7fada162686fc6e8c7b1e2e9e641667208a36d3db610e57e8b549756ffe597656fee5444fe95466f1b88f45366595766f7c4640eea
c97589f8de Fix TapTree derserialization (sanket1729)
Pull request description:
Trees should only be serialized if both of the following conditions
hold:
1) Tree is complete binary tree(is_finalized)
2) Tree does not have any hidden nodes
ACKs for top commit:
tcharding:
ACK c97589f8de
apoelstra:
ACK c97589f8de
Tree-SHA512: 33d16f2d532cb24acba4ab847d493e550f7b279567678f3f2cd7e4161dea8b720a0e35be32b6c506e467c3526a29042aad8f4b5f45133b9a32028d4ee6a48f8e
bd36d846f2 Add temporary warning about edition change (Martin Habovstiak)
Pull request description:
This warns contributors about possible rebases. Changing CONTRIBUTING
should ensure that GitHub will display special warning. This will be
removed after migration is done.
See also https://github.com/rust-bitcoin/rust-bitcoin/discussions/945
ACKs for top commit:
sanket1729:
ACK bd36d846f2
apoelstra:
ACK bd36d846f2
Tree-SHA512: 8fd0c9a0803933c3996340ceb46f6d72ec1896936267c5c14ac1a7c48499a24babad1e096fa3a903fb4aedff6b915fd0ba92dd53f8cc3fd1fa0dbccb9bf7732b
This warns contributors about possible rebases. Changing CONTRIBUTING
should ensure that GitHub will display special warning. This will be
removed after migration is done.
This creates a few primitive functions for handling iterators and uses
them to avoid repeated code. As a result not only is the code simpler
but also fixes a forgotten bound check. Thanks to a helper function
which always does bounds check correctly this can no longer be
forgotten.
38f09d2d93 minor changelog fixes (Andrew Poelstra)
785be0e27a release 0.28.0 (Andrew Poelstra)
c9385969f9 release: do CHANGELOG for 0.28.0 (Andrew Poelstra)
Pull request description:
See discussion here https://github.com/rust-bitcoin/rust-bitcoin/discussions/941 about whether we want to cut the release now or do another slew of breaking changes between RCs.
ACKs for top commit:
sanket1729:
utACK 38f09d2d93. The diff from the previous ACKs is minor easy to review.
Tree-SHA512: e1ebdd637918337bb9d40f35fdcc3bf50fcab2bce095ed41026502af9446d8b27ddc1b87150205a79a4ac30baea96163219e772a757ab85649d503d97cdcb691
This refactors `blockdata::script::Instructions` to use
`::core::slice::Iter<'a, u8>` instead of `&'a [u8]` to better express
the intention and to avoid some slicing mistakes. Similarly to a
previous change this uses a macro to deduplicate the common logic and
the new `read_uint_iter` internal function to automatically advance the
iterator.
Addresses:
https://github.com/rust-bitcoin/rust-bitcoin/pull/662#pullrequestreview-768320603
7969b7a43e Make TaprooBuilder::finalize able to return keyspend only (Jeremy Rubin)
Pull request description:
ACKs for top commit:
JeremyRubin:
> ACK 7969b7a
sanket1729:
ACK 7969b7a43e
apoelstra:
ACK 7969b7a43e
Tree-SHA512: 26d0b730590f610a858061394faafaa74b13dd353f34ccf1c6166d0cbb62937010eed5661a887f7bea4f983ac9eab8cdca10a5fe7bd74f2dd5701a7782cbac64
3c59897598 Removed IntoIterator for TapTree implementation (Dr Maxim Orlovsky)
7a5482d23a Rename LeafInfo into ScriptLeaf (Dr Maxim Orlovsky)
2b8d96581a Rename TapTree::iter into TapTree::script_leaves (Dr Maxim Orlovsky)
6f871ba47d Add convenience LeafInfo::depth method (Dr Maxim Orlovsky)
3c502ffc2d Making all LeafInfo fields private (Dr Maxim Orlovsky)
d655ff3e93 Make TapTreeIterator use LeafInfo (Dr Maxim Orlovsky)
79345fcd02 LeafInfo field accessor methods (Dr Maxim Orlovsky)
5958466678 Make LeafInfo::leaf_hash public and change its name and return type (Dr Maxim Orlovsky)
c83893d497 Make taproot LeafInfo public (Dr Maxim Orlovsky)
Pull request description:
This PR makes existing taproot script iterator to iterate `LeafScript` values instead of constructed `(u8, &Script)`. First, this is more idiomatic (iterator should not construct value but iterate through real internal representation); second information about merkle path of the scripts is required for me downstream to implement OP_RETURN taproot commitments.
The PR also removes unnecessary iterator type, replacing it with a slice iterator type from the core rust library.
I am asking to include this PR into RC fix scope, since it is required downstream.
ACKs for top commit:
sanket1729:
ACK 3c59897598. Reviewed the range-diff with the post that I previously ACKed
Tree-SHA512: 99e341443987204a8aba20869c750bd80a725f3d49d1b5731d554dff7377181b02a4517f8b390101afb2957135dbb255c6e360f90cadd6ee07b17eb14fd30af5
In the future, TapTree may iterate over different node types, and that's why it does not have `iter()` function; using instead `script_leafs`. Thus, we should not have IntoIterator implementation as well
Previously used depth and script tuple missed information about the leaf version.
All three comprises already existing type `LeafInfo` which was made public in
previous commits.
In order to sign a utxo that does a p2wpkh spend we need to create the
script that can be used to create a sighash. In the libbitcoin docs this
is referred to as the 'script code' [0].
The script is the same as a p2pkh script but the pubkey_hash is found in
the scriptPubkey.
Add a `Script` conversion method that checks if `self` is a v0 p2wpkh
script and if so extracts the pubkey_hash and returns the required
script.
[0] https://github.com/libbitcoin/libbitcoin-system/wiki/P2WPKH-Transactions#spending-a-p2wpkh-output
Programmers are inherently lazy and for good reason. I'm yet to see
anyone write `PartiallySignedTransaction` in code that uses
`rust-bitcoin`, its too obvious to add a type alias for PSBTs, let's
just do it ourselves to save everyone else having to do so.
Add public type alias `Psbt` for `PartiallySignedTransaction`.
Trees should only be serialized if both of the following conditions
hold:
1) Tree is complete binary tree(is_finalized)
2) Tree does not have any hidden nodes
c036b0db6f Unit test for failing TapTree on builder containing hidden nodes. (Dr Maxim Orlovsky)
77715311cf Prevent TapTree from hidden parts (Dr Maxim Orlovsky)
b0f3992db1 Rename TaprootBuilder::is_complete into is_finalized (Dr Maxim Orlovsky)
efa800fb1f Make TapTree::from_inner return a proper error type (Dr Maxim Orlovsky)
e24c6e23e3 TapTree serialization roundtrip unit test (Dr Maxim Orlovsky)
56adfa4527 TaprootBuilder::has_hidden_nodes method (Dr Maxim Orlovsky)
e69701e089 Rename taproot `*_hidden` API into `*_hidden_nodes` (Dr Maxim Orlovsky)
6add0dd9dc Track information about hidden leaves in taproot NodeInfo (Dr Maxim Orlovsky)
Pull request description:
Closes#928
ACKs for top commit:
sanket1729:
ACK c036b0db6f. Reviewed the range diff
apoelstra:
ACK c036b0db6f
Tree-SHA512: 3a8193e6d6dd985da30a2094d1111471b5971f422525870003b77b6ac47cd4ad6e718d46a6d86bbb5e92e5253ac53804badf67edd98bbccbdc11e6383c675663
29843c41ef Allow deprecated function call (Tobin Harding)
Pull request description:
We have a deprecated function call because of the MSRV, tell clippy to ignore it.
ACKs for top commit:
Kixunil:
ACK 29843c41ef conditioned on adding this to MSRV bump TODO list.
sanket1729:
ACK 29843c41ef.
Tree-SHA512: 3e2bf95d05c3baff4f01c447717dde4e78d686cbc6f720591f6656ce1e5d8cf3a276a0c3dc0016dea357b61350091778d4500a59427ea9863eb5bb9344b822e4
This adds tests for the previously untested reject message
(de)serialization. The two reject messages were received from an
older Bitcoin Core peer that still sends reject messages.
46c34b3fb7 Fix code comments referring to sighash (Tobin Harding)
8f36c3979c Use sighash not sig_hash in identifiers (Tobin Harding)
c3a167b96b Rename SigHash -> Sighash (Tobin Harding)
52b711c084 Rename InvalidSigHashType -> InvalidSighashType (Tobin Harding)
b84f25584e Rename SigHashCache -> SighashCache (Tobin Harding)
e37652578b Rename PsbtSigHashType -> PsbtSighashType (Tobin Harding)
c19ec339ef Rename NonStandardSigHashType -> NonStandardSighashType (Tobin Harding)
130e27349e Rename SigHashTypeParseError -> SighashTypeParseError (Tobin Harding)
6caba2ed24 Rename SchnorrSigHashType -> SchnorrSighashType (Tobin Harding)
5522454583 Rename EcdsaSigHashType -> EcdsaSighashType (Tobin Harding)
Pull request description:
Our usage of `SigHash` implies that 'sighash' is _two_ words; 'sighash' is a well known word in the Bitcoin ecosystem it should appear in identifiers as `Sighash`.
Change various types, variants, and code comments to use sighash as a single word.
- Patches 1-8 are code changes `s/SigHash/Sighash/g`
- Patch 9 is code changes `s/sig_hash/sighash/g`
- Patch 11 is docs fixes
Fixes: #911
## Note to reviewers
I've been particularly pedantic with the patch separation because we are so close to release.
Done as separate patches to make review easier if review is to be done by reading the diffs. Perhaps at least one person could verify this PR programmatically by doing
- Reset the last 2 patches (those are easy to do manually)
- Check out master
- Do `s/SigHash/Sighash/g` on all source files (bash function below)
- Use `git diff branchA..branchB` to verify
The difference between the two branches should only include comment lines (last three patches) and these seven instances of `SigHash:
```
CHANGELOG.md:82:- [Add FromStr/Display implementation for SigHashType](a4a7035a94)
CHANGELOG.md:93:- [Introduce `SigHashCache` structure](https://github.com/rust-bitcoin/rust-bitcoin/pull/390) to replace `SighashComponents` and support all sighash modes
CHANGELOG.md:121: - `SigHash`
src/blockdata/transaction.rs:1190: "SigHash_None",
src/blockdata/transaction.rs:1191: "SigHash_NONE",
src/util/sighash.rs:1175: "SigHash_None",
src/util/sighash.rs:1176: "SigHash_NONE",
```
In case its useful, the shell function I used to do these changes is:
```bash
function search-and-replace() {
if (($# != 2))
then
echo "Usage: $0 <this> <that>"
return
fi
local this="$1"
local that="$2"
# For all files containing $this, replace $this with $that.
for file in $(git grep -l "$this")
do
perl -pi -e "s/$this/$that/g" "$file"
done
}
```
ACKs for top commit:
dr-orlovsky:
ACK 46c34b3fb7
apoelstra:
ACK 46c34b3fb7
Tree-SHA512: fe7e25e9cfb5155e4921de5ac185dbf9f4ca0770846d7892f6968b44fc5431f3f1a183380107449e90f7ea662094c60b118dc0468230384e8f9a8ef98d5ee0a0
f27c4a541d Added push_x_only_key(..) and its test. (mpls)
Pull request description:
**Issue**
I can not use [`XOnlyPublicKey`](ae985dd191/src/key.rs (L973)) in my Scripts which prevents me from working with Taproot.
**Cause**
The current version of [`script::Builder`](0a2d45de09/src/blockdata/script.rs (L121)) does not accept `XOnlyPublicKey`s.
**Solution**
So, I created a function `push_xkey(self, key: &XOnlyPublicKey)` based on the existing [`push_key`](0a2d45de09/src/blockdata/script.rs (L914)) function. I also augmented an [existing test](0a2d45de09/src/blockdata/script.rs (L1108)) in an attempt to reach testing parity with existing code.
After toying around with `push_xkey`, it seems to work on my end.
ACKs for top commit:
dr-orlovsky:
ACK f27c4a541d
sanket1729:
utACK f27c4a541d. Thanks a lot for keeping up the iterations with prompt responses
Tree-SHA512: 064958d49edc1d3636a21e428d62c2e9bcd9b13bd226c5821db9e04ce78663a11fcf601c7667b564f88e845207219a052e1c7413f50e5d27c79003e8129825ed
Andrew Poelstra
Martin Habovstiak
sanket1729
Dr Maxim Orlovsky
Tobin Harding
0xb10c
Jeremy Rubin