Skip to content

Commit 961901f

Browse files
laanwjlaanwj
committed
Merge #11117: Prepare for non-Base58 addresses
864cd27 Move CBitcoinAddress to base58.cpp (Pieter Wuille) 5c8ff0d Introduce wrappers around CBitcoinAddress (Pieter Wuille) Pull request description: This patch removes the need for the intermediary Base58 type `CBitcoinAddress`, by providing {`Encode`,`Decode`,`IsValid`}`Destination` functions that directly operate on the conversion between `std::string`s and `CTxDestination`. As a side, it also fixes a number of indentation issues, and removes probably several unnecessary implicit `CTxDestination`<->`CBitcoinAddress` conversions. This change is far from complete. In follow-ups I'd like to: * Split off the specific address and key encoding logic from base58.h, and move it to a address.h or so. * Replace `CTxDestination` with a non-`boost::variant` version (which can be more efficient as `boost::variant` allocates everything on the heap, and remove the need for `boost::get<...>` and `IsValidDestination` calls everywhere). * Do the same for `CBitcoinSecret`, `CBitcoinExtKey`, and `CBitcoinExtPubKey`. However, I've tried to keep this patch to be minimally invasive, but still enough to support non-Base58 addresses. Perhaps a smaller patch is possible to hack Bech32 support into `CBitcoinAddress`, but I would consider that a move in the wrong direction. Tree-SHA512: c2c77ffb57caeadf2429b1c2562ce60e8c7be8aa9f8e51b591f354b6b441162625b2efe14c023a1ae485cf2ed417263afa35c892891dfaa7844e7fbabccab85e
2 parents 39ae413 + 864cd27 commit 961901f

26 files changed

+323
-301
lines changed

‎src/base58.cpp

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,30 @@ int CBase58Data::CompareTo(const CBase58Data& b58) const
212212

