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
3717a549f9 hashes: Fix stale repository name (Tobin C. Harding)
Pull request description:
The repository name is stale since we moved the `hashes` crate into the `rust-bitcoin` repo a while ago.
ACKs for top commit:
apoelstra:
ACK 3717a549f9
Kixunil:
ACK 3717a549f9
Tree-SHA512: ac4553547f1912c8242e3b87d3cc8951598999b5512ad1b49494b3c504449939e0e60905d96464b22c71b67ca975d18814a92af6e0aa66ff4f46effb97ac0733
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
0046bb8ad8 Fix usage of cfg(rust_1_53) (Tobin C. Harding)
c3450f3913 Remove stale usage of doc(cfg) (Tobin C. Harding)
Pull request description:
These build cfg options are not features, fix broken usage. And remove stale docsrs attribute while we are at it. Bad rust-bitcoin devs.
Found while reviewing #1870
ACKs for top commit:
apoelstra:
ACK 0046bb8ad8
Kixunil:
ACK 0046bb8ad8
Tree-SHA512: 7053affef6654ff203c93590bf081e165f019feb040aa8c55259ffe4e15eaf0e7522f6e5a4f6f62e8f578420b0313f4b7b17c46b741b7fcfd05750e5c5976589
3af9258025 embedded: Document how to clean up linker flags (Tobin C. Harding)
Pull request description:
Our embedded crate includes instructions to source a shell script that sets the `RUSTFLAGS` env var. Having the env var set like this in ones environment breaks linkage when trying to do "regular" builds.
Document how to clean up.
ACKs for top commit:
apoelstra:
ACK 3af9258025
Kixunil:
ACK 3af9258025
Tree-SHA512: 72758fba4dede873da299f01d75fd64b549fe21f954c2720ba3b7fc3c29fd4ed28fd0a749bbe987a7de1551aa32192253dd0033a18c877e877c9960343a5b07c
ca06d64455 rename bitcoin-private to bitcoin-internals (Andrew Poelstra)
Pull request description:
Since we now have control over the name on crates.io.
ACKs for top commit:
tcharding:
ACK ca06d64455
Kixunil:
ACK ca06d64455
Tree-SHA512: 8b3285b5ce9e3f1788ab9a3efa29b3e96f1f21dcfc0bb13f5940caf22cd43f74463b28bd85309e0200956dc11fe0102fd041b79bcd5f45fb8edb8449893bd287
Our embedded crate includes instructions to source a shell script that
sets the `RUSTFLAGS` env var. Having the env var set like this in ones
environment breaks linkage when trying to do "regular" builds.
Document how to clean up.
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`.
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
b03c24db8c Add a checked version of weight mul fee_rate (yancy)
Pull request description:
Add a checked version of fee_rate * weight. While I like the trait version of just being able to multiply `feerate * weight`, it's not really very useful imo since a large input feerate could cause an overflow. Instead of changing the trait in https://github.com/rust-bitcoin/rust-bitcoin/pull/1849 (not idiomatic enough I guess) I added a `checked_weight_mul` method to `FeeRate`.
ACKs for top commit:
apoelstra:
ACK b03c24db8c
Kixunil:
ACK b03c24db8c
Tree-SHA512: 231ade94291becadcea9ea2a40a5daf96b77f01a29cca2494d7bbe4f7de5b412fa8fc816ea249268569f5378410185d9349fd687533bf3a422a752997e107a2b
c2f33f60b8 Add changelo entry for removal of FromHex (Tobin C. Harding)
Pull request description:
During the last round of releases (bitcoin 0.30, hashes 0.12) we removed the `FromHex` implementation from all types except vecs and arrays. We added `FromStr` impls for types that roundtrip with `Display`.
We never added a changelog mention to either `bitcoin` or `hashes`, lets retroactively add an entry.
Fix: #1747
ACKs for top commit:
Kixunil:
ACK c2f33f60b8
apoelstra:
ACK c2f33f60b8
Tree-SHA512: 7affc492104be2be17861c55511407e66d8c821f35b18eeb62f6852a669253d23af67cbd44323ff568d2f143af57e37212455c7af3ed0b1d58313135b0783ea6
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.
During the last round of releases (bitcoin 0.30, hashes 0.12) we removed
the `FromHex` implementation from all types except vecs and arrays. We
added `FromStr` impls for types that roundtrip with `Display`.
We never added a changelog mention to either `bitcoin` or `hashes`, lets
retroactively add an entry.
Fix: #1747
6cec19f6d3 Update documentation of `InputWeightPrediction` (Martin Habovstiak)
6fec1789b9 Fix associated constants of `InputWeightPrediction` (Martin Habovstiak)
Pull request description:
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.
Closes https://github.com/rust-bitcoin/rust-bitcoin/issues/1834
ACKs for top commit:
apoelstra:
ACK 6cec19f6d3
tcharding:
ACK 6cec19f6d3
Tree-SHA512: 4e05be2c05795e8c4f9c67374491be71d2bde8233446f22bf458cfeb542d81043f03f90f65e59e769fce8275925dff28fc3dfce48fd62bd20be7fca22b71ca3c
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
0555695757 Fix formatting (Martin Habovstiak)
Pull request description:
This slipped in and is breaking builds.
ACKs for top commit:
RCasatta:
ACK 0555695757
apoelstra:
ACK 0555695757
Tree-SHA512: de93667f10fa99c51f1119353e3d59c587b7eb737a9bb92355e1f57bf2beadd1515133431bd510e6dd9b81a0d08fdd263e663117cae288eec92d207a9dabc0b3
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.
a1aaf5f12c ci: fix run syntax in fuzz job (Andrew Poelstra)
Pull request description:
I think our CI failures started with #1821 when we added an extra `run:` line to `fuzz.yml` without prefixing it with `-`.
Fix the syntax to use a multi-line `run` instead.
ACKs for top commit:
tcharding:
ACK a1aaf5f12c
Kixunil:
ACK a1aaf5f12c
Tree-SHA512: 7214291dc629b6417f8dfaf4a68bfcdc23078a57759717cef92344e1873d475f597fe447e037d15c568a1d5b04ec764ebf8477d18274f52a4b93c50d8a1615f5
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
639c548aed docs: Add doc comments for external crates (yancy)
Pull request description:
A new version of rust nightly is causing a test failure for external crates that are not documented. This is currently causing a failure on my branch here: https://github.com/rust-bitcoin/rust-bitcoin/pull/1838
As a side note, it feels like this should be locked to a specific version of nightly in the CI tests and then updated as a choir instead of having it create a new problem in a branch that's not related.
ACKs for top commit:
apoelstra:
ACK 639c548aed
tcharding:
ACK 639c548aed
Tree-SHA512: 764999edc448bff88013301bef287b16ac750af560931470257f2b4475747d52546e73d7fd9a7bdf2cedb18178f370150351e40f176f3da929edf686f02f5c1a
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
2f7bf1e7be readme: Document that we do not support altcoins (Tobin C. Harding)
5d66f72e5c readme: Enforce 100 column width (Tobin C. Harding)
Pull request description:
Improve the readme by doing:
- Patch 1: Enforce column width to 100
- Patch 2: Update policy on altcoin support (or lack of)
Excuse the OCD leading to patch one, its formatting only no content change.
ACKs for top commit:
Kixunil:
Enthusiastic ACK 2f7bf1e7be
apoelstra:
ACK 2f7bf1e7be
Tree-SHA512: e63a2fba5a2334742bd6f701b04d37af8ffcc85de71e0ca40457a1198eff2d26b673a718a352ba2129e5168c69ee723c78dc45ecd3dda9eaed9446ae7f4df2e0
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.
6cab7beba3 Deprecate min/max_value methods (Tobin C. Harding)
5fbbd483ea Use MIN/MAX consts instead of min/max_value (Tobin C. Harding)
3885f4d430 Add MIN/MAX consts to amounts (Tobin C. Harding)
Pull request description:
The new MSRV (1.48.0) uses associated consts MAX/MIN instead of functions, we had functions to be compliant with the old MSRV.
~Remove all methods `min_value` and `max_value` including calls to these methods on stdlib types.~
PR is now split into three patches:
- patch 1: Add missing associated consts MIN/MAX as needed
- patch 2: Use consts instead of method calls
- patch 3: Deprecate methods `min_value` and `max_value`
ACKs for top commit:
sanket1729:
ACK 6cab7beba3
apoelstra:
ACK 6cab7beba3
Kixunil:
ACK 6cab7beba3
Tree-SHA512: 60949d1bb971e0dfbab7f573b4447f889b5fa1a5f1c9ac9325a2970fe17a19ccc93418dba57f07bed7e13864b130de48b6b3741d1d80266c6144237dd4565ff7
Andrew Poelstra
startup-dreamer
Tobin C. Harding
yancy
Martin Habovstiak
Riccardo Casatta
sanket1729
kshitjj
Wilmer Paulino