Error types conventionally include `Error` as a suffix.
Rename `NonStandardSighashType` to `NonStandardSighashTypeError`.
While we are at it make the inner type private to the crate, there is no
need to leak the inner values type.
07041d584d Apply rustfmt (The rustfmt Tyranny)
dada6d65b7 script: Move some inspector methods from ScriptBuf to Script (Steven Roose)
Pull request description:
Noticed that these methods belong in Script.
ACKs for top commit:
tcharding:
ACK 07041d584d
sanket1729:
ACK 07041d584d.
apoelstra:
ACK 07041d584d
Tree-SHA512: cdcbdf22f0457123205621ec2834164c4598be1e5b221cf859d60e88110b19f8c1e484e86f60653af237e9c2acbcdbe5d2b4c98ccf239924386639c4ba6222f7
As part of an ongoing effort to make our error types stable and useful
add a stand set of derives to all error types in the library.
`#[derive(Debug, Clone, PartialEq, Eq)]`
Add `Copy` if possible and the error type does not include
`#[non_exhaustive]`.
If an error type includes `io::Error` it only gets `#[derive(Debug)]`.
bb8bd16302 internals: Remove hex module (Tobin C. Harding)
2268b44911 Depend on hex-conservative (Tobin C. Harding)
db50509cd3 Add usage docs to the "core2" feature (Tobin C. Harding)
Pull request description:
Use the newly released `hex-conservative` crate, by doing the following:
- Depend on `hex-conservative` in `bitcoin` and `hashes`
- Re-export `hex-conservative` as `hex` from both crate roots.
- Remove all the old hex code from `hashes`
- Remove all the old hex code from `internals`
- Remove the now unused `internals::prelude`
- Fix all the import statements (makes up the bulk of the lines changes in this patch)
ACKs for top commit:
apoelstra:
ACK bb8bd16302
sanket1729:
utACK bb8bd16302
Tree-SHA512: ec83b3941cae6f32272471779f28461bb04959a3f6a126a68bbf2c748d83ff9518ff8932d9e937a6f389c10028bf3eb58c6b6d71ea066924dd7a34faaec7a087
1edc5f4098 Fill in deprecated since NEXT-RELEASE placeholder (Tobin C. Harding)
Pull request description:
As we gear up for the v0.31.0 release we can fill in the NEXT-RELEASE placeholders.
ACKs for top commit:
sanket1729:
utACK 1edc5f4098
apoelstra:
ACK 1edc5f4098
Tree-SHA512: 61a78e9df0d563a1ebefef3d6c3447b767c5d878f05b45e9a20aecdcef897cc9d8c40d499882a7b8d74d5e99b25a256946c39edf2f05abab370cc0223346f66b
5c8933001c Avoid serialize inner data in RawNetworkMessage (Riccardo Casatta)
bc66ed82b2 Impl Encodable for NetworkMessage (Riccardo Casatta)
8560baaca2 Make fields of RawNetworkMessage non public (Riccardo Casatta)
Pull request description:
This PR removes the need to serialize the inner NetworkMessage in the RawNetworkMessage encoding, thus saving memory and reducing allocations.
To achieve this payload_len and checksum are kept in the RawNetworkMessage and checksum kept in CheckedData, to preserve invariants fields of the struct are made non-public.
ACKs for top commit:
apoelstra:
ACK 5c8933001c
tcharding:
ACK 5c8933001c
Tree-SHA512: aca3c7ac13d2d71184288f7815449e72c4c04fc617a65effba592592ef4ec50f18b6f83dbff58e9c4237cb1fe8e7af52cd43db9036658bdaf7888c07011e46cc
This type was defined in the `transaction` module because it was
originally used in a function that had been deprecated in favour of
moving the logic to the `sighash` module.
We just removed the deprecated code so we can now move this type to the
`sighash` module where it is used.
We only keep deprecated code around for one release so we can now remove
code deprecated in v0.30.0
Done in preparation as we gear up for v0.31.0 release.
RawNetworkMessage keep the payload_len and its checksum in the struct, thus
is not needed to serialize the inner network message
pub in fields of both RawNetworkMessage and CheckedData are removed so that
invariant are preserved.
provide accessor method and new for downstream libs.
This is done in order to more easily change the struct without impacting
downstream and also in order to add another field while preserving struct
invariant in future commit.
We have just released the `hex-conservative` crate, we can now use it.
Do the following:
- Depend on `hex-conservative` in `bitcoin` and `hashes`
- Re-export `hex-conservative` as `hex` from both crate roots.
- Remove all the old hex code from `hashes`
- Fix all the import statements (makes up the bulk of the lines changed
in this patch)
7b402e930c schemars: Add pinning docs (Tobin C. Harding)
0848ab7e25 Fix clippy warnings for embedded build (Tobin C. Harding)
5b1443a91c hashes/embedded: Add script dir and README (Tobin C. Harding)
94732aecbf Add patch section to test crates (Tobin C. Harding)
512d982275 Remove path field from internals dependency (Tobin C. Harding)
Pull request description:
Do a bunch of infrastructure fixes that either are needed for adding additional crate deps (hex) or updating deps (internals, hashes), or just make the tests more maintainable.
ACKs for top commit:
apoelstra:
ACK 7b402e930c
sanket1729:
ACK 7b402e930c
Tree-SHA512: 9349bb20225363914acc774cca672a23e6562fb02aea644777c558074d5eeb65289d68a93b5be59a93e9b32167e2494f6599caedc8a0d9cfbee2f94d406edbfc
6640074d34 bitcoin/bip32: add DerivationPath::to_u32_vec (Marko Bencun)
Pull request description:
This is useful to pass the keypath to other libraries which expect it to be represented with a list of u32 ints.
Fixes#1944
ACKs for top commit:
apoelstra:
ACK 6640074d34
RCasatta:
ACK 6640074d34
Tree-SHA512: c2327716370558dd9d7e0419f898707ba5e56555284ea7ca746c973080061aae53674b41d8fe7c68a00d7c4bec1e4bb53e8991141749a87dfa40febe9f456369
e30c492faf witness: clean up Debug implementation (Andrew Poelstra)
Pull request description:
The previous code seems to have been rebased/iterated on too many times, and had room for significant simplification. By inlining the indentation logic we can eliminate 40 LOC and also clean up the output by removing trailing spaces.
Fixes#1937
It is not good form to add unit tests for debug output but you can test this locally with the patch
```
diff --git a/bitcoin/src/blockdata/witness.rs b/bitcoin/src/blockdata/witness.rs
index d0b7408c..a2c38af0 100644
--- a/bitcoin/src/blockdata/witness.rs
+++ b/bitcoin/src/blockdata/witness.rs
@@ -619,6 +619,9 @@ mod test {
"304402207c800d698f4b0298c5aac830b822f011bb02df41eb114ade9a6702f364d5e39c0220366900d2a60cab903e77ef7dd415d46509b1f78ac78906e3296f495aa1b1b54101")
];
assert_eq!(witness.to_vec(), expected_witness);
+
+ println!("{:?}", witness);
+ panic!();
}
#[test]
```
And by sticking `{:#?}` in there to see the alternate output.
ACKs for top commit:
tcharding:
tACK e30c492faf
RCasatta:
ACK e30c492faf
Tree-SHA512: 0ec07885f5c75f3f34965852cf5b42b63290295d1f56e9fef7d5b3610b8ac8d318cbf8f184da5b8a9ed5b352bb2c0402797b41714cb9d5488e93c2e290340c2a
9787ba6c96 Rename Script::empty to Script::new (Tobin C. Harding)
Pull request description:
The `empty` constructor is mis-named for the following reasons:
- Non-uniform with `ScriptBuf::new`
- Non-standard with respect to stdlib which uses `Path::new` and `PathBuf::new` (on which we based the `Scritp`/`ScriptBuf`)
Rename the function to `new`, put it at the top of the impl block while we are at it.
ACKs for top commit:
apoelstra:
ACK 9787ba6c96
RCasatta:
ACK 9787ba6c96
Tree-SHA512: 2dee0f61fa9097a48369a0df802ebf238b00ad3e9ed520fbf31affa1cb2a1820cbb910b525be63513e4586acb2aa0b593cecddcad0b6cd894cdac0ba7fcf0871
Pull all the code that depends on `bitcoinconsensus` out into a separate
module `consensus::validation`.
Leave transaction testing of bitcoinconsensus code in the transaction
module.
There is not need to return the general `script::Error` from the
transaction verify functions. We can better describe the error path by
returning a custom error.
There is no need no nest the `bitcoinconsensus::Error` type within the
`script::Error`, it is the only error type returned by the verify
functions so just return it directly.
92749d29e4 Rename PartiallySignedTransaction to Psbt (Tobin C. Harding)
Pull request description:
Last release we added a type alias for `Psbt`, now lets just rename the type and be done with it.
Includes re-export at the crate root because `bitcoin::Psbt` is clear and obvious.
ACKs for top commit:
sanket1729:
ACK 92749d29e4.
apoelstra:
ACK 92749d29e4
Tree-SHA512: 2ded728409829709a46acd2a83ce9a91839bce222264b2fca122b346ec4f45a52c3f970eb05001794e2f355ce9391df1a184b57baf24589e8a5c3f77f72f6ec7
Last release we added a type alias for `Psbt`, now lets just rename the
type and be done with it.
Includes re-export at the crate root because `bitcoin::Psbt` is clear
and obvious.
The previous code seems to have been rebased/iterated on too many times,
and had room for significant simplification. By inlining the indentation
logic we can eliminate 40 LOC and also clean up the output by removing
trailing spaces.
71c0043127 Remove docsrs attributes (Tobin C. Harding)
Pull request description:
Somehow when we started using `doc_auto_cfg` we forgot to remove a bunch of docsrs attributes.
ACKs for top commit:
apoelstra:
ACK 71c0043127
sanket1729:
utACK 71c0043127
Tree-SHA512: 16ff8eec0f6cd392d496f8f07cc0773bbda28f7c71022ae6b5e2c47a98d40c94a9169c60c0d8fa5a819f07910593d65a47b69bdc748d64cda0aac3323e9599a6
Currently the test `hex` macro is only available when the `test`
compiler configuration option is set but we are using it in benches
code, this works for use because `cargo bench` sets `test` for the
current crate, however it breaks downstream crates.
Fix: #1830
d45dbef3e7 Manually implement Debug on Witness (Tobin C. Harding)
Pull request description:
The current derived debug implementation on `Witness` prints the content field as an array of integers. We can do better than this by manually implementing `Debug`.
With this applied `Witness` is printed as follows: (first line is `{:?}` and the next is `{:#?}`):
Using `{:?}`:
```
Witness: { indices: 3, indices_start: 8, witnesses: [[0x00], [0x02, 0x03], [0x04, 0x05]] }
```
Using `{:#?}`:
```
Witness: {
indices: 3,
indices_start: 8,
witnesses: [
[0x00],
[0x02, 0x03],
[0x04, 0x05],
],
}
```
ACKs for top commit:
sanket1729:
tested ACK d45dbef3e7. This would be helpful for debugging downstream.
apoelstra:
ACK d45dbef3e7
Tree-SHA512: eacf4fa8e3f38c4e9ddc45de78afb8eab5b5b196b77a6612f61860e0e4e7ba96de2e7f434b92816e0b00532e73c05378cafc046ec9c34108e9d9216fb36c524a
Add rustdocs to `WitnessProgram` commenting on why we carry the witness
version number around with the witness program. This is mainly a dev
comment but it helps document the invariants so make it a rustdoc
comment.
From BIP 141:
> A scriptPubKey (or redeemScript as defined in BIP16/P2SH) that
> consists of a 1-byte push opcode (for 0 to 16) followed by a data push
> between 2 and 40 bytes gets a new special meaning. The value of the
> first push is called the "version byte". The following byte vector
> pushed is called the "witness program".
`WitnessVersion` and `WitnessProgram` are scriptPubkey concerns and
scriptPubkey is basically synonymous with address so in one way it makes
sense that these types are in `address` however we are in the process of
overhauling the `Address` (and `AddressInner`) types so lets move the
witness stuff to `script` and put it in individual sub-modules.
This move helps simplify the address error type also.
Note please, there are a bunch of formatting changes in here in the
error type that I cannot explain and could not remove.
The current derived debug implementation on `Witness` prints the content
field as an array of integers. We can do better than this by manually
implementing `Debug`.
With this applied `Witness` is printed as follows: (first line is `{:?}`
and the next is `{:#?}`):
Using `{:?}`:
```
Witness: { indices: 3, indices_start: 8, witnesses: [[0x00], [0x02, 0x03], [0x04, 0x05]] }
```
Using `{:#?}`:
```
Witness: {
indices: 3,
indices_start: 8,
witnesses: [
[0x00],
[0x02, 0x03],
[0x04, 0x05],
],
}
```
The `empty` constructor is mis-named for the following reasons:
- Non-uniform with `ScriptBuf::new`
- Non-standard with respect to stdlib which uses `Path::new` and
`PathBuf::new` (on which we based the `Scritp`/`ScriptBuf`)
Rename the function to `new`, put it at the top of the impl block while
we are at it.
Expose signature verification functionality for ECDSA signatures on the
`PublicKey` type.
We should have an identical function on `XOnlyPublicKey` but this will
have to be done in `secp2561`.
7cdc90565f Mutate mul_u64 with mutagen (Tobin C. Harding)
Pull request description:
Add the `mutate` attribute to mutate `mul_u64`. Add non-doc comments listing the two false positives. These are identical but we list them twice so when devs grep for `mutagen false pos` the same number of lines for each function is displayed as is displayed by the `mutagen` run. This coding false positives thing is also introduced in PR #1655.
ACKs for top commit:
apoelstra:
ACK 7cdc90565f
sanket1729:
utACK 7cdc90565f
Tree-SHA512: d066beb2f9ba198f5af36258ba15cfbd2c7c9ce7596f6340ed1fe2f62a2b0b5296eeb2cb4be30146d019671f1858521c29db917936895b5b3fd36bdb0bd07e57
9f7449b572 Use from_int_btc function for const context (yancy)
f93e67977a Add from_int_btc function to Amount (yancy)
Pull request description:
Followup PR from https://github.com/rust-bitcoin/rust-bitcoin/pull/1811
Added a `const` associated function `from_int_btc()` for Amount. `panic()` in const context is only available after 1.57+ so a work around is provided.
ACKs for top commit:
tcharding:
ACK 9f7449b572
apoelstra:
ACK 9f7449b572
Tree-SHA512: 7755234f2e573577d754f0224083cb7acc059e58833790fe344b0d9bad0acd89b0f74054d9efcba2133576222c7e9ab8dc3d81ddc10fbdcd4f83638d697118c4
2b6bcf085c Implement support for `alloc`-free parse errors (Martin Habovstiak)
783e1e81dc Move `impl_std_error` to `bitcoin-internals` (Martin Habovstiak)
Pull request description:
This implements various helpers for parse errors that will not require `alloc`. This PR is useless while all of the crates require `alloc` and is thus a draft so that you can look at the design.
ACKs for top commit:
tcharding:
ACK 2b6bcf085c
apoelstra:
ACK 2b6bcf085c
Tree-SHA512: 776838a754b2c17263cf167c8cf8a3e69e51d8de45eb08d072ef930cbd1149360da2cb5fc430ce58f31c2b07dbf06c9f8384c567358873a3440e85632fcc2af8
6a18997e3c Removed only available in 1.46.0 line (TATHAGATA ROY)
Pull request description:
Fix: #1850
Removed "*Important: only available in Rust 1.46+*" on the file transaction.rs from lines 1288 and 1407 respectively.
ACKs for top commit:
Kixunil:
ACK 6a18997e3c
apoelstra:
ACK 6a18997e3c
tcharding:
ACK 6a18997e3c
sanket1729:
ACK 6a18997e3c
Tree-SHA512: 1395384ffe301b628687cc6d154e191b6a4415acd33eb4209065c5bf94115c3210ea1d28f7d7186e41665b39b5bebae849c3fa5394786ce24bdcd57b765cdbd3
d961b9c4ee Fix minor comments on count_sigops PR (junderw)
Pull request description:
Fixing some comments that were left on #1890
ACKs for top commit:
yancyribbens:
ACK d961b9c4ee
apoelstra:
ACK d961b9c4ee
tcharding:
ACK d961b9c4ee
Tree-SHA512: caa04428eb7c09915964e4a7bae2d1fca2426317f3620d16e73e992269a99d7adb3d360affb954a173835661a9960cf760d29ae9861816b1a898c01428b0f2d6
202d1cd581 Rename taproot::Error to SigFromSliceError (Tobin C. Harding)
29678cb82b Correctly document InvalidSighashType variant (Tobin C. Harding)
13d5c0536b Remove explicit error conversion (Tobin C. Harding)
d86517ae4f taproot: Use error variants locally (Tobin C. Harding)
Pull request description:
First three patches are preparatory cleanup, last patch renames `crypto::taproot::Error` to `SigFromSliceError`. See commit log for justification of the `Sig` prefix.
Done as part of the great error cleanup.
ACKs for top commit:
apoelstra:
ACK 202d1cd581
Kixunil:
ACK 202d1cd581
Tree-SHA512: 87aef07d2a3518c68c070e348d2331a9fbf1dc5cd36fd4d966607ddb0531eca9dc6be08f1923f941d33973f173b915490de9ef0cad724cce7bf108cdc8a82af0
638445f8a9 Feature: Add opcodes::All::decode_pushnum and Script::count_sigops (junderw)
Pull request description:
Planning to also add methods for the various parts of Transaction etc. to eventually allow for easier sigops calculation.
Bare multisig is making a comeback, which is causing a large amount of transactions' effective vSizes (for fee calculation) to be dependent on the sigop count.
This is a first step at making those transactions easier to estimate fees for / template blocks for etc.
ACKs for top commit:
Kixunil:
ACK 638445f8a9
tcharding:
ACK 638445f8a9
Tree-SHA512: 5e87d0f5ab58ed22ed50e43eac392b9b84ebccab5086553a6234d825766842057ab89bd0753f3c9de50d9ab17536182b8f64a57e8d5632a55494180f2cc26bbd
The `as_script_map` is a getter not a conversion function (to/into/as),
as such it should not include the prefix `as_`.
Deprecate `as_script_map` in favour of `script_map`.
This error type is only used in the `from_slice` function. Use prefix
`Sig` because `taproot::FromSliceError` does not fully express how the
error came about.
Use specific identifier for the error, this aids usage but also prevents
us later adding "random" other variants into this error and using it in
other functions.
06afd52a12 Improve hashes::Error (Tobin C. Harding)
Pull request description:
We are trying to make error types stable on the way to v1.0
The current `hashes::Error` is a "general" enum error type with a single variant, better to use a struct and make the error usecase specific.
Improve the `hashes::Error` by doing:
- Make it a struct
- Rename to `FromSliceError`
- Move it to the crate root (remove `error` module)
Includes usage in `bitcoin`.
ACKs for top commit:
apoelstra:
ACK 06afd52a12
Kixunil:
ACK 06afd52a12
Tree-SHA512: 20a517daaf3e9e09744e2a65cde6e238c8f2d1224899a6c04142a3a4e635d54112b0a2e846d25256071bb27cb70f7482380f98e9a535a5498aa4c7dc0d52cc54
0f74eb6876 Remove the unused crate::Error (Tobin C. Harding)
74154c2294 Add block::ValidationError (Tobin C. Harding)
3a9b5526b3 Move BlockHash From impls (Tobin C. Harding)
Pull request description:
Remove the `crate::Error` and replace its usage with `block::ValidationError`.
ACKs for top commit:
apoelstra:
ACK 0f74eb6876
Kixunil:
ACK 0f74eb6876
Tree-SHA512: 80b2c98d3d8f7c3f060c8ea2d94e1ebe118c07d0dcf91f6d13aed00df2cb0b15bf5e295ec0976d88d81e029cf7d3e8e4a1fe70120db57e49bbd8dd229291836b
Add a `ValidationError` to the `block` module and remove the two
variants out of `crate::Error`.
This error is only used by the `validate_pow` function, a specific error
better serves our purposes.
We are trying to make error types stable on the way to v1.0
The current `hashes::Error` is a "general" enum error type with a single
variant, better to use a struct and make the error usecase specific.
Improve the `hashes::Error` by doing:
- Make it a struct
- Rename to `FromSliceError`
- Move it to the crate root (remove `error` module)
Includes usage in `bitcoin`.
No idea why this re-export is here, the `Prevouts` type is not even used
in the `psbt` module.
Remove the re-export of `crate::sighash::Prevouts` from `pstb`.
013dffa65d tests: Use script hash methods (Tobin C. Harding)
Pull request description:
The `ScriptBuf` type can be serialized using it's `to_bytes` function. Do not use the `psbt::Serialize` trait to do so in test code.
No logic changes, since the impl of `psbt::Serialize` for `ScriptBuf` just calls `to_bytes`.
ACKs for top commit:
Kixunil:
ACK 013dffa65d
apoelstra:
ACK 013dffa65d
Tree-SHA512: 08959e065f1528f2ca69c12d5e34bceea3ddd17eefcee45094f071b3fa7357dbf289f6fe54d18fea02eecd1d0a7cd04598bbf014a5f10676387dbe27bb490395
We have methods to convert a script to a `WScritpHash` and `ScriptHash`,
no need to do this manually, let alone use the `psbt::Serialize` trait
to do so.
These constants had an error that they had `script_size` set to 0 which
was incorrect because it's not length of the script but serialized size.
Rather than just bumping the value this uses the `from_slice` method
which is less error-prone.
This also deletes a useless test of the constants.
Closes#1834
8835d5d2f1 make bip21 schema lowercase (Riccardo Casatta)
Pull request description:
The spec RFC3986 specifies the scheme is case insensitive and we were uppercasing to optimize QR code representation.
Unfortunately, common platform such as Android seems to fail to recognize uppercase schema, so for compatibility reason we use lowercase.
close#1843
ACKs for top commit:
Kixunil:
ACK 8835d5d2f1
apoelstra:
ACK 8835d5d2f1
Tree-SHA512: 02d228b52fe4df20edb71ba8e2ab8a2bae4b912252e30a3150ee3af74e65a6e91b165c9579273b57e894366c9792a8312ea973723cd8c5d98037aaba80d7cf07
6c6a89b1d1 Add sub-sat fractions parsing regression test (Martin Habovstiak)
f1a3dc6719 Allow parsing sub-sat denoms with decimal points (Martin Habovstiak)
b3d9a267ea Add a few more amount parsing tests (Martin Habovstiak)
Pull request description:
Numbers with only zeros after decimal points are valid if they are also
multiples of `10^precision` (e.g. 1000 for msats). These were
artificially disallowed as "too precise" which was at least misleading.
This change allows parsing such numbers.
And yes, I know this is not perfectly efficient (unless the compiler figures out some magic opts) but so isn't the rest of the code. TBH this parsing code drives me crazy and I'd love to rewrite it to be more efficient and readable.
ACKs for top commit:
apoelstra:
ACK 6c6a89b1d1
Tree-SHA512: 03cf4b416f2eac25e0aac57ef964ed06fa36c7fe8244bdcf97852cc58e1613b1ec6132379b834da58ad3240fdd61508a384202f63aa9ffa335c18cd7b2b724d3
The spec RFC3986 specifies the scheme is case insensitive and we were uppercasing
to optimize QR code representation.
Unfortunately, common platform such as Android seems to fail to recognize
uppercase schema, so for compatibility reason we use lowercase.
75b3f19b96 Move and rename TxOut default trait to a const called NULL (yancy)
Pull request description:
Create an associated constant `const TxOut::NULL` for consensus signing code and remove the default trait. Note I tried to deprecate the `default()` fn instead of just removing it but it doesn't seem to be possible. Also because `TxOut::NULL` is `const`, `ScriptBuf::new()` needed to be changed to `const fn`.
ACKs for top commit:
apoelstra:
ACK 75b3f19b96
Kixunil:
ACK 75b3f19b96
Tree-SHA512: ff61a2b1641a1ba32f183c27205af2d868dbc2eb47cf758c3d8315329d2c23e0b8a82ea0ab59d1de9add0d238f927165e2e4df014aab1ef066d74d4feda0700b
995c797e0d feat: generate PrivateKey (kshitjj)
Pull request description:
added a function to generate a private key
Resolves: #1823
ACKs for top commit:
apoelstra:
ACK 995c797e0d
tcharding:
ACK 995c797e0d
Tree-SHA512: 29ba54be8cb777e71a4683835686cbf2978b23736f629d7bbff468074235fece261ca170c23f358d1bd878987566d09e4488c3f1a106c59a5c8bdf52b98abffe
Numbers with only zeros after decimal points are valid if they are also
multiples of `10^precision` (e.g. 1000 for msats). These were
artificially disallowed as "too precise" which was at least misleading.
This change allows parsing such numbers.
8e6f953aa7 Expose valid (min, max) difficulty transition thresholds (Wilmer Paulino)
Pull request description:
Once `U256` was made private, we lost the ability to check whether a valid difficulty transition was made in the chain, since `Target` doesn't expose any operations. We only choose to expose `Shl<u32>` and `Shr<u32>` such that we can compute the min and max target thresholds allowed for a difficulty transition.
This is something we realized was missing after bumping to `rust-bitcoin v0.30.0` in `rust-lightning`, specifically for our `lightning-block-sync` crate. It may also be worth having a helper in `rust-bitcoin` that checks a header properly builds upon the previous, but that can be left for future work.
ACKs for top commit:
Kixunil:
ACK 8e6f953aa7
sanket1729:
ACK 8e6f953aa7 . Sorry, was confused by some details.
apoelstra:
ACK 8e6f953aa7
Tree-SHA512: 740dd64089426463dc6a19726d5a562276bd0966e0e31af8e1e67b28d18945644ac0e50be3cf0cc7fa604acc3d2c5b912a77a7caa69d8cff85f70fd57e5328c5
dff757d7db Comment predict_weight (yancy)
Pull request description:
I've been reading over the `predict_weight` function since it is one of the biggest challenges for coin-selection. IE choosing inputs and constructing an optimal selection strategy requires predicting the weight to get the best selection. It's great this work has been done but there are some things I don't understand well enough to comment.
1) why are we looking at the size of VarInt struct here
> let script_size = script_len + VarInt(script_len as u64).len()
2) [predict_weight_internal](36500b4451/bitcoin/src/blockdata/transaction.rs (L1245)) has a bunch of magic numbers. I'd like to be able to comment this as well but I don't fully understand that function.
Also, `Transaction.rs` is a big file and it seems like all of the prediction stuff could be moved to a separate module or maybe a separate crate?
ACKs for top commit:
tcharding:
ACK dff757d7db
Kixunil:
ACK dff757d7db
Tree-SHA512: 8ffa16d500075d691528ce1819b9352a148af431889bebbd7cddcf470bd4e3048ec53a5e778bc3659e33d8c25b68422a93dac1d46b9489ff56f41d88d7f05433
d57ec019d5 Use Amount type for TxOut value field (yancy)
Pull request description:
Propose using `Amount` type for the `TxOut` `value` field. I only implemented `Decodable ` and `Encodable` enough to compile but this needs to completed obviously if using `Amount` seems like a good idea.
ACKs for top commit:
tcharding:
ACK d57ec019d5
apoelstra:
ACK d57ec019d5
Tree-SHA512: df3fd55294d5f9392ca90bb920be8fbb9d7d285d97669412e07d5a099f70f81fd73e7e259679de9c8ce5c6c855e64f62213700f0fb7db415e0c706c509485377
ed6421c939 address: Add generic serde::Serialize for Address (Steven Roose)
814b9917da address: Add Sync, Send, Sized and UnPin marker traits on NetworkValidation (Steven Roose)
Pull request description:
With the new rewrite of Address, `serde::Serialize` is only implemented on `Address<bitcoin::address::NetworkChecked>` and `Address<bitcoin::address::NetworkUnchecked>`. But the compiler has no way of knowing that that are all the possible versions of `Address`, so the generic `Address<impl bitcoin::address::NetworkValidation>` doesn't implement `serde::Serialize`.
ACKs for top commit:
Kixunil:
ACK ed6421c939
tcharding:
ACK ed6421c939
Tree-SHA512: 65e43dff244c94fe08ccb2d985781a2687a1e2db186960a35d4ae89f3b31c5af66892630a3ebaac9cecdc83638487425afa17374869d278648b348869e0ba091
Once `U256` was made private, we lost the ability to check whether a
valid difficulty transition was made in the chain, since `Target`
no longer exposes any arithmetic operations.
Our previous MSRV did not support MIN/MAX associated consts so we had
methods min/max_value. Now that our MSRV is Rust 1.48.0 we can use the
consts.
Deprecate min/max_value methods in favor of MIN/MAX associated conts.
We currently use the functions `min_value` and `max_value` because the
consts were not available in Rust 1.41.1, however we recently bumped the
MSRV so we can use the consts now.
1c3bbd4bf2 internals: Remove attribution from all files (Tobin C. Harding)
99673ab5c4 hashes: Introduce SPDX license identifiers (Tobin C. Harding)
984fe69448 bitcoin: Remove attribution from all files (Tobin C. Harding)
Pull request description:
Please note, whether or not we need a per-file license comment is out of scope for this PR. This PR leaves us with the most simple per-file solution possible and leaves the merit of per-file license comment to be discussed on another day.
Simplify the per-file license stuff by doing:
- Remove the attribution line from each file.
Currently we have a mishmash of attribution lines accompanying the SPDX identifier. These lines are basically meaningless because:
- The date is often wrong
- The original author attributed is not the only contributor to a file
- The term "rust bitcoin developers" is basically just noise
- Introduce SPDX license identifiers into `hashes` and remove attribution line (ie, make `hashes` uniform with `bitcoin`)
Required before merge please:
- [x] ack from apoelstra because as the library original author many of the changes in this PR remove his name
- [x] ack from Kixunil because he had some concerns in the issue descussion
Fix: #1816
ACKs for top commit:
Kixunil:
ACK 1c3bbd4bf2
sanket1729:
ACK 1c3bbd4bf2
apoelstra:
ACK 1c3bbd4bf2
Tree-SHA512: c5ac05c5eb23b3b6a760f707c344b22f5871a4dedee4990b1840f57e4cee1d38560ff4507c354bbf29bc8ff05a179d95d7e100fcf19bd93c5362344a352c7b5a
Currently we have a mishmash of attribution lines accompanying the SPDX
identifier. These lines are basically meaningless because:
- The date is often wrong
- The original author attributed is not the only contributor to a file
- The term "rust bitcoin developers" is basically just noise
Just remove all the attribution lines and be done with it. While we are
at it add an SPDX line to the few files missing it, whether this license
nonsense is even needed is left as an argument for another day.
dd4ad9444e Hardcode expected weight in txin_txout_weight_tests (Peter Todd)
Pull request description:
Rational: the expected weight is fixed so this both ensures we don't accidentally change it somehow, and makes it easier to re-use these test cases in other codebases (eg python-bitcoinlib).
ACKs for top commit:
apoelstra:
ACK dd4ad9444e
tcharding:
ACK dd4ad9444e
Kixunil:
ACK dd4ad9444e
Tree-SHA512: 4769a4bb8695f4f4c95e258bb5f06a232090b14c3d9159d6d5de2d09d7fc934a1b920b90cc09677a88fc0cf37ac21ed27794692dff2c73df4252c9551dc10fc2
Rational: the expected weight is fixed so this both ensures we don't
accidentally change it somehow, and makes it easier to re-use these test
cases in other codebases (eg python-bitcoinlib).
a54e1ceab1 Apply rustfmt (The rustfmt Tyranny)
38d11ce3da ci: Make release CI search for NEXT.RELEASE instead (Steven Roose)
dad3abd20f transaction: Rename is_coin_base to is_coinbase (Steven Roose)
Pull request description:
Alternative to https://github.com/rust-bitcoin/rust-bitcoin/pull/1795.
Keep the old method as deprecated and add doc alias. Also change internal usage of the method.
ACKs for top commit:
tcharding:
ACK a54e1ceab1
apoelstra:
ACK a54e1ceab1
Tree-SHA512: 52d9729bf83da164556d960f8867cb836ff57a0f619da3dd3620efffb28a974aac23b8085863ab0e072a4bdb2f13ac576efa43ad2eec9a271ad044227f4d00a4
8f6317fbab Add predict_weight test for witness address types (yancy)
Pull request description:
Add a predict_weight test for address types with witness data
ACKs for top commit:
Kixunil:
ACK 8f6317fbab
apoelstra:
ACK 8f6317fbab
Tree-SHA512: 954908f068d6b0e8f7cc6bc6bffd110d490d7165cf2544ff0f936264761cb1f8e4da1e37fd5a6a34fd8b05f8d72817a3b340bbf968b29c9f538f10853347fdf0
fabcde036f Use package in manifest and shorten import (Tobin C. Harding)
Pull request description:
We can use `package` to rename `bitcoin_hashes` to `hashes` and `bitcoin_internals` to `internals`. This makes imports more terse with no loss of meaning.
ACKs for top commit:
apoelstra:
ACK fabcde036f
Kixunil:
ACK fabcde036f
Tree-SHA512: bc5bff6f7f6bf3b68ba1e0644a83da014081d8c6c9d578c21cb54fdd56a018f68733dd1135d05b590ba193ed9efd12fa9019182c1fed347e604d8548f6ef9103
29cb34eed7 Refactor Address struct and its methods (Harshil Jani)
Pull request description:
Closes#1755
In this PR the `as_unchecked` is added to the Address struct, which returns a reference to the same address but with the type Address<NetworkUnchecked>. Similarly, the `assume_checked_ref` is added to Address<NetworkUnchecked>, which returns a reference to the same address but with the type Address.
ACKs for top commit:
tcharding:
ACK 29cb34eed7
apoelstra:
ACK 29cb34eed7
Tree-SHA512: 75ba40883d9fb31026b0e94e8d3fdcf808ff38a4d10f61f99a1e14ddc48358da86bad44cd78563dc67aa5d39382a5493e73c03891317f361f590e39b6257cb84
91f45a214f Replace hardcoded values with compile-time hashing (Martin Habovstiak)
095b7958dd Make `sha256t_hash_newtype!` evocative of the output. (Martin Habovstiak)
Pull request description:
The Rust API guidelines state that macros should be evocative of the
output, which is a sensible recommendation. We already had this for
`hash_newtype!` macro but didn't for sha256t version.
This changes the macro to have this syntax:
```rust
sha256t_hash_newtype! {
// Order of these structs is fixed.
/// Optional documentation details here. Summary is auto-generated.
/*pub*/ struct Tag = raw(MIDSTATE_BYTES, LEN);
/// Documentation here
#[hash_newtype(forward)] // optional, default is backward
/*pub*/ struct HashType(/* attributes allowed here */ _);
}
```
Closes https://github.com/rust-bitcoin/rust-bitcoin/issues/1427
Depends on #1769
How do you like the syntax? Is weird `struct Foo = bar(..);` acceptable?
ACKs for top commit:
tcharding:
ACK 91f45a214f
apoelstra:
ACK 91f45a214f
Tree-SHA512: f6b555b20311a4c1cb097bc296c92459f6fbe16ba102d8c977eb2383dbcae3cc8ffce32e3cb771e7e22197fda26379971aa4feaadc5e2e70d37fa690ae8b8859
873501a85f Use slice patterns (Martin Habovstiak)
Pull request description:
Some code looks better with slice patterns. This changes `bip32` to use them.
ACKs for top commit:
apoelstra:
ACK 873501a85f
tcharding:
ACK 873501a85f
Tree-SHA512: 2e91b21d0f943c04f592112f241444ba3b1b1dad47ea6bea162f62cfbcaf98e383257cad144072b3807aaa0c122dabab638ac10b1cc1f5179727a5f05aac0659
This commit refactors the Address struct and its methods to improve
its functionality and usability.The AddressInner struct now holds
the payload and network, and the PhantomData<V> type is used to track
the network validation state.
Also as_unchecked and assume_checked_red methods are added to allow
conversion between checked and unchecked network validation state.
Signed-off-by: Harshil Jani <harshiljani2002@gmail.com>
Previous changes enabled passing the string used as a tag into
`sha256t_hash_newtype!` macro rather than hard-coding midstate. This
commit takes advantage of it and replaces the hard-coded values with
compile-time executed (`const`) hashing.
The Rust API guidelines state that macros should be evocative of the
output, which is a sensible recommendation. We already had this for
`hash_newtype!` macro but didn't for sha256t version.
This changes the macro to have this syntax:
```rust
sha256t_hash_newtype! {
// Order of these structs is fixed.
/// Optional documentation details here. Summary is auto-generated.
/*pub*/ struct Tag = raw(MIDSTATE_BYTES, LEN);
/// Documentation here
#[hash_newtype(forward)] // optional, default is backward
/*pub*/ struct HashType(/* attributes allowed here */ _);
}
```
Closes#1427
We've upgraded MSRV but didn't update clippy config, so some things that
could be improved aren't caught by clippy. This updates the config and
fixes the new issues.
I also `rg '1\.41\.1'`ed for interesting changes and found one
additional improvement.
a189942c64 Use doc_auto_cfg (Tobin C. Harding)
Pull request description:
If we use `#![cfg_attr(docsrs, feature(doc_auto_cfg))]` instead of `#![cfg_attr(docsrs, feature(doc_cfg))]` we no longer need to manually mark types with `#[cfg_attr(docsrs, doc(cfg(feature = "std")))]`.
Sweeeeeet.
Props to pezcore for the lesson :)
ACKs for top commit:
apoelstra:
ACK a189942c64
Kixunil:
ACK a189942c64
Tree-SHA512: 1ced1e09f5d1733b362b83ca650d3f52c89eb57e78e8437f74c496d89776548f8c50feab6750352342e2abe680434681de2c126ce36a81dda21397b9695d4d4e
Signed-off-by: Harshil Jani <harshiljani2002@gmail.com>
implementation of PartialEq<Address> for Address<NetworkUnchecked>
Signed-off-by: Harshil Jani <harshiljani2002@gmail.com>
If we use `#![cfg_attr(docsrs, feature(doc_auto_cfg))]` instead of
`#![cfg_attr(docsrs, feature(doc_cfg))]` we no longer need to manually
mark types with `#[cfg_attr(docsrs, doc(cfg(feature = "std")))]`.
Sweeeeeet.
ed80df5ebc Add `ChainHash::from_genesis_block_hash` (Martin Habovstiak)
Pull request description:
This improves readability of converting `BlockHash` into `ChainHash`. It's useful in e.g. Electrum protocol which sends `BlockHash` (serialized backward).
Closes#1751
ACKs for top commit:
apoelstra:
ACK ed80df5ebc
tcharding:
ACK ed80df5ebc
Tree-SHA512: b03eb9ca0a1eb38196cc8e43751c2def0d00b463b774003c5319bf56599db598ddd4bb0e2ad0bd3803f21a925135992d421848e7b1ba5e7bab3a405f60fc973c
We can use `package` to rename `bitcoin_hashes` to `hashes` and
`bitcoin_internals` to `internals`. This makes imports more terse with
no loss of meaning.
82b6332b91 create a set of recognized denomination forms (yancy)
Pull request description:
I took a stab at restricting the acceptable forms here. There was some consensus that "BtC" was confusing that was discussed in a previous pr https://github.com/rust-bitcoin/rust-bitcoin/pull/1715.
Also, personally I felt that the `PossiblyConfusingDenomination` enum variant was itself confusing. I think it's probably cleaner to just maintain a list of acceptable forms and treat everything else as unknown. For now I just created a const of possibly confusing forms.
ACKs for top commit:
Kixunil:
ACK 82b6332b91
apoelstra:
ACK 82b6332b91
Tree-SHA512: f3c212046e4d8a34ce2d50f34065b1778ae190ca80dd9ad73ef41eb5925705c88c0a764ec149d731eb1735d670c5542415953633a7f3f672d0c1e46b6e004d0b
This improves readability of converting `BlockHash` into `ChainHash`.
It's useful in e.g. Electrum protocol which sends `BlockHash`
(serialized backward).
Closes#1751
122188f7dd Use shorter import statements (Tobin C. Harding)
Pull request description:
Just patch 2, patch 1 is #1728
From the commit log of patch 2
Use shorter import statements
As per discussion [0] use the shorter form for importing crates that we
re-export (`hashes` and `secp256k1`).
[0] https://github.com/rust-bitcoin/rust-bitcoin/discussions/1661
ACKs for top commit:
apoelstra:
ACK 122188f7dd
sanket1729:
utACK 122188f7dd
Tree-SHA512: 3f540464d38c72ba9d68f8ceda8600540bd0c3eef0ba67531c87fa1e0e4f757af7035cf80a1a5f17aa05604a17fdd9ef59bb6bece6b4145d540dac1e5362fc01
Add the `mutate` attribute to mutate `mul_u64`. Add non-doc comments
listing the two false positives. These are identical but we list them
twice so when devs grep for `mutagen false pos` the same number of lines
for each function is displayed as is displayed by the `mutagen` run.
This coding false positives thing is also introduced in PR #1655.
Tobin C. Harding
Andrew Poelstra
The rustfmt Tyranny
Steven Roose
Riccardo Casatta
Marko Bencun
yancy
sanket1729
roy9495
junderw
Martin Habovstiak
startup-dreamer
TATHAGATA ROY
kshitjj
Wilmer Paulino
Peter Todd
Sergi Delgado Segura
Harshil Jani