Skip to content

Commit

Permalink
refactor: Replace BResult with util::Result
Browse files Browse the repository at this point in the history
Replace BResult usages with util::Result and delete BResult class. Changes are
rote and mechanical and don't affect behavior.
  • Loading branch information
ryanofsky committed Jul 22, 2022
1 parent 471535d commit 13aeab1
Show file tree
Hide file tree
Showing 21 changed files with 80 additions and 102 deletions.
4 changes: 2 additions & 2 deletions src/bench/wallet_loading.cpp
Expand Up @@ -46,10 +46,10 @@ static void BenchUnloadWallet(std::shared_ptr<CWallet>&& wallet)
static void AddTx(CWallet& wallet)
{
const auto& dest = wallet.GetNewDestination(OutputType::BECH32, "");
assert(dest.HasRes());
assert(dest);

CMutableTransaction mtx;
mtx.vout.push_back({COIN, GetScriptForDestination(dest.GetObj())});
mtx.vout.push_back({COIN, GetScriptForDestination(*dest)});
mtx.vin.push_back(CTxIn());

wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInactive{});
Expand Down
6 changes: 3 additions & 3 deletions src/interfaces/wallet.h
Expand Up @@ -88,7 +88,7 @@ class Wallet
virtual std::string getWalletName() = 0;

// Get a new address.
virtual BResult<CTxDestination> getNewDestination(const OutputType type, const std::string label) = 0;
virtual util::Result<CTxDestination> getNewDestination(const OutputType type, const std::string label) = 0;

//! Get public key.
virtual bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) = 0;
Expand Down Expand Up @@ -139,7 +139,7 @@ class Wallet
virtual void listLockedCoins(std::vector<COutPoint>& outputs) = 0;

//! Create transaction.
virtual BResult<CTransactionRef> createTransaction(const std::vector<wallet::CRecipient>& recipients,
virtual util::Result<CTransactionRef> createTransaction(const std::vector<wallet::CRecipient>& recipients,
const wallet::CCoinControl& coin_control,
bool sign,
int& change_pos,
Expand Down Expand Up @@ -329,7 +329,7 @@ class WalletLoader : public ChainClient
virtual std::string getWalletDir() = 0;

//! Restore backup wallet
virtual BResult<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) = 0;
virtual util::Result<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) = 0;

//! Return available wallets in wallet directory.
virtual std::vector<std::string> listWalletDir() = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/qt/addresstablemodel.cpp
Expand Up @@ -384,7 +384,7 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con
return QString();
}
}
strAddress = EncodeDestination(op_dest.GetObj());
strAddress = EncodeDestination(*op_dest);
}
else
{
Expand Down
4 changes: 2 additions & 2 deletions src/qt/walletcontroller.cpp
Expand Up @@ -393,8 +393,8 @@ void RestoreWalletActivity::restore(const fs::path& backup_file, const std::stri
QTimer::singleShot(0, worker(), [this, backup_file, wallet_name] {
auto wallet{node().walletLoader().restoreWallet(backup_file, wallet_name, m_warning_message)};

m_error_message = wallet ? bilingual_str{} : wallet.GetError();
if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(wallet.ReleaseObj());
m_error_message = wallet ? bilingual_str{} : util::ErrorString(wallet);
if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(*wallet));

QTimer::singleShot(0, this, &RestoreWalletActivity::finish);
});
Expand Down
4 changes: 2 additions & 2 deletions src/qt/walletmodel.cpp
Expand Up @@ -207,7 +207,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact

auto& newTx = transaction.getWtx();
const auto& res = m_wallet->createTransaction(vecSend, coinControl, !wallet().privateKeysDisabled() /* sign */, nChangePosRet, nFeeRequired);
newTx = res ? res.GetObj() : nullptr;
newTx = res ? *res : nullptr;
transaction.setTransactionFee(nFeeRequired);
if (fSubtractFeeFromAmount && newTx)
transaction.reassignAmounts(nChangePosRet);
Expand All @@ -218,7 +218,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
{
return SendCoinsReturn(AmountWithFeeExceedsBalance);
}
Q_EMIT message(tr("Send Coins"), QString::fromStdString(res.GetError().translated),
Q_EMIT message(tr("Send Coins"), QString::fromStdString(util::ErrorString(res).translated),
CClientUIInterface::MSG_ERROR);
return TransactionCreationFailed;
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/util/wallet.cpp
Expand Up @@ -21,9 +21,9 @@ std::string getnewaddress(CWallet& w)
{
constexpr auto output_type = OutputType::BECH32;
auto op_dest = w.GetNewDestination(output_type, "");
assert(op_dest.HasRes());
assert(op_dest);

return EncodeDestination(op_dest.GetObj());
return EncodeDestination(*op_dest);
}

#endif // ENABLE_WALLET
22 changes: 0 additions & 22 deletions src/util/result.h
Expand Up @@ -215,26 +215,4 @@ template <typename T, typename F>
bilingual_str ErrorString(const Result<T, F>& result) { return ErrorString(result.GetErrors(), result.GetWarnings()); }
} // namespace util

/**
* Backwards-compatible interface for util::Result class. New code should prefer
* util::Result class which supports returning error information along with
* result information and supports returing `void` and `bilingual_str` results.
*/
template<class T>
class BResult {
private:
util::Result<T> m_result;

public:
BResult() : m_result{util::Error{}} {};
BResult(const T& value) : m_result{value} {}
BResult(T&& value) : m_result{std::move(value)} {}
BResult(bilingual_str error) : m_result{util::Error{std::move(error)}} {}
bool HasRes() const { return m_result.has_value(); }
const T& GetObj() const { return m_result.value(); }
T ReleaseObj() { return std::move(m_result.value()); }
const bilingual_str& GetError() const { assert(!HasRes()); return m_result.GetErrors().back(); }
explicit operator bool() const { return HasRes(); }
};

#endif // BITCOIN_UTIL_RESULT_H
4 changes: 2 additions & 2 deletions src/wallet/feebumper.cpp
Expand Up @@ -221,11 +221,11 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
constexpr int RANDOM_CHANGE_POSITION = -1;
auto res = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, new_coin_control, false);
if (!res) {
errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + res.GetError());
errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + util::ErrorString(res));
return Result::WALLET_ERROR;
}

