55c627715f Moving globals into PSBT struct (Dr Maxim Orlovsky)
Pull request description:
I took the most non-invasive approach to reduce diff size. Many parts of the code can be improved in style or further refactored (like some functions are not necessary and can be just moved to be part of other functions), but I'd prefer to do that as a separate PR once this will be merged.
My approach with this PR:
1. Remove `Global` struct by moving its fields right into `PartiallySignedTransaction` - but keep the `util/psbt/map/global.rs` file with all its logic
2. Keep existing `Map for Global` implementation in the same file, but just change it to `Map for PartiallySignedTransaction`
3. With serialization, convert `Global` deserialization into crate-private function and use it from `PartiallySignedTransaction` deserialization
4. Refactor the tests and imports as required to get the thing compile and pass tests
The refactoring will be followed by PR(s) adding support for Taproot
ACKs for top commit:
apoelstra:
ACK 55c627715f
sanket1729:
ACK 55c627715f . Reviewed range diff with ac0c908 that I previously ACKed
Tree-SHA512: 79b329b6e4e60af905e4e00507d6abc558261d921bcf8f5d4ee34dd685322d7a529b18015423da50a388ba6732b7b662a92bc95ad078228cc809254ad010d467
Ambiguous TweakedPublicKey and UntweakedPublicKey type aliases and methods to convert
Use structs for Untweaked and Tweaked key type
swap dangerous api to work on tweaked keys
remove unecessary allocations and rename methods
Use type alias for UntweakedPublicKey
TweakedPublicKey::new(...) method added
minor naming and doc changes
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.
5d71a9dd89 Correct input length check for uin128 fuzzer (Matt Corallo)
9c256cc88e Add a fuzz check for `Uint128::increment` (Matt Corallo)
a15f263c4e Move the `increment` fn into the uint macro to add it to Uint128 (Matt Corallo)
d52b88b525 Fix increment of Uint256 with carry (carolcapps)
Pull request description:
This is #578 with review feedback addressed.
ACKs for top commit:
apoelstra:
ACK 5d71a9dd89
sanket1729:
ACK 5d71a9d
Tree-SHA512: 32e5ea6387943ecad8f190a0de336a545fda72b6ff7388d3479037a5f880434276a7d0607f5cf61710d45e984c01954f4e3199a60c542be48b397717afb3d406
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
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.
This refactors the code to make it possible to use `get_or_insert_with`
instead of unwrapping in `segwit_cache()`. To achieve it `common_cache`
is refactored into two functions: one taking only the required borrows
and the original calling the new one. `segwit_cache` then calls the new
function so that borrows are OK.
Apart from removing unwrap, this avoids calling `common_cache` multiple
times.
There was a question whether this is equally performant. There are
multiple good reasons why it should be:
1. `get_or_insert_with` is marked `#[inline]`
2. Any good optimizer will inline a function that is used exactly once
3. 1 and 2 conclude that the closure will get inlined
4. Computing self.tx can then be moved to the only branch where it is
required.
5. Even if get_or_insert_with didn't get optimized, which is extremely
unlikely, the `tx` field is at the beginning of the struct and it
probably has pointer alignment (`Deref` suggests it's a pointer).
Alignment larger than pointer is not used, so we can expect the
fields to be ordered as-defined. (This is not guaranteed by Rust but
there's not good reason to change the order in this case.) We can
assume that offset to tx is zero in most cases which means no
computation is actually needed so the expression before closure is
no-op short of passing it into the closure as an argument.
At the time of writing `#[inline]` can be seen at
https://doc.rust-lang.org/src/core/option.rs.html#933
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.
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
Fixes#608. In #567 the Display impl for ChildNumber was
consciously changed, assuming the semver break would not
affect any correctly implemented downstream projects. We
were wrong.
This is instead of encode::Errors because the encoders should
not be allowed to return errors that don't originate in the writer
they are writing into.
This is a part of the method definition that has been relied upon for a
while already.
Marko Bencun
sanket1729
Dr. Maxim Orlovsky
Noah Lanson
Tobin Harding
Andrew Poelstra
Clark Moody
Vikas S Shetty
Martin Habovstiak
Riccardo Casatta
Devrandom
elsirion
Sebastian Geisler
Sebastian
Matt Corallo
Roman Zeyde
Steven Roose
carolcapps
Jeremy Rubin
Antoine Poinsot
Nadav Ivgi