b2344e019d units: Assert roundtrip SignedAmount/str overflows (Tobin C. Harding)
baadcf4c0a units: Test that SignedAmount float conversion overflows (Tobin C. Harding)
d768f25da8 units: Remove duplicate assertion (Tobin C. Harding)
1d536ac8b2 units: Enable parsing Amount from u64::MAX (Tobin C. Harding)
Pull request description:
Our `Amount` type uses an internal `u64` and we maintain no invariants on the inner value. Therefore we should be able to parse `u64::MAX`.
Fix the parsing code by removing the explicit, incorrect check and fix unit tests to mirror this behaviour.
Fix: #2297
ACKs for top commit:
Kixunil:
ACK b2344e019d
apoelstra:
ACK b2344e019d
Tree-SHA512: 944f8d0bfedc559f0444f75eca7d3fba042fbc204c4c032d09ff0edc29be280a3707f5b363dbc04f0d7bdf64701c0c4619e2e0de683d804a2663c2a20ac963f6
c6c5f07880 Add automated labeler job (Martin Habovstiak)
Pull request description:
Rather than modifying the labels manually, which we often forget, we can label the PRs automatically. This will make it easier to search PRs.
ACKs for top commit:
apoelstra:
ACK c6c5f07880
tcharding:
ACK c6c5f07880
Tree-SHA512: d3f7ca8c69cc8734d914f871675568ca293e28c3dc7351ee54b9237645204b87d1b47db02863b27d930900ae4b41d6f169f130ed9f4c750d4afbb765921cfb66
22747149a9 Add convenience constants to `Denomination` (Martin Habovstiak)
Pull request description:
`Denomination::Bitcoin` and `Denomination::Satoshi` are often used, especially in test code so this change adds `BTC` and `SAT` - short, readable constants. Notably this doesn't add the other constants as that would lead to either unidiomatic names or confusing casing (MSAT meaning millisat not megasat) and they are not used that much anyway.
ACKs for top commit:
apoelstra:
ACK 22747149a9
tcharding:
ACK 22747149a9
Tree-SHA512: fb49e61c28f5df14d5432a81ec946a367696fe36e96798d35831cd5cb88b813fe3aa1e7be201997956b4d4f6e5431e4da20d0d24e4d6c036aa199fc9b9c87da1
We should not be able to roundtrip a `SignedAmount` value greater than
`MAX`, add a test to prove so.
While we are at it document the assertion above that proves we can parse
a float representing an `Amount` greater than `SignedAmount::MAX`.
fe8d559d69 test: add invalid segwit transaction test (startup-dreamer)
Pull request description:
Tries to close#2183
Added the test for invalid segwit transaction (witness flag is set but no witness is present) using [This suggested hex](https://github.com/rust-bitcoin/rust-bitcoin/issues/2183#issuecomment-1901207149) by Kixunil
ACKs for top commit:
Kixunil:
ACK fe8d559d69
apoelstra:
ACK fe8d559d69
Tree-SHA512: 723027e0ad9944a4763fba1e12398d7bcacdef691a40168f0be7cecdb170936f7e0c3690c4de911d086b8c5d42f5a25784f53fe096404f5cf69d6fc75c645d6e
`Denomination::Bitcoin` and `Denomination::Satoshi` are often used,
especially in test code so this change adds `BTC` and `SAT` - short,
readable constants. Notably this doesn't add the other constants as that
would lead to either unidiomatic names or confusing casing (MSAT meaning
millisat not megasat) and they are not used that much anyway.
47b476ec5b Ignore fuzz dir from coverage reports (pool2win)
50ff68550c Add github action for llvm-cov coverage (pool2win)
Pull request description:
There has been discussion around generating coverage reports for tests. See #1853 and #2353.
This PR adds an action using llvm-cov and coveralls.
The action generates lcov coverage report using llvm-cov and uploads the generated report to coveralls. You can see sample reports for my fork here: https://coveralls.io/github/pool2win/rust-bitcoin
I am using the [cargo-llvm-cov](https://github.com/taiki-e/cargo-llvm-cov) wrapper around llvm-cov.
I also use the coveralls official github action to push the coverage report to coveralls. It removes the need to deal with repository secrets etc, making the action easy to run for all contributors on their forks.
We can move this action later to rust.yml if we need to.
ACKs for top commit:
apoelstra:
ACK 47b476ec5b
Kixunil:
ACK 47b476ec5b
tcharding:
ACK 47b476ec5b
Tree-SHA512: 7a7b8caf94d4b4828416bc1e6df49bfe47fede7b8fa4386d905a6f40439a95df1d3fedecdd6d64675f2f6858851f18d2f8863994aa20ac1ab224f97ca48b4af2
a338a61cc3 Remove quadratic algorithm (Tobin C. Harding)
Pull request description:
Currently we loop over transaction inputs within a loop over transaction inputs - ouch.
Cache the `use_segwit_serialization` outside the iteration loop.
Fix: #2357
ACKs for top commit:
Kixunil:
ACK a338a61cc3
apoelstra:
ACK a338a61cc3
Tree-SHA512: 91d0b46b235db57d9c28fc8da5d43a52c76a29916797a4ec44273b91eb120a928050a79cdbd704b922635dd2130db7b6e7863fd10e878eee52882c661af54c11
e356ff6611 Remove the now unused sighash::Error type (Tobin C. Harding)
c17324c574 Introduce segwit sighash error types (Tobin C. Harding)
f0b567313b Introduce sighash::LegacyError (Tobin C. Harding)
a1b21e2f1d Introduce sighash::TaprootError (Tobin C. Harding)
b0f20903a5 Introduce AnnexError (Tobin C. Harding)
a1a2056829 Add tx_in/tx_out accessor methods on Transaction (Tobin C. Harding)
f08aa16e91 Use Self:: in error return type (Tobin C. Harding)
Pull request description:
Improve the error handling in the `sighash` module by adding small specific error types.
Close: #2150
ACKs for top commit:
Kixunil:
ACK e356ff6611
apoelstra:
ACK e356ff6611
Tree-SHA512: e2e98a4caccae4e4acdc0e577e369fc90ee39a2206a8a1451739695fbe33ec2c3a52482b70cec8f9ee6bdb3ad7a2f4f639e8c87031878cd5d816fae24d913c42
61bf462806 Use full path in all macro usage of Result (josibake)
Pull request description:
Follow-up to https://github.com/rust-bitcoin/rust-bitcoin/pull/2355
Couldn't think of a clever way to do this , so just grepped for all instances of `macro_rules` and added the full path for the imports. Wasn't sure if it was necessary for `fmt::Result`, but went ahead and added the full path for consistency.
Tested locally and confirmed this fixes the issue I was seeing.
ACKs for top commit:
apoelstra:
ACK 61bf462806
Kixunil:
ACK 61bf462806
tcharding:
ACK 61bf462806
Tree-SHA512: 8af105b3e6a36723804b290f8254f52e65cd42a61c323f1190e3bcbcb9e4427ff9b026a4530bafcd14aab644ccd9401fed351494457194c3a68a55f11b2a3d04
In a few places in the codebase we want to grab an reference to an input
by index. To reduce code duplication add two methods on `Transaction`,
each to get a reference to an input or output respectively.
These are public methods, do not use them yet internally.
Currently we loop over transaction inputs within a loop over transaction
inputs - ouch.
Cache the `use_segwit_serialization` outside the iteration loop.
Fix: #2357
bd41c836ab Fix cut'n'pasta error in map variable (Tobin C. Harding)
Pull request description:
We are mapping outputs not inputs.
ACKs for top commit:
apoelstra:
ACK bd41c836ab
Kixunil:
ACK bd41c836ab
Tree-SHA512: 2f2f788662a56077ac7b3623a4c4969d3248a2f374368b6a08e7ae4fd04baefee72c75e961cfb4edb973396627ac869ad7845217572eb21460af8aad58b0fa55
aa6e5cd342 Use full path in all macro usage of Result (Steven Roose)
Pull request description:
Apparently when someone uses a custom `Result` type and then uses some of these macros, they can get type conflict errors.
(Thanks josibake for finding this using the `sha256t_hash_newtype` macro.)
ACKs for top commit:
josibake:
utACK aa6e5cd342
Kixunil:
ACK aa6e5cd342
apoelstra:
ACK aa6e5cd342
Tree-SHA512: 04e51d6a4da520fd03f8c20c41707e43fc8d909de68533959373afd99e654068cedc5a6ca30bdc867a33e7e42b971a3bba623fad0fd294359948018ed55dc662
3ea44a166b Remove usage of Cursor in pubkey sanity checks (Tobin C. Harding)
35b5350088 Remove usage of Cursor in multi key read (Tobin C. Harding)
Pull request description:
We do not need to use `Cursor` in these tests, remove the usage.
PR touches test code only.
ACKs for top commit:
Kixunil:
ACK 3ea44a166b
apoelstra:
ACK 3ea44a166b
Tree-SHA512: 9fefdc4a8d7fcf3dd09fa60b993f5a61e55865b830c9519efd2de99c4e871cf665facc68fd6c06829d166e76e181fe2026ab5475e7fc6241a5d627212a2cff2f
3333dbab24 Use new read_to_limit function (Tobin C. Harding)
f29da57ef6 io: Add functions to read to the end of a reader (Tobin C. Harding)
Pull request description:
Total re-write, now does:
- Add a method to the `io::Take` trait `read_to_end`
- Add a method to the `io::Read` trait `read_to_limit` (with default impl that calls through to `Take::read_to_end`)
- Use the new methods and remove `psbt::read_to_end`
ACKs for top commit:
apoelstra:
ACK 3333dbab24
Tree-SHA512: ce4439a85296db36013e284ebce6f91665b1daa21e6d5a570746a3dea789aec3e6a32d295ab88a8c72b56ee8d50b18e142cbd684980076f2e88c3ae7acf5c322
The `std::io::Read` trait includes `read_to_end` but that method
provides a denial of service attack vector since an unbounded reader
will exhaust all system memory.
Add a method to our `Read` trait called `read_to_limit` that does the
same as `std::io::Read::read_to_end` but with memory exhaustion
protection.
Add a `read_to_end` method on our `Take` trait and call through to it
from the new method on our `Read` trait called `read_to_limit`.
Our decoding code reads bytes in very small chunks. Which is not
efficient when dealing with the OS where the cost of a context switch is
significant. People could already buffer the data but it's easy to
forget it by accident.
This change requires the new `io::BufRead` trait instead of `io::Read`
in all bounds.
Code such as `Transaction::consensus_decode(&mut File::open(foo))` will
break after this is applied, uncovering the inefficiency.
This was originally Kix's work, done before we had the `io` crate.
Changes to `bitcoin` were originally his, any new mistakes are my own.
Changes to `io` are mine.
Co-developed-by: Martin Habovstiak <martin.habovstiak@gmail.com>
2dfe455161 Remove mention of core2 (Tobin C. Harding)
Pull request description:
We no longer depend on `core2`, remove stale code comment mention of the crate.
Fix: #2034
ACKs for top commit:
Kixunil:
ACK 2dfe455161
apoelstra:
ACK 2dfe455161
Tree-SHA512: cb723a384cd69e5b1aa70bdb25f53c818092c465783bd8a9b1ec60af488ed013d39f29057b4b09d6347b8bc52911eb6daf609bd088dec172647dbfedc2ea1791
de9c2bc43d p2p: Improve nonce documentation (Tobin C. Harding)
Pull request description:
Better describe what the nonce is used for.
Note this file has not had its docs manicured so the line length is 80 still, just use the same line length instead of the conventional 100.
Fix: #575
ACKs for top commit:
Kixunil:
ACK de9c2bc43d
apoelstra:
ACK de9c2bc43d
Tree-SHA512: ab18d9893fff4e673373125e607a4a60843a98cf84dc336fba9b6423da24ea3ad4c5fe5846ae8bcef51962dc2f3017157f2d7301c2c2cd1a81a37c3da6823552
The effective_value method is useful for coin selection algorithms. By
providing this effective value method, the effective value of each
output can be known during the coin selection process.
Better describe what the nonce is used for.
Note this file has not had its docs manicured so the line length is 80
still, just use the same line length instead of the conventional 100.
Fix: #575
Tobin C. Harding
Andrew Poelstra
dependabot[bot]
Martin Habovstiak
startup-dreamer
pool2win
josibake
Steven Roose
Thabokani
yancy