Commit Graph

469 Commits

Author SHA1 Message Date
Tobin Harding d1abfd9c30 Add unit test for sighash single bug
When signing a transaction will result in the sighash single bug being
exploitable we should return the 'one array' (equivalent to 1 as a
uint256) as the signature hash.

Add a unit test to verify we return uint256 1 value when use of
SIGHASH_SINGLE is invalid.
2022-03-14 12:10:16 +11:00
Tobin Harding 82f29b4267 Use 1 signature hash for invalid SIGHASH_SINGLE
When signing a transaction will result in the sighash single bug being
exploitable we should return the 1 array (equivalent to 1 as a uint256)
as the signature hash.

Currently we are using the correct array value but are re-hashing it,
instead we should directly return it.
2022-03-14 12:09:39 +11:00
Tobin Harding 3831816a73 Move test helper function
Move helper function to above the test that uses it.

Refactor only, no logic changes.
2022-03-14 12:06:54 +11:00
Tobin Harding 3e21295b88 Remove unnecessary whitespace character
Typically we do not put a whitespace character before a `:` when using
explicit types.
2022-03-14 12:06:54 +11:00
Dr Maxim Orlovsky 7554d76dfe
Make Script::witness_version public 2022-03-13 14:40:03 +01:00
Dr. Maxim Orlovsky 40e6f131ff
Merge rust-bitcoin/rust-bitcoin#867: Cleanup opcodes module
f4886afa66 Add full stops to docs (Tobin Harding)
f01f047b21 Remove unnecessary newlines (Tobin Harding)
8a1cc2ca77 Improve docs on ClassifyContext (Tobin Harding)

Pull request description:

  Do some clean ups to the `blockdata::opcodes` module. Patch 3 is big but it should be quick to review because I made all the boring 'add full stops' changes in a single commit.

ACKs for top commit:
  Kixunil:
    ACK f4886afa66
  apoelstra:
    ACK f4886afa66
  dr-orlovsky:
    ACK f4886afa66

Tree-SHA512: b30f36bd06a028b6bbc24a64849c0788a9223760907bdcb3765af1742a228f630cc7666ed66fa2afd8fb6c96e3cf416e9bd9d2a3b6c72c6e47a16399a856fca1
2022-03-12 12:50:40 +02:00
Dr. Maxim Orlovsky 7e755d3ddd
Merge rust-bitcoin/rust-bitcoin#864: Cleanup block module
146d5e83d1 Improve docs for blockdata::block (Tobin Harding)
f03092c380 Fix erroneous function rustdoc (Tobin Harding)
5464848f45 Refactor check_witness_commitment (Tobin Harding)

Pull request description:

  Do some clean ups to the `blockdata::block` module.

  - Patch 1: Change predicate names (API breaking, could be seen as unnecessarily changing the API), can remove if NACK'd
  - Patch 2: Refactor to assist code clarity
  - Patch 3 and 4: are docs improvements, shouldn't be too controversial

ACKs for top commit:
  apoelstra:
    ACK 146d5e83d1
  dr-orlovsky:
    ACK 146d5e83d1

Tree-SHA512: 65cc414857c4569a389638b53eb99ed629bf67ae1d8ebdc9023e5974bb26902d4de41ec311bef3b5c895229d7d0df78d469a84c1e94fc0b7be7435338f0d510a
2022-03-12 12:41:08 +02:00
Dr. Maxim Orlovsky 501cf63f0d
Merge rust-bitcoin/rust-bitcoin#865: Cleanup transaction docs
e503f14331 Improve docs: blockdata::transaction (Tobin Harding)
f02b3a8472 Add code comment for emtpy input (Tobin Harding)
6a0ec1ac47 Remove redundant _eq (Tobin Harding)
3bcc146a44 Improve docs: encode_signing_data_to/signature_hash (Tobin Harding)

Pull request description:

  Do some cleanups to the docs in `blockdata::transaction`. Patch 1 needs the most careful review please. The rest should not be too controversial.

ACKs for top commit:
  apoelstra:
    ACK e503f14331
  dr-orlovsky:
    ACK e503f14331

Tree-SHA512: 3953226e1b7f0db0371b1902888407a48531688bf8ed08539a0090f369b491b130d70b2fae859878ef178a397cefe0ee2a15f3358afc990a2776194cc2b3882b
2022-03-12 12:12:11 +02:00
Dr. Maxim Orlovsky 64164db4b4
Merge rust-bitcoin/rust-bitcoin#869: Improve docs: script module
4dcbef6ddd Improve docs: script module (Tobin Harding)

Pull request description:

  Improve the docs in the `blockdata::script` module by doing:

  - Use full sentences (use capitals and full stops)
  - Improve grammar/wording if necessary
  - Remove incorrect/unneeded comments
  - Fix layout of rustdoc i.e., use brief and description sections
  - Use 100 line character width if it makes the comment look better
  - Use third person instead of imperative tense

  ## Note to reviewers

  Sorry to be a bore and request review on all these docs fixes, this one is all in a single patch which makes it a bit harder to review. It is very similar in content to all the others that are open right now so I'm going to be a bit rude and leave it like this. Please say if this is even slightly putting too much demand on you review time.

ACKs for top commit:
  apoelstra:
    ACK 4dcbef6ddd
  dr-orlovsky:
    ACK 4dcbef6ddd

