wallet/refactor: change PSBTError to PSBTResult and remove std::optional<common::PSBTResult> and return common::PSBTResult #32958
Conversation
PSBTError now contains the PSBTError::Ok type which means it makes sense to rename it into PSBTResult since there is a case where it does not error
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32958. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
4786148 to
0748f9b
Compare
|
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
1a246a9 to
a92955d
Compare
There was a problem hiding this comment.
Concept ACK
l have looked at 87736a9, 5959096 and 125431e changes, LGTM
makes sense to rename since PSBTError::OK is not an error.
PSBTError is consistent with TransactionError, https://github.com/bitcoin/bitcoin/blob/master/src/node/types.h#L24 so maybe TransactionError should also be renamed in a follow-up PR.
src/qt/psbtoperationsdialog.cpp
Outdated
| @@ -62,7 +62,7 @@ void PSBTOperationsDialog::openWithPSBT(PartiallySignedTransaction psbtx) | |||
| const auto err{m_wallet_model->wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, &n_could_sign, m_transaction_data, complete)}; | |||
There was a problem hiding this comment.
Thanks for the review! pushed an update in d242c75
PSBTResult has an enum value of OK, so it makes sense to return OK
insead of a {}. This change makes it so we check for an OK value instead
of null/{}
a92955d to
d242c75
Compare
Description
This is a follow-up to #31622 (comment) and #31622 (comment)
What this changes
Updates
PSBTErrortoPSBTResultbecause we now have
PSBTResult::OKUpdates
PSBTErrorStringtoPSBTResultStringUpdates
RPCErrorFromPSBTErrortoRPCErrorFromPSBTResultRemoves
std::optional<common::PSBTResult>and instead returns justcommon::PSBTResultreplacing when we return
nullwithPSBTResult::OK