Commit Graph

1472 Commits

Author SHA1 Message Date
Andrew Poelstra 99af5b9cfc
Merge rust-bitcoin/rust-bitcoin#1024: Expose SIGHASH_SINGLE bug in `encode_signing_data_to`
42a91ab32a Expose SIGHASH_SINGLE bug in `encode_signing_data_to` (Dawid Ciężarkiewicz)

Pull request description:

  Via `Option` return value

  Fix #1015

ACKs for top commit:
  tcharding:
    ACK 42a91ab32a
  apoelstra:
    ACK 42a91ab32a

Tree-SHA512: 8e401ba0ee6ed2bdb95ec838440cfd7a0b6414991ada0d941e8b9526ea4a7d9b6ca1fc84318c4b2a317705650cabc957269c1034dd70c92bdeb854ca413e53be
2022-06-23 23:02:50 +00:00
Dawid Ciężarkiewicz a24a3b0194 Forward `consensus_decode` to `consensus_decode_from_finite_reader` 2022-06-23 15:55:21 -07:00
Dawid Ciężarkiewicz 9c754ca4de Take Writer/Reader by `&mut` in consensus en/decoding
Fix #1020 (see more relevant discussion there)

This definitely makes the amount of generics compiler
has to generate by avoding generating the same functions
for `R`, &mut R`, `&mut &mut R` and so on.

old:

```
> ls -al target/release/deps/bitcoin-07a9dabf1f3e0266
-rwxrwxr-x 1 dpc dpc 9947832 Jun  2 22:42 target/release/deps/bitcoin-07a9dabf1f3e0266
> strip target/release/deps/bitcoin-07a9dabf1f3e0266
> ls -al target/release/deps/bitcoin-07a9dabf1f3e0266
-rwxrwxr-x 1 dpc dpc 4463024 Jun  2 22:46 target/release/deps/bitcoin-07a9dabf1f3e0266
```

new:

```

> ls -al target/release/deps/bitcoin-07a9dabf1f3e0266
-rwxrwxr-x 1 dpc dpc 9866800 Jun  2 22:44 target/release/deps/bitcoin-07a9dabf1f3e0266
> strip target/release/deps/bitcoin-07a9dabf1f3e0266
> ls -al target/release/deps/bitcoin-07a9dabf1f3e0266
-rwxrwxr-x 1 dpc dpc 4393392 Jun  2 22:45 target/release/deps/bitcoin-07a9dabf1f3e0266
```

In the unit-test binary itself, it saves ~100KB of data.

I did not expect much performance gains, but turn out I was wrong(*):

old:

```
test blockdata::block::benches::bench_block_deserialize                 ... bench:   1,072,710 ns/iter (+/- 21,871)
test blockdata::block::benches::bench_block_serialize                   ... bench:     191,223 ns/iter (+/- 5,833)
test blockdata::block::benches::bench_block_serialize_logic             ... bench:      37,543 ns/iter (+/- 732)
test blockdata::block::benches::bench_stream_reader                     ... bench:   1,872,455 ns/iter (+/- 149,519)
test blockdata::transaction::benches::bench_transaction_deserialize     ... bench:         136 ns/iter (+/- 3)
test blockdata::transaction::benches::bench_transaction_serialize       ... bench:          51 ns/iter (+/- 8)
test blockdata::transaction::benches::bench_transaction_serialize_logic ... bench:           5 ns/iter (+/- 0)
test blockdata::transaction::benches::bench_transaction_size            ... bench:           3 ns/iter (+/- 0)
```

new:

```
test blockdata::block::benches::bench_block_deserialize                 ... bench:   1,028,574 ns/iter (+/- 10,910)
test blockdata::block::benches::bench_block_serialize                   ... bench:     162,143 ns/iter (+/- 3,363)
test blockdata::block::benches::bench_block_serialize_logic             ... bench:      30,725 ns/iter (+/- 695)
test blockdata::block::benches::bench_stream_reader                     ... bench:   1,437,071 ns/iter (+/- 53,694)
test blockdata::transaction::benches::bench_transaction_deserialize     ... bench:          92 ns/iter (+/- 2)
test blockdata::transaction::benches::bench_transaction_serialize       ... bench:          17 ns/iter (+/- 0)
test blockdata::transaction::benches::bench_transaction_serialize_logic ... bench:           5 ns/iter (+/- 0)
test blockdata::transaction::benches::bench_transaction_size            ... bench:           4 ns/iter (+/- 0)
```

(*) - I'm benchmarking on a noisy laptop. Take this with a grain of salt. But I think
at least it doesn't make anything slower.

While doing all this manual labor that will probably generate conflicts,
I took a liberty of changing generic type names and variable names to
`r` and `R` (reader) and `w` and `W` for writer.
2022-06-23 15:55:14 -07:00
Tobin C. Harding a2a54b3982 Remove unnecessary ? operator
clippy emits:

  warning: question mark operator is useless here

As suggested, remove the `?` operator.
2022-06-23 13:58:29 +10:00
Tobin C. Harding 67ed8f673e Remove unneeded clone
clippy emits:

 warning: using `clone` on type `blockdata::transaction::OutPoint` which
 implements the `Copy` trait

Remove unneeded call to `clone`.
2022-06-23 13:57:31 +10:00
Tobin C. Harding eccd401fc4 Remove unneeded reference
clippy emits:

 warning: this expression creates a reference which is immediately
 dereferenced by the compiler

As suggested, remove the explicit reference.
2022-06-23 13:56:28 +10:00
Tobin C. Harding fd4239f1d2 Use custom digit grouping
clippy emits a bunch of:

 warning: digits grouped inconsistently by underscores

We have a custom grouping elsewhere in this file

  10_000_000_00 sats == 10 BTC

Fix up all instances of large sats amount to uniformly using this format
and add compiler directives where needed to shoosh clippy.
2022-06-23 13:54:34 +10:00
Tobin C. Harding acd551e644 Remove unnecessary 'static lifetime
clippy emits a bunch of:

 warning: statics have by default a `'static` lifetime

