WARNING: This is not like all the other extension traits.
Because of the use of generics on various `Transaction` methods it is
not easily possible to use the `define_extension_trait` macro.
Manually create the extension traits (public and private) for the
`Transaction` type. This is quite ugly but c'est la vie
(Includes two in the `transaction` module and one in the
`consensus_validation` module.)
Currently `script_pubkey_lens` returns a generic `Iterator` using `impl`
syntax. This syntax is not supported in traits and we want to move the
function to the soon-to-be-added `TransactionExt` trait.
Add a struct to hold the iterator returned by `Map`, this is ugly but
its the least ugly thing I could come up with.
Split the `Transaction` impl block into three parts:
- The bits going to `primitives`
- The bits staying in a public extension trait
- The bits staying in a private extension trait
Internal change only.
It is acceptable to use a wildcard import in bench code for the same
reasons it is acceptable in the `tests` module.
In preparation for introducing extension traits in the `transaction`
module use wildcard import in the module's bench code.
c89b816437 psbt: Fix bug in Subtype consensus_encode (Tobin C. Harding)
Pull request description:
In #2906 we switched from using a `u8` for type keys to using a `u64` and encoding as a compact int (inline with the spec). Note that a `u8` encodes to the same bytes as a `u64` when the value is < 252.
In that patch, I introduced a bug because the length returned by `PoprietaryKey::consensus_encode` uses a hard code 1 for the length of the encoding (because of single byte) instead of the variable length for the new compact encoding.
Bug showed up in fuzzing, and was isolated by Jamil - mad props.
Fix: #3501
ACKs for top commit:
jamillambert:
ACK c89b816437
apoelstra:
ACK c89b8164377123eb20476636f2f5271c6a687406; successfully ran local tests
Tree-SHA512: 1b61b6a9ece197d74038ceedb447fd3ca21db8e2a6a96c9281a99ac232c18c3ca55da8e3f46930401714d3575e9a406a36e4f44929ca963208a5df4be6b46cfb
025a8773bf Use fully qualified path in macro (Tobin C. Harding)
Pull request description:
Using fully qualified paths in macros reduces maintenance burden. We have one macro where we use relative path to access the `psbt` module.
Refactor only, no external change.
ACKs for top commit:
apoelstra:
ACK 025a8773bf63aacdaca011ef000f41a85a961567; successfully ran local tests; will one-ACK merge
Tree-SHA512: eb5923a48ae4d82499679a58375ef7d2e8ba85c91671e350f7be19f0372750a269f44dd2f05f4a70ed0c7f277b160400eb41ff1d42b90e6057f1344be7e11a89
66da2266e2 Explicitly re-export stuff from crates down the stack (Tobin C. Harding)
Pull request description:
Up until recently we were using wildcard re-exports for types moved to `units` and `primitives`. We have decided against doing so in favour of explicit re-exports.
Audit `units` and `primitives` using `git grep 'pub enum'` (and `struct`) and explicitly re-export all types.
Remove all wildcards except for the re-exports from `opcodes`, there are too many opcodes, explicitly re-exporting them does not aid clarity.
ACKs for top commit:
apoelstra:
ACK 66da2266e26dfe53947c4606e9d18620931e93cf; successfully ran local tests
Tree-SHA512: 74717f8b127e975e3d131aab884bdfe78e699d88b7ee1db7731ad117437d37684285264001cf6b2182eb1e565171167695e00c4b6aef28a3e26b69d9cebfbb74
9a7b1c232b Wrap the bech32 decoding error (Tobin C. Harding)
Pull request description:
In #2381 we attempted to fully encapsulate the `bech32` crate to help with stabalizing `rust-bitcoin` however we failed to notice the `address:ParseError` has a variant that includes `bech32`. Public enums have public variant internals in Rust. Also the `From<bech32::segtiw::DecodeError` makes `bech32` public.
Closes: #3043
ACKs for top commit:
apoelstra:
ACK 9a7b1c232b494dccdce091a46d916cc411a612a1; successfully ran local tests; will one-ACK merge since this is a gazillion years old and obviously right
Tree-SHA512: b5053aa43107aa47da1fe7e7db0f882cfb231b9769a7b67d8c930532c471df191f588bf98f2b00cc76d5a2e9c74e035ee96128da115363ac3952f96a766494ea
Using fully qualified paths in macros reduces maintenance burden. We
have one macro where we use relative path to access the `psbt` module.
Refactor only, no external change.
In #2906 we switched from using a `u8` for type keys to using a `u64`
and encoding as a compact int (inline with the spec). Note that a `u8`
encodes to the same bytes as a `u64` when the value is < 252.
In that patch, I introduced a bug because the length returned by
`PoprietaryKey::consensus_encode` uses a hard code 1 for the length of
the encoding (because of single byte) instead of the variable length for
the new compact encoding.
Bug showed up in fuzzing, and was isolated by Jamil - mad props.
We do not want `bech32` to appear in the public API of the `address`
module in case `bech32` does not stabalize before the soon-to-be-created
`address` crates does.
We already had a go at removing it but forgot one error variant - wrap
the variant in a new type with a private inner bech32 error field.
Up until recently we were using wildcard re-exports for types moved to
`units` and `primitives`. We have decided against doing so in favour of
explicit re-exports.
Audit `units` and `primitives` using `git grep 'pub enum'` (and
`struct`) and explicitly re-export all types.
Remove all wildcards except for the re-exports from `opcodes`, there are
too many opcodes, explicitly re-exporting them does not aid clarity.
5633b10f5c Manually implement compute_txid and compute_wtxid (Tobin C. Harding)
Pull request description:
We would like to move the `Transaction` type over to `primitives` including the `compute_txid` and `compute_wtxid` functions however currently the implementations, as expected, use `Encodable`.
Manually implement `Encodable` by hashing all the fields in the correct order.
Note we have unit tests already that check the output string of the txid returned so these act as regression tests for this patch.
ACKs for top commit:
apoelstra:
ACK 5633b10f5c826e0b2ac47dd85f697f12710898d7; successfully ran local tests; nice
Tree-SHA512: 66a955d3d896801cfefe0388aade3a31f22fac5b6da7b996be61f374b93772487c0c203320aaf5165fcef26874564bce375ecb364175b0a01c3008b7ea8db981
We would like to move the `Transaction` type over to `primitives`
including the `compute_txid` and `compute_wtxid` functions however
currently the implementations, as expected, use `Encodable`.
Manually implement `Encodable` by hashing all the fields in the correct
order.
Note we have unit tests already that check the output string of the txid
returned so these act as regression tests for this patch.
In preparation for moving the `TxIn` over to `primitives` make the
private `TxIn::BASE_WEIGHT` associated const into a file-scoped constant
because the other alternative is to make it public.
9c2ac46902 Split up ParseError (Jamil Lambert, PhD)
3d994f7bdb Decode an address string based on prefix (Jamil Lambert, PhD)
Pull request description:
When a decoding error occurs for a bech32 address string the error is discarded and the same address string is attempted to be decoded as base58. This then incorrectly returns a base58 error.
Check the string prefix and decode as bech32 or base58 and return the relevant error. If the prefix is unknown return an `UnknownHrpError`.
Close#3044
ACKs for top commit:
tcharding:
ACK 9c2ac46902
apoelstra:
ACK 9c2ac46902ae2e6f2513ee125ea5c89953ac89a2; successfully ran local tests
Tree-SHA512: 40c94328828af86723e84d4196e8949430fb9a15efd8865c18cb5048fe59b8a2514d97f4809d828353b78c010544a8a6d4589a8c9c7fbd75d9d0ecceb3151e8f
bbffa3db43 Remove the IO error from DecodeError (Tobin C. Harding)
713196be0d Return DeserError from encode::deserialize (Tobin C. Harding)
33566ac58c Split encode::Error into two parts (Tobin C. Harding)
b04142c745 Add encode::Error::MissingData variant (Tobin C. Harding)
5a42ef2850 Do not manually map IO error (Tobin C. Harding)
efd7f9f06c Add error constructor parse_failed_error (Tobin C. Harding)
ebfef3f114 Return generic error as Some (Tobin C. Harding)
a6254212dc Move consensus error code to submodule (Tobin C. Harding)
Pull request description:
The `consensus::deserialize` and `consensus::deserilaize_partial` functions should not return an I/O error. Doing so causes various other error types to include an `io::Error` and the `io::Error` is an annoying type to work with.
This PR is a bunch of steps, and it took me a good while with quite a bit of backtracking to get here. As such you may want to review the final state before looking at each patch.
The `consensus` errors can be further cleaned up but I'd prefer not to spend more time on this unless it has some chance of merging.
ACKs for top commit:
apoelstra:
ACK bbffa3db43802b30d23259c0372f16a877a0ef8b; successfully ran local tests
Tree-SHA512: 522fdd29638a214cb7fcee29dd3b9f5c846f041fba087a56a91b83e6d85f033cbed95f659dc4321cd4596943ff233bdd184cdfbfcc787fe89172bb93aa4ab186
Currently `combine_node_hashes` is an associated function, it is also
private. It is called from within other methods of the `TapNodeHash`.
In preparation for moving the `TapNodeHash` to `primitives` while
leaving all the methods in `bitcoin` in an extension trait; move the
associated function out of `TapNodeHash` and make it a stand alone
private function.
e7d326f071 Seal extension traits (Tobin C. Harding)
Pull request description:
The extension traits are temporary just while we try to stabalize `primitives`, they are not intended to be implemented by downstream.
Seal the extension traits so that downstream crates cannot implement them.
Fix: #3231
ACKs for top commit:
apoelstra:
ACK e7d326f071a368389f087ddb10ee9bbf3552c33a; successfully ran local tests; thanks! I know this is tedious and annoying
Tree-SHA512: 365979aeabb7941b9c8fa526f71aaadae3ab1cdd6a39e992c5eea2c1057b4b7c2b3a846ffd96a7eab47b9ad4e3e4de4fb141c24c62747e5cee45c74f52f9a172
The `DecodeError` (badly named) consensus decodes an object from an
iterator that implements `Read`. The `Read` impl never returns a real IO
error, we use the `io::Error` to temporarily wrap the error returned by
the inner iterator and unwrap it in `IterReader::decode`. As such there
is no reason for the `DecodeError` to hold an `encode::Error`, it can
hold an `encode::ParseError`.
The value of this change is easily seen in the removal of calls to
`unreachable`.
The `consensus::encode::Error` contains an IO error but reading from a
buffer only ever errors for EOF. We converted all instances of EOF to
`MissingData` already so now we can split the IO error apart from the
actual encoding errors variants.
The `io::Error` is troublesome because it contains a bunch of stuff that
never happens when reading from a buffer. However the EOF variant can
occur if the buffer is too short. As an initial step towards reducing
usage of the `io::Error` add a `MissingData` variant to the
`encode::Error` and when converting from an IO error map to
`MissingData` if EOF is encountered.
The `encode::Error::ParseFailed` variant holds an inner string, this is
suboptimal.
In an effort to patch the `encode::Error` while mimizing the diffs
required add a helper function that creates the variant. The benefit is
that later patches that effect this variant will only need to update the
constructor function and not every call site.
Internal change only.
The `consensus` module has a bunch of error types, move them all to a
separate module. Add re-exports so the types are still available at the
same place they were. Make the `error` module private and re-export all
errors from the `consensus` module root.
ParseError is too general and the functions returning it do not have an
error path for all variants.
Split out the Bech32 and Base58 related errors into their own enums.
The extension traits are temporary just while we try to stabalize
`primitives`, they are not intended to be implemented by downstream.
Seal the extension traits so that downstream crates cannot implement
them.
Fix: #3231
c1eccfde25 Move Witness to primitives (Tobin C. Harding)
6ce76cd7c8 Add unstable constructor to Witness (Tobin C. Harding)
Pull request description:
Patch 1 introduces a new policy to the codebase, we use `foo__unstable` for public unstable functions and there are zero semver guarantees if you call these functions.
Patch 2 does the move.
Close#3406
ACKs for top commit:
apoelstra:
ACK c1eccfde25fd4c2b19e7ec6759352b46ac246113; successfully ran local tests
Tree-SHA512: 2388066be2b6bb2cf3d6757c8f6334beeda6115ef1ce7c537955d32aa5e466add5162d0d2adee27f416fe622fc93c4e94bd848326463ee55e08d1c0f4e03719c
When a decoding error occurs for a bech32 address string the error is
discarded and the same address string is attempted to be decoded as
base58. This then incorrectly returns a base58 error.
Check the string prefix and decode as bech32 or base58 and return the
relevant error. If the prefix is unknown return an `UnknownHrpError`.
Move the `Witness` over to `primitives` leaving behind any method that
takes or returns a `Script` or a signature.
Includes addition of a feature gate to unit test.
Tobin C. Harding
merge-script
Fmt Bot
Jamil Lambert, PhD
Casey Rodarmor
ChrisCho-H