e6f2d029fa9708f98599c1bd4ef74d232b111c5a Remove genesis_block lazy initialization (Shobit Beltangdy)
Pull request description:
### Description
This commit contains a change to address issue #752
cargo test runs successfully.
### Notes to the reviewers
Hi, newbie here learning Rust and BDK! I've removed the lazy_static block in this commit, and when learning about lazy_static also came across something called [once_cell](https://doc.rust-lang.org/std/cell/struct.OnceCell.html), [soon to be available in stdlib](https://github.com/rust-lang/rust/issues/74465). It's like lazy_static but faster and is not a macro. Shall I keep the lazy_static and create an issue to switch to once_cell in the future, or remove the lazy_static (as I've done) and create an issue anyway?
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
notmandatory:
ACK e6f2d029fa9708f98599c1bd4ef74d232b111c5a
Tree-SHA512: 528f5fdfb0d7d1f7a83869b7a0de1b25dfcfafae2671c9229cdb4e5d80d11e5578d9325b3d95555e59f5dfb4ed6304c0c112a01a56b6a596e50dd62e310c516e
e1fa0b66954cc56dae055464ab5bb40b7a5043be Fix the new release process (Alekos Filini)
Pull request description:
### Description
A few things I noticed while doing the 0.22 release.
Follow-up of #544
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
ACKs for top commit:
notmandatory:
ACK e1fa0b66954cc56dae055464ab5bb40b7a5043be
Tree-SHA512: 5e60e73ba1f820fc39f62b75583e1125f911e578ab7263a20636e2a995d0efa10ba1b3b66a1e03d8a2ed61e32c00b53de9d6bbdb31de94db37c79fa5acdbf483
e2bf9734b16f5a0f419f1c1fc39c9773fd7e1128 Remove redundant duplicated keys check (Alekos Filini)
Pull request description:
### Description
This check is redundant since it's already performed by miniscript (see https://docs.rs/miniscript/7.0.0/miniscript/miniscript/analyzable/enum.AnalysisError.html#variant.RepeatedPubkeys) and it was incorrectly failing on tr descriptors that contain duplicated keys across different taproot leaves
Fixes#760
### Changelog notice
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### Bugfixes:
* [x] This pull request breaks the existing API
* [x] I've added tests to reproduce the issue which are now passing
* [x] I'm linking the issue being fixed by this PR
ACKs for top commit:
danielabrozzoni:
Code review ACK e2bf9734b16f5a0f419f1c1fc39c9773fd7e1128 - the code looks good to me, but I didn't test manually
Tree-SHA512: 3c557d741e34abf6336c7e257867f2c6f91a4be0024317af834f08ba7c0c64557bd74b643005254c9f04e953350b104b0d4d287f0d0528134b357a4adf580f87
97b6fb06aa6d95d1c8cf1498cc4478d0f27423e4 Add a policy example (rajarshimaitra)
da7670801b81e3e67ff776444afc6e6b48fc10bb Update compiler example. (rajarshimaitra)
Pull request description:
### Description
Fixes#729.
There is an "unmaintained" warning in a old version of clap, which triggered the issue.
Ideally, we should not have clap in bdk's dependency. It was only used for the `compiler.rs` example, which was a very tiny clap app compiling miniscript policies, and it wasn't really an example for bdk.
This PR rewrites the example as a `policy.rs` which demos the BDK's Policy module and policy structures.
- Use a `wsh(multi(2, Privkey, Pubkey))` descriptor, which has only one part private and other part public.
- use `into_wallet_descriptor()` to turn that into a `Descriptor` and `KeyMap`.
- Use the `KeyMap` to create a custom signer.
- Extract the descriptor `Policy` structure from the given keymap.
I am not very sure on how much this example is helpful. I still find it hard to read the Policy structure visually. But if Policy is something we want the user to know about descriptors and bdk wallets, this shows how to extract it for a simple multisig condition.
Note: There is no use of `bdk::wallet` in the example. BDK uses the Policy extraction internally while transaction creation. But all these are exposed publicly, so can be used independently too.
### Questions:
- Should we still have a `minscript::policy::compile()` example? Which IIUC is very different from `bdk::policy:Policy`. I didn't include it in this PR, because I am not sure if it fits inside bdk example categories.
- Should we expose `extract_policy` as an wallet API? All though its possible to get policy without creating a wallet, why not let the wallet also spit one out for itself, if its useful?
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
afilini:
ACK 97b6fb06aa6d95d1c8cf1498cc4478d0f27423e4
Tree-SHA512: 8e3719fdad308a347d22377050b2f29e02a884ff7d4e57a05a06d078de0709b5bf70bbcb1a696d1e1cdfe02cdb470e5af643da7c775b67fe318046bd6b80f440
54d768412abf218d2646aba211bb79b9dd287118 Sqlite migrations should either succeed or fail (Vladimir Fomene)
369e17b8012df56587c10cd9ce6651908d6a4be9 Add datatype for is_spent sqlite column (Vladimir Fomene)
Pull request description:
### Description
During table creation, Sqlite does not throw an error when a column datatype is not defined. In addition, the datatype provided during table creation does not put a constraint on the type of data that can be put in that column. So you can easily put a string value in an integer column. Despite this, I think it is important for us to add the datatype for clarity.
### Notes to the reviewers
You can read more about how Sqlite dynamic typing [here](https://www.sqlite.org/faq.html). I have amended our `migrate` code with a new commit. The idea is to run migrations in a transaction so that they either succeed or fail. This prevents us from having the database in an inconsistent state at any point in time.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### Bugfixes:
* [ ] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [ ] I'm linking the issue being fixed by this PR
ACKs for top commit:
rajarshimaitra:
tACK 54d768412abf218d2646aba211bb79b9dd287118
afilini:
ACK 54d768412abf218d2646aba211bb79b9dd287118
Tree-SHA512: bb6c0467f799ca917f8d45c6495b766352b3177fc81952fcdd678208abf092fdeae966686528a5dcb3f342d7171783274df6312a08cbef3580063e059f5f7254
5e9965fca7fb50708af0d293a9f6cd27d0c85fed Enable signing taproot transactions with only `non_witness_utxos` (Alekos Filini)
Pull request description:
### Description
Some wallets may only specify the `non_witness_utxo` for a PSBT input. If that's the case, BDK should still be able to sign.
This was pointed out in the discussion of #734
### Changelog notice
- Enable signing taproot transactions that only specify the `non_witness_utxo`
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### Bugfixes:
* [ ] This pull request breaks the existing API
* [x] I've added tests to reproduce the issue which are now passing
* [x] I'm linking the issue being fixed by this PR
ACKs for top commit:
danielabrozzoni:
tACK 5e9965fca7fb50708af0d293a9f6cd27d0c85fed - the code looks good to me, I played around with the test you provided (inspecting the PSBT, adding/removing the witness and non-witness utxos, etc) and everything works as expected.
Tree-SHA512: 2f205286263bfee4c76de8e8c81ae1349b1c3b255b72045488f8d629c05cab64c6f775307e831674dc036e5a3a760f95d9cdc1beaf48afb4c475aee838131a33
e65edbf53cb14e273ff99ed0576843dd3367385d Change parameter name of database in test funcs (Vladimir Fomene)
88307045b0f049b7f6b9c0cdc577c4a4bb3a041e Add more test to the database module (Vladimir Fomene)
e06c3f945ca5de4ce4f78bc2179c00e42d0bb4e5 Set tx field to none if `include_raw` is false (Vladimir Fomene)
Pull request description:
### Description
This PR add more test to the database module and also fixes certain bugs discovered by the written test. I also amended the name used for the database parameter in the test functions.
### Notes to the reviewers
This contributes to fixing #699
### Changelog notice
<!-- Notice the release manager should include in the release tag message changelog -->
<!-- See https://keepachangelog.com/en/1.0.0/ for examples -->
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
rajarshimaitra:
tACK e65edbf53cb14e273ff99ed0576843dd3367385d
danielabrozzoni:
Code review ACK e65edbf53cb14e273ff99ed0576843dd3367385d
Tree-SHA512: 1ac1475f7d63f25e94ef21342e6f6e243c34c8c9208d11a5492f224026055da2a96f20be83497c1ba361effff9861f4e68920f98feebaf4b201d205c7030c282
Some wallets may only specify the `non_witness_utxo` for a PSBT input.
If that's the case, BDK should still be able to sign.
This was pointed out in the discussion of #734
The current implementation of the `migrate` method for
Sqlite database does not rollback changes when there is
an error while running one of the migration scripts. This
can leave the database in an inconsistent state. This
change ensures that migrations either succeed completely
or fail.
8963e8c9f445f4069262edffd15cbd971c0adc6b Improve docs w.r.t. PSBT finalization (Elias Rohrer)
Pull request description:
### Description
This PR includes just a few tiny changes to the docs trying to make it a bit more clear what PSBT finalization is and what to expect.
### Notes to the reviewers
### Changelog notice
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
afilini:
ACK 8963e8c9f445f4069262edffd15cbd971c0adc6b
Tree-SHA512: ea1a6c715c2832cdf3a428654fb8fad08a0549e46a808ca7a424590145fc2c40164ea08eb636ce5cd648dd6335480accd1d91f9e37e9397feaf0dac2015e8baa
ab41679368be79e8645915b40be554cc07557931 Add fee_amount() and fee_rate() functions to PsbtUtils trait (Steve Myers)
Pull request description:
### Description
The purpose of the PR is to provide a more convenient way to calculate the transaction fee amount and fee rate for a PSBT. This PR adds `fee_amount` and `fee_rate` functions to the existing `PsbtUtils` trait and implements them for `PartiallySignedBitcoinTransaction`. The `fee_rate` value is only valid if the PSBT it is called on is fully signed and finalized.
See related discussion: https://github.com/bitcoindevkit/bdk-ffi/issues/179
### Changelog
Added
- PsbtUtils.fee_amount(), calculates the PSBT total transaction fee amount in Sats.
- PsbtUtils.fee_rate(), calculates the PSBT FeeRate, the value is only accurate AFTER the PSBT is finalized.
### Notes to the reviewers
Ideally I'd like `fee_rate` to return an `Option` and return `None` if the PSBT isn't finalized. But I'm not quite sure how to determine if a PSBT is finalized without having a `Wallet` and running it through the finalize code first. Or there might be a way to fill in missing signatures with properly sized fake data prior to calculating the fee rate. For now I think it's enough to do this simple approach with usage warning in the rust docs.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### New Features:
* [x] I've added tests for the new feature
* [x] I've added docs for the new feature
ACKs for top commit:
afilini:
ACK ab41679368be79e8645915b40be554cc07557931
Tree-SHA512: 5386109c9ffcf63160f18b4c51eb2c582f4121b669c7276aaba489d186cf9b97343d46f887469b1407ccd7a24448b5e7aee4c6832d86768239b21f4e68054a0f
fa998de4b1849adcf43351dbec05b3de43e6694e Add psbt_signer.rs example (Steve Myers)
Pull request description:
### Description
Adding a simple example of how to create a PSBT with a watch only wallet and then sign it with a signing wallet.
### Notes to the reviewers
This example was inspired by a question from a user.
### Changelog notice
none.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### New Features:
* [ ] I've added tests for the new feature
* [ ] I've added docs for the new feature
#### Bugfixes:
* [ ] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [ ] I'm linking the issue being fixed by this PR
ACKs for top commit:
danielabrozzoni:
ACK fa998de4b1849adcf43351dbec05b3de43e6694e
afilini:
ACK fa998de4b1849adcf43351dbec05b3de43e6694e
Tree-SHA512: 2e178ee59ce94eb1e9b5616a499e106e1d2843589036bdb6cff2c987e280588ad9989b026cdbf01290cc60f02eccbc410a3e1d1cd45eb4b8ff04353ae31b23ea
`del_tx` pulls the TransactionDetails object using
`select_transaction_details_by_txid` method which gets the transaction
details' data with a non-None transaction field even if the
`include_raw` argument is `false`. So it becomes necessary to Set
the transaction field in transactiondetails to None in `del_tx`, when
we make a call to it with `include_raw=false`.
PsbtUtils.fee_amount(), calculates the PSBT total transaction fee amount in Sats.
PsbtUtils.fee_rate(), calculates the PSBT FeeRate, the value is only accurate AFTER the PSBT is finalized.
aa0ea6aeff474a9934d4e9eb6e14ea1667573fc5 codecov: warn about missing features (Daniela Brozzoni)
c3a7bbb3ff936031200a7b70d5d9287f7f242513 codecov: slightly change the test features (Daniela Brozzoni)
1c4d47825b0ebc6027f04b8672ccabe83cbd8cee Run code coverage on every PR (Daniela Brozzoni)
Pull request description:
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
notmandatory:
ACK aa0ea6aeff474a9934d4e9eb6e14ea1667573fc5
afilini:
ACK aa0ea6aeff474a9934d4e9eb6e14ea1667573fc5
Tree-SHA512: c2d0d9ad1e956f1c1808b46394f810939154442534241f9a023f18173910339ac236a5b6c66f78207dd2cb90a8f18000fc057b960e3354e2882c627fe1ef2c9f
0010ecd94aae49f389093cd3e8983c405049dbf5 Add badge to README (wszdexdrf)
690411722e2131f69a605a6f3b5e97911778e9ed Change configs for source-base code coverage (wszdexdrf)
Pull request description:
### Description
This also changes the code coverage front end to coveralls instead of codecov, which had some issues with other changes in the PR. This will provide better and more accurate code coverage reports.
### Notes to the reviewers
The tests run before generating the report are not exhaustive (not exhaustive earlier too, but I added as many as I could), and hence the report won't be 100% accurate.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
danielabrozzoni:
ACK 0010ecd94aae49f389093cd3e8983c405049dbf5
Tree-SHA512: 04a21b7481b80287cf8a31276238b0a5958871310664363f54d53779bb2dda6f49198baaf3b5471667fa6131443b022aabd86f2e3b1cfcf5d4aacadf137a166e
d7163c3a976daf1b6a38884d77546a745805bd3c Update electrum-client to 0.11.0 (Alekos Filini)
Pull request description:
### Description
Update electrum-client to 0.11.0
### Changelog notice
- Updated `electrum-client` to `0.11.0`
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### New Features:
* [ ] I've added tests for the new feature
* [ ] I've added docs for the new feature
* [x] I've updated `CHANGELOG.md`
ACKs for top commit:
danielabrozzoni:
utACK d7163c3a976daf1b6a38884d77546a745805bd3c
Tree-SHA512: 3be61935e2bebfc5e4b530d17b9900e3c1bd2ef377fdd18df4303de713b124d51555f48f7aff5f4c2579105e86f52480e6b3f2af9c7494cbdb1dd2a2c5b4e0da
3451d1c12e4271b6067ff246d095eb9fb1ca3bd5 Fix docs.rs features (Alekos Filini)
Pull request description:
### Description
Fix docs.rs features
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### New Features:
* [ ] I've added tests for the new feature
* [ ] I've added docs for the new feature
* [ ] I've updated `CHANGELOG.md`
ACKs for top commit:
danielabrozzoni:
utACK 3451d1c12e4271b6067ff246d095eb9fb1ca3bd5
notmandatory:
ACK 3451d1c12e4271b6067ff246d095eb9fb1ca3bd5
Tree-SHA512: 96dc4f816b21cf20fc2828dcfd56865f3f9add8e4aa643205879c810d09e6f2e73d6643061bf3ca98145b37e726907dedd31a145f92d6646c172a43ae2285aa8
7c57965999ca4a3d640d1dc11d7e10401abcabbf Bump version before making release branch, separate patch_release template (Steve Myers)
3d69f1c291b8d595399bfcb3ce202dca77a3f474 Update DEVELOPMENT_CYCLE.md to work with [patch.crates-io] (Steve Myers)
Pull request description:
### Description
Update DEVELOPMENT_CYCLE and release instructions to make [overriding dependencies] possible for downstream projects with unreleased `bdk` versions for development and testing. Also simplifies the release process by capturing changelog information in the `pull_request_template` and recording release changelog information in the release tag message instead of in a `CHANGELOG.md` file which causes too many merge conflicts and complicates the release process.
Fixes#536Fixes#496
### Notes to the reviewers
The primary changes to our current release process are:
1. Don't add `-dev` or `-rc.x` to unreleased `bdk` cargo versions because those extensions do not work with [overriding dependencies].
2. Increment the `master` branch version as soon as a `release/MAJOR.MINOR` branch is created, the next release `release/MAJOR.MINOR` branch version with be **MAJOR.MINOR.PATCH**, and the `master` branch development version will be **MAJOR.MINOR+1.0**; either version can be used with [overriding dependencies].
4. Remove the `bdk` version from the `src/lib.rs` file so that it doesn't need to be changed on every release, because it isn't needed in the rust docs for most developers and removing it will help simplify the release process.
5. The new release process is now documented as a checklist in a new `release.md` github issue template.
6. Putting changelog information in the release tag message is how the tokio project does it. ~~After this PR is merged I will replace old tags with new ones containing changelog information and then do a new PR to remove the CHANGELOG.md file.~~ After this PR is merged I don't think we need to update old tags, only rename the CHANGELOG.md file to CHANGELOG-OLD.md with a note to check tags for future change log info.
[overriding dependencies]: https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
afilini:
ACK 7c57965999ca4a3d640d1dc11d7e10401abcabbf
Tree-SHA512: 818e2f9bc7a629cbbb190a83b9743e8f4de49a4093beae83ed0b9c506f33e6f96b2c1e376f788536d58c46908d278bde08140f43a625515401ea1f9efdb9153f
bfd7b2f65debe57edc5d563c071f0bfce592a8d2 Allow creating transactions with dust outputs (Liam)
Pull request description:
We needed this for testing our wallet with dust, does this look like a reasonable feature? If so, I'll go ahead and add a test, update the changelog, etc.
ACKs for top commit:
danielabrozzoni:
tACK bfd7b2f65debe57edc5d563c071f0bfce592a8d2
Tree-SHA512: b467b365d8a68f5a868cc5cc88387677533e8fb0bf543bf4c7a5b984f8b28972281029a3be8d2c92cee7d6ee05c243d12af0841e7a7e1d652745567557f2bede
138acc3b7d137788d0518182e2167504e58ebc48 Change `populate_test_db` to not return empty input (wszdexdrf)
d6e1dd104063075f49b617786d82d29c1f9c6a0a Change CI to add test using ledger emulator (wszdexdrf)
76034772cba4d3d6fa1bdcb08977c2b9d7a157c2 Add a custom signer for hardware wallets (wszdexdrf)
Pull request description:
Also adds a new test in CI for building and testing on a virtual
hardware wallet.
### Description
This PR would enable BDK users to sign transactions using a hardware wallet. It is just the beginning hence there are no complex features, but I hope not for long.
I have added a test in CI for building a ledger emulator and running the new test on it. The test is similar to the one on bitcoindevkit/rust-hwi.
### Notes to the reviewers
The PR is incomplete (and wouldn't work, as the rust-hwi in `cargo.toml` is pointing to a local crate, temporarily) as a small change is required in rust-hwi (https://github.com/bitcoindevkit/rust-hwi/pull/42).
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### New Features:
* [x] I've added tests for the new feature
* [x] I've added docs for the new feature
* [x] I've updated `CHANGELOG.md`
ACKs for top commit:
afilini:
ACK 138acc3b7d137788d0518182e2167504e58ebc48
Tree-SHA512: 54337f06247829242b4dc60f733346173d957de8e9f8b80beb91304d679cfb4e0e4db722c967469265a5b6ede2bd641ba5c089760391c671995dc30de37897de
08668ac46247d527cc53af5b6f359b1fa4e3b6aa Set the db sync height (rajarshimaitra)
Pull request description:
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
Fixes#719
Previously we weren't setting the db sync height in populate_test_db
macro even when current height is provided.. This creates a bug that
get_funded_wallet will return 0 balance.
This PR fixes the populate_test_db macro and updates tests which were
previously dependent on the unsynced wallet behavior.
### Notes to the reviewers
<!-- In this section you can include notes directed to the reviewers, like explaining why some parts
of the PR were done in a specific way -->
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### Bugfixes:
* [x] I've added tests to reproduce the issue which are now passing
* [x] I'm linking the issue being fixed by this PR
ACKs for top commit:
afilini:
ACK 08668ac46247d527cc53af5b6f359b1fa4e3b6aa
Tree-SHA512: 1dcc968e4b3551e916b450c5ff2fab6636083f104cc982eb3f7602c624382434e0170d9f0c0a356e6c9c5f834eebe5cb1365b37ef73d7b4ef15d652a364dc2ab
