From fa57170187415bf939885fc71cea5bffc30a32f8 Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Thu, 14 Jun 2012 20:44:04 -0400 Subject: [PATCH 1/9] Document how to build/run unit tests --- doc/unit-tests.txt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 doc/unit-tests.txt diff --git a/doc/unit-tests.txt b/doc/unit-tests.txt new file mode 100644 index 00000000000..1168de7b70a --- /dev/null +++ b/doc/unit-tests.txt @@ -0,0 +1,18 @@ +Compiling/runing bitcoind unit tests +------------------------------------ + +bitcoind unit tests are in the src/test/ directory; they +use the Boost::Test unit-testing framework. + +To compile and run the tests: +cd src +make -f makefile.unix test_bitcoin # Replace makefile.unix if you're not on unix +./test_bitcoin # Runs the unit tests + +If all tests succeed the last line of output will be: +*** No errors detected + +To add more tests, add BOOST_AUTO_TEST_CASE's to the existing +.cpp files in the test/ directory or add new .cpp files that +implement new BOOST_AUTO_TEST_SUITE's (and add them to the +list of includes in test_bitcoin.cpp). From 506bf85de57bdf079824a14e492c112338768c2a Mon Sep 17 00:00:00 2001 From: Philip Kaufmann Date: Sat, 9 Jun 2012 15:41:21 +0200 Subject: [PATCH 2/9] add the slot updateDisplayUnit() to overviewpage, sendcoinsdialog, sendcoinsentry and connect it to displayUnitChanged() - this ensures all fields in the GUI, who use a display unit are imediately updated, when the user changes this setting in the optionsdialog / ensure used fields init with the current set display unit --- src/qt/overviewpage.cpp | 32 +++++++++++++++++++------------- src/qt/overviewpage.h | 2 +- src/qt/sendcoinsdialog.cpp | 14 ++++++++++++-- src/qt/sendcoinsdialog.h | 2 +- src/qt/sendcoinsentry.cpp | 18 ++++++++++++++---- src/qt/sendcoinsentry.h | 1 + 6 files changed, 48 insertions(+), 21 deletions(-) diff --git a/src/qt/overviewpage.cpp b/src/qt/overviewpage.cpp index 5b5a8f5271e..2c3d8a49e3a 100644 --- a/src/qt/overviewpage.cpp +++ b/src/qt/overviewpage.cpp @@ -143,7 +143,7 @@ void OverviewPage::setNumTransactions(int count) void OverviewPage::setModel(WalletModel *model) { this->model = model; - if(model) + if(model && model->getOptionsModel()) { // Set up transaction list TransactionFilterProxy *filter = new TransactionFilterProxy(); @@ -163,17 +163,23 @@ void OverviewPage::setModel(WalletModel *model) setNumTransactions(model->getNumTransactions()); connect(model, SIGNAL(numTransactionsChanged(int)), this, SLOT(setNumTransactions(int))); - connect(model->getOptionsModel(), SIGNAL(displayUnitChanged(int)), this, SLOT(displayUnitChanged())); + connect(model->getOptionsModel(), SIGNAL(displayUnitChanged(int)), this, SLOT(updateDisplayUnit())); + } + + // update the display unit, to not use the default ("BTC") + updateDisplayUnit(); +} + +void OverviewPage::updateDisplayUnit() +{ + if(model && model->getOptionsModel()) + { + if(currentBalance != -1) + setBalance(currentBalance, currentUnconfirmedBalance); + + // Update txdelegate->unit with the current unit + txdelegate->unit = model->getOptionsModel()->getDisplayUnit(); + + ui->listTransactions->update(); } } - -void OverviewPage::displayUnitChanged() -{ - if(!model || !model->getOptionsModel()) - return; - if(currentBalance != -1) - setBalance(currentBalance, currentUnconfirmedBalance); - - txdelegate->unit = model->getOptionsModel()->getDisplayUnit(); - ui->listTransactions->update(); -} diff --git a/src/qt/overviewpage.h b/src/qt/overviewpage.h index 11992271688..99fe4864948 100644 --- a/src/qt/overviewpage.h +++ b/src/qt/overviewpage.h @@ -40,7 +40,7 @@ private: TxViewDelegate *txdelegate; private slots: - void displayUnitChanged(); + void updateDisplayUnit(); }; #endif // OVERVIEWPAGE_H diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 4c58b38b785..ef2f1c31869 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -44,10 +44,11 @@ void SendCoinsDialog::setModel(WalletModel *model) entry->setModel(model); } } - if(model) + if(model && model->getOptionsModel()) { setBalance(model->getBalance(), model->getUnconfirmedBalance()); connect(model, SIGNAL(balanceChanged(qint64, qint64)), this, SLOT(setBalance(qint64, qint64))); + connect(model->getOptionsModel(), SIGNAL(displayUnitChanged(int)), this, SLOT(updateDisplayUnit())); } } @@ -195,7 +196,7 @@ SendCoinsEntry *SendCoinsDialog::addEntry() ui->scrollAreaWidgetContents->resize(ui->scrollAreaWidgetContents->sizeHint()); QCoreApplication::instance()->processEvents(); QScrollBar* bar = ui->scrollArea->verticalScrollBar(); - if (bar) + if(bar) bar->setSliderPosition(bar->maximum()); return entry; } @@ -286,3 +287,12 @@ void SendCoinsDialog::setBalance(qint64 balance, qint64 unconfirmedBalance) int unit = model->getOptionsModel()->getDisplayUnit(); ui->labelBalance->setText(BitcoinUnits::formatWithUnit(unit, balance)); } + +void SendCoinsDialog::updateDisplayUnit() +{ + if(model && model->getOptionsModel()) + { + // Update labelBalance with the current balance and the current unit + ui->labelBalance->setText(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), model->getBalance())); + } +} diff --git a/src/qt/sendcoinsdialog.h b/src/qt/sendcoinsdialog.h index 79125766e37..ed562141910 100644 --- a/src/qt/sendcoinsdialog.h +++ b/src/qt/sendcoinsdialog.h @@ -47,8 +47,8 @@ private: private slots: void on_sendButton_clicked(); - void removeEntry(SendCoinsEntry* entry); + void updateDisplayUnit(); }; #endif // SENDCOINSDIALOG_H diff --git a/src/qt/sendcoinsentry.cpp b/src/qt/sendcoinsentry.cpp index c8242d8352f..599a804c46c 100644 --- a/src/qt/sendcoinsentry.cpp +++ b/src/qt/sendcoinsentry.cpp @@ -68,6 +68,10 @@ void SendCoinsEntry::on_payTo_textChanged(const QString &address) void SendCoinsEntry::setModel(WalletModel *model) { this->model = model; + + if(model && model->getOptionsModel()) + connect(model->getOptionsModel(), SIGNAL(displayUnitChanged(int)), this, SLOT(updateDisplayUnit())); + clear(); } @@ -82,10 +86,8 @@ void SendCoinsEntry::clear() ui->addAsLabel->clear(); ui->payAmount->clear(); ui->payTo->setFocus(); - if(model && model->getOptionsModel()) - { - ui->payAmount->setDisplayUnit(model->getOptionsModel()->getDisplayUnit()); - } + // update the display unit, to not use the default ("BTC") + updateDisplayUnit(); } void SendCoinsEntry::on_deleteButton_clicked() @@ -160,3 +162,11 @@ void SendCoinsEntry::setFocus() ui->payTo->setFocus(); } +void SendCoinsEntry::updateDisplayUnit() +{ + if(model && model->getOptionsModel()) + { + // Update payAmount with the current unit + ui->payAmount->setDisplayUnit(model->getOptionsModel()->getDisplayUnit()); + } +} diff --git a/src/qt/sendcoinsentry.h b/src/qt/sendcoinsentry.h index cdbf8932646..db6cba0d80c 100644 --- a/src/qt/sendcoinsentry.h +++ b/src/qt/sendcoinsentry.h @@ -45,6 +45,7 @@ private slots: void on_payTo_textChanged(const QString &address); void on_addressBookButton_clicked(); void on_pasteButton_clicked(); + void updateDisplayUnit(); private: Ui::SendCoinsEntry *ui; From 7ff54e08aa42f10dae424711a0a3764a67eae295 Mon Sep 17 00:00:00 2001 From: "Ricardo M. Correia" Date: Mon, 14 May 2012 02:50:01 +0200 Subject: [PATCH 3/9] Don't overflow signed ints in CBigNum::setint64(). CBigNum::setint64() does 'n <<= 8', where n is of type "long long". This leads to shifting onto and past the sign bit, which is undefined behavior in C++11 and can cause problems in the future. --- src/bignum.h | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/bignum.h b/src/bignum.h index 4a3fb38b00b..e203b26a054 100644 --- a/src/bignum.h +++ b/src/bignum.h @@ -121,16 +121,22 @@ public: return (n > std::numeric_limits::max() ? std::numeric_limits::min() : -(int)n); } - void setint64(int64 n) + void setint64(int64 sn) { - unsigned char pch[sizeof(n) + 6]; + unsigned char pch[sizeof(sn) + 6]; unsigned char* p = pch + 4; - bool fNegative = false; - if (n < (int64)0) + bool fNegative; + uint64 n; + + if (sn < (int64)0) { - n = -n; + n = -sn; fNegative = true; + } else { + n = sn; + fNegative = false; } + bool fLeadingZeroes = true; for (int i = 0; i < 8; i++) { From b0d9f41cd222de5d13d2b9cec27474009029d5ec Mon Sep 17 00:00:00 2001 From: "Ricardo M. Correia" Date: Thu, 7 Jun 2012 19:11:15 +0200 Subject: [PATCH 4/9] Don't overflow integer on 32-bit machines. This was causing test_bitcoin to abort on a 32-bit system likely due to -ftrapv. --- src/util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util.h b/src/util.h index 284cf33c4b1..dc729683268 100644 --- a/src/util.h +++ b/src/util.h @@ -396,7 +396,7 @@ inline int64 GetPerformanceCounter() #else timeval t; gettimeofday(&t, NULL); - nCounter = t.tv_sec * 1000000 + t.tv_usec; + nCounter = (int64) t.tv_sec * 1000000 + t.tv_usec; #endif return nCounter; } From c3def40293a89307d296ff66653b5927165c3c18 Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Tue, 15 May 2012 15:53:30 -0400 Subject: [PATCH 5/9] Optimize orphan transaction handling Changes suggested by Sergio Demian Lerner to help prevent potential DoS attacks. --- src/main.cpp | 46 ++++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 0f45b2e16b3..b7067b2092f 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -45,7 +45,7 @@ map mapOrphanBlocks; multimap mapOrphanBlocksByPrev; map mapOrphanTransactions; -multimap mapOrphanTransactionsByPrev; +map > mapOrphanTransactionsByPrev; double dHashesPerSec; @@ -160,17 +160,37 @@ void static ResendWalletTransactions() // mapOrphanTransactions // -void AddOrphanTx(const CDataStream& vMsg) +bool AddOrphanTx(const CDataStream& vMsg) { CTransaction tx; CDataStream(vMsg) >> tx; uint256 hash = tx.GetHash(); if (mapOrphanTransactions.count(hash)) - return; + return false; - CDataStream* pvMsg = mapOrphanTransactions[hash] = new CDataStream(vMsg); + CDataStream* pvMsg = new CDataStream(vMsg); + + // Ignore big transactions, to avoid a + // send-big-orphans memory exhaustion attack. If a peer has a legitimate + // large transaction with a missing parent then we assume + // it will rebroadcast it later, after the parent transaction(s) + // have been mined or received. + // 10,000 orphans, each of which is at most 5,000 bytes big is + // at most 500 megabytes of orphans: + if (pvMsg->size() > 5000) + { + delete pvMsg; + printf("ignoring large orphan tx (size: %u, hash: %s)\n", pvMsg->size(), hash.ToString().substr(0,10).c_str()); + return false; + } + + mapOrphanTransactions[hash] = pvMsg; BOOST_FOREACH(const CTxIn& txin, tx.vin) - mapOrphanTransactionsByPrev.insert(make_pair(txin.prevout.hash, pvMsg)); + mapOrphanTransactionsByPrev[txin.prevout.hash].insert(make_pair(hash, pvMsg)); + + printf("stored orphan tx %s (mapsz %u)\n", hash.ToString().substr(0,10).c_str(), + mapOrphanTransactions.size()); + return true; } void static EraseOrphanTx(uint256 hash) @@ -182,14 +202,9 @@ void static EraseOrphanTx(uint256 hash) CDataStream(*pvMsg) >> tx; BOOST_FOREACH(const CTxIn& txin, tx.vin) { - for (multimap::iterator mi = mapOrphanTransactionsByPrev.lower_bound(txin.prevout.hash); - mi != mapOrphanTransactionsByPrev.upper_bound(txin.prevout.hash);) - { - if ((*mi).second == pvMsg) - mapOrphanTransactionsByPrev.erase(mi++); - else - mi++; - } + mapOrphanTransactionsByPrev[txin.prevout.hash].erase(hash); + if (mapOrphanTransactionsByPrev[txin.prevout.hash].empty()) + mapOrphanTransactionsByPrev.erase(txin.prevout.hash); } delete pvMsg; mapOrphanTransactions.erase(hash); @@ -2371,8 +2386,8 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) for (unsigned int i = 0; i < vWorkQueue.size(); i++) { uint256 hashPrev = vWorkQueue[i]; - for (multimap::iterator mi = mapOrphanTransactionsByPrev.lower_bound(hashPrev); - mi != mapOrphanTransactionsByPrev.upper_bound(hashPrev); + for (map::iterator mi = mapOrphanTransactionsByPrev[hashPrev].begin(); + mi != mapOrphanTransactionsByPrev[hashPrev].end(); ++mi) { const CDataStream& vMsg = *((*mi).second); @@ -2396,7 +2411,6 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) } else if (fMissingInputs) { - printf("storing orphan tx %s\n", inv.hash.ToString().substr(0,10).c_str()); AddOrphanTx(vMsg); // DoS prevention: do not allow mapOrphanTransactions to grow unbounded From ce1a071f6d6d1548974796c4327399659415b489 Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Wed, 16 May 2012 11:26:56 -0400 Subject: [PATCH 6/9] Further DoS prevention: Verify signatures last Loop over all inputs doing inexpensive validity checks first, and then loop over them a second time doing expensive signature checks. This helps prevent possible CPU exhaustion attacks where an attacker tries to make a victim waste time checking signatures for invalid transactions. --- src/main.cpp | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index b7067b2092f..00f06334317 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1033,15 +1033,26 @@ bool CTransaction::ConnectInputs(MapPrevTx inputs, if (pindex->nBlockPos == txindex.pos.nBlockPos && pindex->nFile == txindex.pos.nFile) return error("ConnectInputs() : tried to spend coinbase at depth %d", pindexBlock->nHeight - pindex->nHeight); - // Check for conflicts (double-spend) - if (!txindex.vSpent[prevout.n].IsNull()) - return fMiner ? false : error("ConnectInputs() : %s prev tx already used at %s", GetHash().ToString().substr(0,10).c_str(), txindex.vSpent[prevout.n].ToString().c_str()); - // Check for negative or overflow input values nValueIn += txPrev.vout[prevout.n].nValue; if (!MoneyRange(txPrev.vout[prevout.n].nValue) || !MoneyRange(nValueIn)) return error("ConnectInputs() : txin values out of range"); + } + // The first loop above does all the inexpensive checks. + // Only if ALL inputs pass do we perform expensive ECDSA signature checks. + // Helps prevent CPU exhaustion attacks. + for (unsigned int i = 0; i < vin.size(); i++) + { + COutPoint prevout = vin[i].prevout; + assert(inputs.count(prevout.hash) > 0); + CTxIndex& txindex = inputs[prevout.hash].first; + CTransaction& txPrev = inputs[prevout.hash].second; + + // Check for conflicts (double-spend) + if (!txindex.vSpent[prevout.n].IsNull()) + return fMiner ? false : error("ConnectInputs() : %s prev tx already used at %s", GetHash().ToString().substr(0,10).c_str(), txindex.vSpent[prevout.n].ToString().c_str()); + // Verify signature if (!VerifySignature(txPrev, *this, i, fStrictPayToScriptHash, 0)) { From 01473c3f40cea8209186e737ae20289eebcb8898 Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Thu, 17 May 2012 10:12:04 -0400 Subject: [PATCH 7/9] Remove invalid dependent orphans from memory Remove orphan transactions from memory once all of their parent transactions are received and they're still not valid. Thanks to Sergio Demian Lerner for suggesting this fix. --- src/main.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 00f06334317..432ec871f5a 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2378,6 +2378,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) else if (strCommand == "tx") { vector vWorkQueue; + vector vEraseQueue; CDataStream vMsg(vRecv); CTransaction tx; vRecv >> tx; @@ -2392,6 +2393,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) RelayMessage(inv, vMsg); mapAlreadyAskedFor.erase(inv); vWorkQueue.push_back(inv.hash); + vEraseQueue.push_back(inv.hash); // Recursively process any orphan transactions that depended on this one for (unsigned int i = 0; i < vWorkQueue.size(); i++) @@ -2405,19 +2407,27 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) CTransaction tx; CDataStream(vMsg) >> tx; CInv inv(MSG_TX, tx.GetHash()); + bool fMissingInputs2 = false; - if (tx.AcceptToMemoryPool(true)) + if (tx.AcceptToMemoryPool(true, &fMissingInputs2)) { printf(" accepted orphan tx %s\n", inv.hash.ToString().substr(0,10).c_str()); SyncWithWallets(tx, NULL, true); RelayMessage(inv, vMsg); mapAlreadyAskedFor.erase(inv); vWorkQueue.push_back(inv.hash); + vEraseQueue.push_back(inv.hash); + } + else if (!fMissingInputs2) + { + // invalid orphan + vEraseQueue.push_back(inv.hash); + printf(" removed invalid orphan tx %s\n", inv.hash.ToString().substr(0,10).c_str()); } } } - BOOST_FOREACH(uint256 hash, vWorkQueue) + BOOST_FOREACH(uint256 hash, vEraseQueue) EraseOrphanTx(hash); } else if (fMissingInputs) From b199f7547f357711873327347c0f368248e99032 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Wed, 20 Jun 2012 17:59:36 +0000 Subject: [PATCH 8/9] Bump VERSION so we can differentiate between 0.4.7rc2 and 0.4.7rc3 --- src/serialize.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/serialize.h b/src/serialize.h index 8cdfb30b908..c7e64dac768 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -59,7 +59,7 @@ class CDataStream; class CAutoFile; static const unsigned int MAX_SIZE = 0x02000000; -static const int VERSION = 40701; +static const int VERSION = 40703; static const char* pszSubVer = ""; static const bool VERSION_IS_BETA = true; From 3023e782bdaee3448e1543b482cf5cd022c9699f Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Tue, 19 Jun 2012 15:50:12 -0400 Subject: [PATCH 9/9] print large orphan warning BEFORE deleting pvMsg --- src/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.cpp b/src/main.cpp index 432ec871f5a..7ce7c92e5d2 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -179,8 +179,8 @@ bool AddOrphanTx(const CDataStream& vMsg) // at most 500 megabytes of orphans: if (pvMsg->size() > 5000) { - delete pvMsg; printf("ignoring large orphan tx (size: %u, hash: %s)\n", pvMsg->size(), hash.ToString().substr(0,10).c_str()); + delete pvMsg; return false; }