feat: enable random anti-fee sniping

[aagbotemi]

This PR implements Anti-Fee-Sniping with randomization as discussed in issue #4

Notes to the reviewers

The implementation adds randomization to anti-fee-sniping behavior:

1. Uses a 50/50 chance to choose between nLockTime and nSequence (when possible)
2. Adds a 10% chance to set either value further back in time (by a random value between 0-99)
3. Detects taproot inputs and their confirmation status

Changelog notice

Changed

• CreatePsbtError::MissingFullTxForLegacyInput and CreatePsbtError::MissingFullTxForSegwitV0Input now wrap Input in Box (breaking change)

Added

• PsbtParams::enable_anti_fee_sniping field for BIP326 anti-fee-sniping protection
• CreatePsbtError::InvalidLockTime and CreatePsbtError::UnsupportedVersion error variants
• Selection::create_psbt_with_rng method for custom RNG

• I've signed all my commits
• I ran cargo fmt and cargo clippy before committing
• I've added docs for the new feature

Closes #4

[ValuedMammal]

Thanks @aagbotemi I'm still in the process of reviewing. I agree it would be nice to resolve the clippy error - my other suggestion would be to globally allow the lints in lib.rs for now and open an issue to fix them in a future PR, but in general I agree with your approach to simply Box the large enum variants.

[aagbotemi]

Thanks @aagbotemi I'm still in the process of reviewing.

Thank you @ValuedMammal. Clippy error and large enum variant error in the CI build has been fixed. For subsequent ones(if there is), I'll open another for it.

[ValuedMammal]

I have some review notes

• For imports, prefer alloc over std. This way we can actually support no-std.
• I want to see a test of some kind that creates a PSBT with enable_anti_fee_sniping and checks the expected values of the locktime and/or sequence.

[aagbotemi]

I have some review notes

Noted.
alloc is now preferred over std.
adding test to check enable_anti_fee_sniping

[ValuedMammal]

We should also be able to remove the clippy allow attributes in lib.rs.

[aagbotemi]

We should also be able to remove the clippy allow attributes in lib.rs.

Alright, I will remove the clippy allow attributes in lib.rs

[ValuedMammal]

I'm still in favor of using Box on PlanOrPsbtInput and ScriptSource::Descriptor.

[aagbotemi]

I'm still in favor of using Box on PlanOrPsbtInput and ScriptSource::Descriptor.

Fixed! I've updated the code to use Box.

[ValuedMammal]

In general I think the git history would be cleaner if we didn't merge with merge commits but instead rebase the commits in this PR.

[aagbotemi]

In general I think the git history would be cleaner if we didn't merge with merge commits but instead rebase the commits in this PR.

Alright, I've done the cleanup

[ValuedMammal]

Approach ACK.

[aagbotemi]

Currently running cargo build​ is throwing error

error[E0433]: failed to resolve: could not find `rand` in `key`
   --> src/selection.rs:201:74
    |
201 | ...::bitcoin::key::rand::thread_rng())
    |                    ^^^^ could not find `rand` in `key`
    |
note: found an item that was configured out
   --> /home/abiodun/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/bitcoin-0.32.7/src/crypto/key.rs:29:20
    |
29  | pub use secp256k1::rand;
    |                    ^^^^
note: the item is gated behind the `rand-std` feature
   --> /home/abiodun/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/bitcoin-0.32.7/src/crypto/key.rs:28:7
    |
28  | #[cfg(feature = "rand-std")]
    |       ^^^^^^^^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0433`.
error: could not compile `bdk_tx` (lib) due to 1 previous error

The solution I could think of is moving bitcoin from dev-dependencies​ to dependencies​. But it's a very big dependency.

EDIT: another solution is to add the rand crate, which is better (I would like to know if it's okay we want to add a new dependency)

[ValuedMammal]

Looks good overall, not sure if any review comments are still unresolved.

[ValuedMammal]

ACK ed9df3c

• You could make a note somewhere that this also includes an API breaking change in CreatePsbtError
• Following up on the no-std discussion, we should have default features disabled for bdk_coin_select dependency, but I ran into an issue trying to implement it - bitcoindevkit/coin-select#36

[nymius]

ACK ed9df3c

A little bit redundant now, but worth vouching for regardless.

To complement @ValuedMammal, I think is enough to change commit ed9df3c message from feat: enable random anti-fee sniping to feat!: enable random anti-fee sniping and comment about the rationale for the variant additions on the body of the commit message.

[aagbotemi]

make a note somewhere that this also includes an API breaking change in CreatePsbtError

Noted. Thank you.

Following up on the no-std discussion, we should have default features disabled for bdk_coin_select dependency, but I ran into an issue trying to implement it - bitcoindevkit/coin-select#36

no-std support is blocked by bdk_coin_select not having a no-std feature. I will fix it upstream.

change commit ed9df3c message from feat: enable random anti-fee sniping to feat!: enable random anti-fee sniping and comment about the rationale for the variant additions on the body of the commit message.

Thank you for the suggestion. I will fix it

[nymius]

ACK 179860e

[ValuedMammal]

ACK 179860e

[nymius]

@aagbotemi good contribution, well done!