Remove the unnecessary 'static lifetimes.
2022-06-23 13:49:16 +10:00
Tobin C. Harding 3102a48d39 Allow clippy::collapsible_else_if
clippy emits:

  warning: this `else { if .. }` block can be collapsed

In this instance the code is more readable how it is, we should ignore
clippy.

Add compiler directive to quieten warning.
2022-06-23 13:47:22 +10:00
Tobin C. Harding 63f4ff6406 Remove redundant field names
clippy emits two warnings of form:

 warning: redundant field names in struct initialization

Remove the redundant field names.
2022-06-23 13:45:16 +10:00
Andrew Poelstra 66c9fedfdb
Merge rust-bitcoin/rust-bitcoin#1059: Move broken-intra-doc-link lint config to command line
281af7c1b9 Move broken-intra-doc-link lint config to command line (Tobin C. Harding)

Pull request description:

  The docs lint `broken-intra-doc-links` has been changed but the new name
  is not available in our MSRV, this means we get a build warning. We only
  build docs with the nightly toolchain so we can move this lint control
  to the docs build command in `test.sh` instead of doing it crate wide.

  With this patch applied devs risk not noticing docs link issues until
  they hit them on CI _if_ they do not build with the test script or
  explicitly pass in `-- -D rustdoc::broken-intra-doc-links`, which no one
  is going to do. Hence we add a line to the readme with a shell alias
  that can be used to check docs, taken directly from `test.sh`.

ACKs for top commit:
  apoelstra:
    ACK 281af7c1b9
  Kixunil:
    ACK 281af7c1b9

Tree-SHA512: 7c9be3bcf097444a107199c9e9df62324d80b770659556a81eca160b807245e15921cda812f83e8b24d41716273704ff7b78be9300680f1efef3cb1fbffe6afd
2022-06-22 17:05:13 +00:00
Tobin C. Harding 281af7c1b9 Move broken-intra-doc-link lint config to command line
The docs lint `broken-intra-doc-links` has been changed but the new name
is not available in our MSRV, this means we get a build warning. We only
build docs with the nightly toolchain so we can move this lint control
to the docs build command in `test.sh` instead of doing it crate wide.

With this patch applied devs risk not noticing docs link issues until
they hit them on CI _if_ they do not build with the test script or
explicitly pass in `-- -D rustdoc::broken-intra-doc-links`, which no one
is going to do. Hence we add a line to the readme with a shell alias
that can be used to check docs, taken directly from `test.sh`.
2022-06-21 09:21:19 +10:00
Tobin C. Harding 4d2291930b Use fragment-specifier literal
Currently we are using the fragment-specifier `expr` in a bunch of
macros for captures that are only used for literals. If we use `literal`
instead it allows the compiler to give slightly more specific error
messages.

