In preparation for moving the `OutPoint` to `primitives` add an
additional impl block that holds everthing that will stay here in
`bitcoin`.
Internal change only.
a33bcd3654 test: ensure push_int check i32::MIN of overflow error (Chris Hyunhum Cho)
c9988ba8cb refactor: use match for OP_N push in push_int_unchecked (Chris Hyunhum Cho)
Pull request description:
Follow up https://github.com/rust-bitcoin/rust-bitcoin/pull/3392c9988ba8cb
- refactor `push_int_unchecked` with match expression for cleaner code(many thanks for tcharding https://github.com/rust-bitcoin/rust-bitcoin/issues/3407).
a33bcd3654
- ensure newly introduced safe `push_int` function as expected, testing if returns `Error::NumericOverflow` when `n` is `i32::MIN`
ACKs for top commit:
tcharding:
ACK a33bcd3654
apoelstra:
ACK a33bcd3654 successfully ran local tests
Tree-SHA512: 14f19d37f35b47e148b40c5017f0270c534c136d86be0c061cb476e1693130c5fc1bfc45a6f7c75a473022490c5f4e061cbc02640b1a616619ae721116e3cd54
216f6e3c1b Automated update to Github CI to rustc nightly-2024-09-25 (Update Nightly Rustc Bot)
Pull request description:
Automated update to Github CI workflow `rust.yml` by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action
ACKs for top commit:
tcharding:
ACK 216f6e3c1b
Tree-SHA512: c892afaa2424245eaf0fe914e4c2659cd2cf26001512d1c907d9b11e984532772741b06123925b3df8e74c7c62d707efcd43bbbbf007a103f6a34c6bfa9bf193
Rust convention is to not use `test_` prefix on unit tests. Also this
unit test is testing that the `ExactSizedIterator` trait is implemented
and working.
Re-name unit test to `exact_sized_iterator`.
In preparation for moving unit tests to `primitives` give the serde
tests some love by doing:
- Split them up to do one thing only
- Round trip arbitrary witness
- Use better names
Make an effort to clean up the encoding unit test, by doing:
- Remove element accessor assertions (tested already above)
- Add roundtrip encoding assertion
b8dab65c3d CI: Use latest run_task version (Tobin C. Harding)
Pull request description:
Recently we discovered a bug in the `run_task` script where we forgot to use `--no-default-features` in a subset of test runs (the feature matrix stuff).
Upgrade to use the latest version of `run_task.sh`.
ACKs for top commit:
apoelstra:
ACK b8dab65c3d successfully ran local tests; nice
Tree-SHA512: 9b8e6eba81a81658a98f0c8e355f97f2b27ee9a730cb3b008d0c60e81a308cdb2af11587a5e63fe2a9fc35fedef0a9e00f8b721b91cf182214dbe738f8315bec
33edaf935d fix: check overflow for push_int with push_int_unchecked (Chris Hyunhum Cho)
876146154b refactor: use push_lock_time instead of push_int (Chris Hyunhum Cho)
Pull request description:
Fix the issue https://github.com/rust-bitcoin/rust-bitcoin/issues/1530. In the discussion of https://github.com/rust-bitcoin/rust-bitcoin/issues/1530, the suggested solution is to implement `ScriptInt` type to embrace the various type of integer(`i32, u32, i64, u64, i128, u128, isize, usize...`) to support both script number and locktime number.
However, as `push_locktime` and `push_sequence` implemented, there’s no need to support `u32` of lock time for `push_int` anymore. Therefore, I’ve just changed the type of parameter to `i32`, and only check if it’s `i32::MIN`(which overflows 4 bytes sign-magnitude integer).
~I also added push_uint method to use internally for `push_locktime` and `push_sequence`, which have a dependency on `push_int` method.~
UPDATE: also add `push_int_unchecked` for those who want to push the integer out of range(and helper for `push_locktime` and `push_sequence`, which has the same functionality of former `push_int`.
ACKs for top commit:
tcharding:
ACK 33edaf935d
Tree-SHA512: 89b02bd3faf1e0a1ed530b7210250f0db33886d2acd553d07761f4aef0bb6388b22ddc06a88de05acfe465305db4cf34822fb6547576aae2aa224b4d0045fa07
d0a30096b4 transaction: Remove Default implementations (Tobin C. Harding)
Pull request description:
Currently we provide `Default` implementations for a couple of types in the `transaction` module, the values returned are meaningless and it seems these impls were added to make writing test code easier. In hindsight this was the wrong thing to do.
Break the API and remove the `Default` implementations for `OutPoint` and `TxIn`.
Add an associated const `TxIn::EMPTY_COINBASE` that is, as the name suggests, an empty transaction input with the prevout set to all zeros as for the coinbase transaction.
ACKs for top commit:
apoelstra:
ACK d0a30096b4 successfully ran local tests
Tree-SHA512: f76c2984d5f57fb6cb0792b4feb555434dd4ad3439926d30fe1be42432ace990888ad61d724fe701cb30b6f201f66b4670b7c59cc8de82d7a61ee71883a7fbd7
8b9d657667 hashes: Remove unused file (Tobin C. Harding)
Pull request description:
Recently we moved the tests from `impls` to `tests/io.rs` but forgot to delete the original file.
Remove the already unused `impls.rs` file.
ACKs for top commit:
apoelstra:
ACK 8b9d657667 successfully ran local tests
Tree-SHA512: f701825239aff18faa0cd7adf69e1a8830dfb532df9d52c6d545e6352054ae6cb8c625c9fc05b7db374822d3478acf16858bc38ffbecf45575f555eb80f010e1
In preparation for releasing `units v0.2.0` bump the version number,
add a changelog entry, update the lock files, and depend on the new
version in all crates that depend on `units`.
When we do patch version releases (on a separate branch) the release
patches typically include a changelog entry that does not appear on
`master` - this seems like a process fail. Anyways, grab the release
notes for `v0.1.2` and add them to the changelog file. Intentionally do
not cherrypick the release patch because that may make the git index
hard to understand.
The current unit test is incorrect, the indices field of a witness with
a single element starts at 1 because 0 is encode as a single
byte (compact encoded integer).
Fix the debug test and add a test that pushes an empty slice.
cb2146d5fa Implement FeeRate checked_sub (yancy)
212a751929 Implement FeeRate checked_add (yancy)
c967eabd43 Implement FeeRate SubAssign (yancy)
c3a8bfa98d Implement FeeRate AddAssign (yancy)
0e70870056 Implement FeeRate subtraction (yancy)
86359fe364 Implement FeeRate addition (yancy)
Pull request description:
I can't think of a reason to not support addition and subtraction with `fee_rate`. If two fee rates are added, the resulting units `sat/wu` units are viable.
Without this, I have to do some shenanigans like:
```rust
let tmp = fee_rate.to_sat_per_kwu();
fee_rate = FeeRate::from_sat_per_kwu(tmp - 1);
```
ACKs for top commit:
apoelstra:
ACK cb2146d5fa successfully ran local tests
tcharding:
ACK cb2146d5fa
Tree-SHA512: 582c5e75e836df4a8aa00b928a9e81a04ba8c14a8986fac323f608d05468531b17034f42db23af57468b3220151bb6b37b90da4f1621571f011b89491070c619
Recently we removed the `all_zeros` function from `OutPoint` in favour
of a more meaningfully named associated const. We can do the same for
`BlockHash`, the all zeros has is used for the previous blockhash of the
genesis block, add a const named as such.
In test code where we use the `all_zeros` function, just use the more
explicit form `from_byte_array([0; 32])`.
Currently we provide `Default` implementations for a couple of types in
the `transaction` module, the values returned are meaningless and it
seems these impls were added to make writing test code easier. In
hindsight this was the wrong thing to do.
Break the API and remove the `Default` implementations for `OutPoint`
and `TxIn`.
Add an associated const `TxIn::EMPTY_COINBASE` that is, as the name
suggests, an empty transaction input with the prevout set to all
zeros as for the coinbase transaction.
At some stage we named the compact encoding `VarInt` (which makes sense
because the compact size encoding is a variable length integer encoding).
However it turns out the term "varint" is used in Core for a different
encoding so this may lead to confusion.
While we fix this naming thing observe also that the `VarInt` type is
unnecessarily complicated, all we need to be able to do is encode and
decode integers in compact form as specified by Core. We can do this
simply by extending our `WriteExt` and `ReadExt` traits.
Add `emit_compact_size` and `read_compact_size` to emit and read compact
endcodings respectively.
Includes addition of `internals::compact_size::encoded_size_const`.
Patch originally written by Steven, Tobin cherry-picked and did a bunch
of impovements after the varint vs compact_size thing (#1016).
ref: https://en.bitcoin.it/wiki/Protocol_documentation#Variable_length_integer
Co-developed-by: Tobin C. Harding <me@tobin.cc>
We would like to add a `emit_varint` function, however doing so requires
that we can get access to the length of a slice when we are encoding it
so we can use `emit_slice` to implement `emit_varint`. It would be
easier to do so if `emit_slice` returned the length of the slice.
In preparation for adding `emit_varint` (and removing the `VarInt` type)
return the encoded length of a slice from `WriteExt::emit_slice`.
(Patch originally written by Steven, cherry-pick and patch description
written by Tobin.)
Co-developed-by: Tobin C. Harding <me@tobin.cc>
Recently we discovered a bug in the `run_task` script where we forgot to
use `--no-default-features` in a subset of test runs (the feature matrix
stuff).
Upgrade to use the latest version of `run_task.sh`.
58704c2eff Remove schemars all together (Tobin C. Harding)
Pull request description:
We introduced schemars as a personal favor to a user, and it broke our CI repeatedly but eventually it seemed like it was stable (mainly, our MSRV caught up with its MSRV) so we just let it slide. In the end having schemars on hashes but nowhere else in the rust-bitcoin ecosystem did not prove that useful.
Remove schemars all together.
Fix: #3393
ACKs for top commit:
apoelstra:
ACK 58704c2eff successfully ran local tests
Tree-SHA512: 11c136797f28903c7d6b5199ad55d86bc4bc29ee8dd6f0d575e029f4dbebebabed57ebce6cf773b286297ea84f18d0b6cc58e150299e99457e048226478b49cc
7d2c57d345 Soften wording on MSRV (Tobin C. Harding)
Pull request description:
Currently we use the word "always" in the MSRV docs, we mean to say "there are no set of features that will break the build" but this usage of "always" may be interpreted as "always, for all time" which is definitely not what we mean.
Soften the wording on MSRV by removing the word always.
ACKs for top commit:
apoelstra:
ACK 7d2c57d345 successfully ran local tests; will one-ACK merge because it's trivially correct
Tree-SHA512: 8d85d748e90e8082861005e6787d1c6be95745ebb442c98f638c5efb9940fc5ac7860d786b47bff88ba59c17a2a8c115d2c740108772dd63dc24f3e911555473
cbfddb0394 hashes: Rename length field and use u64 (Tobin C. Harding)
Pull request description:
The hash engine types have a `length` field that is used to cache the number of bytes hashed so far, as such it is an arbitrary number and could use a `u64` instead of `usize`.
While we are at it rename `length` to `bytes_hashed` to remove any ambiguity of what this field is. Note this field is private, we already have the public getter `n_bytes_hashes` to get the value.
Introduce a private function `incomplete_block_size`, the purpose of this function is to put all the casts in one place so they can be well documented and easily understood.
Fix: #3016
ACKs for top commit:
apoelstra:
ACK cbfddb0394 successfully ran local tests
Tree-SHA512: a9d932938afcbd6dfb9db471a02fa7e3fff8f0659906627001ad241390b9af57088fd34afeae551c70c2c49783e6296f110b57ff9de6fed2609f4648ec8fd934
18110a51f2 Bump version of internals to 0.4.0 (Tobin C. Harding)
Pull request description:
In preparation for releasing `internals v0.4.0` bump the version number, add a changelog entry, update the lock files, and depend on the new version in all crates that depend on `internals`.
ACKs for top commit:
apoelstra:
ACK 18110a51f2 successfully ran local tests; lots of nice stuff here
Tree-SHA512: a4d3d5279b7d7fa993cbc3b7b34fc6dc4024dd54c0bfa1ecd0f0d5f09b984871f156c3695092a1f6c44b7571f8b2051699040f5f77636d44d4cae6c972ab597f
184dfbcdb6 primitives: Move optional dependency to correct place (Tobin C. Harding)
Pull request description:
We keep optional deps in a separate group to non-optional deps, move the new `arbitrary` dependency.
Internal change only.
ACKs for top commit:
shinghim:
ACK 184dfbcdb6
apoelstra:
ACK 184dfbcdb6 successfully ran local tests
Tree-SHA512: 8540ee9ce9663d60d1a8d08e94738c7a3e3341b640ec6cbc04c304cb12daa4c95ae6cd1e4ad9fde361065510b2e34c65f6f08db1ae305976fce4525aa98756a3
Currently we use the word "always" in the MSRV docs, we mean to say
"there are no set of features that will break the build" but this usage
of "always" may be interpreted as "always, for all time" which is
definitely not what we mean.
Soften the wording on MSRV by removing the word always.
We introduced schemars as a personal favor to a user, and it broke our
CI repeatedly but eventually it seemed like it was stable (mainly, our
MSRV caught up with its MSRV) so we just let it slide. In the end having
schemars on hashes but nowhere else in the rust-bitcoin ecosystem did
not prove that useful.
Remove schemars all together.
Fix: #3393
bd32d1db78 Add stricter doc test to justfile (Jamil Lambert, PhD)
Pull request description:
In order to show warnings for unused imports etc. in doc examples as mentioned in #3362, an extra test has been added to the justfile for `just lint` and the doc test in `just sane` has been removed since it calls `just lint`.
ACKs for top commit:
apoelstra:
ACK bd32d1db78 successfully ran local tests
tcharding:
ACK bd32d1db78
Tree-SHA512: 088244aa9cd07e4aab197d1ad675c6ed778f10cec5304bfcecdd9a0526cd6395f14db69e71a464acd1e41ce54610a51257481602348f3ee65b0228550cc6e12c
In order to show warnings for unused imports etc. in doc examples an
extra test has been added to the justfile for `just lint` and the doc
test in `just sane` has been removed since it calls `just lint`.
46386337b0 Ignore doc compile error (Jamil Lambert, PhD)
Pull request description:
With the stricter doc tests required to pick up unused imports etc. the code under the `compile_fail` tag also creates an Error.
Changed `compile_fail` to `ignore` to remove the Error.
ACKs for top commit:
tcharding:
ACK 46386337b0
apoelstra:
ACK 46386337b0 successfully ran local tests
Tree-SHA512: 67513464dede8d4bd9f9fa4363ccb58e774b180f331edc4486290784f61e931195225765768030d2709f618ba4afe17d034d4bbc4d649a02e7a34b47b48bd297
f6abdcc001 Allow unused in `macros.rs` docs (Jamil Lambert, PhD)
fd89ddf401 Remove or fix unused variables and methods in docs (Jamil Lambert, PhD)
ff6b1d4f19 Remove unused variables and methods from docs (Jamil Lambert, PhD)
e58cda6f92 Remove `unused_imports` in docs (Jamil Lambert, PhD)
Pull request description:
As mentioned in #3362 examples in documentation are not linted in the same way as other code, but should still contain correctly written code.
#![doc(test(attr(warn(unused))))] has been added to all lib.rs files
In the docs throughout all crates:
- Unused imports have been removed.
- Unused variables, structs and enums have been used e.g. with an `assert_eq!` or prefixed with `_`
- Unused methods have been called in the example code.
ACKs for top commit:
tcharding:
ACK f6abdcc001
apoelstra:
ACK f6abdcc001 successfully ran local tests
Tree-SHA512: c3de1775ecde6971056e9fed2c9fa1621785787a6a6ccbf3a6dbd11e18d42d4956949f3f8adfc75d94fd25db998b04adb1c346cc2c2ba47f4dc37402e1388277
With the stricter doc tests required to pick up unused imports etc. the
`compile_fail` tag also creates an Error.
Changed `compile_fail` to `ignore` to remove the Error.
Tobin C. Harding
Shing Him Ng
merge-script
Update Nightly Rustc Bot
Chris Hyunhum Cho
Steven Roose
yancy
Jamil Lambert, PhD