Skip to content

Commit 111ea3a

Browse files
committed
wallet: refactor GetNewDestination, use BResult
1 parent 2235172 commit 111ea3a

File tree

13 files changed

+71
-85
lines changed

13 files changed

+71
-85
lines changed

src/bench/wallet_loading.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,11 @@ static void BenchUnloadWallet(std::shared_ptr<CWallet>&& wallet)
4747

4848
static void AddTx(CWallet& wallet)
4949
{
50-
bilingual_str error;
51-
CTxDestination dest;
52-
wallet.GetNewDestination(OutputType::BECH32, "", dest, error);
50+
const auto& dest = wallet.GetNewDestination(OutputType::BECH32, "");
51+
assert(dest.HasRes());
5352

5453
CMutableTransaction mtx;
55-
mtx.vout.push_back({COIN, GetScriptForDestination(dest)});
54+
mtx.vout.push_back({COIN, GetScriptForDestination(dest.GetObj())});
5655
mtx.vin.push_back(CTxIn());
5756

5857
wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInactive{});

src/interfaces/wallet.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ class Wallet
8888
virtual std::string getWalletName() = 0;
8989

9090
// Get a new address.
91-
virtual bool getNewDestination(const OutputType type, const std::string label, CTxDestination& dest) = 0;
91+
virtual BResult<CTxDestination> getNewDestination(const OutputType type, const std::string label) = 0;
9292

9393
//! Get public key.
9494
virtual bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) = 0;

src/qt/addresstablemodel.cpp

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -370,23 +370,21 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con
370370
else if(type == Receive)
371371
{
372372
// Generate a new address to associate with given label
373-
CTxDestination dest;
374-
if(!walletModel->wallet().getNewDestination(address_type, strLabel, dest))
375-
{
373+
auto op_dest = walletModel->wallet().getNewDestination(address_type, strLabel);
374+
if (!op_dest) {
376375
WalletModel::UnlockContext ctx(walletModel->requestUnlock());
377-
if(!ctx.isValid())
378-
{
376+
if (!ctx.isValid()) {
379377
// Unlock wallet failed or was cancelled
380378
editStatus = WALLET_UNLOCK_FAILURE;
381379
return QString();
382380
}
383-
if(!walletModel->wallet().getNewDestination(address_type, strLabel, dest))
384-
{
381+
op_dest = walletModel->wallet().getNewDestination(address_type, strLabel);
382+
if (!op_dest) {
385383
editStatus = KEY_GENERATION_FAILURE;
386384
return QString();
387385
}
388386
}
389-
strAddress = EncodeDestination(dest);
387+
strAddress = EncodeDestination(op_dest.GetObj());
390388
}
391389
else
392390
{

src/test/util/wallet.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,10 @@ const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqq
2020
std::string getnewaddress(CWallet& w)
2121
{
2222
constexpr auto output_type = OutputType::BECH32;
23-
CTxDestination dest;
24-
bilingual_str error;
25-
if (!w.GetNewDestination(output_type, "", dest, error)) assert(false);
23+
auto op_dest = w.GetNewDestination(output_type, "");
24+
assert(op_dest.HasRes());
2625

27-
return EncodeDestination(dest);
26+
return EncodeDestination(op_dest.GetObj());
2827
}
2928

3029
#endif // ENABLE_WALLET

src/wallet/interfaces.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,11 +146,10 @@ class WalletImpl : public Wallet
146146
void abortRescan() override { m_wallet->AbortRescan(); }
147147
bool backupWallet(const std::string& filename) override { return m_wallet->BackupWallet(filename); }
148148
std::string getWalletName() override { return m_wallet->GetName(); }
149-
bool getNewDestination(const OutputType type, const std::string label, CTxDestination& dest) override
149+
BResult<CTxDestination> getNewDestination(const OutputType type, const std::string label) override
150150
{
151151
LOCK(m_wallet->cs_wallet);
152-
bilingual_str error;
153-
return m_wallet->GetNewDestination(type, label, dest, error);
152+
return m_wallet->GetNewDestination(type, label);
154153
}
155154
bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) override
156155
{

src/wallet/rpc/addresses.cpp

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,12 @@ RPCHelpMan getnewaddress()
5858
output_type = parsed.value();
5959
}
6060

61-
CTxDestination dest;
62-
bilingual_str error;
63-
if (!pwallet->GetNewDestination(output_type, label, dest, error)) {
64-
throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, error.original);
61+
auto op_dest = pwallet->GetNewDestination(output_type, label);
62+
if (!op_dest) {
63+
throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original);
6564
}
6665

67-
return EncodeDestination(dest);
66+
return EncodeDestination(op_dest.GetObj());
6867
},
6968
};
7069
}
@@ -106,12 +105,11 @@ RPCHelpMan getrawchangeaddress()
106105
output_type = parsed.value();
107106
}
108107