const auto& txr = res.GetObj();
const auto& txr = *res;
// Write back new fee if successful
new_fee = txr.fee;

Expand Down
16 changes: 8 additions & 8 deletions src/wallet/interfaces.cpp
Expand Up @@ -146,7 +146,7 @@ class WalletImpl : public Wallet
void abortRescan() override { m_wallet->AbortRescan(); }
bool backupWallet(const std::string& filename) override { return m_wallet->BackupWallet(filename); }
std::string getWalletName() override { return m_wallet->GetName(); }
BResult<CTxDestination> getNewDestination(const OutputType type, const std::string label) override
util::Result<CTxDestination> getNewDestination(const OutputType type, const std::string label) override
{
LOCK(m_wallet->cs_wallet);
return m_wallet->GetNewDestination(type, label);
Expand Down Expand Up @@ -249,17 +249,17 @@ class WalletImpl : public Wallet
LOCK(m_wallet->cs_wallet);
return m_wallet->ListLockedCoins(outputs);
}
BResult<CTransactionRef> createTransaction(const std::vector<CRecipient>& recipients,
util::Result<CTransactionRef> createTransaction(const std::vector<CRecipient>& recipients,
const CCoinControl& coin_control,
bool sign,
int& change_pos,
CAmount& fee) override
{
LOCK(m_wallet->cs_wallet);
const auto& res = CreateTransaction(*m_wallet, recipients, change_pos,
auto res = CreateTransaction(*m_wallet, recipients, change_pos,
coin_control, sign);
if (!res) return res.GetError();
const auto& txr = res.GetObj();
if (!res) return {util::Error{}, std::move(res)};
const auto& txr = *res;
fee = txr.fee;
change_pos = txr.change_pos;

Expand Down Expand Up @@ -569,12 +569,12 @@ class WalletLoaderImpl : public WalletLoader
options.require_existing = true;
return MakeWallet(m_context, LoadWallet(m_context, name, true /* load_on_start */, options, status, error, warnings));
}
BResult<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) override
util::Result<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) override
{
DatabaseStatus status;
bilingual_str error;
BResult<std::unique_ptr<Wallet>> wallet{MakeWallet(m_context, RestoreWallet(m_context, backup_file, wallet_name, /*load_on_start=*/true, status, error, warnings))};
if (!wallet) return error;
util::Result<std::unique_ptr<Wallet>> wallet{MakeWallet(m_context, RestoreWallet(m_context, backup_file, wallet_name, /*load_on_start=*/true, status, error, warnings))};
if (!wallet) return {util::Error{error}};
return wallet;
}
std::string getWalletDir() override
Expand Down
8 changes: 4 additions & 4 deletions src/wallet/rpc/addresses.cpp
Expand Up @@ -60,10 +60,10 @@ RPCHelpMan getnewaddress()

auto op_dest = pwallet->GetNewDestination(output_type, label);
if (!op_dest) {
throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original);
throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, util::ErrorString(op_dest).original);
}

return EncodeDestination(op_dest.GetObj());
return EncodeDestination(*op_dest);
},
};
}
Expand Down Expand Up @@ -107,9 +107,9 @@ RPCHelpMan getrawchangeaddress()