The benefits of this change are minor. Of note, this patch found one
unusual macro call site (removed unnecessary `&`).

The macros changed are all internal macros, this is not a breaking change.
2022-06-20 14:43:06 +10:00
Tobin C. Harding 7a3bb7d3ec Replace runtime size check with compile time check
Add a macro `const_assert` that uses some const declaration trickery to
trigger a compile time error if a boolean expression is false.

Replace runtime checks using `debug_assert_eq!` with the newly defined
`const_assert!` macro.
2022-06-20 14:40:51 +10:00
Andrew Poelstra e7a3bdda66
Merge rust-bitcoin/rust-bitcoin#1053: Implement iter::size_hint and ExactSizeIterator for Witness Iter
ec8dadaf86 Implement iter::size_hint and ExactSizeIterator for Witness Iter (Riccardo Casatta)

Pull request description:

  close https://github.com/rust-bitcoin/rust-bitcoin/issues/1050

  I don't think we need to change the `collect` since it use the `size_hint()` lower bound to initially allocate

  // with size_hint
  // test blockdata::witness::benches::bench_big_witness_to_vec              ... bench:         313 ns/iter (+/- 13)
  // test blockdata::witness::benches::bench_witness_to_vec                  ... bench:         204 ns/iter (+/- 11)

  // without
  // test blockdata::witness::benches::bench_big_witness_to_vec              ... bench:         489 ns/iter (+/- 28)
  // test blockdata::witness::benches::bench_witness_to_vec                  ... bench:         221 ns/iter (+/- 102)

  The reason why the small witness doesn't get big perf boost is because by default vec allocates 4 slots

ACKs for top commit:
  Kixunil:
    ACK ec8dadaf86
  apoelstra:
    ACK ec8dadaf86

Tree-SHA512: dbe09ba6ebd4014fe0639412894beedab6cc7e844a5ec1697af8f0694b62ae5d423a801df1b48ac7029444c01877975e2d5168728f038fbd4f5808bda90e0f2f
2022-06-16 13:35:24 +00:00
Andrew Poelstra e00b0c39ca
Merge rust-bitcoin/rust-bitcoin#1042: Refactor `serve_tcp` code
abfeb32e35 Remove unnecessary local variable (Tobin C. Harding)
04b09a4e8d Remove unused loop (Tobin C. Harding)
380e0016cc Use write_all instead of write (Tobin C. Harding)

Pull request description:

  Done while clearing clippy warnings, done as a separate PR because its not a simple glance to review like the others.

  Remove 2 clippy warnings and remove unnecessary local variable.

ACKs for top commit:
  apoelstra:
    ACK abfeb32e35
  Kixunil:
    ACK abfeb32e35

Tree-SHA512: 965708999c067dd8c156bbc54b711f608d524fab7051a0e56066f53b5c8d7bea1c233f04e77873b2624cd22e26a58f1d22f47870d2afe4347aa85335c3142245
2022-06-16 12:53:20 +00:00
Riccardo Casatta ec8dadaf86
Implement iter::size_hint and ExactSizeIterator for Witness Iter
// with size_hint
// test blockdata::witness::benches::bench_big_witness_to_vec              ... bench:         313 ns/iter (+/- 13)
// test blockdata::witness::benches::bench_witness_to_vec                  ... bench:         204 ns/iter (+/- 11)

// without
// test blockdata::witness::benches::bench_big_witness_to_vec              ... bench:         489 ns/iter (+/- 28)
// test blockdata::witness::benches::bench_witness_to_vec                  ... bench:         221 ns/iter (+/- 102)
2022-06-16 10:42:58 +02:00
Andrew Poelstra f30df076d0
Merge rust-bitcoin/rust-bitcoin#1051: Add getter methods for PartialMerkleTree fields
9ff0e06810 Add getter methods for PartialMerkleTree fields (eunoia_1729)

Pull request description:

  Resolves: #1046

ACKs for top commit:
  apoelstra:
    ACK 9ff0e06810
  tcharding:
    ACK 9ff0e06810

