dec05b63e9 Refactor witness_version and is_witness_program (Tobin C. Harding)
dac552b436 Add unit tests for shortest/longest witness program (Tobin C. Harding)
Pull request description:
Refactor `witness_version` and `is_witness_program`.
- Patch 2 adds a couple of preparatory unit tests.
- Patch 2 does the refactor
Fix: #2618
ACKs for top commit:
apoelstra:
ACK dec05b63e9
sanket1729:
ACK dec05b63e9
Tree-SHA512: 3db0a1d8175cbb2fd18f3254854d02db3ad7efa2620b12f08d9727ef6bb5854f0a015917e57023cd2196a36d13276e80536a0e96318c44a1173da4f6793ca370
04715e3e60 absolute: make is_* methods uniform with the ones from relative (Andrew Poelstra)
878b865f85 relative locktime: introduce is_* methods to check units (Andrew Poelstra)
c2f87c7ab3 relative locktime: add is_implied_by method for sequences (Andrew Poelstra)
319e102fed relative locktime: use From/TryFrom to convert between relative locktimes and Sequence (Andrew Poelstra)
0ed26915f6 relative locktime: add conversions to/from sequence (Andrew Poelstra)
5c8fb5c11b relative locktime: add consensus encode/decode functions (Andrew Poelstra)
ac968e02b6 relative locktime: constify a bunch of constructors (Andrew Poelstra)
f27e675e1e relative locktime: add "obvious" constructors (Andrew Poelstra)
f02b1dac5b relative locktime: copy comments and PartialOrd impl from absolute locktimes (Andrew Poelstra)
2ff5085e70 locktimes: run cargo fmt (Andrew Poelstra)
Pull request description:
While implementing https://github.com/rust-bitcoin/rust-miniscript/pull/654 I ran into a number of limitations of the `relative::LockTime` API. This fixes these by
* Copying a ton of functions from `absolute::LockTime` to `relative::LockTime`, adjusting comments and functionality accordingly.
* Adding conversion functions to/from `Sequence` numbers, as well as a method to check whether a locktime is satisfied by a given sequence number.
Fixes#2547Fixes#2545Fixes#2540
ACKs for top commit:
tcharding:
ACK 04715e3e60
sanket1729:
ACK 04715e3e60
Tree-SHA512: 70740eaa3a83dc1e7312b99e907ccdcef4eeb6191ae881d81712707ad6fb949c4e28183ab6f9258c6cde1ef8fdd5dc6476439e705a9e02a939b7832430a608d4
The "One ACK carve-out" has 3 rules and then there is a separate
"Refactor carve-out" that covers things that are not only refactoring -
this makes it hard to reference the carve-outs in github because its a
bit confusing.
Merge the carve-outs into a single "one ACK carve-out" with multiple
rules. Use rule 0 for the original refactor carve-out stuff because it
makes the diff smaller and all good lists start with 0.
Also remove mention of the refactor carve-out from rule 3.
When we renamed `dust_value` to `minimal_non_dust` we forgot to keep the
original and deprecated it, doing so assists with the upgrade path.
Pub back in deprecated `dust_value`, linking to the rename.
8bd0394b0a Document how to write commits (Tobin C. Harding)
Pull request description:
Reviewers often find themselves linking to blog posts to encourage newer devs to improve their commit logs, we can save everyones time by putting the links in the contributing docs, then we can just point devs there.
ACKs for top commit:
apoelstra:
ACK 8bd0394b0a
Tree-SHA512: b2713c2882c3153152091bd2a72a473e151cf1e3e93288ae17b773c9f2944805a78c991437c9951d9fe5478c76a31d5c0f2ac0212bda6cbca531681a7b41f988
Adds constructors to allow directly creating locktimes from time or
block counts; adds a flooring constructor to Time to match the ceiling
one; adds an explicit constructor to Height since the From<u16> was not
very discoverable.
These two functions are related. We cannot, by definition, get the
witness version from a script that is not a witness program but
currently the code is not linking these two things.
Refactor by doing:
- Move the check of the witness program bip rules to `witness_version`
- Call `witness_version().is_some()` in the predicate
Improve the docs while we are at it to include the bip text in the
rustdoc. Note I didn't bother referencing the segwit bip number, this
bip text is pretty well known.
Add two unit tests that verify we can correctly determine if a
shortest allowed and longest allowed script is a witness program.
Done in preparation for patching the `witness_version` function.
In preparation for release add a changlelog entry and bump the version.
I'm not 100% sure that this release is API breaking, dependencies
definitely changed. The rest might be only additives but I didn't bother
looking exactly because I think its better to bump the minor version and
err on the side of caution.
Note the hashes 0.13.0 dependency stays in the dependency graph because
of secp, we can update secp after releasing `hashes` then update the
secp dependency in `rust-bitcoin` thereby removing the `hashes v0.13.0`
dependency - phew.
d38cb8af9e fuzz: Use path in manifest instead of version (Tobin C. Harding)
Pull request description:
Using `path` instead of `version` makes the `fuzz` crate easier to maintain because we don't have to update the version number to do releases.
ACKs for top commit:
apoelstra:
ACK d38cb8af9e neat, I did not know you could do this
Tree-SHA512: dd3d4526e6599c0752aef07393238b381b166d0d5b436a4babe967c6b282aa47079583b36ecea7a17279b1de135591950c07c9d6f2ba8e50d349bc62ca77f5c6
50e772fe79 Revert "ci: introduce `classify-pr.sh` script which determines whether a PR should have CI run" (Andrew Poelstra)
ae381fcc01 Revert "ci: gate CI workflow on source being changed" (Andrew Poelstra)
495d7e8acd Revert "ci: gate fuzztesting on whether source code changed" (Andrew Poelstra)
ec3e4e8801 Revert "ci: gate coverage analysis on whether source code changed" (Andrew Poelstra)
Pull request description:
This PR did not work on the "master" CI runs and I really don't care enough to figure it out (or even how to test it). Just revert it.
ACKs for top commit:
tcharding:
ACK 50e772fe79
Tree-SHA512: c56b77dab93e9917b2420f5b7f178bfa05500f8329852296102d72df33c5de87d45ee79f235a4ce30e629bc79b2b290a2fcb97d7808c406bfe07c9fe7e1cc997
We are currently using the `base58::Error` type to create errors in
`bitcoin`, these are bitcoin errors not `base58` errors.
Note that we add what looks like duplicate
`InvalidBase58PayloadLengthError` types but they are different because
of the expected length. This could have been a field but I elected not
to do so for two reasons:
1. We will need to do so anyways if we crate smash more
2. The `crypto::key` one can have one of two values 33 or 34.
With this applied we can remove the now unused error variants from
`base58::Error`.
In preparation for improving the `base58` error types crate an `error`
module and move the single current error type there. Make the module
public and reexport the type.
6b09857f55 base58: Re-name crate to base58ck (Tobin C. Harding)
Pull request description:
The current name `base58check` is taken, as is `base58`. Use `base58ck` instead.
Add a brief section to the readme about the crate naming.
ACKs for top commit:
apoelstra:
ACK 6b09857f55
sanket1729:
ACK 6b09857f55
Tree-SHA512: 86ee08105906a6f3403dc2602e827b0d46226798ecdedb420ad3ac4b657d6a00e25eabcdfbdb9f8e89bdc3a38e608189f1e073e65593f89a2ad853e8ff027f69
b816c0bb01 hash_types: add unit tests for display of all hash types in the library (Andrew Poelstra)
Pull request description:
This can be checked against the 0.29.x branch, and against the commit prior to #1659 (40c246743b^) and you will see that it is consistent EXCEPT:
* In rust-bitcoin 0.29.x we did not have multiple sighash types, only `Sighash`; we now have `LegacySighash`, `SegwitV0Sighash`, and `TapSighash`.
* In #1565 we deliberately changed the display direction of the sighashes, to match BIP 143.
Fixes#2495.
ACKs for top commit:
tcharding:
That's a win. ACK b816c0bb01
Tree-SHA512: 5b44f40165699910ea9ba945657cd1f960cf00a0b4dfa44c513feb3b74cda33ed80d3551042c15b85d6e57c30a54242db202eefd9ec8c8b6e1498b5578e52800
32f9b1a231 ci: gate coverage analysis on whether source code changed (Andrew Poelstra)
2203c02347 ci: gate fuzztesting on whether source code changed (Andrew Poelstra)
09f7fc3cff ci: gate CI workflow on source being changed (Andrew Poelstra)
9aca8a18c7 ci: introduce `classify-pr.sh` script which determines whether a PR should have CI run (Andrew Poelstra)
Pull request description:
Fixes#2523
ACKs for top commit:
tcharding:
ACK 32f9b1a231
Tree-SHA512: 7d53365bdf4e8ae5480221e43b4c515d4b16048ec1a00cb62ab65b2d2b91a37f8945c39b619a82bfe4ca74f3508176d9879481b908bb8d3fe8d214b173d3bf18
0d517dcfdd Re-export P2shError (Tobin C. Harding)
646ee1a837 Put re-exports in alphabetic order (Tobin C. Harding)
Pull request description:
As with the rest of the errors in `address::error` that are returned by a pubic function from the `address` module.
Note please, this PR just makes the `address/mod.rs` file uniform, debating the merit of the re-exports is out of scope.
ACKs for top commit:
apoelstra:
ACK 0d517dcfdd
Tree-SHA512: a65ebe6de62b83c6d3723c7c97a0953ddb8da51964ef54e69865855502ae5fa51b2b49c6fd408c452414b56518f4c7ab763faca925e8d81f0fe806af1df92aa2
911f8cbd6a fix: Manually implement serde traits (Tobin C. Harding)
Pull request description:
Currently we are deriving the serde traits for the `absolute::{Height, Time}` types, this is incorrect because we maintain an invariant on the inner `u32` of both types that it is above or below the threshold.
Manually implement the serde traits and pass the deserialized `u32` to `from_consensus` to maintain the invariant.
Close: #2559
ACKs for top commit:
apoelstra:
ACK 911f8cbd6a
sanket1729:
ACK 911f8cbd6a.
Tree-SHA512: 28f9f3e23b2016e00216e5e94f35e22a5bfa6b29e1dd30fa3351575eafc2e47dbd98aa8e51631de2f09eec8d881b0e5e79231185c7a293c1ba0170a90fbbd19d
Currently we are deriving the serde traits for the `absolute::{Height,
Time}` types, this is incorrect because we maintain an invariant on
the inner `u32` of both types that it is above or below the threshold.
Manually implement the serde traits and pass the deserialized `u32` to
`from_consensus` to maintain the invariant.
Close: #2559
290e4418e6 units: Fix cargo cult programming (Tobin C. Harding)
Pull request description:
When creating the ParseIntError in `hex_u32` I (tobin) just cargo cult programmed the generic stuff without thinking.
- The `is_signed` field is used to denote whether we were attempting to parse a signed or unsigned integer, it should be `false`.
- The `bits` field should be directly set to 32.
ACKs for top commit:
apoelstra:
ACK 290e4418e6
sanket1729:
ACK 290e4418e6
Tree-SHA512: 7dfd9f0cd98eff1c2b27a92dac5c4e2fe0fa4ae724528ef741fe43d8d923e2d31cbdcd4e540ecfba1b953860dc2b728a958e756e2d2012d9a9e715c0ca3c5068
When creating the ParseIntError in `hex_u32` I (Tobin) just cargo cult
programmed the generic stuff without thinking.
- The `is_signed` field is used to denote whether we were attempting to
parse a signed or unsigned integer, it should be `false`.
- The `bits` field should be directly set to 32.
16a813734c Implement consensus deserialize_hex (Tobin C. Harding)
Pull request description:
We have `serialize_hex` and `deserialize` but no `deserialize_hex`, add it.
Move the `IterReader` out of `consensus::serde` to the `consensus` module.
Add some additional logic to the `DecodeError`, I'm not sure why this wasn't there before?
Use the `HexSliceToBytesIter` by way of the `IterReader` to deserialize an arbitrary hex string. Add unit tests to check that we consume all bytes when deserializing a fixed size object (a transaction).
ACKs for top commit:
apoelstra:
ACK 16a813734c
sanket1729:
ACK 16a813734c
Tree-SHA512: 121285cb328ca01bf9fd2a716e6d625fa93113a11613d44c576e3e49a9d06dc181165d2d9bfb9beea7c3d2aff264f64ade4965acd594b05ce0d1660e7493d2e4
13ec770f10 io: Bump version to 0.1.2 (Tobin C. Harding)
Pull request description:
There have only been two PRs merged that touched the `io` crate since it as last released. The changes are additive so we can do a pre-0.1 point release.
In preparation for release bump the version and add a changelog entry.
ACKs for top commit:
apoelstra:
ACK 13ec770f10
sanket1729:
utACK 13ec770f10
Tree-SHA512: 319edb80cedffe796df798ba5fc640953ccfde12295d0bf5b7987ab69b1b677108a7351d4d1d335e4136801589503666172891d2b6ee73ed8be58ddafc9547fe
d887423efc Enforce displaying Amount with trailing zeros (448 OG)
Pull request description:
It is common to display bitcoins using trailing zeros upto 8 decimals. This commit enforces:
- Displaying Amount in BTC with trailing zeroes by eight decimal places if a precision on the Amount is not specified.
- Displaying Amount in BTC upto the precision specified truncating the insignificant zeros.
- Displaying amount in BTC without any decimals if the remainder of the amount divided by the satoshis in 1 BTC is equal to zero using formula `satoshis.rem_euclid(Amount::ONE_BTC.to_sat()) != 0`
These are not breaking changes and all previous tests pass.
A testcase is added to for changes introduced.
Resolves: #2136
ACKs for top commit:
sanket1729:
ACK d887423efc
apoelstra:
ACK d887423efc
Tree-SHA512: c32e41216477f60a8d95f164bf4a1f6644ea14adc7e169743ce419b0f26ecb6603f3a516f9b18d6508c04ce658f6a0a78ff3b0b062aeb7101b28bbb1e9d522bc
It is common to display bitcoins using trailing zeros upto 8 decimals.
This commit enforces:
- Displaying Amount in BTC with trailing zeroes by eight decimal places
if a precision on the Amount is not specified.
- Displaying Amount in BTC upto the precision specified truncating the insignificant zeros.
- Displaying amount in BTC without any decimals if the remainder of the amount
divided by the satoshis in 1 BTC is equal to zero using formula `satoshis.rem_euclid(Amount::ONE_BTC.to_sat()) != 0`
These are not breaking changes and all previous tests pass.
A testcase is added to for changes introduced.
Resolves: #2136
438c000316 fix: small typo in SECURITY.md (Jose Storopoli)
Pull request description:
Just caught a small typo in SECURITY.md
ACKs for top commit:
apoelstra:
ACK 438c000316
Tree-SHA512: d2bcc4d5d02b531cebaa8290c2055360753ef06102fee7c16a0345dbecd1769a79e820b7139e7fc60a0d931d7a35b6fb5e7bac2ee32dc4eb55460b05b7e22064
cbee9781e8 Move unit types to units (Tobin C. Harding)
5bd0d7194b Remove unused absolute::Error (Tobin C. Harding)
Pull request description:
Move the following unit types to the new `units` crate:
- `locktime::absolute::{Height, Time}`
- `locktime::relative::{Height, Time}`
- `FeeRate`
- `Weight`
Also move the `parse` module as well as constants as required.
Do minimal changes to get things building:
- Feature gate on "alloc" as needed.
- Remove rustdocs that use `bitcoin` types.
- Re-export units types so this is a non-breaking change.
- Fix import paths.
Patch 1 was originally #2526, putting it in via this PR to try and speed up the process.
Close: #2282
ACKs for top commit:
sanket1729:
ACK cbee9781e8
apoelstra:
ACK cbee9781e8 lgtm. this is a good start. I think the LockTime types should follow Height and Time
Tree-SHA512: 6b0d63c7b054008598d7fa81be7d8c112f2778883b5529d79d446617b94b3c196c9ac735f840d1dfb488700894d3161c6976d44ab0e12ac3af4008068eac5f87
Andrew Poelstra
Tobin C. Harding
448 OG
Jose Storopoli