refactor: Replace BResult with util::Result

Rename `BResult` class to `util::Result` and update the class interface to be
more compatible with `std::optional` and with a full-featured result class
implemented in https://github.com/bitcoin/bitcoin/pull/25665. Motivation for
this change is to update existing `BResult` usages now so they don't have to
change later when more features are added in #25665.

This change makes the following improvements originally implemented in #25665:

- More explicit API. Drops potentially misleading `BResult` constructor that
  treats any bilingual string argument as an error. Adds `util::Error`
  constructor so it is never ambiguous when a result is being assigned an error
  or non-error value.

- Better type compatibility. Supports `util::Result<bilingual_str>` return
  values to hold translated messages which are not errors.

- More standard and consistent API. `util::Result` supports most of the same
  operators and methods as `std::optional`. `BResult` had a less familiar
  interface with `HasRes`/`GetObj`/`ReleaseObj` methods. The Result/Res/Obj
  naming was also not internally consistent.

- Better code organization. Puts `src/util/` code in the `util::` namespace so
  naming reflects code organization and it is obvious where the class is coming
  from. Drops "B" from name because it is undocumented what it stands for
  (bilingual?)

- Has unit tests.
This commit is contained in:
Ryan Ofsky
2022-07-21 08:30:39 -04:00
parent 4a4289e2c9
commit a23cca56c0
24 changed files with 248 additions and 126 deletions

View File

@@ -172,7 +172,7 @@ protected:
public:
explicit ScriptPubKeyMan(WalletStorage& storage) : m_storage(storage) {}
virtual ~ScriptPubKeyMan() {};
virtual BResult<CTxDestination> GetNewDestination(const OutputType type) { return Untranslated("Not supported"); }
virtual util::Result<CTxDestination> GetNewDestination(const OutputType type) { return util::Error{Untranslated("Not supported")}; }
virtual isminetype IsMine(const CScript& script) const { return ISMINE_NO; }
//! Check that the given decryption key is valid for this ScriptPubKeyMan, i.e. it decrypts all of the keys handled by it.
@@ -360,7 +360,7 @@ private:
public:
using ScriptPubKeyMan::ScriptPubKeyMan;
BResult<CTxDestination> GetNewDestination(const OutputType type) override;
util::Result<CTxDestination> GetNewDestination(const OutputType type) override;
isminetype IsMine(const CScript& script) const override;
bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override;
@@ -568,7 +568,7 @@ public:
mutable RecursiveMutex cs_desc_man;
BResult<CTxDestination> GetNewDestination(const OutputType type) override;
util::Result<CTxDestination> GetNewDestination(const OutputType type) override;
isminetype IsMine(const CScript& script) const override;
bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override;