Tree-SHA512: 849f2bf87ab99a8f975ee9c78eaaf3b8d392c3338317d94234ceb9da0d78bddd488c67dfb3ded1c572d5ce400c10263d75815315a4cd52b80cd6b60e12d55078
2022-06-15 13:27:10 +00:00
eunoia_1729 9ff0e06810
Add getter methods for PartialMerkleTree fields 2022-06-11 15:36:59 +05:30
eunoia_1729 24fdb53c9c
Fix incorrect argument passed to Error::InvalidSegwitV0ProgramLength 2022-06-11 07:04:20 +05:30
Tobin C. Harding 5ce34011f2 Implement std::error::Error for ParseAmount
The `ParseAmountError` does not implement `std::error::Error`, must have
been missed when we did the rest.

Implement `std::error::Error` for `ParseAmount`.
2022-06-10 12:09:00 +10:00
eunoia_1729 66e852cd19
Update format of ExcessiveScriptSize error message 2022-06-09 01:22:23 +05:30
eunoia_1729 89bd4b61a4
Modify from_script functions in address.rs to return result
Modify from_script functions to return result instead of option so that, in case of errors, there is more
information on what went wrong.

Resolves: #1022
2022-06-09 01:13:51 +05:30
Tobin C. Harding 271d0ba068 Allow many arguments in test function
This is a unit test helper function, it is ok to have a whole bunch of
arguments.
2022-06-07 15:34:59 +10:00
Tobin C. Harding c0c88fe87d Use vec instead of pushing to a mutable vector
Clippy emits:

  warning: calls to `push` immediately after creation

Use `vec` instead of pushing to a mutable vector.
2022-06-07 15:34:59 +10:00
Tobin C. Harding 73066e7e48 Use values() to iterate map values
Clippy emits:

  warning: you seem to want to iterate on a map's values

As suggested, iterate using `values`.
2022-06-07 15:34:59 +10:00
Tobin C. Harding 38ff025122 Remove useless use of vec!
Clippy warns of useless use of `vec!` macro, remove it.
2022-06-07 15:34:58 +10:00
Tobin C. Harding d8e82d5cd4 Remove length comparison to zero
Clippy emits:

  warning: length comparison to zero

Remove length comparison to zero, use `!is_empty`.
2022-06-07 15:26:59 +10:00
Tobin C. Harding c1f34f5c0e Return Address directly
Clippy emits:

  warning: returning the result of a `let` binding from a block

Remove the local binding, return the `Address` directly.
2022-06-07 15:26:59 +10:00
Tobin C. Harding ff8d585c17 Use flat_map instead of map().flatten()
Clippy emits:

  warning: called `map(..).flatten()` on `Iterator`

As suggested, use `flat_map` instead of chaining `map` with `flatten`.
2022-06-07 15:26:59 +10:00
Tobin C. Harding b24a112f08 Remove calls to clone from types that implement Copy
Clippy emits:

  warning: using `clone` on type `blockdata::transaction::OutPoint`
  which implements the `Copy` trait

Remove calls to `clone` from types that implement `Copy`.
2022-06-07 15:26:59 +10:00
Tobin C. Harding 2b8d93ec4b Remove unnecessary explicit reference
Clippy warns about creating a reference that is immediately
de-referenced.

Remove unnecessary explicit `&`, while we are at it remove unnecessary
explicit types that appear on the same lines of code.
2022-06-07 15:26:59 +10:00
Tobin C. Harding ef90e3d4ed Use plus-equals operator
Clippy emits:

  warning: manual implementation of an assign operation

Use the more conventional `+=` operator.
2022-06-07 15:15:26 +10:00
Tobin C. Harding 922b820105 Replace assert!(false) with panic!
Clippy emits:

  warning: `assert!(false)` should probably be replaced

As suggested, replace assert with a call to panic.
2022-06-07 15:15:26 +10:00
Tobin C. Harding a8039e1742 Remove redundant clone
Clippy emits:

  warning: redundant clone

Remove the redundant calls to clone.
2022-06-07 15:15:26 +10:00
Tobin C. Harding cf8de73169 Remove unnecessary cast of integer literal
Clippy emits:

  warning: casting integer literal to `usize` is unnecessary

Remove the unnecessary cast.
2022-06-07 15:15:26 +10:00
Tobin C. Harding 999ac450bb Do not use assert_eq with literal bool
Clippy emits:

  warning: used `assert_eq!` with a literal bool