Tree-SHA512: 49fa1d88c4b97decbc563747ba166fe95698da6a634801ccf5f99fd67a4a907067dbf0a4d64e7773d5d5b04aef404167b6cc911382363247d15a61cef5d8965c
2022-03-12 12:08:32 +02:00
Tobin Harding 4dcbef6ddd Improve docs: script module
Improve the docs in the `blockdata::script` module by doing:

- Use full sentences (use capitals and full stops)
- Improve grammar/wording if necessary
- Remove incorrect/unneeded comments
- Fix layout of rustdoc i.e., use brief and description sections
- Use 100 line character width if it makes the comment look better
- Use third person instead of imperative tense
2022-03-10 07:45:38 +11:00
Tobin Harding e503f14331 Improve docs: blockdata::transaction
Improve the rustdocs for the `blockdata::transaction` module:

- Use full sentences (capitalisation and full stop)
- Use third person tense instead of imperative
- Improve wording/grammar
- Use backticks in links
- Use 100 character column width if it improves readability

Nothing too controversial here :)
2022-03-10 07:20:40 +11:00
Tobin Harding f02b3a8472 Add code comment for emtpy input
The line of code `let mut have_witness = self.input.is_empty();` is
puzzling if one does not know _why_ we serialize in BIP141 style when
there are no inputs.

Add a code comment to save devs spending time trying to work out _why_
this is correct.
2022-03-10 07:14:06 +11:00
Tobin Harding 6a0ec1ac47 Remove redundant _eq
`assert!` already checks a boolean, it is redundant to use `assert_eq!`
and pass in `true`.

Remove redundant usage of `assert_eq!(foo, true)`.
2022-03-10 07:13:55 +11:00
Tobin Harding 3bcc146a44 Improve docs: encode_signing_data_to/signature_hash
The two methods `encode_signing_data_to` and `signature_hash` use the
same docs (one is a public helper for the other). The docs have gotten a
bit stale (refer to deprecated types).

Instead of duplicating all the text, add a statement pointing readers
from the docs of `signature_hash` to the docs on
`encode_signing_data_to`.
2022-03-10 07:13:06 +11:00
Tobin Harding aaf587d320 Use correct opcode count
Code comment contains an off-by-one error, update it to the correct
value '61'.
2022-03-10 06:42:09 +11:00
Tobin Harding 146d5e83d1 Improve docs for blockdata::block
Improve the rustdocs for the `blockdata::block` module:

- Use full sentences (capitalisation and full stop)
- Use third person tense instead of imperative
- Improve wording if needed
2022-03-10 06:39:32 +11:00
Tobin Harding f03092c380 Fix erroneous function rustdoc
The returns part of the function docs appears to be stale, remove it.
Improve wording of rustdocs while we are at it.
2022-03-10 06:39:32 +11:00
Tobin Harding 5464848f45 Refactor check_witness_commitment
Currently function contains nested `if` clauses that arguably obfuscate
the code. We can make the code easier to read by pulling out the error
paths and returning them higher up in the function.

Refactor only, no logic changes.
2022-03-10 06:39:04 +11:00
Tobin Harding f4886afa66 Add full stops to docs
Add full stops to all lines of rustdocs in the `blockdata::opcodes`
module.
2022-03-09 13:55:14 +11:00
Tobin Harding f01f047b21 Remove unnecessary newlines
Whitespace only, no code changes.
2022-03-09 13:55:14 +11:00
Tobin Harding 8a1cc2ca77 Improve docs on ClassifyContext
Improve docs on `ClassifyContext` by doing:

- Separate brief doc line from the rest
- Use uniform backticks on opcodes
2022-03-09 13:55:14 +11:00
Tobin Harding df7bb03a67 Simplify read_scriptbool
Simplify `read_scriptbool` by doing:

- Use `split_last` to get at the last element
- Mask the last byte against ^0x80 instead of using two equality
  statements
2022-01-26 16:52:41 +11:00
Tobin Harding 4b6e86658d Refactor is_provably_unspendable
Refactor with the aim of making the code easier to read. Code path is
covered by current unit tests.

Refactor only, no logic changes.
2022-01-25 10:21:06 +11:00
Tobin Harding e54a2d653b Put && operator at front of line
In an effort to make code containing multi-line logical AND clearer to
read put the operator at the start of the line.
2022-01-25 10:21:06 +11:00
Tobin Harding f5512c4931 Refactor is_p2pkh
Refactor with the aim of simplifying `is_p2kh`. This function is covered
sufficiently by current unit tests.

Refactor only, no logic changes.
2022-01-25 10:16:56 +11:00
Tobin Harding 373ea89a9a Simplify read_scriptbool
Refactor and simplify the logical operators in `read_scriptbool`.

Refactor only, no logic changes.
2022-01-25 10:16:56 +11:00
Tobin Harding 654b2772b8 Add passing unit tests for read_scriptbool
In preparation for refactoring `read_scriptbool` add passing unit
tests.
2022-01-25 10:16:54 +11:00
Casey Rodarmor 94dd57de12 Add subsidy halving interval constant 2022-01-20 08:52:46 -08:00
Tobin Harding 8d780e8172 Use 4 character indentation
Bizarrely the impl block for `EcdsaSigHashType` uses 5 character
indentation.

