We only simulate a single connection in the test function `serve_tcp`.
Remove the unused loop (includes an unconditional break after first
iteration) and use `next` directly.
Found by clippy. Refactor only, no logic changes.
Clippy emits:
warning: digits grouped inconsistently by underscores
Add allow directive for grouping that aims to make explicit 100,000,000
sats/per bitcoin.
28049ce2d9 Document `Txid` being displayed backwards (Dawid Ciężarkiewicz)
Pull request description:
Fix#958
I hope putting it on the most notorious type where people actually notice it is enough. I couldn't find a good way to put it on all other `sha256d` automatically, and copy pasting it seems not worth it.
ACKs for top commit:
tcharding:
ACK 28049ce2d9
apoelstra:
ACK 28049ce2d9
Tree-SHA512: a5acf5d7a73361a6c48b45ed264fafb911930ae9f1bdb03895dc39c679d508dc56dbf44896fd38cf6569abb652e7fce721028ef06344462747a77078ef5a8f4f
99aab446c3 Remove network::Error (Tobin C. Harding)
Pull request description:
The `network::Error` is not used, remove it.
(This description has been changed, the thumbs up emojis were put on the previous PR description.)
ACKs for top commit:
sanket1729:
reACK 99aab446c3
apoelstra:
ACK 99aab446c3
Tree-SHA512: 2342531160966860b7b65f8c5df10e169876ec446e6fd30093d5d81d0b0304cad04e2c2057eb3ca6b23a2fc56453c91ad4ddf426d3796fb301acb7f7d03a66b9
43b684bbe6 Add non_exhaustive compiler directive to AddressType (Tobin C. Harding)
Pull request description:
Add non_exhaustive compiler directive to AddressType
Currently adding variants to enums is a breaking change. In an effort to
reduce the upgrade burden on users we can use the `non_exhaustive`
compiler directive so that adding a new variant does not cause
downstream code to break.
Add `non_exhaustive` to the `AddressType` since it may be extended in
the future.
ACKs for top commit:
sanket1729:
ACK 43b684bbe6
Kixunil:
ACK 43b684bbe6
apoelstra:
ACK 43b684bbe6
Tree-SHA512: 2b2a15fb501d23058acca94318776ffcccedf463d43d07afa290fba46a7bd58b3a730f6e1f25605ef399afcfdb5de4c7ad67eaa0adff0ba39b0096cbcec10f57
Currently adding variants to enums is a breaking change. In an effort to
reduce the upgrade burden on users we can use the `non_exhaustive`
compiler directive so that adding a new variant does not cause
downstream code to break.
Add `non_exhaustive` to the `AddressType` since it may be extended in
the future.
57dd6739c3 Do not print error when displaying for std builds (Tobin C. Harding)
b80cfeed85 Bind to error_kind instead of e (Tobin C. Harding)
241ec72497 Bind to b instead of e (Tobin C. Harding)
01f481bf5c Bind to s instead of e (Tobin C. Harding)
5c6d369289 network: Remove unused error variants (Tobin C. Harding)
e67e97bb37 Put From impl below std::error::Error impl (Tobin C. Harding)
6ca98e5275 Remove error TODO (Tobin C. Harding)
Pull request description:
As part of the ongoing error improvement work and as a direct result of [this comment](https://github.com/rust-bitcoin/rust-bitcoin/pull/987#issuecomment-1135563287) improve the `Display` implementations of all our error types so as to not repeat the source error when printing.
The first 5 patches are trivial clean ups around the errors. Patch 6 is the real work.
EDIT: ~CC @Kixunil, have I got the right idea here bro?~ Patch 6 now includes a macro as suggested.
ACKs for top commit:
Kixunil:
ACK 57dd6739c3
apoelstra:
ACK 57dd6739c3
sanket1729:
ACK 57dd6739c3. Did not check if we covered all cases. We need to remember to use `write_err!` instead of `write!` in future.
Tree-SHA512: 1ed26b0cc5f9a0f71684c431cbb9f94404c116c9136be696434c56a2f56fd93cb5406b0955edbd0dc6f8612e77345c93fa70a70650118968cc58e680333a41de
1875c912c3 Extend docstring for more types (Dawid Ciężarkiewicz)
325ea8fb7d Add "Relevant BIPs` to `Address` (Dawid Ciężarkiewicz)
7c2ca3d20b Add `BlockHeader` Bitcoin Core reference link (Dawid Ciężarkiewicz)
f4922f6fe7 Update `BlockHeader::version` documentation (Dawid Ciężarkiewicz)
Pull request description:
This is meant to make it more educational, and handy even for experienced developers.
A first step to make https://docs.rs/bitcoin (or `cargo doc --open`) a go-to place for
convenient Bitcoin documentation.
ACKs for top commit:
tcharding:
tACK 1875c912c3
apoelstra:
ACK 1875c912c3
sanket1729:
utACK 1875c912c3. Thanks for doing this.
Tree-SHA512: 8457e120f9979bfd95e55e8b18faf6131610aa2241f8e5fc4630fe61dc7e16ddfc35fb6eff46339804016db7b176465943cc0c02d84dcf478ed55da9f5e06fc5
2e7effc604 Feature `use-serde` renamed to `serde` (Martin Habovstiak)
Pull request description:
Features activating external crates are supposed to have same name as
those crates. However we depend on same feature in other crates so we
need a separate feature. After MSRV bump it is possible to rename the
crates and features so we can now fix this inconsistency.
Sadly, derive can't see that the crate was renamed so all derives must
be told to use the other one.
Replaces #373
ACKs for top commit:
apoelstra:
ACK 2e7effc604
Tree-SHA512: b20364b9e8f30c2269bef915e821b2b2ec929e71dd0e88af2bc3a021821f87011d35e095cb8efe99add77a23dde940a17537eb387fb4582b05c57c8679969eb0
8e29f2b493 Add ChainHash type (Tobin Harding)
cd8f511fcb blockdata: constants: Use wildcard import in unit tests (Tobin Harding)
71bf19621a Use fully qualified path in macro (Tobin Harding)
Pull request description:
The Lightning network defines a type called 'chain hash' that is used to uniquely represent the various Bitcoin networks as a 32 byte hash value. Chain hash is now being used by the DLC folks, as such it is useful to have it implemented in rust-bitcoin.
One method of calculating a chain hash is by hashing the genesis block for the respective network.
Add a `ChainHash` type that can be used to get the unique identifier of each of the 4 Bitcoin networks we support. Add a method that calculates the chain hash for a network using the double sha256 of the genesis block. Do so using hard coded consts and add unit tests (regression/sanity) that show these hard coded byte arrays match the hash of the data we return for the genesis block for the respective network.
The chain hash for the main Bitcoin network can be verified from LN docs (BOLT 0), add a link to this document.
Closes: #481
ACKs for top commit:
Kixunil:
ACK 8e29f2b493
sanket1729:
ACK 8e29f2b493.
Tree-SHA512: 8156bb55838b73694ddf77a606cbe403f53a31d363aa0dee11b97dc31aa9b62609d7d84b8f0f92c08e90372a3e8c7b416fb07989d6da9633763373b41339b1f5
6c10d77ecb Address::from_script() - Check witness v0 program lengths. (Noah)
Pull request description:
Adds a check in `Address::from_script()` that checks if segwit v0 scripts have a valid length.
Fix: #995
ACKs for top commit:
tcharding:
ACK 6c10d77ecb
sanket1729:
ACK 6c10d77ecb. Left a comment can be addressed in separate PR.
apoelstra:
ACK 6c10d77ecb
Tree-SHA512: 32aebb13477958b1455c688f668aaa3d3af4db0a7936b9549bcd1d03bd0e16635b8471549d96f1e8d408d6501e8fb515df2eb86b17a08c3152774a5be78ae8b1
99f565f932 Add non_exhaustive to all error enums (Tobin C. Harding)
Pull request description:
Adding an error variant to a public enum is an API breaking change, this means making, what could be, small refactorings or improvements harder. If we use `non_exhaustive` for error types then we mitigate this cost.
There is a tradeoff however, downstream users who explicitly match on our public error types must include a wildcard pattern.
ACKs for top commit:
apoelstra:
ACK 99f565f932
Kixunil:
ACK 99f565f932
Tree-SHA512: ff329f87d52b3fbe24654f32e4062ddae73173cba5a13d511591158e68ee278e9bdc0a70a3e0b42d6606b369255923f9c46d8b3d1b2ff75f8461a82567df80cd
5fbb211085 Use fn name to_ instead of as_ (Tobin Harding)
8ffa32315d Use fn name to_ instead of into_ (Tobin Harding)
6874ce91e2 Remove as_inner (Tobin C. Harding)
Pull request description:
Rust has naming conventions surrounding conversion functions
We have a handful of methods that are not following convention. This PR is done as three patches, separated by incorrect function name (`into_` or `as_`) and by whether or not the original method needs deprecating. Can be squashed if folks prefer.
From the docs: https://rust-lang.github.io/api-guidelines/naming.html
<h2><a class="header" href="https://rust-lang.github.io/api-guidelines/naming.html#ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv" id="ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv">Ad-hoc conversions follow <code>as_</code>, <code>to_</code>, <code>into_</code> conventions (C-CONV)</a></h2>
<p>Conversions should be provided as methods, with names prefixed as follows:</p>
Prefix | Cost | Ownership
-- | -- | --
as_ | Free | borrowed -> borrowed
to_ | Expensive | borrowed -> borrowed
| | | borrowed -> owned (non-Copy types)
| | | owned -> owned (Copy types)
into_ | Variable | owned -> owned (non-Copy types)
EDIT: I did actually audit all uses of `to_` when I first did this, I did this by grepping for `fn to_` and checking the output against the table.
ACKs for top commit:
apoelstra:
ACK 5fbb211085
Kixunil:
ACK 5fbb211085
Tree-SHA512: f750b2d1a10bc1d4bb030d8528a582701cc3d615aa8a8ab391324dae639544bb3629a19b372784e1e274a8ddcc613c621c7aae21a3ea54fde356a6aa5e611ac0
a6efe982bd Use write_all to write whole buffer (Tobin C. Harding)
51c60b8507 Allow no is_empty method for VarInt (Tobin C. Harding)
841f1f5832 Implement Default for TaprootBuilder (Tobin C. Harding)
f81d4aa9bd Remove unnecessary call to clone (Tobin C. Harding)
27649ba182 Use copied instead of map to copy (Tobin C. Harding)
62ccc9102c Use iter().flatten().any() instead of if let Some (Tobin C. Harding)
4b28a1bb97 Remove unneeded return statement (Tobin C. Harding)
16cac3cd70 Derive Default for Witness (Tobin C. Harding)
c75189841a Remove unnecessary closure (Tobin C. Harding)
dfff85352a Ignore bytes written for sighash_single bug output (Tobin C. Harding)
14c72e755b Use contains combinator instead of manual range (Tobin C. Harding)
b7d6c3e02c Remove additional reference (Tobin C. Harding)
1940b00132 Implement From instead of Into (Tobin C. Harding)
fcd0f4deac Use struct field init shorthand (Tobin C. Harding)
641960f037 Use rustfmt::skip (Tobin C. Harding)
3cd00e5d47 Remove unnecessary whitespace (Tobin C. Harding)
Pull request description:
Clear all current Clippy warnings, codebase wide. Possibly contentious patches include:
- [commit](fcd0f4deac): `fcd0f4d Use struct field init shorthand`
- [commit](14c72e755b): `14c72e7 Use contains combinator instead of manual range`
- [commit](3b3c37803a): `3b3c378 Use iter().flatten() instead of if let Some`
## Notes
Please note commit `dfff8535 Ignore bytes written for sighash_single bug output` touches the same lines of code as commit `a6efe982 Use write_all to write whole buffer`.
ACKs for top commit:
apoelstra:
ACK a6efe982bd
Kixunil:
ACK a6efe982bd
Tree-SHA512: 5351a82fd3deadb8e53911c43b5a60a9517d5c57014f5fa833b79b32c0a4606ada0bcd28e06ce35d47aa74115c7cf70c27a1ba9c561a3424ac85a4f69774014d
Adding an error variant to a public enum is an API breaking change, this
means making what could be small refactorings or improvements harder. If
we use `non_exhaustive` for error types then we mitigate this cost.
There is a tradeoff however, downstream users who explicitly match on
our public error types must include a wildcard pattern.
As things are right now, memory exhaustion protection in `Decodable`
is based on checking input-decoded lengths against arbitrary limits,
and ad-hoc wrapping collection deserialization in `Take`.
The problem with that are two-fold:
* Potential consensus bugs due to incorrect limits.
* Performance degradation when decoding nested structured,
due to recursive `Take<Take<..>>` readers.
This change introduces a systematic approach to the problem.
A concept of a "size-limited-reader" is introduced to rely on
the input data to finish at enforced limit and fail deserialization.
Memory exhaustion protection is now achived by capping allocations
to reasonable values, yet allowing the underlying collections
to grow to accomodate rare yet legitmately oversized data (with tiny
performance cost), and reliance on input data size limit.
A set of simple rules allow avoiding recursive `Take` wrappers.
Fix#997
A better way to write a byte string is to use write all so that
`ErrorKind::Interupted` is not returned.
Use `write_all` to write the non-sense (error indication) string to the
writer when we hit the SIGHASH_SINGLE bug.
Rust convention is to use `to_` for conversion methods that convert from
an owned type to an owned `Copy` type. `as_` is for borrowed to borrowed
types.
Re-name and deprecate conversion methods that use `as_` for owned to
owned `Copy` types to use `to_`.
Rust convention is to use `to_` for conversion methods that convert from
an owned type to an owned `Copy` type. `into_` is for owned to owned
non-`Copy` types.
Re-name and deprecate conversion methods that use `into_` for `Copy`
types to use `to_`.
`self` and the referenced type returned by `as_inner` are both `Copy`
types. There is no need to provide an reference getter method to a
`Copy` type since implementing `Copy` implies that copying is cheap.
We implement `source` for all our error types. This means that we should
not display the source error explicitly because users can call `source`
to get the source error.
However, `std::Error::source()` is only available for "std" builds, so
that we do not loose the error source information in "no-std" builds add
a macro that conditionally adds the source onto the error message.
Features activating external crates are supposed to have same name as
those crates. However we depend on same feature in other crates so we
need a separate feature. After MSRV bump it is possible to rename the
crates and features so we can now fix this inconsistency.
Sadly, derive can't see that the crate was renamed so all derives must
be told to use the other one.
Currently we allow multiple trailing colons when matching within the
`check_format_non_negative` macro. We can be more restrictive with no
loss of usability.
Use `$(;)?` instead of `$(;)*` to match against 0 or 1 semi-colons
instead of 0 or more.
Clippy emits:
warning: struct `VarInt` has a public `len` method, but no `is_empty`
method
However, `VarInt` has no concept of 'is empty' so add a compiler
directive to allow the lint.
Clippy emits:
warning: you should consider adding a `Default` implementation for
`TaprootBuilder`
As suggested, implement `Default` or `TaprootBuilder`.
Clippy emits:
warning: you are using an explicit closure for copying elements
In one instance we have `map` followed by `flatten`, this can be
replaced by the `flat_map` combinator.
As suggested use `copied` combinator.
Clippy emits:
warning: unnecessary `if let` since only the `Some` variant of the
iterator element is used
Use combinator chain `iter().flatten().any()` to check for an node with
hidden nodes.
Clippy emits:
error: written amount is not handled
This code is explicitly writing garbage to the writer, no need to handle
the number of bytes written.
Clippy emits:
warning: this expression creates a reference which is immediately
dereferenced by the compiler
As suggested, remove the additional reference.
07c75304d2 Refactor address byte swapping (Tobin C. Harding)
Pull request description:
Refactor address byte swapping
When encoding a `network::Address` two of the fields are encoded
big-endian instead of little-endian as is done by `consensus_encode`. In
order to achieve this we have a helper function `addr_to_be` that swaps
the bytes. This function is miss-named because it is not converting to a
specific endian-ness (which implies different behaviour on machines with
different endian-ness) but is reversing the byte order irrespective of
the underlying architecture.
- Remove function `addr_to_be`
- Inline the endian-ness code when encoding an address
- Remove TODO and use `to_be_bytes` when encoding port
- Add a function for reading big-endian bytes `read_be_address`
- Use `read_be_address` when decoding `Address` and `Addrv2`
Refactor only, no logic changes. Code path is already covered by
unit tests.
ACKs for top commit:
apoelstra:
ACK 07c75304d2
Kixunil:
ACK 07c75304d2
Tree-SHA512: 186bc86512e264a7b306f3bc2e18d1619f3cd84fc54412148cfc2663e8d6e9616ea9e2fe19eafec72d76cc11367a9b39cac2b73210d9e43eb8f453bd253b33de
97a5bb1439 Implement std::error::source codebase wide (Tobin C. Harding)
0a9191b429 Add parenthesis around left hand side of companion (Tobin C. Harding)
7cf8af2f86 Put Error impl block below Display (Tobin C. Harding)
2384712364 Re-order Display match arms (Tobin C. Harding)
Pull request description:
Now that we have MSRV of 1.41.1 we should use `source` instead of `cause`. Audit the whole codebase and implement `source` for _every_ error type we have.
The first three patches are preparatory cleanup, patch 3 is particularly shameful (adds parenthesis to make my editor work).
CC @Kixunil because he is championing the error stuff.
ACKs for top commit:
apoelstra:
ACK 97a5bb1439
Tree-SHA512: 46313a28929445f32e01e30ca3b0246b30bc9d5e43db5754d4b441e9c30d3e427efaf247100eb6b452f98beec5a4fcde1daba7943a772114aa34f78ab52cbc60
9896f27eae psbt: Improve documentation (Tobin C. Harding)
33a50831ce sighash: Improve documentation (Tobin Harding)
Pull request description:
Done while working on sighash and PSBT signing. Just the usual docs fixes. Note, does not do the whole `psbt` module just the file mentioned.
ACKs for top commit:
apoelstra:
ACK 9896f27eae
Tree-SHA512: 5fbfa258cdb216189922a49a42b7ab9fb78faeee72d82f8cb99a1b3d930d170074013e317b0e7af259a404ac4db93841b4d2b525e933c5e145da71e7522800fd
58f94bee9b Remove sha256t_hash_newtype macro (Tobin C. Harding)
Pull request description:
Since commit `commit 275adc6c335a4326699cfbd444949e1725864ea1` on `bitcoin_hashes` we have the identical implementation of the macro `sha256t1_hash_newtype` in this crate and in `bitcoin_hashes`.
Remove the `sha256t_hash_newtype` macro from this crate in favour of the one in `bitcoin_hashes`.
ACKs for top commit:
apoelstra:
ACK 58f94bee9b
sanket1729:
ACK 58f94bee9b
Tree-SHA512: ec08fd25c1cca71a07ea61cb5838ce8962daae7cbb84d8beccc3d0d285439909721edd643292a8f3f6989e1c2c41fda9addfd5cdb063ef53ebc6ef646da79cf3
90b4f1cde8 Clear TapTreeIter clippy warning (Tobin C. Harding)
e6084a1af8 Improve documentation around EcdsaSig (Tobin Harding)
Pull request description:
Do a couple of trivial docs fixes, done during other work.
- Patch 1 improves docs on the `EcdsaSig` struct
- Patch 2 clears a clippy warning during docs build - no sure if the solution is the best available though
ACKs for top commit:
apoelstra:
re-ACK 90b4f1cde8
sanket1729:
ACK 90b4f1cde8
Tree-SHA512: 0647dc2e6550938ccca658a9dddffba7175d5c4eb8cec0e165d3a7fa8f2b1dfb902e795aca77d96a6c31092baf64244fa1d7151a304134d3b1895619a2823338
7ca30b6aa8 Move Address::payload_as_bytes to Payload::as_bytes (Fredrik Meringdal)
525ea00e0f Make Address::get_payload_bytes public (Fredrik Meringdal)
Pull request description:
Hi, thanks for the amazing work on this crate.
I am trying to upgrade from v0.27 to v0.28, but unable to do so because the `Address::get_payload_bytes` was made private. My use-case is that I have a script hash address and an `Address` and need to compare the two, and in order to do so I need access to the payload bytes of `Address`.
I hope you will consider making this function public again 🙏
ACKs for top commit:
apoelstra:
ACK 7ca30b6
tcharding:
ACK 7ca30b6aa8
sanket1729:
ACK 7ca30b6aa8. Sorry for the delay and congratz on your first time contribution
Tree-SHA512: 02af4565853d93506751ed7cb004f52cb5d8c7936067e06b3e237b448ccdf5716470448eeccbe211958e095b66bb37c7027800c0470c6988dc18d8bd5b48f459
Parenthesis are not needed around this expression but my editor is going
mad and cannot format the code without them. Since it does not hurt
readability add parenthesis around the expression.
When encoding a `network::Address` two of the fields are encoded
big-endian instead of little-endian as is done by `consensus_encode`. In
order to achieve this we have a helper function `addr_to_be` that swaps
the bytes. This function is miss-named because it is not converting to a
specific endian-ness (which implies different behaviour on machines with
different endian-ness) but is reversing the byte order irrespective of
the underlying architecture.
- Remove function `addr_to_be`
- Inline the endian-ness code when encoding an address
- Remove TODO and use `to_be_bytes` when encoding port
- Add a function for reading big-endian bytes `read_be_address`
- Use `read_be_address` when decoding `Address` and `Addrv2`
Refactor only, no logic changes. Code path is already covered by
unit tests.
The Lightning network defines a type called 'chain hash' that is used to
uniquely represent the various Bitcoin networks as a 32 byte hash value.
Chain hash is now being used by the DLC folks, as such it is useful to
have it implemented in rust-bitcoin.
One method of calculating a chain hash is by hashing the genesis block
for the respective network.
Add a `ChainHash` type that can be used to get the unique identifier of
each of the 4 Bitcoin networks we support. Add a method that returns
the chain hash for a network using the double sha256 of the genesis
block. Do so using hard coded consts and add unit
tests (regression/sanity) that show these hard code byte arrays match
the hash of the data we return for the genesis block for the respective
network.
The chain hash for the main Bitcoin network can be verified from LN
docs (BOLT 0), add a link to this document.
Since commit `commit 275adc6c335a4326699cfbd444949e1725864ea1` on
`bitcoin_hashes` we have the identical implementation of the macro
`sha256t1_hash_newtype` in this crate and in `bitcoin_hashes`.
Remove the `sha256t_hash_newtype` macro from this crate in favour of the
one in `bitcoin_hashes`.
Clippy emits warning:
public documentation for `script_leaves` links to private item `TapTreeIter`
I'm not exactly sure why this is but adding the generic type place
holder clears the warning.
Improve documentation in `psbt/mod.rs` by doing:
- Use full sentences (full stops and capitalisation)
- Use 100 line column width
- Use back ticks and links as appropriate
- Use `Errors` section
- Use third person tense to describe functions
Improve the rustdoc documentation in the `sighash` module by doing:
- Improve grammar
- Use full sentences (full stops and capitalisation)
- Use 100 line column width
- Use back ticks and links as appropriate
- Improve correctness of `SigHashCache::new` function
Use cargo to upgrade from edition 2015 to edition 2018.
cargo fix --edition
No manual changes made. The result of the command above is just to fix
all the use statements (add `crate::`) and fix the fully qualified path
formats i.e., `::Foo` -> `crate::Foo`.
We do this all over the place in rust-lightning, and its probably
the most common thing to do with a `Witness` so I figured I'd
upstream the util method to do this. It also avoids an allocation
compared to the naive approach of `SerializedSignature.to_vec()`
with two pushes, which is nice.
4f1200d629 Added `amount::Display` - configurable formatting (Martin Habovstiak)
Pull request description:
This significatnly refactors the formatting code to make formatting more
configurable. The main addition is the `Display` type which is a
builder that can configure denomination or other things (possibly more
in the future).
Further, this makes all representations of numbers minimal by default,
so should be documented as a possibly-breaking change.
Because of the effort to support all other `fmt::Formatter` options this
required practically complete rewrite of `fmt_satoshi_in`. As a
byproduct I took the opportunity of removing one allocation from there.
Closes#709
ACKs for top commit:
tcharding:
ACK 4f1200d629
dr-orlovsky:
ACK 4f1200d629
sanket1729:
ACK 4f1200d629
Tree-SHA512: 3fafdf63fd720fd4514e026e9d323ac45dfcd3d3a53a4943178b1e84e4cf7603cb6235ecd3989d46c4ae29453c4b0bb2f2a5996fbddf341cd3f68dc286062144
2c28d3b448 Fix handling of empty slice in Instructions (Martin Habovštiak)
e6ff754b73 Fix doc of take_slice_or_kill (Martin Habovštiak)
0ec6d96a7b Cleanup after `Instructions` refactoring (Martin Habovstiak)
bc763259fe Move repeated code to functions in script (Martin Habovstiak)
1f55edf718 Use iterator in `blockdata::script::Instructions` (Martin Habovstiak)
Pull request description:
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
ACKs for top commit:
tcharding:
ACK 2c28d3b448
sanket1729:
ACK 2c28d3b448. I don't want to hold ACKs on minor things as they can be in a fixup later.
Tree-SHA512: 9dc770b9f7958efbd0df2cc2d3546e23deca5df2f94ea2c42b089df628f4b99f08032ca4aa8822caf6643a8892903e1bda41228b78c8519b90bcaa1255d9acc6
76fcf81474 Override default visit_byte_buf on Script (ass3rt)
add100c20d Removed reimplementations of default methods (ass3rt)
7db03f27e4 Disable Serde's default-features (ass3rt)
Pull request description:
With this patch, existing users of the `use-serde` feature will no longer be
compiling with `serde/std` enabled, but this allows dependent projects
to import serde and enable `serde/alloc` as required by some no-std targets.
ACKs for top commit:
Kixunil:
ACK 76fcf81474
tcharding:
ACK 76fcf81474
apoelstra:
ACK 76fcf81474
Tree-SHA512: 5748e64e1f91f19dbfbf32bead6e6d759e448e92ed0dab731b3059f6b37bd811fad6654edc8fbd113e3be17fefaf9fc4912145d6b61484ced0517712361ecfdc
7854bd7918 Fix `no_std` MSRV Fixes#690, #947 (mcroad)
Pull request description:
`rust-bitcoin` does not work with rust 1.29 under a `no_std` environment. This could be considered a bug. However, `no_std` support is a recent addition and this is likely not breaking anyone's builds.
A decision needs to be made, either `no_std` MSRV is the current stable version while keeping the `std` MSRV as 1.29, or it needs to be fixed.
This pr adds `no_std` to the 1.29 test suite.
This came as I try to get rust-bitcoin/rust-miniscript#277 working and got stuck on the issue of testing `no_std` under 1.29.
ACKs for top commit:
Kixunil:
ACK 7854bd7918
tcharding:
ACK 7854bd7918
sanket1729:
ACK 7854bd7918
apoelstra:
ACK 7854bd7918
Tree-SHA512: 1614fb2193f760ed340592bdb94d076066f6f783bc1dc2b145d97f7151a28316e56b1975f1ad948460eb26db04e7e9382e60076686a681e46dcf33521fda5fca
831b0267de Use contains() instead of manual range (Tobin C. Harding)
6410095687 Use chunks_exact (Tobin C. Harding)
3a0097ba49 Use trim_start_matches (Tobin C. Harding)
0a19710906 Use vec! macro instead of new followed by push (Tobin C. Harding)
Pull request description:
Now that 0.28 is out we do not need to support Rust 1.29 on `master`.
Remove trivial MSRV `TODO`s from the code. (All these changes only rely on MSRV bumping to 1.31 so are easily within bounds.)
ACKs for top commit:
Kixunil:
ACK 831b0267de
sanket1729:
ACK 831b0267de
Tree-SHA512: f1ea594216ba7dfa24696b964ce296a8aea72dd2e16e11d3a43fe8b90c851abf59b1612b2b1311146e8070112f3834762584e4f0515b8f546f72af169eb4bda9
5afb0eaf40 API to get an iterator for funding utxos in psbt (violet360)
Pull request description:
### Current status
The API returns a vector of UTXOs and has return type `Result<Vec<&TxOut>, Error>`
### Expected
The return statement should be of type `sighash::Prevouts` as pointed in #849
ACKs for top commit:
Kixunil:
ACK 5afb0eaf40
tcharding:
ACK 5afb0eaf40
sanket1729:
ACK 5afb0eaf40. Thanks for being patient with this.
Tree-SHA512: 724fc3dffdbb1331584f89bbe84527e1af0d193a344fe43b36f2f2a628652d259001a3abf6b3909df53524cd3fbdbe3af760b7004d40d3bee1848fbb83efff5b
31571cafbd util::amount: Make from_sat constructor constant (Steven Roose)
Pull request description:
Currently unmergable because of MSRV but I heard talk about bumping it, so once it's bumped, this is a very much needed change :)
ACKs for top commit:
tcharding:
ACK 31571cafbd
apoelstra:
ACK 31571cafbd
Tree-SHA512: f254eb10a4349d890e29ea5fae77536429c7e731362cf2edcf2fe98ec9cbf2d8bbf65f98dfc8f0b80bac89960de688005d066a116d6cb62cca1efa9c1151f2ae
7307363c2e Use qualified path instead of alias (Tobin C. Harding)
80e0fb7673 Remove unnecessary 'as' statement (Tobin C. Harding)
21e1b9dbbd Use secp256k1 qualified path instead of underscore (Tobin C. Harding)
Pull request description:
Three trivial clean ups of import aliases.
ACKs for top commit:
apoelstra:
ACK 7307363c2e
sanket1729:
ACK 7307363c2e. These are clean improvements
Kixunil:
ACK 7307363c2e
Tree-SHA512: f6ed3ede11d2803dbcb4584f11632fc47d28e525b5bf4de7794d400117f2d7c9ffce5bdff274877a63a519d5799bba2224fc39105d623da4bccad863005e171f
It is more typical in this repo to use `module::Error` instead of a type
alias when importing.
Use `hex::Error` directly instead of `use hex::Error as HexError`.
d882b68a2c Add Script conversion method p2wpkh_script_code (Tobin Harding)
Pull request description:
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] (also described in BIP143)
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.
Includes a link to BIP143
[0] https://github.com/libbitcoin/libbitcoin-system/wiki/P2WPKH-Transactions#spending-a-p2wpkh-output
ACKs for top commit:
apoelstra:
ACK d882b68a2c
sanket1729:
code review ACK d882b68a2c.
Tree-SHA512: 9a3244b5aac4e2911edf4d3bb634d3d2b98006b864280a2a04b45c55c263c2541bf25f01196f2a65bf9acbdd0cf28c69c3a020a7e6c8da6fddf7c7cfbb62836d
f92854a805 Add PSBT alias (Tobin Harding)
Pull request description:
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`.
ACKs for top commit:
apoelstra:
ACK f92854a805
sanket1729:
ACK f92854a805
Tree-SHA512: 1f56ac236d34a89bbb557ada147f05d8a8ce961dad3ad921f10f26c597b91ecc8e15070f8825774745e5333ba5282962830a3cc0c53b93f147be93ab566b1b9e
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
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.
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
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
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
Tobin C. Harding
Dawid Ciężarkiewicz
Andrew Poelstra
Steven Roose
sanket1729
eunoia_1729
Noah
Martin Habovstiak
Riccardo Casatta
Matt Corallo
Fredrik Meringdal
mcroad
ass3rt
violet360
Dr Maxim Orlovsky
0xb10c
Jeremy Rubin