109-
CTxDestination dest;
110-
bilingual_str error;
111-
if (!pwallet->GetNewChangeDestination(output_type, dest, error)) {
112-
throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, error.original);
108+
auto op_dest = pwallet->GetNewChangeDestination(output_type);
109+
if (!op_dest) {
110+
throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original);
113111
}
114-
return EncodeDestination(dest);
112+
return EncodeDestination(op_dest.GetObj());
115113
},
116114
};
117115
}

src/wallet/scriptpubkeyman.cpp

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,26 +21,22 @@ namespace wallet {
2121
//! 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.
2222
const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000;
2323

24-
bool LegacyScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error)
24+
BResult<CTxDestination> LegacyScriptPubKeyMan::GetNewDestination(const OutputType type)
2525
{
2626
if (LEGACY_OUTPUT_TYPES.count(type) == 0) {
27-
error = _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types");
28-
return false;
27+
return _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types");;
2928
}
3029
assert(type != OutputType::BECH32M);
3130

3231
LOCK(cs_KeyStore);
33-
error.clear();
3432

3533
// Generate a new key that is added to wallet
3634
CPubKey new_key;
3735
if (!GetKeyFromPool(new_key, type)) {
38-
error = _("Error: Keypool ran out, please call keypoolrefill first");
39-
return false;
36+
return _("Error: Keypool ran out, please call keypoolrefill first");
4037
}
4138
LearnRelatedScripts(new_key, type);
42-
dest = GetDestinationForKey(new_key, type);
43-
return true;
39+
return GetDestinationForKey(new_key, type);
4440
}
4541

4642
typedef std::vector<unsigned char> valtype;
@@ -1658,12 +1654,11 @@ std::set<CKeyID> LegacyScriptPubKeyMan::GetKeys() const
16581654
return set_address;
16591655
}
16601656

1661-
bool DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error)
1657+
BResult<CTxDestination> DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type)
16621658
{
16631659
// Returns true if this descriptor supports getting new addresses. Conditions where we may be unable to fetch them (e.g. locked) are caught later
16641660
if (!CanGetAddresses()) {
1665-
error = _("No addresses available");
1666-
return false;
1661+
return _("No addresses available");
16671662
}
16681663
{
16691664
LOCK(cs_desc_man);
@@ -1681,15 +1676,14 @@ bool DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDest
16811676
std::vector<CScript> scripts_temp;
16821677
if (m_wallet_descriptor.range_end <= m_max_cached_index && !TopUp(1)) {
16831678
// We can't generate anymore keys
1684-
error = _("Error: Keypool ran out, please call keypoolrefill first");
1685-
return false;
1679+
return _("Error: Keypool ran out, please call keypoolrefill first");
16861680
}
16871681
if (!m_wallet_descriptor.descriptor->ExpandFromCache(m_wallet_descriptor.next_index, m_wallet_descriptor.cache, scripts_temp, out_keys)) {
16881682
// We can't generate anymore keys
1689-
error = _("Error: Keypool ran out, please call keypoolrefill first");
1690-
return false;
1683+
return _("Error: Keypool ran out, please call keypoolrefill first");
16911684
}
16921685

1686+
CTxDestination dest;
16931687
std::optional<OutputType> out_script_type = m_wallet_descriptor.descriptor->GetOutputType();
16941688
if (out_script_type && out_script_type == type) {
16951689
ExtractDestination(scripts_temp[0], dest);
@@ -1698,7 +1692,7 @@ bool DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDest
16981692
}
16991693
m_wallet_descriptor.next_index++;
17001694
WalletBatch(m_storage.GetDatabase()).WriteDescriptor(GetID(), m_wallet_descriptor);
1701-
return true;
1695+
return dest;
17021696
}
17031697
}
17041698

