040b14ef1a Implement basic support for fast hex encoding (Martin Habovstiak)
Pull request description:
There's a `hex` module in `bitcoin_hashes` which is a bit out of place and not very fast - it passes each *digit* through dynamic dispatch not only adding overhead but also not allowing `String` to make better allocations.
This change adds basic support for hex encoding using a stack-allocated buffer which minimizes the overhead of dynamic dispatch. It also provides a new `DisplayHex` trait designed to replace `ToHex` found in `bitcoin_hashes`.
ACKs for top commit:
apoelstra:
ACK 040b14ef1a
tcharding:
ACK 040b14ef1a
Tree-SHA512: ffb2f46d5e0aa97c73a14067bdde92b7577651de1daee89fcb38b6e464b78a6ce40d39ac6bd573a9eec7274aa4e112ebc537604dca2c0d38da7d647cb548482a
Currently in one of the rustdoc examples showing lock satisfaction we
use two locks with the same value, this obfuscates which lock is doing
the satisfying and which lock is being satisfied.
Increment the value in one of the locks so it is obvious which lock is
which.
There's a `hex` module in `bitcoin_hashes` which is a bit out of place
and not very fast - it passes each *digit* through dynamic dispatch not
only adding overhead but also not allowing `String` to make better
allocations.
This change adds basic support for hex encoding using a stack-allocated
buffer which minimizes the overhead of dynamic dispatch. It also
provides a new `DisplayHex` trait designed to replace `ToHex` found in
`bitcoin_hashes`.
`Cargo.toml` claimed that this crate works with very old versions of
`serde` which wasn't the case. This commit changes the versions to
minimal known-to-work values.
834bbf461f Introduce bitcoin-internals crate (Tobin C. Harding)
12c5fb042e Refactor import statements (Tobin C. Harding)
022730bd8d Add a workspace to the top level directory. (Tobin C. Harding)
a59e0544ec Remove trailing whitespace (Tobin C. Harding)
Pull request description:
Add a new crate `bitcoin-internals` to be used for internal code needed by multiple soon-to-be-created crates. Add the `write_err` macro to `bitcoin-internals`, nothing else.
### Notes
This PR uses a `path` dependency which means `rust-bitcoin` cannot be released in its current state, will need to be changed once we release the `bitcoin-internals` crate on `crates.io`.
Please note also the testing strategy imposed by this PR - each new crate to have its own `contrib/test.sh` run from the top level `test.sh`.
Please excuse the mid-series import statement refactor, I had a moment of weakness and got uncontrollably triggered by the messy imports in one file, I managed to keep myself together on the others :)
ACKs for top commit:
apoelstra:
ACK 834bbf461f
Kixunil:
ACK 834bbf461f
Tree-SHA512: 327cced077b6d8bf6b4407a930ff8c0a6a8488256453cab168fc0d172ebb03976f8e64ff26a9b178f856c2ffa4342b5ef1a2c6eabcadf9bfa3cf787157a1d1ec
Add a new crate `bitcoin-internals` to be used for internal code needed
by multiple soon-to-be-created crates.
Add the `write_err` macro to `bitcoin-internals`, nothing else.
This patch uses a `path` dependency which means `rust-bitcoin` cannot be
released in its current state, will need to be changed once we release
the `bitcoin-internals` crate on `crates.io`.
Create a directory `bitcoin` and move into it the following as is with
no code changes:
- src
- Cargo.toml
- contrib
- test_data
- examples
Then do:
- Add a workspace to the repository root directory.
- Add the newly created `bitcoin` crate to the workspace.
- Exclude `fuzz` and `embedded` crates from the workspace.
- Add a contrib/test.sh script that runs contrib/test.sh in each
sub-crate
- Fix the bitcoin/contrib/test.sh script
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.
Andrew Poelstra
Tobin C. Harding
Martin Habovstiak
sanket1729
Noah Lanson
hrouis