bitcoindevkit
Reintroduce a way verify a transaction
Describe the enhancement
In pre-1.0 there was a function to verify a transaction against consensus rules and there should also be a convenient way to do this in BDK 1.0.
See original function bdk::wallet::verify::verify_tx.
Use case
This function is useful for verifying a transaction is valid before broadcasting it.
Additional context
https://docs.rs/bdk/latest/src/bdk/wallet/verify.rs.html#34-73
Automated payjoin receivers must check that the "original psbt" transaction to be augmented can be broadcast as fallback in case the sender doesn't broadcast the payjoin they propose so that they can still get paid in the failure case. This function is useful in that case, and I'd hope to see a reference to testmempoolaccept bitcoind rpc because that's where I looked for this functionality at first.
That RPC also tests against the UTXO set which is important for this use case.
Should this be a new build feature in bdk, or should we just include the bitcoinconsensus feature in the dependency for rust-bitcoin?
rust bitcoin offers what looks like a fairly convenient API as a method on Transaction.
If we want something more granular, we would likely want to pull in rust-bitcoinconsensus as a new dep.
As a first step, I like the idea of making verify_tx a method on Wallet that takes a &Transaction as param.
pub fn verify_tx(&self, tx: &Transaction) -> Result<(), VerifyTxError> {
tx.verify(|op: &OutPoint| -> Option<TxOut> { self.tx_graph().get_txout(*op).cloned() })
.map_err(VerifyTxError::BitcoinConsensus)
}
This verify method belongs on TxGraph in bdk_chain feature gated on the bitcoinconsensus feature. Wallet can just call it internally.
There's some more discussion for the original feature in bitcoindevkit/bdk#352.
@notmandatory this is very relevant to #1374. The problem we should solve here is the RBF feature being mis-designed. You shouldn't just RBF a transaction by looking at the inputs and outputs without understanding the semantics of it. How do we know which outputs and inputs have to be there and which can we change. We guess and provide awkward APIs to let the developer hint us (allow_shrinking). Applications should store the purpose of a transaction at the application layer i.e. "paying john x BTC" and recreate a transaction serving the same purpose from scratch with the constraint that it has to spend one of the existing transaction's inputs. There are much fewer ways to screw this approach up and it is more powerful (you can replace multiple txs at once).
I see how this is valuable especially since we had it in the pre-1.0.0 wallet API, so I agree this should stay in the 1.0.0-alpha milestone.
Since this can be enabled with a new feature flag without an api change I propose we move this to a post 1.0 milestone.