Skip to content

Commit 831a13b

Browse files
sipadagurval
sipa
authored and
dagurval
committed
Introduce wrappers around CBitcoinAddress
Summary: This patch removes the need for the intermediary Base58 type CBitcoinAddress, by providing {Encode,Decode,IsValid}Destination function that directly operate on the conversion between strings and CTxDestination. This is a port of bitcoin/bitcoin#11117 Depends on D539 Test Plan: make check, rpc tests, manual QT testing Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Subscribers: deadalnix Differential Revision: https://reviews.bitcoinabc.org/D540
1 parent e2ecbe0 commit 831a13b

25 files changed

+317
-319
lines changed

‎src/base58.cpp‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,33 @@ int CBase58Data::CompareTo(const CBase58Data &b58) const {
208208
}
209209

210210
namespace {
211+
/**
212+
* base58-encoded Bitcoin addresses.
213+
* Public-key-hash-addresses have version 0 (or 111 testnet).
214+
* The data vector contains RIPEMD160(SHA256(pubkey)), where pubkey is the
215+
* serialized public key.
216+
* Script-hash-addresses have version 5 (or 196 testnet).
217+
* The data vector contains RIPEMD160(SHA256(cscript)), where cscript is the
218+
* serialized redemption script.
219+
*/
220+
class CBitcoinAddress : public CBase58Data {
221+
public:
222+
bool Set(const CKeyID &id);
223+
bool Set(const CScriptID &id);
224+
bool Set(const CTxDestination &dest);
225+
bool IsValid() const;
226+
bool IsValid(const CChainParams &params) const;
227+
228+
CBitcoinAddress() {}
229+
CBitcoinAddress(const CTxDestination &dest) { Set(dest); }
230+
CBitcoinAddress(const std::string &strAddress) { SetString(strAddress); }
231+
CBitcoinAddress(const char *pszAddress) { SetString(pszAddress); }
232+
233+
CTxDestination Get() const;
234+
bool GetKeyID(CKeyID &keyID) const;
235+
bool IsScript() const;
236+
};
237+
211238
class CBitcoinAddressVisitor : public boost::static_visitor<bool> {
212239
private:
213240
CBitcoinAddress *addr;
@@ -308,3 +335,21 @@ bool CBitcoinSecret::SetString(const char *pszSecret) {
308335
bool CBitcoinSecret::SetString(const std::string &strSecret) {
309336
return SetString(strSecret.c_str());
310337
}
338+
339+
std::string EncodeDestination(const CTxDestination &dest) {
340+
CBitcoinAddress addr(dest);
341+
return addr.IsValid() ? addr.ToString() : "";
342+
}
343+
344+
CTxDestination DecodeDestination(const std::string &str) {
345+
return CBitcoinAddress(str).Get();
346+
}
347+
348+
bool IsValidDestinationString(const std::string &str,
349+
const CChainParams &params) {
350+
return CBitcoinAddress(str).IsValid(params);
351+
}
352+
353+
bool IsValidDestinationString(const std::string &str) {
354+
return CBitcoinAddress(str).IsValid();
355+
}

‎src/base58.h‎

Lines changed: 6 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -106,32 +106,6 @@ class CBase58Data {
106106
bool operator>(const CBase58Data &b58) const { return CompareTo(b58) > 0; }
107107
};
108108

109-
/** base58-encoded Bitcoin addresses.
110-
* Public-key-hash-addresses have version 0 (or 111 testnet).
111-
* The data vector contains RIPEMD160(SHA256(pubkey)), where pubkey is the
112-
* serialized public key.
113-
* Script-hash-addresses have version 5 (or 196 testnet).
114-
* The data vector contains RIPEMD160(SHA256(cscript)), where cscript is the
115-
* serialized redemption script.
116-
*/
117-
class CBitcoinAddress : public CBase58Data {
118-
public:
119-
bool Set(const CKeyID &id);
120-
bool Set(const CScriptID &id);
121-
bool Set(const CTxDestination &dest);
122-
bool IsValid() const;
123-
bool IsValid(const CChainParams &params) const;
124-
125-
CBitcoinAddress() {}
126-
CBitcoinAddress(const CTxDestination &dest) { Set(dest); }
127-
CBitcoinAddress(const std::string &strAddress) { SetString(strAddress); }
128-
CBitcoinAddress(const char *pszAddress) { SetString(pszAddress); }
129-
130-
CTxDestination Get() const;
131-
bool GetKeyID(CKeyID &keyID) const;
132-
bool IsScript() const;
133-
};
134-
135109
/**
136110
* A base58-encoded secret key
137111
*/
@@ -182,4 +156,10 @@ typedef CBitcoinExtKeyBase<CExtPubKey, BIP32_EXTKEY_SIZE,
182156
CChainParams::EXT_PUBLIC_KEY>
183157
CBitcoinExtPubKey;
184158

159+
std::string EncodeDestination(const CTxDestination &dest);
160+
CTxDestination DecodeDestination(const std::string &str);
161+
bool IsValidDestinationString(const std::string &str);
162+
bool IsValidDestinationString(const std::string &str,
163+
const CChainParams &params);
164+
185165
#endif // BITCOIN_BASE58_H

‎src/bitcoin-tx.cpp‎

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -277,13 +277,11 @@ static void MutateTxAddOutAddr(CMutableTransaction &tx,
277277

278278
// extract and validate ADDRESS
279279
std::string strAddr = vStrInputParts[1];
280-
CBitcoinAddress addr(strAddr);
281-
if (!addr.IsValid()) {
280+
CTxDestination destination = DecodeDestination(strAddr);
281+
if (!IsValidDestination(destination)) {
282282
throw std::runtime_error("invalid TX output address");
283283
}
284-
285-
// build standard output script via GetScriptForDestination()
286-
CScript scriptPubKey = GetScriptForDestination(addr.Get());
284+
CScript scriptPubKey = GetScriptForDestination(destination);
287285

288286
// construct TxOut, append to transaction output list
289287
CTxOut txout(value, scriptPubKey);
@@ -310,7 +308,6 @@ static void MutateTxAddOutPubKey(CMutableTransaction &tx,
310308
}
311309

312310
CScript scriptPubKey = GetScriptForRawPubKey(pubkey);
313-
CBitcoinAddress addr(scriptPubKey);
314311

315312
// Extract and validate FLAGS
316313
bool bScriptHash = false;
@@ -320,10 +317,9 @@ static void MutateTxAddOutPubKey(CMutableTransaction &tx,
320317
}
321318

322319
if (bScriptHash) {
323-
// Get the address for the redeem script, then call
324-
// GetScriptForDestination() to construct a P2SH scriptPubKey.
325-
CBitcoinAddress redeemScriptAddr(scriptPubKey);
326-
scriptPubKey = GetScriptForDestination(redeemScriptAddr.Get());
320+
// Get the ID for the script, and then construct a P2SH destination for
321+
// it.
322+
scriptPubKey = GetScriptForDestination(CScriptID(scriptPubKey));
327323
}
328324

329325
// construct TxOut, append to transaction output list
@@ -387,10 +383,9 @@ static void MutateTxAddOutMultiSig(CMutableTransaction &tx,
387383
CScript scriptPubKey = GetScriptForMultisig(required, pubkeys);
388384

389385
if (bScriptHash) {
390-
// Get the address for the redeem script, then call
391-
// GetScriptForDestination() to construct a P2SH scriptPubKey.
392-
CBitcoinAddress addr(scriptPubKey);
393-
scriptPubKey = GetScriptForDestination(addr.Get());
386+
// Get the ID for the script, and then construct a P2SH destination for
387+
// it.
388+
scriptPubKey = GetScriptForDestination(CScriptID(scriptPubKey));
394389
}
395390

396391
// construct TxOut, append to transaction output list
@@ -450,8 +445,7 @@ static void MutateTxAddOutScript(CMutableTransaction &tx,
450445
}
451446

452447
if (bScriptHash) {
453-
CBitcoinAddress addr(scriptPubKey);
454-
scriptPubKey = GetScriptForDestination(addr.Get());
448+
scriptPubKey = GetScriptForDestination(CScriptID(scriptPubKey));
455449
}
456450

457451
// construct TxOut, append to transaction output list

‎src/core_write.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,8 @@ void ScriptPubKeyToUniv(const CScript &scriptPubKey, UniValue &out,
176176

177177
UniValue a(UniValue::VARR);
178178
for (const CTxDestination &addr : addresses) {
179-
a.push_back(CBitcoinAddress(addr).ToString());
179+
a.push_back(EncodeDestination(addr));
180180
}
181-
182181
out.pushKV("addresses", a);
183182
}
184183

‎src/qt/addresstablemodel.cpp‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -81,14 +81,14 @@ class AddressTablePriv {
8181
LOCK(wallet->cs_wallet);
8282
for (const std::pair<CTxDestination, CAddressBookData> &item :
8383
wallet->mapAddressBook) {
84-
const CBitcoinAddress &address = item.first;
85-
bool fMine = IsMine(*wallet, address.Get());
84+
const CTxDestination &address = item.first;
85+
bool fMine = IsMine(*wallet, address);
8686
AddressTableEntry::Type addressType = translateTransactionType(
8787
QString::fromStdString(item.second.purpose), fMine);
8888
const std::string &strName = item.second.name;
8989
cachedAddressTable.append(AddressTableEntry(
9090
addressType, QString::fromStdString(strName),
91-
QString::fromStdString(address.ToString())));
91+
QString::fromStdString(EncodeDestination(address))));
9292
}
9393
}
9494
// qLowerBound() and qUpperBound() require our cachedAddressTable list
@@ -233,7 +233,7 @@ bool AddressTableModel::setData(const QModelIndex &index, const QVariant &value,
233233
/* For SetAddressBook / DelAddressBook */
234234
LOCK(wallet->cs_wallet);
235235
CTxDestination curAddress =
236-
CBitcoinAddress(rec->address.toStdString()).Get();
236+
DecodeDestination(rec->address.toStdString());
237237
if (index.column() == Label) {
238238
// Do nothing, if old label == new label
239239
if (rec->label == value.toString()) {
@@ -244,7 +244,7 @@ bool AddressTableModel::setData(const QModelIndex &index, const QVariant &value,
244244
strPurpose);
245245
} else if (index.column() == Address) {
246246
CTxDestination newAddress =
247-
CBitcoinAddress(value.toString().toStdString()).Get();
247+
DecodeDestination(value.toString().toStdString());
248248
// Refuse to set invalid address, set error status and return false
249249
if (boost::get<CNoDestination>(&newAddress)) {
250250
editStatus = INVALID_ADDRESS;
@@ -335,8 +335,7 @@ QString AddressTableModel::addRow(const QString &type, const QString &label,
335335
// Check for duplicate addresses
336336
{
337337
LOCK(wallet->cs_wallet);
338-
if (wallet->mapAddressBook.count(
339-
CBitcoinAddress(strAddress).Get())) {
338+
if (wallet->mapAddressBook.count(DecodeDestination(strAddress))) {
340339
editStatus = DUPLICATE_ADDRESS;
341340
return QString();
342341
}
@@ -356,15 +355,15 @@ QString AddressTableModel::addRow(const QString &type, const QString &label,
356355
return QString();
357356
}
358357
}
359-
strAddress = CBitcoinAddress(newKey.GetID()).ToString();
358+
strAddress = EncodeDestination(newKey.GetID());
360359
} else {
361360
return QString();
362361
}
363362

364363
// Add entry
365364
{
366365
LOCK(wallet->cs_wallet);
367-
wallet->SetAddressBook(CBitcoinAddress(strAddress).Get(), strLabel,
366+
wallet->SetAddressBook(DecodeDestination(strAddress), strLabel,
368367
(type == Send ? "send" : "receive"));
369368
}
370369
return QString::fromStdString(strAddress);
@@ -382,8 +381,7 @@ bool AddressTableModel::removeRows(int row, int count,
382381
}
383382
{
384383
LOCK(wallet->cs_wallet);
385-
wallet->DelAddressBook(
386-
CBitcoinAddress(rec->address.toStdString()).Get());
384+
wallet->DelAddressBook(DecodeDestination(rec->address.toStdString()));
387385
}
388386
return true;
389387
}
@@ -393,9 +391,9 @@ bool AddressTableModel::removeRows(int row, int count,
393391
QString AddressTableModel::labelForAddress(const QString &address) const {
394392
{
395393
LOCK(wallet->cs_wallet);
396-
CBitcoinAddress address_parsed(address.toStdString());
394+
CTxDestination destination = DecodeDestination(address.toStdString());
397395
std::map<CTxDestination, CAddressBookData>::iterator mi =
398-
wallet->mapAddressBook.find(address_parsed.Get());
396+
wallet->mapAddressBook.find(destination);
399397
if (mi != wallet->mapAddressBook.end()) {
400398
return QString::fromStdString(mi->second.name);
401399
}

‎src/qt/bitcoinaddressvalidator.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,9 @@ QValidator::State BitcoinAddressCheckValidator::validate(QString &input,
7676
int &pos) const {
7777
Q_UNUSED(pos);
7878
// Validate the passed Bitcoin address
79-
CBitcoinAddress addr(input.toStdString());
80-
if (addr.IsValid()) return QValidator::Acceptable;
79+
if (IsValidDestinationString(input.toStdString())) {
80+
return QValidator::Acceptable;
81+
}
8182

8283
return QValidator::Invalid;
8384
}

‎src/qt/coincontroldialog.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -741,8 +741,8 @@ void CoinControlDialog::updateView() {
741741
QString sAddress = "";
742742
if (ExtractDestination(out.tx->tx->vout[out.i].scriptPubKey,
743743
outputAddress)) {
744-
sAddress = QString::fromStdString(
745-
CBitcoinAddress(outputAddress).ToString());
744+
sAddress =
745+
QString::fromStdString(EncodeDestination(outputAddress));
746746

747747
// if listMode or change => show bitcoin address. In tree mode,
748748
// address is not shown again for direct wallet address outputs

‎src/qt/guiutil.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,9 @@ static std::string DummyAddress(const CChainParams &params) {
122122
for (int i = 0; i < 256; ++i) { // Try every trailing byte
123123
std::string s = EncodeBase58(sourcedata.data(),
124124
sourcedata.data() + sourcedata.size());
125-
if (!CBitcoinAddress(s).IsValid()) return s;
125+
if (!IsValidDestinationString(s)) {
126+
return s;
127+
}
126128
sourcedata[sourcedata.size() - 1] += 1;
127129
}
128130
return "";
@@ -246,7 +248,7 @@ QString formatBitcoinURI(const SendCoinsRecipient &info) {
246248
}
247249

248250
bool isDust(const QString &address, const CAmount &amount) {
249-
CTxDestination dest = CBitcoinAddress(address.toStdString()).Get();
251+
CTxDestination dest = DecodeDestination(address.toStdString());
250252
CScript script = GetScriptForDestination(dest);
251253
CTxOut txOut(amount, script);
252254
return txOut.IsDust(dustRelayFee);

‎src/qt/paymentserver.cpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -223,11 +223,12 @@ void PaymentServer::ipcParseCommandLine(int argc, char *argv[]) {
223223

224224
SendCoinsRecipient r;
225225
if (GUIUtil::parseBitcoinURI(arg, &r) && !r.address.isEmpty()) {
226-
CBitcoinAddress address(r.address.toStdString());
227-
228-
if (address.IsValid(Params(CBaseChainParams::MAIN))) {
226+
if (IsValidDestinationString(r.address.toStdString(),
227+
Params(CBaseChainParams::MAIN))) {
229228
SelectParams(CBaseChainParams::MAIN);
230-
} else if (address.IsValid(Params(CBaseChainParams::TESTNET))) {
229+
} else if (IsValidDestinationString(
230+
r.address.toStdString(),
231+
Params(CBaseChainParams::TESTNET))) {
231232
SelectParams(CBaseChainParams::TESTNET);
232233
}
233234
}
@@ -420,8 +421,8 @@ void PaymentServer::handleURIOrFile(const QString &s) {
420421
// normal URI
421422
SendCoinsRecipient recipient;
422423
if (GUIUtil::parseBitcoinURI(s, &recipient)) {
423-
CBitcoinAddress address(recipient.address.toStdString());
424-
if (!address.IsValid()) {
424+
if (!IsValidDestinationString(
425+
recipient.address.toStdString())) {
425426
Q_EMIT message(
426427
tr("URI handling"),
427428
tr("Invalid payment address %1").arg(recipient.address),
@@ -545,8 +546,7 @@ bool PaymentServer::processPaymentRequest(const PaymentRequestPlus &request,
545546
CTxDestination dest;
546547
if (ExtractDestination(sendingTo.first, dest)) {
547548
// Append destination address
548-
addresses.append(
549-
QString::fromStdString(CBitcoinAddress(dest).ToString()));
549+
addresses.append(QString::fromStdString(EncodeDestination(dest)));
550550
} else if (!recipient.authenticatedMerchant.isEmpty()) {
551551
// Unauthenticated payment requests to custom bitcoin addresses are
552552
// not supported (there is no good way to tell the user where they

‎src/qt/sendcoinsdialog.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -828,18 +828,17 @@ void SendCoinsDialog::coinControlChangeEdited(const QString &text) {
828828
CoinControlDialog::coinControl->destChange = CNoDestination();
829829
ui->labelCoinControlChangeLabel->setStyleSheet("QLabel{color:red;}");
830830

831-
CBitcoinAddress addr = CBitcoinAddress(text.toStdString());
831+
const CTxDestination dest = DecodeDestination(text.toStdString());
832832

833833
if (text.isEmpty()) {
834834
// Nothing entered
835835
ui->labelCoinControlChangeLabel->setText("");
836-
} else if (!addr.IsValid()) {
836+
} else if (!IsValidDestination(dest)) {
837837
// Invalid address
838838
ui->labelCoinControlChangeLabel->setText(
839839
tr("Warning: Invalid Bitcoin address"));
840840
} else {
841841
// Valid address
842-
const CTxDestination dest = addr.Get();
843842
if (!model->IsSpendable(dest)) {
844843
ui->labelCoinControlChangeLabel->setText(
845844
tr("Warning: Unknown change address"));

0 commit comments

Comments
 (0)