Use `assert!` instead of `assert_eq!(foo, true)`.
2022-06-07 15:15:25 +10:00
Tobin C. Harding abfeb32e35 Remove unnecessary local variable
In test functions; we bind to `istream` only to re-bind immediately to
`stream`, this is unnecessary and adds no additional information to the
code.
2022-06-07 14:56:06 +10:00
Tobin C. Harding 04b09a4e8d Remove unused loop
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.
2022-06-07 14:56:06 +10:00
Tobin C. Harding 380e0016cc Use write_all instead of write
In this unit test we want to write all the pieces, use `write_all`.

Clears clippy warning about not using return value of `write`.
2022-06-07 14:55:54 +10:00
Tobin C. Harding 827fcd8a89 Allow unusual digit grouping
Clippy emits:

  warning: digits grouped inconsistently by underscores

Add allow directive for grouping that aims to make explicit 100,000,000
sats/per bitcoin.
2022-06-07 14:29:15 +10:00
Tobin C. Harding 242c640603 Remove redundant field names
Clippy emits:

  warning: redundant field names in struct initialization

As suggested, remove redundant field names in struct initialization.
2022-06-07 14:26:57 +10:00
Tobin C. Harding 0f8f4c5609 Collapse if statements
Clippy emits:

  warning: this `if` statement can be collapsed

As suggested, collapse the if statements into a single statement, with
no loss of clarity.
2022-06-07 14:24:44 +10:00
Tobin C. Harding 229fcb9f1f Use if let instead of destructuring pattern
Clippy emits:

 warning: you seem to be trying to use `match` for destructuring a
 single pattern. Consider using `if let`

As suggested, use `if let`.
2022-06-07 14:22:38 +10:00
Dawid Ciężarkiewicz 42a91ab32a Expose SIGHASH_SINGLE bug in `encode_signing_data_to`
Via `Option` return value

Fix #1015
2022-06-06 19:17:01 -07:00
Andrew Poelstra a9365375c1
Merge rust-bitcoin/rust-bitcoin#1037: Document `Txid` being displayed backwards
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
2022-06-06 11:35:13 +00:00
Dawid Ciężarkiewicz 28049ce2d9 Document `Txid` being displayed backwards
Fix #958
2022-06-04 12:08:16 -07:00
Andrew Poelstra de17554a3c
Merge rust-bitcoin/rust-bitcoin#1033: Avoid allocation in build_scriptint
c80dbc2169 Avoid allocation in build_scriptint (Steven Roose)

Pull request description:

  Hehe, reason for party, let's invite apoelstra !

ACKs for top commit:
  apoelstra:
    ACK c80dbc2169
  tcharding:
    ACK c80dbc2169

Tree-SHA512: 8446e765d8b9fa562f636817327db6fad4bb9c906d3f69fda76e61cd258fc4c296e6ffaa440a357125c2ab45603eb05c58cb8d6822deea2fe5746e5c7c3f1e4d
2022-06-03 18:27:21 +00:00
Steven Roose c80dbc2169
Avoid allocation in build_scriptint 2022-06-02 15:36:45 +01:00
Andrew Poelstra 21f4493813
Merge rust-bitcoin/rust-bitcoin#1032: Remove network::Error
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
2022-06-02 14:14:41 +00:00
Andrew Poelstra adf3958127
Merge rust-bitcoin/rust-bitcoin#1011: Add non_exhaustive compiler directive to `AddressType`
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
2022-06-02 14:13:52 +00:00
Tobin C. Harding 43b684bbe6 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.
2022-06-02 12:43:25 +10:00
Tobin C. Harding 99aab446c3 Remove network::Error
The `network::Error` is not used, remove it.
2022-06-02 09:49:33 +10:00
Andrew Poelstra 09b4198b16
Merge rust-bitcoin/rust-bitcoin#1031: refactor: remove SchnorrSignatureHash::Reserved variant
69707b0ccd refactor: remove SchnorrSignatureHash::Reserved variant (eunoia_1729)

Pull request description:

  Resolves #1030

ACKs for top commit:
  sanket1729:
    utACK 69707b0ccd. For reference to other reviewers:
  tcharding:
    ACK 69707b0ccd
  apoelstra:
    ACK 69707b0ccd

