d8116de56a Use Borrow trait bound for bip158 Iterator::Item (Tobin C. Harding)
Pull request description:
Currently the generic iterators in `bip158` return items of type slice reference. We can make the code cleaner (no explicit lifetimes) and more general if the generic iterator argument iterates over `Borrow<[u8]>`
ACKs for top commit:
Kixunil:
ACK d8116de56a
apoelstra:
ACK d8116de56a
Tree-SHA512: ce8111130d70a8c6f8d97f2c3f6c07bffd534461fdb599185129fc3cc4fc6c209e3187444082e47b7b55e4ec71fd8652fb2b86d2037573b99560ffd094175dc1
a9d1e08833 Add a comment to delineate test_macros re-export (Tobin C. Harding)
Pull request description:
The formatter removes an empty line that is clearly better to have. Add a rustdoc comment to at least delineate the line a bit.
ACKs for top commit:
Kixunil:
ACK a9d1e08833
apoelstra:
ACK a9d1e08833
Tree-SHA512: 0d962200a9d359825f3db50c2fcf3cf254f4f2ae55148d4a6524062bad8536160ae5f17256d4a86d746e66e5892e969b8efc14372f046ab7b4ce2950ce4bd25b
Currently the generic iterators in `bip158` return items of type slice
reference. We can make the code cleaner (no explicit lifetimes) and more
general if the generic iterator argument iterates over `Borrow<[u8]>`.
Note, includes adding `Borrow` to `crate::prelude`.
b9a512330a Simplify `PublicKey` serialization code (Martin Habovstiak)
Pull request description:
There were a bunch of `if self.serialized` expressions repeated in the code. This change simplifies it by putting the `if` in a function that calls a closure with a slice containing the serialized bytes.
ACKs for top commit:
apoelstra:
ACK b9a512330a
sanket1729:
utACK b9a512330a. I don't have strong preferences on naming. `serialize_and_call` sounds better because so far I have only seen `with_*` used with constructors.
tcharding:
ACK b9a512330a
Tree-SHA512: 5d90a73f87fa36bd45a5765542ec24994d601d32ad6529fff5d5140ded2b86eb40e00bfba4955e94d2c468983e3e7683ca5f8ece7a205d9b9944334a19b06015
81f81caa04 implement `From<PublicKey>` for `PubkeyHash` (Noah Lanson)
Pull request description:
Part of #1245.
Do we also want a method on `PubkeyHash` or is the `From` impl sufficient?
ACKs for top commit:
tcharding:
ACK 81f81caa04
Kixunil:
ACK 81f81caa04
sanket1729:
ACK 81f81caa04
apoelstra:
ACK 81f81caa04
Tree-SHA512: 760ea2991de60fd72d0aae18d375024f001f331d8a57b1e6e39b5933efa3a5af379effecb89cde0defb960b25c997b417fc126248589953655fb83db8b8dd608
86218ad240 Use macro to implement `std::erorr::Error` (Martin Habovstiak)
Pull request description:
There was a bunch of manual implemntations that can be converted to
macro call. This commit replaces them except for enums because those are
currently not supported by the macro and we want to protect against
forgetting to handle newly added variants.
Depends on #1129 and is **not** urgent for the next release.
ACKs for top commit:
apoelstra:
ACK 86218ad240
tcharding:
ACK 86218ad240
Tree-SHA512: a4ca91e023d66b5ad9408004b201cfe4cea85efb5a6e7f2241367934a954659c9196561295a491d2b2ed15c1a69c0ffb17a297d710cec4ce1d0f1ec8c12492e6
57c4283220 Remove unused consensus encoding error variants (Martin Habovstiak)
Pull request description:
The error variants `UnexpectedNetworkMagic` and `UnknownNetworkMagic` were not only duplicated but also completely unused. This removes both of them.
ACKs for top commit:
tcharding:
ACK 57c4283220
apoelstra:
ACK 57c4283220
Tree-SHA512: 3e686f54dee038500712e6960aeb3a633d95a6302f782b9b756a8a32f6b9eff805a7b63f2190c57ed806d6dfd7585b2f335bd8d5c5bfbe2c1b27e4761fd96625
0e425acad3 Run cargo fmt (Tobin C. Harding)
Pull request description:
A few trivial formatting errors have snuck into the codebase.
Run `cargo +nightly fmt`
FTR we have an open PR to run formatter in CI: https://github.com/rust-bitcoin/rust-bitcoin/pull/1111
ACKs for top commit:
Kixunil:
ACK 0e425acad3
apoelstra:
ACK 0e425acad3
Tree-SHA512: b42ba57fab25baf6ed7cfb3c207389df09d192840c591178fb3f43f1f0ff3e9381e1416c976fa4da8104dd3c18a32f54338ed98b997a31aba5c37a1b6c261210
840520042a BIP 34: Check last minimal encoding of encoded heights in coinbase tx (sanket1729)
fd3e01d6e3 Fix minimality of reading/writing CScriptNums (sanket1729)
Pull request description:
See: https://github.com/bitcoin/bips/blob/master/bip-0034.mediawiki
The BIP was updated with commit: 08844fd6ef
Found while reviewing #1240
Also cleanly deals with negative heights as errors.
ACKs for top commit:
apoelstra:
ACK 840520042a
tcharding:
ACK 840520042a
Tree-SHA512: 85eb1fbb428fb600f864390c3ac2e8e62efc1fc4b03b842e384ddb4ed9273c66d2274d0de219f1c7a6bad7b02bd8723f8f257c864577614df674b44bfc02010a
There was a bunch of manual implemntations that can be converted to
macro call. This commit replaces them except for enums because those are
currently not supported by the macro and we want to protect against
forgetting to handle newly added variants.
There were a bunch of `if self.serialized` expressions repeated in the
code. This change simplifies it by putting the `if` in a function that
calls a closure with a slice containing the serialized bytes.
947dcf5675 Add new type for block version (Noah Lanson)
Pull request description:
Added new type `BlockVersion` for block header version field with inspiration from [Kixunil 's comment on #1215.](https://github.com/rust-bitcoin/rust-bitcoin/issues/1215#issue-1352273532)
I previously knew very little about the upgrading process so it was fun learning about it in writing this PR, however that also means it's probably not perfect yet, especially around BIP-9 signalling (fingers crossed it's all correct).
API for the type is kept simple but happy to change it up to add more functionality.
ACKs for top commit:
sanket1729:
ACK 947dcf5675.
apoelstra:
ACK 947dcf5675
Tree-SHA512: 1ce9720d50f0ab50e08308e56bdc047567b64dbe446dcdcd9db2f14f5c9d4603a7b9d58a5fa38b769581789fdbc4e1ef6083be32b9b59cef59714e07b2f8be5f
d780ea3c48 Use correct path for bip158 in rustdoc (Tobin C. Harding)
Pull request description:
Recently we moved the `bip158` module but I missed one of the import statements in rustdoc - if only I could work out how to lint rustdocs.
ACKs for top commit:
apoelstra:
ACK d780ea3c48
sanket1729:
ACK d780ea3c48
Tree-SHA512: 2fcb7f1ad8883d17370b37786be34597ae6ab0a858d4636db831d80e779888307bc99cf247449ae285e43719cf955ea3bb9ebb3f045aafe644c815c5f5e21724
37cd6503f1 ci: Remove TOOLCHAIN env var (Tobin C. Harding)
8eb6c23eaf ci: Remove inner quotes from shell variable (Tobin C. Harding)
58ede47ef1 ci: Use set -ex instead of /bin/sh -ex (Tobin C. Harding)
4c4846f82a Remove double blank line at EOF (Tobin C. Harding)
Pull request description:
Clean up the CI script. These patches are part of #1225, are beneficial to have before that PR resolves, and are trivial.
ACKs for top commit:
sanket1729:
utACK 37cd6503f1
apoelstra:
ACK 37cd6503f1
Tree-SHA512: f6b3379c8a1be7a3519461faeed35e35560e4228bddb59ef28ab3fb3ef2679afabfaccfeaffd78adbb66879aed02c092f3d9d1d1eb3193c811067ee75e876732
01161e66ee Run formmater on bip158 (Tobin C. Harding)
95393aadbc Move bip158 module to crate root (Tobin C. Harding)
Pull request description:
We are attempting to flatten the `util` module.
Move the `bip158` module to the crate root out of `util`.
Currently `src/util/` is ignored by the formatter so this move requires `bip158` module to be formatted. Formatting is done as a separate patch so reviewers can run `cargo +nightly fmt` and compare the diffs if so desired.
ACKs for top commit:
apoelstra:
ACK 01161e66ee
sanket1729:
ACK 01161e66ee
Tree-SHA512: 192279d8d1466aa939ecad1f7beae12c4c7b5067871f6f1297fc80094a4ab736de4c1ed82a0b1d9d8e1cdba15c0342b90722002a9ba02ab1eff901edbd0fb356
Cargo already has an env var for controlling the toolchain, we should
use it instead of our current custom `TOOLCHAIn`.
While we are at it fix the missing `$` from variables (found by
`shellcheck`).
Use RUSTUP_TOOLCHAIN to allow control of the toolchain.
shellcheck emits:
SC2027: The surrounding quotes actually unquote this. Remove or escape them.
Remove inner quotes from shell variable so the feature is correctly printed.
Setting `-ex` in the shebang makes it a little hard to see since folks
are used to glancing up and looking for `set -ex`. Both styles achieve
the same result, so this does not change the logic of the script.
a013847bd3 Correctly handle bicoinconsensus::Error (Tobin C. Harding)
Pull request description:
With the latest release of `rust-bitcoinconsensus` the
`bicoinconsensus::Error` now implements `std::error::Error`.
Correctly handle `bicoinconsensus::Error` by returning `Some(e)` and
using `write_err` as is standard across the code base.
ACKs for top commit:
Kixunil:
ACK a013847bd3
apoelstra:
ACK a013847bd3
Tree-SHA512: d9ea4ce0b01277f7c6e2f1a8b877720dba111ecfbb5de7fe5bb35bf659f5c47ee4436f021f9293717000b4cc3ccfc5dc6a893e879aca4db12bb7570be8ccaee0
With the latest release of `rust-bitcoinconsensus` the
`bicoinconsensus::Error` now implements `std::error::Error`.
Correctly handle `bicoinconsensus::Error` by returning `Some(e)` and
using `write_err` as is standard across the code base.
fea908c8af Add private::Sealed trait bound to SerdeAmount (Tobin C. Harding)
08e4b28dd0 Add integer serialization tests (Tobin C. Harding)
a8f9e8ad96 Add serde roundtrip tests (Tobin C. Harding)
Pull request description:
Remove some easy todos from the code. The three patches are unrelated except each removes a single `TODO`. Patch 1 and 2 are unit tests and trivial to review. Patch 3 requires more thought, it was't clear exactly why it was left as a todo and not done at the time.
cc JeremyRubin for patch 3 as the original author of that code.
ACKs for top commit:
apoelstra:
utACK fea908c8af
Tree-SHA512: 70a041820fe52bc3d70cc110c16acfa86469fd8556793c4204671ddcb7457f336e57b43b0f03cb68d3c6b96217502f29f1628d1acbe7381a9e9d8d21c67759d2
As suggested by the todo seal the `SerdeAmount` so users of the library
explicitly cannot implement it.
Its not obvious to me why this wasn't done at the time.
Remove the associated TODO from the code.
Add some negative integer unit tests and remove the TODO.
Note, this is not really that necessary because we are going to move to
using the stdlib `to_le_bytes` methods so we don't really need any of
these test except to test that we are calling the correct be/le method.
They do no harm however.
d25aba8ca0 fuzz: use travis-fuzz.sh in CI (Andrew Poelstra)
407cbca111 fuzz: remove mysteriously-not-necessary quotes from gh action script (Andrew Poelstra)
0a32525862 fuzz: disable features in honggfuzz (Andrew Poelstra)
c7910f4561 Revert "Temporarily disable fuzzing" (Andrew Poelstra)
Pull request description:
This reverts commit 720ea29865.
There is a new version of honggfuzz out. Maybe things work now.
ACKs for top commit:
sanket1729:
utACK d25aba8ca0
tcharding:
ACK d25aba8ca0
Tree-SHA512: d9d5cb89e5bcacfdd84bf293267ee558cffed4454908add9b6c4a172d3d06404564d1bb8c3f59f29a621369b901a9cdf31b8229b299320ac061f378a73bfa80c
pmt_tests with different tx counts were executed in a single test, not taking advantage of the parallelism provided by unit tests.
A macro was added to define a unit test per transaction count.
We recently added the `Sequence` type and we explicitly did not include
relative lock time logic.
Add a new module `relative` and new type `LockTime` to represent
relative lock times as defined by BIP 68.
There were some duplicated hex macros in the `address` module as well as
several macros outputing specific types instead of working with any type
deserializable from hex.
This cleanup moves the macros and deduplicates them, introduces a new
`TestFromHex` trait to generalize parsing hex and rewrites the code to
use hex parsing generally. `hex_script` is still kept because its usage
is very frequent.
Several macros used in tests had `#[cfg(test)]` condition on each of
them. This deduplicates them by putting them into a separate module and
reexporting (with one condition for module and one for re-export).
We are attempting to flatten the `util` module; move the `bip158` module
to the crate root out of `util`.
Currently `src/util/` is ignored by the formatter so this move causes
the `bip32` module to be formatted.
Tobin C. Harding
Andrew Poelstra
sanket1729
Noah Lanson
Martin Habovstiak
hrouis