Skip to content

Commit 1ad01a4

Browse files
furszyfurszy
committed
[Refactor] Wallet key creation, stop throwing a runtime_error if the wallet is locked, returning false plus the status error message.
1 parent 9defbad commit 1ad01a4

File tree

10 files changed

+89
-23
lines changed

10 files changed

+89
-23
lines changed

‎src/Makefile.am‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ BITCOIN_CORE_H = \
104104
primitives/transaction.h \
105105
core_io.h \
106106
crypter.h \
107+
pairresult.h \
107108
denomination_functions.h \
108109
obfuscation.h \
109110
obfuscation-relay.h \

‎src/pairresult.h‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright (c) 2019 The PIVX developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#ifndef FURSZY_PIVX_PAIRRESULT_H
6+
#define FURSZY_PIVX_PAIRRESULT_H
7+
8+
9+
class PairResult {
10+
11+
public:
12+
PairResult(bool res):result(res){}
13+
PairResult(bool res, std::string* statusStr):result(res), status(statusStr){}
14+
15+
bool result;
16+
std::string* status = nullptr;
17+
};
18+
19+
20+
#endif //FURSZY_PIVX_PAIRRESULT_H

‎src/qt/pivx/masternodewizarddialog.cpp‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "qt/pivx/forms/ui_masternodewizarddialog.h"
77
#include "qt/pivx/qtutils.h"
88
#include "optionsmodel.h"
9+
#include "pairresult.h"
910
#include "activemasternode.h"
1011
#include <QFile>
1112
#include <QIntValidator>
@@ -178,7 +179,14 @@ bool MasterNodeWizardDialog::createMN(){
178179
std::string port = portStr.toStdString();
179180

180181
// New receive address
181-
CBitcoinAddress address = walletModel->getNewAddress(alias);
182+
CBitcoinAddress address;
183+
PairResult r = walletModel->getNewAddress(address, alias);
184+
185+
if (!r.result) {
186+
// generate address fail
187+
inform(tr(r.status->c_str()));
188+
return false;
189+
}
182190

183191
// const QString& addr, const QString& label, const CAmount& amount, const QString& message
184192
SendCoinsRecipient sendCoinsRecipient(QString::fromStdString(address.ToString()), QString::fromStdString(alias), CAmount(10000) * COIN, "");

‎src/qt/pivx/receivewidget.cpp‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "qt/pivx/furlistrow.h"
1212
#include "walletmodel.h"
1313
#include "guiutil.h"
14+
#include "pairresult.h"
1415

1516
#include <QModelIndex>
1617
#include <QColor>
@@ -147,8 +148,17 @@ void ReceiveWidget::loadWalletModel(){
147148
void ReceiveWidget::refreshView(QString refreshAddress){
148149
try {
149150
QString latestAddress = (refreshAddress.isEmpty()) ? this->addressTableModel->getLastUnusedAddress() : refreshAddress;
150-
if (latestAddress.isEmpty()) // new default address
151-
latestAddress = QString::fromStdString(walletModel->getNewAddress("Default").ToString());
151+
if (latestAddress.isEmpty()) { // new default address
152+
CBitcoinAddress newAddress;
153+
PairResult r = walletModel->getNewAddress(newAddress, "Default");
154+
// Check for generation errors
155+
if (!r.result) {
156+
ui->labelQrImg->setText(tr("No available address, try unlocking the wallet"));
157+
inform(tr("Error generating address"));
158+
return;
159+
}
160+
latestAddress = QString::fromStdString(newAddress.ToString());
161+
}
152162
ui->labelAddress->setText(latestAddress);
153163
int64_t time = walletModel->getKeyCreationTime(CBitcoinAddress(latestAddress.toStdString()));
154164
ui->labelDate->setText(GUIUtil::dateTimeStr(QDateTime::fromTime_t(static_cast<uint>(time))));
@@ -225,7 +235,15 @@ void ReceiveWidget::onLabelClicked(){
225235
void ReceiveWidget::onNewAddressClicked(){
226236
try {
227237
if (!verifyWalletUnlocked()) return;
228-
CBitcoinAddress address = walletModel->getNewAddress("");
238+
CBitcoinAddress address;
239+
PairResult r = walletModel->getNewAddress(address, "");
240+
241+
// Check for validity
242+
if(!r.result) {
243+
inform(r.status->c_str());
244+
return;
245+
}
246+
229247
updateQr(QString::fromStdString(address.ToString()));
230248
ui->labelAddress->setText(!info->address.isEmpty() ? info->address : tr("No address"));
231249
updateLabel();

‎src/qt/pivx/requestdialog.cpp‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,17 @@ void RequestDialog::onNextClicked(){
9292
info = new SendCoinsRecipient();
9393
info->label = ui->lineEditLabel->text();
9494
info->message = ui->lineEditDescription->text();
95-
info->address = QString::fromStdString(walletModel->getNewAddress((info->label.isEmpty() ? "" : info->label.toStdString())).ToString());
95+
96+
CBitcoinAddress address;
97+
PairResult r = walletModel->getNewAddress(address, (info->label.isEmpty() ? "" : info->label.toStdString()));
98+
99+
if (!r.result) {
100+
// TODO: notify user about this error
101+
close();
102+
return;
103+
}
104+
105+
info->address = QString::fromStdString(address.ToString());
96106
int displayUnit = walletModel->getOptionsModel()->getDisplayUnit();
97107
bool isValueValid = true;
98108
CAmount value = GUIUtil::parseValue(

‎src/qt/walletmodel.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -831,12 +831,12 @@ int64_t WalletModel::getKeyCreationTime(const CBitcoinAddress& address){
831831
return 0;
832832
}
833833

834-
CBitcoinAddress WalletModel::getNewAddress(std::string label) const{
835-
return wallet->getNewAddress(label);
834+
PairResult WalletModel::getNewAddress(CBitcoinAddress& ret, std::string label) const{
835+
return wallet->getNewAddress(ret, label);
836836
}
837837

838-
CBitcoinAddress WalletModel::getNewStakingAddress(std::string label) const{
839-
return wallet->getNewStakingAddress(label);
838+
PairResult WalletModel::getNewStakingAddress(CBitcoinAddress& ret,std::string label) const{
839+
return wallet->getNewStakingAddress(ret, label);
840840
}
841841

842842
// returns a list of COutputs from COutPoints

‎src/qt/walletmodel.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "allocators.h" /* for SecureString */
1515
#include "swifttx.h"
1616
#include "wallet/wallet.h"
17+
#include "pairresult.h"
1718

1819
#include <map>
1920
#include <vector>
@@ -245,11 +246,11 @@ class WalletModel : public QObject
245246
int64_t getCreationTime() const;
246247
int64_t getKeyCreationTime(const CPubKey& key);
247248
int64_t getKeyCreationTime(const CBitcoinAddress& address);
248-
CBitcoinAddress getNewAddress(std::string label = "") const;
249+
PairResult getNewAddress(CBitcoinAddress& ret, std::string label = "") const;
249250
/**
250251
* Return a new address used to receive for delegated cold stake purpose.
251252
*/
252-
CBitcoinAddress getNewStakingAddress(std::string label = "") const;
253+
PairResult getNewStakingAddress(CBitcoinAddress& ret, std::string label = "") const;
253254
bool isMine(CBitcoinAddress address);
254255
bool isUsed(CBitcoinAddress address);
255256
void getOutputs(const std::vector<COutPoint>& vOutpoints, std::vector<COutput>& vOutputs);

‎src/wallet/rpcwallet.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,11 @@ CBitcoinAddress GetNewAddressFromAccount(const std::string purpose, const UniVal
8989
if (!params.isNull() && params.size() > 0)
9090
strAccount = AccountFromValue(params[0]);
9191

92-
return pwalletMain->getNewAddress(strAccount, purpose, addrType);
92+
CBitcoinAddress address;
93+
PairResult r = pwalletMain->getNewAddress(address, strAccount, purpose, addrType);
94+
if(!r.result)
95+
throw JSONRPCError(RPC_WALLET_UNLOCK_NEEDED, *r.status);
96+
return address;
9397
}
9498

9599
UniValue getnewaddress(const UniValue& params, bool fHelp)

‎src/wallet/wallet.cpp‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -101,29 +101,32 @@ std::vector<CWalletTx> CWallet::getWalletTxs()
101101
return result;
102102
}
103103

104-
CBitcoinAddress CWallet::getNewAddress(std::string label){
105-
return getNewAddress(label, "receive");
104+
PairResult CWallet::getNewAddress(CBitcoinAddress& ret, std::string label){
105+
return getNewAddress(ret, label, "receive");
106106
}
107107

108-
CBitcoinAddress CWallet::getNewStakingAddress(std::string label){
109-
return getNewAddress(label, "coldstaking", CChainParams::Base58Type::STAKING_ADDRESS);
108+
PairResult CWallet::getNewStakingAddress(CBitcoinAddress& ret, std::string label){
109+
return getNewAddress(ret, label, "coldstaking", CChainParams::Base58Type::STAKING_ADDRESS);
110110
}
111111

112-
CBitcoinAddress CWallet::getNewAddress(const std::string addressLabel, const std::string purpose,
112+
PairResult CWallet::getNewAddress(CBitcoinAddress& ret, const std::string addressLabel, const std::string purpose,
113113
const CChainParams::Base58Type addrType)
114114
{
115115
LOCK2(cs_main, cs_wallet);
116116

117-
if (!pwalletMain->IsLocked())
118-
pwalletMain->TopUpKeyPool();
117+
// Perform all the possibly checks before try to get/generate a key and fail with a runtime-error.
118+
if (IsLocked())
119+
return PairResult(false, new std::string("Cannot create key, wallet locked"));
119120

121+
pwalletMain->TopUpKeyPool();
120122
// Generate a new key that is added to wallet
121123
CPubKey newKey = GenerateNewKey();
122124
CKeyID keyID = newKey.GetID();
123125

124126
pwalletMain->SetAddressBook(keyID, addressLabel, purpose);
125127

126-
return CBitcoinAddress(keyID, addrType);
128+
ret = CBitcoinAddress(keyID, addrType);
129+
return PairResult(true);
127130
}
128131

129132
CPubKey CWallet::GenerateNewKey()

‎src/wallet/wallet.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "key.h"
1616
#include "keystore.h"
1717
#include "main.h"
18+
#include "pairresult.h"
1819
#include "primitives/block.h"
1920
#include "primitives/transaction.h"
2021
#include "zpiv/zerocoin.h"
@@ -366,10 +367,10 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
366367
// keystore implementation
367368
// Generate a new key
368369
CPubKey GenerateNewKey();
369-
CBitcoinAddress getNewAddress(const std::string addressLabel, const std::string purpose,
370+
PairResult getNewAddress(CBitcoinAddress& ret, const std::string addressLabel, const std::string purpose,
370371
const CChainParams::Base58Type addrType = CChainParams::PUBKEY_ADDRESS);
371-
CBitcoinAddress getNewAddress(std::string label);
372-
CBitcoinAddress getNewStakingAddress(std::string label);
372+
PairResult getNewAddress(CBitcoinAddress& ret, std::string label);
373+
PairResult getNewStakingAddress(CBitcoinAddress& ret, std::string label);
373374
CBitcoinAddress GenerateNewAutoMintKey();
374375
int64_t GetKeyCreationTime(CPubKey pubkey);
375376
int64_t GetKeyCreationTime(const CBitcoinAddress& address);

0 commit comments

Comments
 (0)