Add bumpfee finish related error #516
Conversation
reez
changed the title
WalkthroughThe recent updates involve replacing the Changes
Possibly related issues
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (4)
Files skipped from review as they are similar to previous changes (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| impl From<BuildFeeBumpError> for CreateTxError { | ||
| fn from(error: BuildFeeBumpError) -> Self { | ||
| match error { | ||
| BuildFeeBumpError::UnknownUtxo(outpoint) => CreateTxError::UnknownUtxo { | ||
| outpoint: outpoint.to_string(), | ||
| }, | ||
| BuildFeeBumpError::TransactionNotFound(txid) => CreateTxError::UnknownUtxo { | ||
| outpoint: txid.to_string(), | ||
| }, | ||
| BuildFeeBumpError::TransactionConfirmed(txid) => CreateTxError::UnknownUtxo { | ||
| outpoint: txid.to_string(), | ||
| }, | ||
| BuildFeeBumpError::IrreplaceableTransaction(txid) => CreateTxError::UnknownUtxo { | ||
| outpoint: txid.to_string(), | ||
| }, | ||
| BuildFeeBumpError::FeeRateUnavailable => CreateTxError::FeeRateTooLow { | ||
| required: "unavailable".to_string(), | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
Refine error mappings from BuildFeeBumpError to CreateTxError.
The current implementation maps several distinct errors to CreateTxError::UnknownUtxo, which could be confusing. Consider providing more specific error messages for different scenarios such as transaction not found, transaction confirmed, and irreplaceable transaction.
There was a problem hiding this comment.
My first thought is to remove the allow_shrinking() method. See #457 and bitcoindevkit/bdk#1386. The method is being removed/rewritten. Would you mind doing this as part of this PR?
absolutely, will update the PR with that change- |
Description
This removes
Alpha3Errorcompletely by updating this last method that used it. ✅BumpFeeTxBuilderhas afinishmethod with a few things it hits under the hood like:TxIdfrom a stringallow_shrinkingPsbtI added a few
Froms, but did not add one for this line:... because if
Txid::from_strfails it seems likely it's because the string provided does not correctly represent a transaction identifier, and which is why I choseUnknownUtxo(and didn't create aFrom) to indicate that. But let me know if you think there is a better way to handle this (How to handle failing onfrom_strand/or if aFrommakes sense on that line of code)? Totally open to something better.Notes to the reviewers
Changelog notice
Checklists
All Submissions:
cargo fmtandcargo clippybefore committingNew Features:
Bugfixes:
Summary by CodeRabbit
Alpha3ErrorwithCreateTxErrorto provide more specific error feedback.