Use 4 character indentation as is typical.
2022-01-19 14:30:36 +11:00
sanket1729 85009a7b50 Update documentation of from_u32_consensus 2022-01-15 06:15:14 +05:30
Dr Maxim Orlovsky b9170162d5 Re-org keys and ecdsa mods - pt.1
This commit tries to achieve separation of signature- and key-related types, previously mixed in a single ECDSA module.

Rationale: bitcoin key types are not specific for signature algorithm.

This is achieved through
- Remove key mod with its content moved to ecdsa mod
- Re-export keys under key module in util mod - to make git generate diff for the rename of ecdsa mod in the next commit correctly.
2022-01-14 01:35:48 +01:00
Dr Maxim Orlovsky 6364ebd927 Code style fixups to taproot key functions 2022-01-13 17:48:13 +01:00
Andrew Poelstra 907b3a7a6a
Merge rust-bitcoin/rust-bitcoin#775: Issue #394 - Refactor Block::merkle_root()
cd2435c807 Change deprecated version to 0.28. Remove redundant Block::merkle_root() logic. (Nils Loewen)
05788285f5 Issue #394 - Refactor Block::merkle_root() to Block::compute_merkle_root() and deprecate Block::merkle_root(). (Nils Loewen)

Pull request description:

  Refactor `Block::merkle_root()` to `Block::compute_merkle_root()` and deprecate `Block::merkle_root()`.

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

Tree-SHA512: 820d85d5a25b7316046d5df8e3ab1e8cd57f72c7fa63f0d6826b965c5da3ef1bfacd4704180810aa8c0a7a224dcd62f398a2cf93fc344e2b0d52d9d7024c6c27
2022-01-13 14:32:30 +00:00
sanket1729 7d62277f83
Merge rust-bitcoin/rust-bitcoin#696: Taproot tweaks generalization & KeyPair support
7405836411 Fix warning about deprecated method use (Dr Maxim Orlovsky)
f39b1300fa CI: do not fail fast (Dr Maxim Orlovsky)
f77c57195a Making Script method new_* names more consistent (Dr Maxim Orlovsky)
91b68a468d Taproot-related methods for Script type (Dr Maxim Orlovsky)
599c5f9488 Generalizing taproot key tweaking for KeyPairs (Dr Maxim Orlovsky)

Pull request description:

  * Adds taproot-related methods to `Script`
  * Fixes API for existing taproot methods
  * Generalizes `TapTweak` trait to work with both public keys and key pairs

  ~~UPD: PR is pending https://github.com/rust-bitcoin/rust-secp256k1/pull/342~~

ACKs for top commit:
  sanket1729:
    ACK 7405836411
  apoelstra:
    ACK 7405836411

Tree-SHA512: 4a76dfffa1452baadc15e19812831ef9d2e66794c090a8fc123388d7119b2c8a1f0420ce723ad22e01683c8198711fe62e0cdf00c9ad2d2974606383baaf1cb0
2022-01-13 10:06:30 +05:30
Nils Loewen cd2435c807
Change deprecated version to 0.28. Remove redundant Block::merkle_root() logic. 2022-01-12 16:23:35 -08:00
Nils Loewen 05788285f5
Issue #394 - Refactor Block::merkle_root() to
Block::compute_merkle_root() and deprecate Block::merkle_root().
2022-01-12 13:37:23 -08:00
Dr Maxim Orlovsky f77c57195a Making Script method new_* names more consistent 2022-01-11 16:10:29 +01:00
Dr Maxim Orlovsky 91b68a468d Taproot-related methods for Script type 2022-01-11 16:10:29 +01:00
Andrew Poelstra d9bc48f12f
Merge rust-bitcoin/rust-bitcoin#771: Add Witness::new()
5d27c15717 Add Witness::new() (sanket1729)

Pull request description:

  Rebasing rust-miniscript on rust-bitcoin master. Expect more trivial PRs :)

ACKs for top commit:
  dr-orlovsky:
    ACK 5d27c15717
  apoelstra:
    ACK 5d27c15717

Tree-SHA512: f1b6b1bba1ba6f153416be514b53d64bb6ba0c1cc1f182857f5010a048004f884d5c36ef96f6e27837bf682d781e3a4375bf2303e1f53685019406e88e192a03
2022-01-11 14:49:14 +00:00
Dr Maxim Orlovsky eb09019720 Rename inner key field in PrivateKey and PublicKey
Closes #532
2022-01-11 08:39:52 +01:00
sanket1729 5d27c15717 Add Witness::new()
I think it is more natural to write Wintess::new() followed by Witness::push()
then Witness::default(). In any case, there is no harm in having additional constructors.
2022-01-11 07:37:20 +05:30
Dr Maxim Orlovsky a6e8f581db PSBT BIP32 keys moved to Secp256k1 from bitcoin ECDSA
Fourth step in implementation of Schnorr key support after #588.

While PSBT BIP174 does not specify whether uncompressed keys are supported in BIP32-related fields, from BIP32 it follows that it is impossible to use uncompressed keys within the extended keys.  This PR fixes this situation and is a companion to BIP174 PR clarifying key serialization: https://github.com/bitcoin/bips/pull/1100
2022-01-10 10:16:57 +01:00
Dr Maxim Orlovsky daf0eacf3d Improve NonStandardSigHashType 2022-01-07 21:49:05 +01:00
Andrew Poelstra f332a1967e
Merge rust-bitcoin/rust-bitcoin#750: Use `test_data` for big objects, add big block for benchmarking
247a14f4c3 Use test big block for bench_stream_reader instead of making one (Riccardo Casatta)
b92dfbb63f exclude test_data when publishing the crate (Riccardo Casatta)
f5a9681a2a include a big block in test_data, use it for ser/de benchmark (Riccardo Casatta)
09dada55d6 Move bip158 test vectors to test_data (Riccardo Casatta)
06d1a820c3 Remove testnet block hex from tests, use test_data with include_bytes! (Riccardo Casatta)

