In an effort to reduce the build times of `rocksdb` we can set
`default-features` to false.
Please note, the build speed up is minimil
With default features:
```
cargo check --features compact_filters 890.91s user 47.62s system 352% cpu 4:26.55 total
```
Without default features:
```
cargo check --features compact_filters 827.07s user 47.63s system 352% cpu 4:08.39 total
```
Enable `snappy` since it seems like this is the current default compression
algorithm, therefore this patch (hopefully) makes no changes to the usage of the
`rocksdb` library in `bdk`. From the `rocksdb` code:
```
/// Sets the compression algorithm that will be used for compressing blocks.
///
/// Default: `DBCompressionType::Snappy` (`DBCompressionType::None` if
/// snappy feature is not enabled).
///
/// # Examples
///
/// ```
/// use rocksdb::{Options, DBCompressionType};
///
/// let mut opts = Options::default();
/// opts.set_compression_type(DBCompressionType::Snappy);
/// ```
pub fn set_compression_type(&mut self, t: DBCompressionType) {
....
```
Currently we have duplicate code for converting the fee estimate we get
back from esplora into a fee rate. This logic can be moved to a separate
function and live in the `esplora` module.
Our features are a bit convoluted, most annoyingly we cannot build with
`--all-features`. However we can make life for users a little easier.
Explicitly we want users to be able to:
- Use async-interface/WASM without using esplora (to implement their own blockchain)
- Use esplora in an ergonomic manner
Currently using esplora requires either reqwest or ureq. Instead of
making the user add all the features manually we can add features that
add the required feature sets, this makes it easier for users to
understand what is required and also makes usage easier.
With this patch applied we can do
- `cargo check --no-default-features --features=use-esplora-reqwest`
- `cargo check --no-default-features --features=use-esplora-ureq`
- `cargo check --features=use-esplora-ureq`
- `cargo check --no-default-features --features=async-trait`
The `Blockchain` implementation for connecting to an Esplora instance is
currently based on `reqwest`. Some users may not wish to use reqwest.
`ureq` is a simple HTTP client (no async) that is useful when `reqwest`
is not suitable.
- Move `esplora.rs` -> `esplora/reqwest.rs`
- Add an implementation based on the `reqwest` esplora code but using `ureq`
- Add feature flags and conditional includes to re-export everything to
the `esplora` module so we don't effect the rest of the code base.
- Remove the forced dependency on `tokio`.
- Make esplora independent of async-interface
- Depend on local version of macros crate
Add a unit test that passes a required utxo to the coin selection
algorithm that is less than the required spend. This tests that we get
that utxo included as well as tests that the rest of the coin selection
algorithm code also executes (i.e., that we do not short circuit
incorrectly).
If the required UTXO set is already bigger (including fees) than the
amount required for the transaction we can return early, no need to go
through the BNB algorithm or random selection.
It was just pointed out that we are calculating the virtual bytes
incorrectly by forgetting to take the ceiling after division by 4 [1]
Add helper functions to encapsulate all weight unit -> virtual byte
calculations including fee to and from fee rate. This makes the code
easier to read, easier to write, and gives us a better chance that bugs
like this will be easier to see.
As an added bonus we can also stop using f32 values for fee amount,
which is by definition an amount in sats so should be a u64. This
removes a bunch of casts and the need for epsilon comparisons and just
deep down feels nice :)
[1] https://github.com/bitcoindevkit/bdk/pull/386#discussion_r670882678
We have an attribute `no_run` that builds but does not run example code
in Rustdocs, this keeps the examples building as the codebase evolves.
use `no_run` and fix example code so it builds cleanly during test run.
Some examples that require the `electrum` feature to be available have
been feature-gated to make sure they aren't accidentally compiled when
that feature is not enabled.
Co-authored-by: Alekos Filini <alekos.filini@gmail.com>
The transaction builder changed a while ago, looks like some of the
example code did not get updated.
Update the transaction creation code to use a mutable builder.
The lint `manual_map` is new so we cannot explicitly allow it and
maintain backwards comparability. Instead, allow all lints for
`get_utxo_for` with a comment explaining why.
Clippy emits:
warning: struct constructor field order is inconsistent with struct
definition field order
As suggested, re-order the fields to be consistent with the struct
definition.
It is redundant to pass true/false to `assert_eq!` since `assert!`
already asserts true/false.
This may, however, be controversial if someone thinks that
```
assert_eq!(foo, false);
```
is more clear than
```
assert!(!foo);
```
Use `assert!` directly instead of `assert_eq!` with true/false argument.
We recently converted uses of `PSBT` -> `Psbt` inline with idiomatic
Rust acronym identifiers. Do the same to `PSBTKey`.
Use `PsbtKey` instead of `PSBTKey` when aliasing the import of
`psbt::raw::Key` from `bitcoin` library.
Idiomatic Rust uses lowercase for acronyms for all characters after the
first e.g. `std::net::TcpStream`. PSBT (Partially Signed Bitcoin
Transaction) should be rendered `Psbt` in Rust code if we want to write
idiomatic Rust.
Use `Psbt` instead of `PSBT` when aliasing the import of
`PartiallySignedTransaction` from `bitcoin` library.
Currently we use `F: f` for the argument that is the default function
passed to `map_or_else` and pass a closure for the second argument. This
bent my brain while reading the documentation because the docs use
`default: D` for the first and `f: F` for the second. Although this is
totally trivial it makes deciphering the combinator chain easier if we
name the arguments the same way the Rust docs do.
Use `default: D` for the identifier of the default function passed into `map_or_else`.
Currently we have a unit test to test that signers are sorted by
ordering. We call `add_external` to add them but currently we add them
in the same order we expect them to be in. This means if the
implementation happens to insert them simply in the order they are
added (i.e. insert to end of list) then this test will still pass.
Insert in a mixed order, including one lower followed by one higher -
this ensures we are not inserting at the front or at the back but are
actually sorting based on the `SignerOrdering`.
If we give the `DummySigner` a valid identifier then we can use this to
do comparison.
Half the time we do comparison we only have a `dyn Signer` so we cannot
use `PartialEq`, add a helper function to check equality (this is in
test code so its not toooo ugly).
Thanks @afilini for the suggestion.
We build against various targets on CI, in order to not abuse CI its
nice to see if code is good before pushing.
Add a script that runs `cargo check` with various combinations of
features and targets in order to enable thorough checking of the project
source code during development.
Clippy emits:
warning: Question mark operator is useless here
No need to use the `?` operator inside an `Ok()` statement when
returning, just return directly.
Clippy emits error:
comparing trait object pointers compares a non-unique vtable address
The vtable is an implementation detail, it may change in future. we
should not be comparing vtable addresses for equality. Instead we can
get a pointer to the data field of a fat pointer and compare on that.
Remove the TODO; refactor matching to correctly handle conditionally
built `Sled` variants. Use `unreachable` instead of `unimplemented` with
a comment hinting that this is a bug, this makes it explicit, both at
runtime and when reading the code, that this match arm should not be hit.
The `ChunksIterator` constructor is only used when either `electrum` or
`esplora` features are enabled. Conditionally build it so that we do not
get a clippy warning when building without these features.
Clippy complains about use of a mutex, suggesting we use an
`AtomicUsize`. While the same functionality _could_ be achieved using an
`AtomicUsize` and a CAS loop it makes the code harder to reason about
for little gain. Lets just quieten clippy with an allow attribute and
document why we did so.
Clippy emits warning:
warning: field assignment outside of initializer for an instance
created with Default::default()
Do as suggested by clippy and use the default init pattern.
```
let foo = Foo {
bar: ...,
Default::default()
}
```
As suggested by clippy we can use `.next()` on an iterator instead of
`nth(0)`. Although arguably no clearer, and certainly no worse, it keeps
clippy quiet and a clean lint is a good thing.
Clippy emits warning:
warning: avoid using `collect()` when not needed
As suggested by clippy just use `count` directly on the iterator instead
of `collect` followed by `len`.
It seems the documentation of this project uses arbitrarily long
lines (i.e. no set column width) along with the occasional newline
before some sentences (within a paragraph). When to split a sentence
onto a newline does not seem to follow any discernible pattern.
There are a few instances of newline characters appearing randomly in
the middle of a sentence and since, as observed above, there is no
fixed column width is use these new lines are out of place.
Remove them so the documentation is slightly more uniform and nice to
read in an editor.
This patch is whitespace only, no other textual changes.
Tobin Harding