213213
namespace
214214
{
215+
/** base58-encoded Bitcoin addresses.
216+
* Public-key-hash-addresses have version 0 (or 111 testnet).
217+
* The data vector contains RIPEMD160(SHA256(pubkey)), where pubkey is the serialized public key.
218+
* Script-hash-addresses have version 5 (or 196 testnet).
219+
* The data vector contains RIPEMD160(SHA256(cscript)), where cscript is the serialized redemption script.
220+
*/
221+
class CBitcoinAddress : public CBase58Data {
222+
public:
223+
bool Set(const CKeyID &id);
224+
bool Set(const CScriptID &id);
225+
bool Set(const CTxDestination &dest);
226+
bool IsValid() const;
227+
bool IsValid(const CChainParams &params) const;
228+
229+
CBitcoinAddress() {}
230+
CBitcoinAddress(const CTxDestination &dest) { Set(dest); }
231+
CBitcoinAddress(const std::string& strAddress) { SetString(strAddress); }
232+
CBitcoinAddress(const char* pszAddress) { SetString(pszAddress); }
233+
234+
CTxDestination Get() const;
235+
bool GetKeyID(CKeyID &keyID) const;
236+
bool IsScript() const;
237+
};
238+
215239
class CBitcoinAddressVisitor : public boost::static_visitor<bool>
216240
{
217241
private:
@@ -318,3 +342,25 @@ bool CBitcoinSecret::SetString(const std::string& strSecret)
318342
{
319343
return SetString(strSecret.c_str());
320344
}
345+
346+
std::string EncodeDestination(const CTxDestination& dest)
347+
{
348+
CBitcoinAddress addr(dest);
349+
if (!addr.IsValid()) return "";
350+
return addr.ToString();
351+
}
352+
353+
CTxDestination DecodeDestination(const std::string& str)
354+
{
355+
return CBitcoinAddress(str).Get();
356+
}
357+
358+
bool IsValidDestinationString(const std::string& str, const CChainParams& params)
359+
{
360+
return CBitcoinAddress(str).IsValid(params);
361+
}
362+
363+
bool IsValidDestinationString(const std::string& str)
364+
{
365+
return CBitcoinAddress(str).IsValid();
366+
}

‎src/base58.h

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -95,30 +95,6 @@ class CBase58Data
9595
bool operator> (const CBase58Data& b58) const { return CompareTo(b58) > 0; }
9696
};
9797

98-
/** base58-encoded Bitcoin addresses.
99-
* Public-key-hash-addresses have version 0 (or 111 testnet).
100-
* The data vector contains RIPEMD160(SHA256(pubkey)), where pubkey is the serialized public key.
101-
* Script-hash-addresses have version 5 (or 196 testnet).
102-
* The data vector contains RIPEMD160(SHA256(cscript)), where cscript is the serialized redemption script.
103-
*/
104-
class CBitcoinAddress : public CBase58Data {
105-
public:
106-
bool Set(const CKeyID &id);
107-
bool Set(const CScriptID &id);
108-
bool Set(const CTxDestination &dest);
109-
bool IsValid() const;
110-
bool IsValid(const CChainParams &params) const;
111-
112-
CBitcoinAddress() {}
113-
CBitcoinAddress(const CTxDestination &dest) { Set(dest); }
114-
CBitcoinAddress(const std::string& strAddress) { SetString(strAddress); }
115-
CBitcoinAddress(const char* pszAddress) { SetString(pszAddress); }
116-
117-
CTxDestination Get() const;
118-
bool GetKeyID(CKeyID &keyID) const;
119-
bool IsScript() const;
120-
};
121-
12298
/**
12399
* A base58-encoded secret key
124100
*/
@@ -167,4 +143,9 @@ template<typename K, int Size, CChainParams::Base58Type Type> class CBitcoinExtK
167143
typedef CBitcoinExtKeyBase<CExtKey, BIP32_EXTKEY_SIZE, CChainParams::EXT_SECRET_KEY> CBitcoinExtKey;
168144
typedef CBitcoinExtKeyBase<CExtPubKey, BIP32_EXTKEY_SIZE, CChainParams::EXT_PUBLIC_KEY> CBitcoinExtPubKey;
169145

146+
std::string EncodeDestination(const CTxDestination& dest);
147+
CTxDestination DecodeDestination(const std::string& str);
148+
bool IsValidDestinationString(const std::string& str);
149+
bool IsValidDestinationString(const std::string& str, const CChainParams& params);
150+
170151
#endif // BITCOIN_BASE58_H

‎src/bitcoin-tx.cpp

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -271,11 +271,11 @@ static void MutateTxAddOutAddr(CMutableTransaction& tx, const std::string& strIn
271271

272272
// extract and validate ADDRESS
273273
std::string strAddr = vStrInputParts[1];
274-
CBitcoinAddress addr(strAddr);
275-
if (!addr.IsValid())
274+
CTxDestination destination = DecodeDestination(strAddr);
275+
if (!IsValidDestination(destination)) {
276276
throw std::runtime_error("invalid TX output address");
277-
// build standard output script via GetScriptForDestination()
278-
CScript scriptPubKey = GetScriptForDestination(addr.Get());
277+
}
278+
CScript scriptPubKey = GetScriptForDestination(destination);
279279

280280
// construct TxOut, append to transaction output list
281281
CTxOut txout(value, scriptPubKey);
@@ -314,10 +314,8 @@ static void MutateTxAddOutPubKey(CMutableTransaction& tx, const std::string& str
314314
scriptPubKey = GetScriptForWitness(scriptPubKey);
315315
}
316316
if (bScriptHash) {
317-
// Get the address for the redeem script, then call
318-
// GetScriptForDestination() to construct a P2SH scriptPubKey.
319-
CBitcoinAddress redeemScriptAddr(scriptPubKey);
320-
scriptPubKey = GetScriptForDestination(redeemScriptAddr.Get());
317+
// Get the ID for the script, and then construct a P2SH destination for it.
318+
scriptPubKey = GetScriptForDestination(CScriptID(scriptPubKey));
321319
}
322320

323321
// construct TxOut, append to transaction output list
@@ -381,10 +379,8 @@ static void MutateTxAddOutMultiSig(CMutableTransaction& tx, const std::string& s
381379
scriptPubKey = GetScriptForWitness(scriptPubKey);
382380
}
383381
if (bScriptHash) {
384-
// Get the address for the redeem script, then call
385-
// GetScriptForDestination() to construct a P2SH scriptPubKey.
386-
CBitcoinAddress addr(scriptPubKey);
387-
scriptPubKey = GetScriptForDestination(addr.Get());
382+
// Get the ID for the script, and then construct a P2SH destination for it.
383+
scriptPubKey = GetScriptForDestination(CScriptID(scriptPubKey));
388384
}
389385

390386
// construct TxOut, append to transaction output list
@@ -444,11 +440,10 @@ static void MutateTxAddOutScript(CMutableTransaction& tx, const std::string& str
444440
}
445441

446442
if (bSegWit) {
447-
scriptPubKey = GetScriptForWitness(scriptPubKey);
443+
scriptPubKey = GetScriptForWitness(scriptPubKey);
448444
}
449445
if (bScriptHash) {
450-
CBitcoinAddress addr(scriptPubKey);
451-
scriptPubKey = GetScriptForDestination(addr.Get());
446+
scriptPubKey = GetScriptForDestination(CScriptID(scriptPubKey));
452447
}
453448

454449
// construct TxOut, append to transaction output list

‎src/core_write.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,9 @@ void ScriptPubKeyToUniv(const CScript& scriptPubKey,
148148
out.pushKV("type", GetTxnOutputType(type));
149149

150150
UniValue a(UniValue::VARR);
151-
for (const CTxDestination& addr : addresses)
152-
a.push_back(CBitcoinAddress(addr).ToString());
151+
for (const CTxDestination& addr : addresses) {
152+
a.push_back(EncodeDestination(addr));
153+
}
153154
out.pushKV("addresses", a);
154155
}
155156

‎src/qt/addresstablemodel.cpp

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -82,14 +82,14 @@ class AddressTablePriv
8282
LOCK(wallet->cs_wallet);
8383
for (const std::pair<CTxDestination, CAddressBookData>& item : wallet->mapAddressBook)
8484
{
85-
const CBitcoinAddress& address = item.first;
86-
bool fMine = IsMine(*wallet, address.Get());
85+
const CTxDestination& address = item.first;
86+
bool fMine = IsMine(*wallet, address);
8787
AddressTableEntry::Type addressType = translateTransactionType(
8888
QString::fromStdString(item.second.purpose), fMine);
8989
const std::string& strName = item.second.name;
9090
cachedAddressTable.append(AddressTableEntry(addressType,
9191
QString::fromStdString(strName),
92-
QString::fromStdString(address.ToString())));
92+
QString::fromStdString(EncodeDestination(address))));
9393
}
9494
}
9595
// qLowerBound() and qUpperBound() require our cachedAddressTable list to be sorted in asc order
@@ -246,7 +246,7 @@ bool AddressTableModel::setData(const QModelIndex &index, const QVariant &value,
246246
if(role == Qt::EditRole)
247247
{
248248
LOCK(wallet->cs_wallet); /* For SetAddressBook / DelAddressBook */
249-
CTxDestination curAddress = CBitcoinAddress(rec->address.toStdString()).Get();
249+
CTxDestination curAddress = DecodeDestination(rec->address.toStdString());
250250
if(index.column() == Label)
251251
{
252252
// Do nothing, if old label == new label
@@ -257,7 +257,7 @@ bool AddressTableModel::setData(const QModelIndex &index, const QVariant &value,
257257
}
258258
wallet->SetAddressBook(curAddress, value.toString().toStdString(), strPurpose);
259259
} else if(index.column() == Address) {
260-
CTxDestination newAddress = CBitcoinAddress(value.toString().toStdString()).Get();
260+
CTxDestination newAddress = DecodeDestination(value.toString().toStdString());
261261
// Refuse to set invalid address, set error status and return false
262262
if(boost::get<CNoDestination>(&newAddress))
263263
{
@@ -358,7 +358,7 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con
358358
// Check for duplicate addresses
359359
{
360360
LOCK(wallet->cs_wallet);
361-
if(wallet->mapAddressBook.count(CBitcoinAddress(strAddress).Get()))
361+
if(wallet->mapAddressBook.count(DecodeDestination(strAddress)))
362362
{
363363
editStatus = DUPLICATE_ADDRESS;
364364
return QString();
@@ -384,7 +384,7 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con
384384
return QString();
385385
}
386386
}
387-
strAddress = CBitcoinAddress(newKey.GetID()).ToString();
387+
strAddress = EncodeDestination(newKey.GetID());
388388
}
389389
else
390390
{
@@ -394,7 +394,7 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con
394394
// Add entry
395395
{
396396
LOCK(wallet->cs_wallet);
397-
wallet->SetAddressBook(CBitcoinAddress(strAddress).Get(), strLabel,
397+
wallet->SetAddressBook(DecodeDestination(strAddress), strLabel,
398398
(type == Send ? "send" : "receive"));
399399
}
400400
return QString::fromStdString(strAddress);
@@ -412,7 +412,7 @@ bool AddressTableModel::removeRows(int row, int count, const QModelIndex &parent
412412
}
413413
{
414414
LOCK(wallet->cs_wallet);
415-
wallet->DelAddressBook(CBitcoinAddress(rec->address.toStdString()).Get());
415+
wallet->DelAddressBook(DecodeDestination(rec->address.toStdString()));
416416
}
417417
return true;
418418
}
@@ -423,8 +423,8 @@ QString AddressTableModel::labelForAddress(const QString &address) const
423423
{
424424
{
425425
LOCK(wallet->cs_wallet);
426-
CBitcoinAddress address_parsed(address.toStdString());
427-
std::map<CTxDestination, CAddressBookData>::iterator mi = wallet->mapAddressBook.find(address_parsed.Get());
426+
CTxDestination destination = DecodeDestination(address.toStdString());
427+
std::map<CTxDestination, CAddressBookData>::iterator mi = wallet->mapAddressBook.find(destination);
428428
if (mi != wallet->mapAddressBook.end())
429429
{
430430
return QString::fromStdString(mi->second.name);

‎src/qt/bitcoinaddressvalidator.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,9 @@ QValidator::State BitcoinAddressCheckValidator::validate(QString &input, int &po
8989
{
9090
Q_UNUSED(pos);
9191
// Validate the passed Bitcoin address
92-
CBitcoinAddress addr(input.toStdString());
93-
if (addr.IsValid())
92+
if (IsValidDestinationString(input.toStdString())) {
9493
return QValidator::Acceptable;
94+
}
9595

9696
return QValidator::Invalid;
9797
}

‎src/qt/coincontroldialog.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -660,7 +660,7 @@ void CoinControlDialog::updateView()
660660
QString sAddress = "";
661661
if(ExtractDestination(out.tx->tx->vout[out.i].scriptPubKey, outputAddress))
662662
{
663-
sAddress = QString::fromStdString(CBitcoinAddress(outputAddress).ToString());
663+
sAddress = QString::fromStdString(EncodeDestination(outputAddress));
664664

665665
// if listMode or change => show bitcoin address. In tree mode, address is not shown again for direct wallet address outputs
666666
if (!treeMode || (!(sAddress == sWalletAddress)))

‎src/qt/guiutil.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,9 @@ static std::string DummyAddress(const CChainParams &params)
112112
sourcedata.insert(sourcedata.end(), dummydata, dummydata + sizeof(dummydata));
113113
for(int i=0; i<256; ++i) { // Try every trailing byte
114114
std::string s = EncodeBase58(sourcedata.data(), sourcedata.data() + sourcedata.size());
115-
if (!CBitcoinAddress(s).IsValid())
115+
if (!IsValidDestinationString(s)) {
116116
return s;
117+
}
117118
sourcedata[sourcedata.size()-1] += 1;
118119
}
119120
return "";
@@ -248,7 +249,7 @@ QString formatBitcoinURI(const SendCoinsRecipient &info)
248249

249250
bool isDust(const QString& address, const CAmount& amount)
250251
{
251-
CTxDestination dest = CBitcoinAddress(address.toStdString()).Get();
252+
CTxDestination dest = DecodeDestination(address.toStdString());
252253
CScript script = GetScriptForDestination(dest);
253254
CTxOut txOut(amount, script);
254255
return IsDust(txOut, ::dustRelayFee);

‎src/qt/paymentserver.cpp

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -218,17 +218,15 @@ void PaymentServer::ipcParseCommandLine(int argc, char* argv[])
218218
SendCoinsRecipient r;
219219
if (GUIUtil::parseBitcoinURI(arg, &r) && !r.address.isEmpty())
220220
{
221-
CBitcoinAddress address(r.address.toStdString());
222221
auto tempChainParams = CreateChainParams(CBaseChainParams::MAIN);
223222

224-
if (address.IsValid(*tempChainParams))
225-
{
223+
if (IsValidDestinationString(r.address.toStdString(), *tempChainParams)) {
226224
SelectParams(CBaseChainParams::MAIN);
227-
}
228-
else {
225+
} else {
229226
tempChainParams = CreateChainParams(CBaseChainParams::TESTNET);
230-
if (address.IsValid(*tempChainParams))
227+
if (IsValidDestinationString(r.address.toStdString(), *tempChainParams)) {
231228
SelectParams(CBaseChainParams::TESTNET);
229+
}
232230
}
233231
}
234232
}
@@ -441,8 +439,7 @@ void PaymentServer::handleURIOrFile(const QString& s)
441439
SendCoinsRecipient recipient;
442440
if (GUIUtil::parseBitcoinURI(s, &recipient))
443441
{
444-
CBitcoinAddress address(recipient.address.toStdString());
445-
if (!address.IsValid()) {
442+
if (!IsValidDestinationString(recipient.address.toStdString())) {
446443
Q_EMIT message(tr("URI handling"), tr("Invalid payment address %1").arg(recipient.address),
447444
CClientUIInterface::MSG_ERROR);
448445
}
@@ -560,7 +557,7 @@ bool PaymentServer::processPaymentRequest(const PaymentRequestPlus& request, Sen
560557
CTxDestination dest;
561558
if (ExtractDestination(sendingTo.first, dest)) {
562559
// Append destination address
563-
addresses.append(QString::fromStdString(CBitcoinAddress(dest).ToString()));
560+
addresses.append(QString::fromStdString(EncodeDestination(dest)));
564561
}
565562
else if (!recipient.authenticatedMerchant.isEmpty()) {
566563
// Unauthenticated payment requests to custom bitcoin addresses are not supported

‎src/qt/sendcoinsdialog.cpp

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

780-
CBitcoinAddress addr = CBitcoinAddress(text.toStdString());
780+
const CTxDestination dest = DecodeDestination(text.toStdString());
781781

782782
if (text.isEmpty()) // Nothing entered
783783
{
784784
ui->labelCoinControlChangeLabel->setText("");
785785
}
786-
else if (!addr.IsValid()) // Invalid address
786+
else if (!IsValidDestination(dest)) // Invalid address
787787
{
788788
ui->labelCoinControlChangeLabel->setText(tr("Warning: Invalid Bitcoin address"));
789789
}
790790
else // Valid address
791791
{
792-
const CTxDestination dest = addr.Get();
793792
if (!model->IsSpendable(dest)) {
794793
ui->labelCoinControlChangeLabel->setText(tr("Warning: Unknown change address"));
795794

0 commit comments

Comments
 (0)