Pull request description:

  In the first two commits I moved some data from source files to the newly introduced `test_data` dir, including it with `include_[str|bytes]!` macro.

  The second-to-last commit introduces a big block in test_data which is very handy in ser/de benchmark (I used it for #672) because with smaller blocks you may not notice performance improvements.

  Since I don't want to pollute the package the last commit excludes the `test_data` dir from the published package. I think it's fine to do it because dependent packages don't run dependencies tests.

ACKs for top commit:
  apoelstra:
    ACK 247a14f4c3
  Kixunil:
    tACK 247a14f4c3

Tree-SHA512: a2beb635b0a358737d0b57d3e7205b1ddf87652b9a8c889ce63e2867659a8eaf7e43a5b87a453345d56d953745913f40b58596f449e5fbc87340e0dd2aef0727
2022-01-07 20:22:02 +00:00
Riccardo Casatta 9e7bb0967c
Merge rust-bitcoin/rust-bitcoin#753: Put rustdocs above attributes
533120899e Put rustdocs above attributes (Tobin Harding)

Pull request description:

  (Trivial / Very low priority PR)

  Rust idiomatic style is to put the rustdoc _above_ any attributes on types, functions, etc.

  Audit the codebase and move comments/attributes to the correct place. Add a trailing full stop at times to neaten things up a little extra.

  Done after discussion [here](https://github.com/rust-bitcoin/rust-secp256k1/pull/353#discussion_r778393138)

ACKs for top commit:
  Kixunil:
    ACK 533120899e
  RCasatta:
    ACK 533120899e

Tree-SHA512: 7cd00dc46de813cbe3f96417bb4b13980064e10110b421224496c8b64bbe87b61b6c757cc621fde1d05754be6ecdc08acdb51fd8978e3f820d2d93f7104062d1
2022-01-06 14:09:23 +01:00
Riccardo Casatta 247a14f4c3
Use test big block for bench_stream_reader instead of making one 2022-01-06 13:55:50 +01:00
Riccardo Casatta f5a9681a2a
include a big block in test_data, use it for ser/de benchmark 2022-01-06 13:48:02 +01:00
Riccardo Casatta 06d1a820c3
Remove testnet block hex from tests, use test_data with include_bytes! 2022-01-06 13:47:51 +01:00
Riccardo Casatta 9c3a27a326
Merge rust-bitcoin/rust-bitcoin#680: Deprecate `StreamReader`
e860333bf3 Fix typos (Riccardo Casatta)
9189539715 Use BufReader internally in StreamReader to avoid performance regression on existing callers (Riccardo Casatta)
5dfb93df71 Deprecate StreamReader (Riccardo Casatta)
9ca6c75b18 Bench StreamReader (Riccardo Casatta)

Pull request description:

  `StreamReader` performance is extremely poor in case the object decoded is "big enough" for example a full Block.

  In the common case, the buffer is 64k, so to successfully parse a 1MB block 16 decode attempts are made.
  Even if a user increases the buffer size, `read` is not going to necessarily fill the buffer, as stated in the doc https://doc.rust-lang.org/stable/std/io/trait.Read.html#tymethod.read. In my tests, the reads are 64kB even with a 1MB buffer.

  I think this is the root issue of the performance issue found in electrs in https://github.com/romanz/electrs/issues/547 and they now have decided to decode the TCP stream with their own code in cd0531b8b7 and 05e0221b8e.

  Using directly `consensus_encode` seems to make more sense (taking care of using `BufRead` if necessary) so the `StreamReader` is deprecated

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

Tree-SHA512: a15a14f3f087be36271da5008d8dfb63866c9ddeb5ceb0e328b4a6d870131132a8b05103f7a3fed231f5bca099865efd07856b4766834d56ce2384b1bcdb889b
2022-01-06 13:28:32 +01:00
Tobin Harding 533120899e Put rustdocs above attributes
Rust idiomatic style is to put the rustdoc _above_ any attributes on
types, functions, etc.

Audit the codebase and move comments/attributes to the correct place.
Add a trailing full stop at times to neaten things up a little extra.
2022-01-06 13:04:47 +11:00
Dr. Maxim Orlovsky 86055d9df5
Merge rust-bitcoin/rust-bitcoin#672: New Witness struct to improve ser/de perfomance
106acdc3ac Add fuzzing for Witness struct (Riccardo Casatta)
2fd0125bfa Introduce Witness struct mainly to improve ser/de performance while keeping most usability. (Riccardo Casatta)

Pull request description:

  At the moment the Witness struct is  `Vec<Vec<u8>>`, the vec inside a vec cause a lot of allocations, specifically:

  - empty witness -> 1 allocation, while an empty vec doesn't allocate, the outer vec is not empty
  - witness with n elements -> n+1 allocations

  The proposed Witness struct contains the serialized format of the witness. This reduces the allocations to:

  - empty witness -> 0 allocations
  - witness with n elements -> 1 allocation for most common cases (you don't know how many bytes is long the entire witness beforehand, thus you need to estimate a good value, not too big to avoid wasting space and not too low to avoid vector reallocation, I used 128 since it covers about 80% of cases on mainnet)

  The inconvenience is having slightly less comfortable access to the witness, but the iterator is efficient (no allocations) and you can always collect the iteration to have a Vec of slices. If you collect the iteration you end up doing allocation anyway, but the rationale is that it is an operation you need to do rarely while ser/de is done much more often.

  I had to add a bigger block to better see the improvement (ae860247e191e2136d7c87382f78c96e0908d700), these are the results of the benches on my machine:

  ```
  RCasatta/master_with_block
  test blockdata::block::benches::bench_block_deserialize                 ... bench:   5,496,821 ns/iter (+/- 298,859)
  test blockdata::block::benches::bench_block_serialize                   ... bench:     437,389 ns/iter (+/- 31,576)
  test blockdata::block::benches::bench_block_serialize_logic             ... bench:     108,759 ns/iter (+/- 5,807)
  test blockdata::transaction::benches::bench_transaction_deserialize     ... bench:         670 ns/iter (+/- 49)
  test blockdata::transaction::benches::bench_transaction_get_size        ... bench:           7 ns/iter (+/- 0)
  test blockdata::transaction::benches::bench_transaction_serialize       ... bench:          51 ns/iter (+/- 5)
  test blockdata::transaction::benches::bench_transaction_serialize_logic ... bench:          13 ns/iter (+/- 0)

  branch witness_with_block (this one)
  test blockdata::block::benches::bench_block_deserialize                 ... bench:   4,302,788 ns/iter (+/- 424,806)
  test blockdata::block::benches::bench_block_serialize                   ... bench:     366,493 ns/iter (+/- 42,216)
  test blockdata::block::benches::bench_block_serialize_logic             ... bench:      84,646 ns/iter (+/- 7,366)
  test blockdata::transaction::benches::bench_transaction_deserialize     ... bench:         648 ns/iter (+/- 77)
  test blockdata::transaction::benches::bench_transaction_get_size        ... bench:           7 ns/iter (+/- 0)
  test blockdata::transaction::benches::bench_transaction_serialize       ... bench:          50 ns/iter (+/- 5)
  test blockdata::transaction::benches::bench_transaction_serialize_logic ... bench:          14 ns/iter (+/- 0)
  ```

  With an increased performance to deserialize a block of about 21% and to serialize a block of about 16% (seems even higher than expected, need to do more tests to confirm, I'll appreciate tests results from reviewers)

ACKs for top commit:
  apoelstra:
    ACK 106acdc3ac
  sanket1729:
    ACK 106acdc3ac
  dr-orlovsky:
    utACK 106acdc3ac

Tree-SHA512: e4f23bdd55075c7ea788bc55846fd9e30f9cb76d5847cb259bddbf72523857715b0d4dbac505be3dfb9d4b1bcae289384ab39885b4887e188f8f1c06caf4049a
2021-12-30 01:55:44 +02:00
Riccardo Casatta 2fd0125bfa
Introduce Witness struct mainly to improve ser/de performance while keeping most usability.
Witness struct is in place of the Vec<Vec<u8>> we have before this commit.

from_vec() and to_vec() methods are provided to switch between this type and Vec<Vec<u8>>

Moreover, implementation of Default, Iterator and others allows to have similar behaviour but
using a single Vec prevent many allocations during deserialization which in turns results in
better performance, even 20% better perfomance on recent block.

last() and second_to_last() allows to access respective element without going through costly Vec
transformation
2021-12-28 09:56:38 +01:00
sanket1729 b945a5e5c6
Merge rust-bitcoin/rust-bitcoin#665: transactions: add a note about `get_vsize` and standardness rules
826fed53f2 transactions: add a note about `get_vsize` and standardness rules (Antoine Poinsot)

Pull request description:

  If they ever hit a discrepancy they must really be doing something dodgy but hey :)

ACKs for top commit:
  dr-orlovsky:
    ACK 826fed53f2

Tree-SHA512: c618a80b047797625a233939d2c1146e8b4ce44215648841813f78178577afc844f5e561e4e60b4084e315735894ecb354af8d81f4702f5354e5d5cd05b52ac4
2021-12-28 02:35:54 +05:30
Martin Habovstiak 779d4110c6 Fixed a bunch of clippy lints, added clippy.toml
This is the initial step towards using and maybe enforcing clippy.
It does not fix all lints as some are not applicable. They may be
explicitly ignored later.
2021-12-21 22:50:13 +01:00
Riccardo Casatta fe43e3c9d7
Merge rust-bitcoin/rust-bitcoin#710: Refactor bitcoin_merkle_root functions
b454cf8e15 Return None from merkle_root functions (Tobin Harding)
7a8b017ea3 Use correct spelling of merkle (Tobin Harding)

Pull request description:

  ~Do two minor refactorings to the `bitcoin_merkle_root[_inline] functions.~

  This PR has grown, is no longer a refactoring because the two functions have been changed to return an `Option`.

  First patch is cleanup. Here is the commit message for the second patch
  ```
  The merkle_root of an empty tree is undefined, this is the only error
  case we have for the two `bitcoin_merkle_root*` functions. We can fully
  describe this error case by returning an `Option` if args are found to
  be empty.

  While we are at it, refactor out a recursive helper function to make
  reading the code between the two functions easier.
  ```

ACKs for top commit:
  Kixunil:
    ACK b454cf8e15
  dr-orlovsky:
    ACK b454cf8e15

Tree-SHA512: 961714a8b0eb0dad493a1548317d875d64ca22d2d584c905c502369b5f6e5a9f8be1edd7345136b44964dc0bde7a4c43bfaff4287d1dbf7fd736da79818074e3
2021-12-16 09:48:29 +01:00
sanket1729 94cfe79170 Rename existing SigHashType to EcdsaSigHashType 2021-12-15 20:00:52 +05:30
Riccardo Casatta 9ca6c75b18
Bench StreamReader 2021-12-12 17:33:24 +01:00
Tobin Harding b454cf8e15 Return None from merkle_root functions
The merkle_root of an empty tree is undefined, this is the only error
case we have for the two `bitcoin_merkle_root*` functions. We can fully
describe this error case by returning an `Option` if args are found to
be empty. We can do the same for the wrapper functions in `block`
module.

While we are at it, refactor out a recursive helper function to make
reading the code between the two functions easier.
2021-12-10 11:24:30 +11:00
Martin Habovstiak 04a8f89f05 Implement `FusedIterator` for `Instructions`
`Instructions` guarantee to return `None` from empty iterator so we
should signal this in type system so that the code can be optimized
better. This also adds a test to make sure this property holds.
2021-12-01 21:38:46 +01:00
Tobin Harding 4031fbf4ba Use expect for concensus_encode on sinks
Calls to `unwrap` outside of tests are typically unfavourable.

Sink writers do not error. We can use `expect` with a descriptive
message string to indicate this.
2021-11-25 10:04:21 +11:00
Tobin Harding fa513bb5b5 Use expect for concensus_encode on engines
Calls to `unwrap` outside of tests are typically unfavourable.

Hash engines do not error when calling `consensus_encode`. Instead of
the current usage of `unwrap` we can use `expect` with a descriptive
string as is done in other parts of the codebase.
2021-11-25 10:01:41 +11:00
Tobin Harding 3f5caa501f Clean up module level rustdocs
Docs can always do with a bit of love.

Clean up the module level (`//!`) rustdocs for all public modules.

I claim uniform is better than any specific method/style. I tried to fit
in with what ever was either most sane of most prevalent, therefore
attaining uniformity without unnecessary code churn (one exception being
the changes to headings described below).

Notes:

* Headings - use heading as a regular sentence for all modules e.g.,

```
//! Bitcoin network messages.
```

as opposed to
```
//! # Bitcoin Network Messages
```

It was not clear which style to use so I picked a 'random' mature
project and copied their style.

* Added 'This module' in _most_ places as the start of the module
description, however I was not religious about this one.

* Fixed line length if necessary since most of our code seems to follow
short (80 char) line lengths for comments anyways.

* Added periods and fixed obvious (and sometimes not so obvious)
grammatically errors.

* Added a trailing `//!` to every block since this was almost universal
already. I don't really like this one but I'm guessing it is Andrew's
preferred style since its on the copyright notices as well.
2021-11-06 10:59:53 +11:00
Martin Habovstiak 49bd3af449 Refactor Script::bytes_to_asm_fmt to use iterator
This refactors `Script::bytes_to_asm_fmt`` function to use an iterator
instead of index. Such change makes it easier to reason about overflows
or out-of-bounds accesses. As a result this also fixes three unlikely
overflows and happens to improve formatting to not output space at the
beginning in some weird cases.

To improve robustness even better it also moves `read_uint`
implementation to internal function which returns a more specific error
type which can be exhaustively matched on to guarantee correct error
handling. Probably because of lack of this the code was previously
checking the same condition twice, the second time being unreachable and
attempting to behave differently than the first one.

Finally this uses macro to deduplicate code which differs only in single
number, ensuring the code stays in sync across all branches.
2021-09-30 14:33:10 +02:00
Andrew Poelstra 9fe840c20e
Merge pull request #644 from sanket1729/tap_opcodes
Add OP_CHECKSIGADD and OP_SUCCESSxxx
2021-09-24 22:47:48 +00:00
Antoine Poinsot 826fed53f2
transactions: add a note about `get_vsize` and standardness rules
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
2021-09-24 19:48:36 +02:00
Andrew Poelstra 29549ccc73
Merge rust-bitcoin/rust-bitcoin#658: Check for overflow in Script::bytes_to_asm_fmt()
76cf74fa9b Added test for the overflow bug and few others (Martin Habovstiak)
a0e1d2e706 Check for overflow in Script::bytes_to_asm_fmt() (Martin Habovstiak)

Pull request description:

  This adds an overflow check in `Script::bytes_to_asm_fmt()` motivated by
  `electrs` issue. While it was not tested yet, I'm very confident that
  overflow is the cause of panic there and even if not it can cause panic
  becuase the public function takes unvalidated byte array and reads
  `data_len` from it.

  The `electrs` issue: https://github.com/romanz/electrs/issues/490

  ~~Strangely, this breaks a test case and I can't see why. I'm publishing in case someone wants to help.~~

  Edit: One damn character. :D Should be OK now.

ACKs for top commit:
  apoelstra:
    ACK 76cf74fa9b

Tree-SHA512: 4ffeca442a71b10c132f055f056128ae64e66cbdc1891662c3a4e743b82fa5d27075a44513e844be37888b33068eef3bbf6bcced5def70c17c9c5bd5b9d870cc
2021-09-20 14:22:44 +00:00
Martin Habovstiak 76cf74fa9b Added test for the overflow bug and few others
This adds a test case for script formatting which caused overflow in the
past and a few others from the same "interesting" transaction. Note that
to trigger the bug one has to run the test on 32 bit architecture.
2021-09-19 15:45:17 +02:00
Martin Habovstiak a0e1d2e706 Check for overflow in Script::bytes_to_asm_fmt()
This adds an overflow check in `Script::bytes_to_asm_fmt()` motivated by
`electrs` issue. While it was not tested yet, I'm very confident that
overflow is the cause of panic there and even if not it can cause panic
becuase the public function takes unvalidated byte array and reads
`data_len` from it.

The `electrs` issue: https://github.com/romanz/electrs/issues/490
2021-09-19 13:33:37 +02:00
Dr. Maxim Orlovsky b7f984972a
Merge pull request #655 from vss96/Limit-Script-Size 2021-09-16 10:42:08 +02:00
Andrew Poelstra b6b60fc4aa
Merge rust-bitcoin/rust-bitcoin#628: Adds Taproot BIP341 signature message and create a unified sighash cache for legacy, segwit and taproot inputs
c704ee7ffe [docs-only] Use backtick in addition to square parentheses for types references, clarify legacy, non_exhaustive comment, remove std:: (Riccardo Casatta)
f223be618f Rename access_witness to witness_mut and return Option (Riccardo Casatta)
c9bc0b928a [fmt-only] autoformatting with `rustfmt src/util/sighash.rs` (Riccardo Casatta)
07774917c2 Use get_or_insert_with in segwit_cache (Martin Habovstiak)
497dbfb7c3 Use get_or_insert_with in common_cache() (Martin Habovstiak)
ca80a5a030 Use get_or_insert_with in taproot_cache (Martin Habovstiak)
6e06a32ccc Wrap ErrorKind in Io enum variant, fix doc comment for the IO variant (Riccardo Casatta)
1a2b54ff23 introduce constant KEY_VERSION_0 (Riccardo Casatta)
417cfe31e3 Derive common traits for structs and enum, make internal struct not pub (Riccardo Casatta)
55ce3dd6ae Fix validation error if SINGLE with missing corresponding output, remove check_index and check with get().ok_or(), more details in errors (Riccardo Casatta)
2b3b22f559 impl Encodable for Annex to avoid allocation (Riccardo Casatta)
1a7afed068 Add Reserved variant to SigHashType for future use (ie SIGHASH_ANYPREVOUT) (Riccardo Casatta)
53d0e176d3 Deprecate bip143::SigHashCache in favor of sighash::SigHashCache (Riccardo Casatta)
15e3caf62d [test] Test also sighash legacy API with legacy tests (Riccardo Casatta)
24acfe3672 Implement Bip341 signature hash, create unified SigHashCache for taproot, segwit and legacy inputs (Riccardo Casatta)
683b9c14ff add [En|De]codable trait for sha256::Hash (Riccardo Casatta)

Pull request description:

  Adds https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki message signature algorithm

  The base is taken from `bip143::SigHashCache`, some code results duplicated but I think it's more clear to keep things separated

  Would mark some bullet point on https://github.com/rust-bitcoin/rust-bitcoin/issues/503

  Test vectors are taken by running d1e4c56309/test/functional/feature_taproot.py with a modified `TaprootSignatureHash` function to print intermediate values that I cannot found in the bip341 [test vector json](https://raw.githubusercontent.com/bitcoin-core/qa-assets/main/unit_test_data/script_assets_test.json)

  UPDATE: Latest version includes the suggestion from @sanket1729 to create a unified tool for signature message hash for legacy, segwit, and taproot inputs. In particular, makes sense for mixed segwit v0 and taproot v1 inputs because cached values could be shared

ACKs for top commit:
  sanket1729:
    ACK c704ee7ffe. Reviewed the diff from a37de1ade475e0c31c932121abaa7aec701b9987 which I previously ACKed
  dr-orlovsky:
    utACK c704ee7ffe by diffing it to 6e06a32ccc having my ACK before.
  apoelstra:
    ACK c704ee7ffe

Tree-SHA512: 35530995fe9d078acd0178cfca654ca980109f4502c91d578c1a0d5c6cafacab7db1ffd6216288eac99f6a763776cbc0298cfbdff00b5a83e98ec4b15aa764e8
2021-09-15 17:47:17 +00:00
Vikas S Shetty 48c732e934 Changes for checking script size and returning Error appropriately 2021-09-15 16:16:20 +05:30
Dr Maxim Orlovsky c75f3ef4a8
Handling CHECKMULTISIG(VERIFY) ops in TapScript context; refactoring classifier 2021-09-14 13:58:16 +02:00
Martin Habovstiak 95fb4e01f9 Document cargo features
This documents cargo features in two ways: explictly in text and in code
using `#[doc(cfg(...))]` attribute where possible. Notably, this is
impossible for `serde` derives. The attribute is contitional and only
activated for docs.rs or explicit local builds.

This change also adds `package.metadata.docs.rs` field to `Cargo.toml`
which instructs docs.rs to build with relevant features and with
`docsrs` config activated enabling `#[doc(cfg(...))] attributes.

I also took the opportunity to fix a few missing spaces in nearby code.
2021-09-14 12:24:57 +02:00
sanket1729 b0ad6748e4 Add tests for opcode classification 2021-09-13 07:45:15 -07:00
sanket1729 c252b36786 Add CHECKSIGADD and update classify API 2021-09-13 07:45:00 -07:00
Dr. Maxim Orlovsky 697e8f5194
Merge pull request #626 from visvirial/impl-vsize
Implement `Block.get_strippedsize()` and `Transaction.get_vsize()`
2021-09-11 00:09:22 +02:00
Dr Maxim Orlovsky ecc400826c
Updating Script::is_witness_program to use new WitnessVersion 2021-08-10 10:34:15 +02:00
Dr Maxim Orlovsky 64c1ec0b76
WitnessVersion type 2021-08-10 10:34:15 +02:00
Dr. Maxim Orlovsky 4e3c2c32fc
Merge pull request #632 from tcharding/prefixes 2021-08-09 23:26:15 +02:00
Dr. Maxim Orlovsky 33393e0bf4
Merge pull request #621 from RCasatta/verify_with_amount 2021-08-09 23:23:09 +02:00
Dr. Maxim Orlovsky 808e1708c3
Merge pull request #596 from RCasatta/script_ser 2021-08-09 23:22:06 +02:00
Tobin Harding adc1543e7c
Use consts for address prefix values
Instead of using magic numbers we can define constants for the address
prefix bytes. This makes it easier for future readers of the code to see
what these values are if they don't know them and/or see that they are
correct if they do know them.
2021-07-22 10:31:14 +10:00
Riccardo Casatta 15e3caf62d
[test] Test also sighash legacy API with legacy tests 2021-07-21 12:05:37 +02:00
Devrandom 4826d0c6cc no_std support
Based on the original work by Justin Moon.

*MSRV unchanged from 1.29.0.*

When `std` is off, `no-std` must be on, and we use the [`alloc`](https://doc.rust-lang.org/alloc/) and core2 crates. The `alloc` crate requires the user define a global allocator.

* Import from `core` and `alloc` instead of `std`
* `alloc` only used if `no-std` is on
* Create `std` feature
* Create `no-std` feature which adds a core2 dependency to polyfill `std::io` features. This is an experimental feature and should be
used with caution.
* CI runs tests `no-std`
* MSRV for `no-std` is 1.51 or so
2021-07-15 09:04:49 +02:00
Vis Virial cdf7be4765
Add extra checks for `test_segwit_transaction()`. 2021-06-29 07:59:22 +09:00
Vis Virial 2bda871628
Remove `#[inline]` from `Transaction.get_strippedsize()`. 2021-06-29 07:40:13 +09:00
Vis Virial c9dead410a
Implement `Transaction.get_strippedsize()`.
`Block.get_strippedsize()` is also simplified and optimized.
2021-06-29 07:34:37 +09:00
Vis Virial 1bf9147a6e
Optimize `Transaction.get_vsize()` (thanks @TheBlueMatt). 2021-06-29 07:14:01 +09:00
Vis Virial 2085dc32a7
Refactoring: define `Block.get_base_size()`. 2021-06-28 20:07:07 +09:00
Vis Virial 4ac9cef9e9
Implement `Block.get_strippedsize()` and `Transaction.get_vsize()`. 2021-06-28 20:03:42 +09:00
Riccardo Casatta a86bced568
use Amount type in verify 2021-06-21 14:43:09 +02:00
Sebastian e334c9deea
Merge pull request #616 from sgeisler/2021-06-dust-value-amount
Use `Amount` type for dust value calculation
2021-06-16 20:51:23 +02:00
Riccardo Casatta 4a4460b1a3
Add test for script serialize/deserialize 2021-06-16 10:17:56 +02:00
Riccardo Casatta fadd368911
use different ser/de for Script in case of non human readable format 2021-06-16 10:17:40 +02:00
Sebastian Geisler 9981da2ec8 Use `Amount` type for dust value calculation 2021-06-12 21:15:13 +02:00
Devrandom 95aa3bf153 std -> core 2021-06-11 17:28:04 +02:00
Sebastian d7eb15c6f1
Merge pull request #584 from darosior/max_tx_weight_const
Introduce some policy constants from Bitcoin Core
2021-05-29 02:49:46 +02:00
Andrew Poelstra 052aaf1d80
Merge pull request #559 from LNP-BP/fix/error-derives-2
Fix/error derives 2
2021-05-19 15:02:10 +00:00
Antoine Poinsot 2e9d62a9c7 blockdata/script: use policy's constant in dust computation
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
2021-05-18 14:52:58 +02:00
Antoine Poinsot 2b777485fa policy: introduce p2p constants from the reference implementation
This introduces some constants defined by Bitcoin Core which as a
consequence define some network rules in a new 'policy' module.

Only some were picked, which are very unlikely to change. Nonetheless a
Warning has been put in the module documentation.

Script-level constants are left into rust-miniscript where they are
already defined (src/miniscript/limits.rs).
2021-05-18 14:52:56 +02:00