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
Add a `BufRead` trait for types that perform buffered reading.
Implement it for:
- `Take`
- `Cursor`
- `std::io::BufRead` readers
- (in no-std builds) for slice of u8s
d480adaf25 io: Simplify crate docs and add README (Tobin C. Harding)
Pull request description:
Simplify the docs in `lib.rs` and copy them into a minimal README file.
ACKs for top commit:
apoelstra:
ACK d480adaf25
Kixunil:
ACK d480adaf25
Tree-SHA512: 17b6bed688854f9b0cafa0a0320683e75cf0d8f190a4f526d982292c0ae0e4834e4f239e451a1ded124d44d6f0c8f248893eeb27f0a7a5a0b97a757515f732ee
518f0970c9 Implement ArbitaryOrd for absolute::LockTime (Tobin C. Harding)
Pull request description:
At times we would like to provide types that do not implement `PartialOrd` and `Ord` because it does not make sense. I.e we do not want users writing `a < b`. This could range from kind-of-iffy to down-right-buggy (like comparing absolute locktimes).
However this decision effects downstream users who may not care about what the ordering means they just need to use it for some other reason e.g., to use as part of a key for a `BTreeMap` (as we do in `miniscript` requiring the `AbsLockTime` type).
A solution to this problem is to provide a wrapper data type that adds `PartialOrd` and `Ord` implementations. I wrote the `ordered` crate is for this very purpose.
ACKs for top commit:
apoelstra:
ACK 518f0970c9
Kixunil:
ACK 518f0970c9
Tree-SHA512: 05c753e650b6e2f181caf7dc363c4f8ec89237b42883bd695a64da0661436c9a7e715347f8fcf4fb19ce069cbf75a93032052e946f05fd8029f61860cf9c6225
271b45299f Improve Signature field names (Tobin C. Harding)
Pull request description:
Applies to both `ecdsa::Signature` and `taproot::Signature`.
Re-name the `Signature` fields with more descriptive names. The names used were decided upon in the issue discussion.
Impove rustdocs while we are at it.
Note, the change to `sign-tx-segwit-v0` is refactor only, the diff does not show it but we have a local variable already called `sighash_type` that is equal to `EcdsaSighashType::All`.
Fix: #2139
ACKs for top commit:
Kixunil:
ACK 271b45299f
apoelstra:
ACK 271b45299f
Tree-SHA512: b27221e922206b2510b073c0f38678f97f0c946707e3e679eee181faa170348101198706f9ca5803a55c799b0b330cc263cc103cd9beefff4e5c58d8fea77b8d
Applies to both `ecdsa::Signature` and `taproot::Signature`.
Re-name the `Signature` fields with more descriptive names. The
names used were decided upon in the issue discussion.
Impove rustdocs while we are at it.
Note, the change to `sign-tx-segwit-v0` is refactor only, the diff does
not show it but we have a local variable already called `sighash_type`
that is equal to `EcdsaSighashType::All`.
Includes a function argument rename as well, just to be uniform.
Fix: #2139
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
e762c53725 fix nightly clippy issues (Andrew Poelstra)
Pull request description:
I don't mind making these PRs, but I think we should start pinning our nightlies, and using a script to update the pin every week (or day, or whatever). When our CI breaks due to tooling changes, it means that our release branches will have permanently broken CI jobs, which I'd like to avoid in the future.
If the automatic pin update failed, we'd close it and manually create one that fixed the nw issues.
ACKs for top commit:
tcharding:
ACK e762c53725
Tree-SHA512: c61af0d69746258268b28be2ef7bd703e121c02fcf48c0625019e51407275b2b501aa31b2e65a548f8aaa903c49eda2d426c53e9530ba79b9bf1d82a690f4372
3a562db39d docs: Remove pinning from hashes readme (Tobin C. Harding)
Pull request description:
We do not need to pin with the current MSRV, remove the stale mention of it from the `hashes` readme file.
ACKs for top commit:
apoelstra:
ACK 3a562db39d will merge with one-ACK carveout
Tree-SHA512: a3f2017ab40e28d1e047ec5466e4b101dfc13280226c71ec5b36d46f54bed359eccbce16d811cee100fb134f3091f7c986c7b2afa3adaa15942bee9e08e730f1
08d2b203a5 Remove rustdoc about attribute (Tobin C. Harding)
eea0b697bf Fix stale docs (Tobin C. Harding)
Pull request description:
Do two miner docs fixes.
- Patch 1: Fixes some stale docs I left in when refactoring the `AddressInner` type.
- Patch 2: Uses code comments instead of rustdoc and fixes the issue linked below.
Fix: #2315
ACKs for top commit:
sanket1729:
utACK 08d2b203a5
Tree-SHA512: e68276473b451c02e230364d386d5b4b610c8ec73ff30e97e111cd339cc2238b65e08c9a83aac5f9d5d6c2e1dd1ee10fec727dfec3bca3cd7317b7049d689f4a
At times we would like to provide types that do not implement
`PartialOrd` and `Ord` because it does not make sense. I.e., we do not
want users writing `a < b`. This could range from kind-of-iffy to
down-right-buggy (like comparing absolute locktimes).
However this decision effects downstream users who may not care about
what the ordering means they just need to use it for some other reason
e.g., to use as part of a key for a `BTreeMap` (as we do in `miniscript`
requiring the `AbsLockTime` type).
A solution to this problem is to provide a wrapper data type that adds
`PartialOrd` and `Ord` implementations. I wrote the `ordered` crate is
for this very purpose.
Feature gate a new dependency on `ordered` and implement `ArbitraryOrd`
for `absolute::LockTime`.
b02c7d1d33 Derive Copy for WitnessProgram (Tobin C. Harding)
Pull request description:
Recently we started using our custom `ArrayVec` for the `program` field of `WitnessProgram`, this means we can now derive `Copy`.
Fix: #2313
ACKs for top commit:
apoelstra:
ACK b02c7d1d33
sanket1729:
ACK b02c7d1d33
Tree-SHA512: 5741081d578f7b056c156d046dc3b0817b4b13cf69dcc1dfb8c7f4dbe8a4f9ed6c8802aaaf2b0084dbf3984d3fde807a02dbaa8c3bd29c220b3b32d3cb7c9f38
a8d50a5541 Remove Push enum (Tobin C. Harding)
Pull request description:
The `Push` enum is only ever used to get access to one of its variants. Since it is a private type we can remove it entirely and just return `PushBytes` from the `last_pushdata` function.
Needs careful review but I believe the function name is still correctly descriptive.
This was discovered by of a new nightly clippy warning.
ACKs for top commit:
apoelstra:
ACK a8d50a5541 Looks good to me. The latest compiler complains about the currently-unused variants.
sanket1729:
ACK a8d50a5541.
Tree-SHA512: 7f96057b0f6f5673252578253ad4f1789793dbf6e917d3974274dedf942da27e6247946262a0669eb500d47987788fcca0e020ed16c0d672188e95ee31163242
089ce8f0fb Deprecate `Script::is_provably_unspendable` (Martin Habovstiak)
Pull request description:
This method is not really that useful because it checked an arbitrary condition. There already exists `OP_RETURN` semantics and the method didn't cover all possible ways the script may be invalid.
This deprecates the method and documents why.
Closes#2191
ACKs for top commit:
apoelstra:
ACK 089ce8f0fb
tcharding:
ACK 089ce8f0fb
sanket1729:
ACK 089ce8f0fb
Tree-SHA512: 044f1c06fb8cbea4f84817be41bf10315f690b2a42748a07c1dd1eb0ba10932456780956fc628fec4bf57fe0722129537874a77be482d6660f9e02de5fc5a8a0
774b405ba9 2024-01-07 automated rustfmt nightly (Fmt Bot)
Pull request description:
Automated nightly `rustfmt` changes by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action
ACKs for top commit:
Kixunil:
ACK 774b405ba9
sanket1729:
ACK 774b405ba9. I was missing for 3 months and we have cool new stuff
Tree-SHA512: ff905c8971bc92c0716c969222d5425715773803ea05336e1bbae35c4e5cfb5cf975451452ebb769296fa84eabbbfa03672bbc0aa161f871edeefaaeb594c2ca
The `Push` enum is only ever used to get access to one of its variants.
Since it is a private type we can remove it entirely and just return
`PushBytes` from the `last_pushdata` function.
Needs careful review but I believe the function name is still correctly
descriptive.
josibake
Andrew Poelstra
Steven Roose
Thabokani
Tobin C. Harding
yancy
dependabot[bot]
Fmt Bot
GoodDaisy