Refactor: Updated TransactionError to use util::Expected and removed TransactionError:OK #35105

Summary

Similar to #32958 and motivated by #32958#pullrequestreview-3107029707

We now are using util::Expected and removed TransactionError::OK

This makes the enum less confusing because it no longer includes OK.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #34806 (refactor: logging: Various API improvements by ajtowns)
• #33117 (Interfaces: Expose UTXO Snapshot Loading and Add Progress Notifications by D33r-Gee)
• #29278 (Wallet: Add maxfeerate wallet startup option by ismaelsadeeq)

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.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

Approach NACK

I do not think this is the best future proof way to accomplish the goal. In C++ 23 std::expected is introduced, which is a better way (imo) to do for explicit error handling. E.g link link2

Yes we are currently on C++ 20 but a "polyfill" std::expected is already introduced in the code as util::Expected<T, E>

This makes the upgrade path to C++23 an easy search and replace and is a better way to do explicit error handling / remove the ok from the errors.

@janb84 thanks for the review!

I agree and I pushed 592a833 to instead use util::Expected.

Feel free to give this another look and let me know if it seems fine to you