Tree-SHA512: d5afe9217e15db8dfd7d712e2fa69d22acda0f93fd7efbc80cf9459303a135a69c8ddf43842584769accb8cffba722ec202dc51f100e0f50262b2561a2fb1699
2022-06-01 23:27:30 +00:00
sanket1729 165cae959a
Merge rust-bitcoin/rust-bitcoin#1003: Improve error `Display` implementations
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
2022-06-01 15:17:24 -07:00
sanket1729 911a9ec579
Merge rust-bitcoin/rust-bitcoin#1018: Add more documentation to some core types
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
2022-06-01 15:06:35 -07:00
eunoia_1729 69707b0ccd
refactor: remove SchnorrSignatureHash::Reserved variant 2022-06-02 03:18:58 +05:30
sanket1729 471f90923d
Merge rust-bitcoin/rust-bitcoin#1006: Feature `use-serde` renamed to `serde`
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
2022-06-01 14:29:02 -07:00
sanket1729 e9a3379228
Merge rust-bitcoin/rust-bitcoin#878: Add chain hash type using consts
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
2022-06-01 12:34:07 -07:00
Andrew Poelstra 57eaf13c86
Merge rust-bitcoin/rust-bitcoin#1021: Enforce segwit v0 script validity when creating address.
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
2022-06-01 16:24:18 +00:00
Andrew Poelstra 50489c8d5e
Merge rust-bitcoin/rust-bitcoin#1026: Add non_exhaustive to all error enums
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
2022-06-01 16:22:44 +00:00
Andrew Poelstra 95548afccf
Merge rust-bitcoin/rust-bitcoin#798: Audit conversion methods
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
2022-06-01 16:19:36 +00:00
Andrew Poelstra 8f81fc5aa7
Merge rust-bitcoin/rust-bitcoin#1004: Clear Clippy warnings
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
2022-06-01 16:18:46 +00:00
Noah 6c10d77ecb Address::from_script() - Check witness v0 program lengths. 2022-05-31 18:13:52 +10:00
Tobin C. Harding 99f565f932 Add non_exhaustive to all error enums
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.
2022-05-31 14:29:50 +10:00
Dawid Ciężarkiewicz 082e185711 Add `consensus_decode_from_finite_reader` optimization
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
2022-05-30 21:15:34 -07:00
Tobin C. Harding a6efe982bd Use write_all to write whole buffer
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.
2022-05-31 12:15:16 +10:00
Tobin Harding 5fbb211085 Use fn name to_ instead of as_
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_`.
2022-05-31 10:07:24 +10:00
Tobin Harding 8ffa32315d Use fn name to_ instead of into_
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_`.
2022-05-31 10:07:24 +10:00
Tobin C. Harding 6874ce91e2 Remove as_inner
`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.
2022-05-31 10:07:24 +10:00
Dawid Ciężarkiewicz 1875c912c3 Extend docstring for more types 2022-05-27 20:34:36 -07:00
Dawid Ciężarkiewicz 325ea8fb7d Add "Relevant BIPs` to `Address` 2022-05-27 20:34:36 -07:00
Dawid Ciężarkiewicz 7c2ca3d20b Add `BlockHeader` Bitcoin Core reference link 2022-05-27 20:34:36 -07:00
Dawid Ciężarkiewicz f4922f6fe7 Update `BlockHeader::version` documentation 2022-05-27 20:34:36 -07:00
Tobin C. Harding 57dd6739c3 Do not print error when displaying for std builds
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.
2022-05-27 08:38:52 +10:00
Martin Habovstiak 2e7effc604 Feature `use-serde` renamed to `serde`
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.
2022-05-26 10:10:47 +02:00
Tobin C. Harding 88ce8fe923 Match against an optional single trailing colon
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.
2022-05-26 11:26:42 +10:00
Tobin C. Harding b80cfeed85 Bind to error_kind instead of e
To make it explicit that this is not an error bind to the local variable
`error_kind` instead of `e`.
2022-05-26 10:06:18 +10:00
Tobin C. Harding 51c60b8507 Allow no is_empty method for VarInt
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.
2022-05-26 08:50:48 +10:00
Tobin C. Harding 841f1f5832 Implement Default for TaprootBuilder
Clippy emits:

  warning: you should consider adding a `Default` implementation for
  `TaprootBuilder`