@@ -1769,9 +1763,14 @@ bool DescriptorScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, Walle
17691763
bool DescriptorScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error)
17701764
{
17711765
LOCK(cs_desc_man);
1772-
bool result = GetNewDestination(type, address, error);
1766+
auto op_dest = GetNewDestination(type);
17731767
index = m_wallet_descriptor.next_index - 1;
1774-
return result;
1768+
if (op_dest) {
1769+
address = op_dest.GetObj();
1770+
} else {
1771+
error = op_dest.GetError();
1772+
}
1773+
return op_dest.HasRes();
17751774
}
17761775

17771776
void DescriptorScriptPubKeyMan::ReturnDestination(int64_t index, bool internal, const CTxDestination& addr)

src/wallet/scriptpubkeyman.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <script/standard.h>
1212
#include <util/error.h>
1313
#include <util/message.h>
14+
#include <util/result.h>
1415
#include <util/time.h>
1516
#include <wallet/crypter.h>
1617
#include <wallet/ismine.h>
@@ -171,7 +172,7 @@ class ScriptPubKeyMan
171172
public:
172173
explicit ScriptPubKeyMan(WalletStorage& storage) : m_storage(storage) {}
173174
virtual ~ScriptPubKeyMan() {};
174-
virtual bool GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) { return false; }
175+
virtual BResult<CTxDestination> GetNewDestination(const OutputType type) { return Untranslated("Not supported"); }
175176
virtual isminetype IsMine(const CScript& script) const { return ISMINE_NO; }
176177

177178
//! Check that the given decryption key is valid for this ScriptPubKeyMan, i.e. it decrypts all of the keys handled by it.
@@ -359,7 +360,7 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProv
359360
public:
360361
using ScriptPubKeyMan::ScriptPubKeyMan;
361362

362-
bool GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) override;
363+
BResult<CTxDestination> GetNewDestination(const OutputType type) override;
363364
isminetype IsMine(const CScript& script) const override;
364365

365366
bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override;
@@ -567,7 +568,7 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
567568

568569
mutable RecursiveMutex cs_desc_man;
569570

570-
bool GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) override;
571+
BResult<CTxDestination> GetNewDestination(const OutputType type) override;
571572
isminetype IsMine(const CScript& script) const override;
572573

573574
bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override;

src/wallet/test/coinselector_tests.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,9 @@ static void add_coin(std::vector<COutput>& coins, CWallet& wallet, const CAmount
7474
tx.vout.resize(nInput + 1);
7575
tx.vout[nInput].nValue = nValue;
7676
if (spendable) {
77-
CTxDestination dest;
78-
bilingual_str error;
79-
const bool destination_ok = wallet.GetNewDestination(OutputType::BECH32, "", dest, error);
80-
assert(destination_ok);
81-
tx.vout[nInput].scriptPubKey = GetScriptForDestination(dest);
77+
auto op_dest = wallet.GetNewDestination(OutputType::BECH32, "");
78+
assert(op_dest.HasRes());
79+
tx.vout[nInput].scriptPubKey = GetScriptForDestination(op_dest.GetObj());
8280
}
8381
uint256 txid = tx.GetHash();
8482

src/wallet/test/fuzz/notifications.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,14 @@ struct FuzzedWallet {
6969
CScript GetScriptPubKey(FuzzedDataProvider& fuzzed_data_provider)
7070
{
7171
auto type{fuzzed_data_provider.PickValueInArray(OUTPUT_TYPES)};
72-
CTxDestination dest;
73-
bilingual_str error;
72+
BResult<CTxDestination> op_dest;
7473
if (fuzzed_data_provider.ConsumeBool()) {
75-
assert(wallet->GetNewDestination(type, "", dest, error));
74+
op_dest = wallet->GetNewDestination(type, "");
7675
} else {
77-
assert(wallet->GetNewChangeDestination(type, dest, error));
76+
op_dest = wallet->GetNewChangeDestination(type);
7877
}
79-
assert(error.empty());
80-
return GetScriptForDestination(dest);
78+
assert(op_dest.HasRes());
79+
return GetScriptForDestination(op_dest.GetObj());
8180
}
8281
};
8382

0 commit comments

Comments
 (0)