auto op_dest = pwallet->GetNewChangeDestination(output_type);
if (!op_dest) {
throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original);
throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, util::ErrorString(op_dest).original);
}
return EncodeDestination(op_dest.GetObj());
return EncodeDestination(*op_dest);
},
};
}
Expand Down
6 changes: 3 additions & 3 deletions src/wallet/rpc/spend.cpp
Expand Up @@ -158,14 +158,14 @@ UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vecto
constexpr int RANDOM_CHANGE_POSITION = -1;
auto res = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, coin_control, true);
if (!res) {
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, res.GetError().original);
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, util::ErrorString(res).original);
}
const CTransactionRef& tx = res.GetObj().tx;
const CTransactionRef& tx = res->tx;
wallet.CommitTransaction(tx, std::move(map_value), {} /* orderForm */);
if (verbose) {
UniValue entry(UniValue::VOBJ);
entry.pushKV("txid", tx->GetHash().GetHex());
entry.pushKV("fee_reason", StringForFeeReason(res.GetObj().fee_calc.reason));
entry.pushKV("fee_reason", StringForFeeReason(res->fee_calc.reason));
return entry;
}
return tx->GetHash().GetHex();
Expand Down
20 changes: 10 additions & 10 deletions src/wallet/scriptpubkeyman.cpp
Expand Up @@ -21,10 +21,10 @@ namespace wallet {
//! Value for the first BIP 32 hardened derivation. Can be used as a bit mask and as a value. See BIP 32 for more details.
const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000;

BResult<CTxDestination> LegacyScriptPubKeyMan::GetNewDestination(const OutputType type)
util::Result<CTxDestination> LegacyScriptPubKeyMan::GetNewDestination(const OutputType type)
{
if (LEGACY_OUTPUT_TYPES.count(type) == 0) {
return _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types");;
return {util::Error{_("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types")}};
}
assert(type != OutputType::BECH32M);

Expand All @@ -33,7 +33,7 @@ BResult<CTxDestination> LegacyScriptPubKeyMan::GetNewDestination(const OutputTyp
// Generate a new key that is added to wallet
CPubKey new_key;
if (!GetKeyFromPool(new_key, type)) {
return _("Error: Keypool ran out, please call keypoolrefill first");
return {util::Error{_("Error: Keypool ran out, please call keypoolrefill first")}};
}
LearnRelatedScripts(new_key, type);
return GetDestinationForKey(new_key, type);
Expand Down Expand Up @@ -1654,11 +1654,11 @@ std::set<CKeyID> LegacyScriptPubKeyMan::GetKeys() const
return set_address;
}

BResult<CTxDestination> DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type)
util::Result<CTxDestination> DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type)
{
// Returns true if this descriptor supports getting new addresses. Conditions where we may be unable to fetch them (e.g. locked) are caught later
if (!CanGetAddresses()) {
return _("No addresses available");
return {util::Error{_("No addresses available")}};
}
{
LOCK(cs_desc_man);
Expand All @@ -1676,11 +1676,11 @@ BResult<CTxDestination> DescriptorScriptPubKeyMan::GetNewDestination(const Outpu
std::vector<CScript> scripts_temp;
if (m_wallet_descriptor.range_end <= m_max_cached_index && !TopUp(1)) {
// We can't generate anymore keys
return _("Error: Keypool ran out, please call keypoolrefill first");
return {util::Error{_("Error: Keypool ran out, please call keypoolrefill first")}};
}
if (!m_wallet_descriptor.descriptor->ExpandFromCache(m_wallet_descriptor.next_index, m_wallet_descriptor.cache, scripts_temp, out_keys)) {
// We can't generate anymore keys
return _("Error: Keypool ran out, please call keypoolrefill first");
return {util::Error{_("Error: Keypool ran out, please call keypoolrefill first")}};
}

CTxDestination dest;
Expand Down Expand Up @@ -1766,11 +1766,11 @@ bool DescriptorScriptPubKeyMan::GetReservedDestination(const OutputType type, bo
auto op_dest = GetNewDestination(type);
index = m_wallet_descriptor.next_index - 1;
if (op_dest) {
address = op_dest.GetObj();
address = *op_dest;
} else {
error = op_dest.GetError();
error = util::ErrorString(op_dest);
}
return op_dest.HasRes();
return op_dest;
}

void DescriptorScriptPubKeyMan::ReturnDestination(int64_t index, bool internal, const CTxDestination& addr)
Expand Down
6 changes: 3 additions & 3 deletions src/wallet/scriptpubkeyman.h
Expand Up @@ -172,7 +172,7 @@ class ScriptPubKeyMan
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.
Expand Down Expand Up @@ -360,7 +360,7 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProv
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;
Expand Down Expand Up @@ -568,7 +568,7 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan

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;
Expand Down

0 comments on commit 13aeab1

Please sign in to comment.