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.
34c95ad729 Automated update to Github CI to rustc nightly-2024-09-18 (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 34c95ad729
Tree-SHA512: da84f2a273c2e26a1305935bd2e1abc35dea9b6dc45f54414339ede5b91346ab3b96188bcb4b6567bc13a53e630b3e0efa0584cd7714f10bc4ae0483e8fb4541
f5cae1cddd Comment from_str methods (yancy)
Pull request description:
Follow up from https://github.com/rust-bitcoin/rust-bitcoin/pull/3346
ACKs for top commit:
tcharding:
ACK f5cae1cddd
apoelstra:
ACK f5cae1cddd successfully ran local tests
Tree-SHA512: 2b95381e5281754e2b3a49aa8dfaac5742c244970fb54f68024dc23b61a74955ae95b9a0e7ae848095ac0686df5faf93faf7de3371b2f341b108cc10e5d4a9cd
8c29fe08f8 Revise doc comment (yancy)
Pull request description:
Update doc comment to make clear that the ceiling is computed instead of the default behavior for integer division.
ACKs for top commit:
tcharding:
ACK 8c29fe08f8
apoelstra:
ACK 8c29fe08f8 successfully ran local tests
Tree-SHA512: 3793dccab5b5a3e59b3949ecb54475c76263e1debcc18df42f3b0251189435ba87e537c4b5d80c91f4b916449618a75e6efac32b4ba2fc29c42563e1b0fb4a89
The example code in macros.rs is much clearer as it is than changing
it to remove the warnings created by adding the warn attribute to the
`lib.rs` file.
The example code was enclosed within a function block and
`#[allow(unused)]` added. And a warn attribute added to `lib.rs`.
Examples in documentation are not linted in the same way as other code,
but should still contain correctly written code.
Throughout all of the crates except internals (another commit) unused
variables have been prefixed with `_`, unused imports have been removed,
and a warn attribute added to all of the `lib.rs` files.
Tobin C. Harding
Update Nightly Rustc Bot
Chris Hyunhum Cho
merge-script
Steven Roose
yancy
Jamil Lambert, PhD