As suggested, implement `Default` or `TaprootBuilder`.
2022-05-26 08:50:48 +10:00
Tobin C. Harding f81d4aa9bd Remove unnecessary call to clone
Clippy emits:

warning: using `clone` on type `secp256k1::XOnlyPublicKey` which
implements the `Copy` trait

As suggested, remove call to `clone`.
2022-05-26 08:50:48 +10:00
Tobin C. Harding 27649ba182 Use copied instead of map to copy
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.
2022-05-26 08:50:48 +10:00
Tobin C. Harding 62ccc9102c Use iter().flatten().any() instead of if let Some
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.
2022-05-26 08:50:43 +10:00
Tobin C. Harding 4b28a1bb97 Remove unneeded return statement
Clippy emits:

  warning: unneeded `return` statement

As suggested, remove the unneeded return statement.
2022-05-25 13:32:42 +10:00
Tobin C. Harding 16cac3cd70 Derive Default for Witness
No need for an explicit `Default` implementation for `Witness`, it can
be derived. Found by Clippy.
2022-05-25 13:31:15 +10:00
Tobin C. Harding c75189841a Remove unnecessary closure
Clippy emits:

  warning: unnecessary closure used to substitute value for
  `Option::None`

As suggested, use `ok_or` removing the unnecessary closure.
2022-05-25 13:30:07 +10:00
Tobin C. Harding dfff85352a Ignore bytes written for sighash_single bug output
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.
2022-05-25 13:25:23 +10:00
Tobin C. Harding 14c72e755b Use contains combinator instead of manual range
Clippy emits:

  warning: manual `RangeInclusive::contains` implementation

As suggested, use `contains` combinator instead of manual range check.
2022-05-25 13:24:15 +10:00
Tobin C. Harding b7d6c3e02c Remove additional reference
Clippy emits:

  warning: this expression creates a reference which is immediately
  dereferenced by the compiler

As suggested, remove the additional reference.
2022-05-25 13:21:25 +10:00
Tobin C. Harding 1940b00132 Implement From instead of Into
Implementing `From` gives us an implementation of `Into` for free so is
therefore superior.

Found by Clippy.
2022-05-25 13:19:17 +10:00
Tobin C. Harding fcd0f4deac Use struct field init shorthand
Clippy emits:

  warning: redundant field names in struct initialization

As suggested use struct field init shorthand.
2022-05-25 13:15:20 +10:00
Tobin C. Harding 641960f037 Use rustfmt::skip
Clippy emits:

  warning: `cfg_attr` is deprecated for rustfmt and got replaced by tool
  attributes

As suggested use `rustfmt::skip`.
2022-05-25 13:13:13 +10:00
Tobin C. Harding 3cd00e5d47 Remove unnecessary whitespace 2022-05-25 13:12:31 +10:00
Tobin C. Harding 241ec72497 Bind to b instead of e
This error variant contains a byte value not an error type, bind to
local variable `b` instead of `e` to make this explicit.
2022-05-25 12:53:33 +10:00
Tobin C. Harding 01f481bf5c Bind to s instead of e
This error variant contains a string not an error type, bind to local
variable `s` instead of `e` to make this explicit.
2022-05-25 12:33:56 +10:00
Tobin C. Harding 5c6d369289 network: Remove unused error variants
Remove unused error variants from `network::Error`.
2022-05-25 12:32:35 +10:00
Tobin C. Harding e67e97bb37 Put From impl below std::error::Error impl
As we do for all the other error types put the `From` impl blocks below
the `std::error::Erro` impl block.

Refactor only, no logic changes.
2022-05-25 12:31:32 +10:00
Tobin C. Harding 6ca98e5275 Remove error TODO
Remove the TODO from comments and raise a GitHub issue to track it.

  https://github.com/rust-bitcoin/rust-bitcoin/issues/1002
2022-05-25 11:59:58 +10:00
Andrew Poelstra 324fa0f7be
Merge rust-bitcoin/rust-bitcoin#968: Refactor address byte swapping
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
2022-05-24 17:41:12 +00:00
Andrew Poelstra 0e82376bf8
Merge rust-bitcoin/rust-bitcoin#987: Implement `std::error::Error` for the new MSRV
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
2022-05-21 14:08:52 +00:00