diff --git a/src/init.cpp b/src/init.cpp index 472af5331..d619cb412 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -582,6 +582,28 @@ bool AppInit2(boost::thread_group& threadGroup) const char* pszP2SH = "/P2SH/"; COINBASE_FLAGS << std::vector(pszP2SH, pszP2SH+strlen(pszP2SH)); + // Fee-per-kilobyte amount considered the same as "free" + // If you are mining, be careful setting this: + // if you set it to zero then + // a transaction spammer can cheaply fill blocks using + // 1-satoshi-fee transactions. It should be set above the real + // cost to you of processing a transaction. + if (mapArgs.count("-mintxfee")) + { + int64 n = 0; + if (ParseMoney(mapArgs["-mintxfee"], n) && n > 0) + CTransaction::nMinTxFee = n; + else + return InitError(strprintf(_("Invalid amount for -mintxfee=: '%s'"), mapArgs["-mintxfee"].c_str())); + } + if (mapArgs.count("-minrelaytxfee")) + { + int64 n = 0; + if (ParseMoney(mapArgs["-minrelaytxfee"], n) && n > 0) + CTransaction::nMinRelayTxFee = n; + else + return InitError(strprintf(_("Invalid amount for -minrelaytxfee=: '%s'"), mapArgs["-minrelaytxfee"].c_str())); + } if (mapArgs.count("-paytxfee")) { diff --git a/src/main.cpp b/src/main.cpp index 45fb7af00..e2bed5278 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -48,6 +48,11 @@ bool fBenchmark = false; bool fTxIndex = false; unsigned int nCoinCacheSize = 5000; +/** Fees smaller than this (in satoshi) are considered zero fee (for transaction creation) */ +int64 CTransaction::nMinTxFee = 10000; // Override with -mintxfee +/** Fees smaller than this (in satoshi) are considered zero fee (for relaying) */ +int64 CTransaction::nMinRelayTxFee = 10000; + CMedianFilter cPeerBlockCounts(8, 0); // Amount of blocks that other nodes claim to have map mapOrphanBlocks; @@ -352,9 +357,22 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) ////////////////////////////////////////////////////////////////////////////// // -// CTransaction +// CTransaction / CTxOut // +bool CTxOut::IsDust() const +{ + // "Dust" is defined in terms of CTransaction::nMinRelayTxFee, + // which has units satoshis-per-kilobyte. + // If you'd pay more than 1/3 in fees + // to spend something, then we consider it dust. + // A typical txout is 33 bytes big, and will + // need a CTxIn of at least 148 bytes to spend, + // so dust is a txout less than 54 uBTC + // (5430 satoshis) with default nMinRelayTxFee + return ((nValue*1000)/(3*((int)GetSerializeSize(SER_DISK,0)+148)) < CTransaction::nMinRelayTxFee); +} + bool CTransaction::IsStandard() const { if (nVersion > CTransaction::CURRENT_VERSION) @@ -384,7 +402,7 @@ bool CTransaction::IsStandard() const BOOST_FOREACH(const CTxOut& txout, vout) { if (!::IsStandard(txout.scriptPubKey)) return false; - if (txout.nValue == 0) + if (txout.IsDust()) return false; } return true; @@ -574,8 +592,8 @@ bool CTransaction::CheckTransaction(CValidationState &state) const int64 CTransaction::GetMinFee(unsigned int nBlockSize, bool fAllowFree, enum GetMinFee_mode mode) const { - // Base fee is either MIN_TX_FEE or MIN_RELAY_TX_FEE - int64 nBaseFee = (mode == GMF_RELAY) ? MIN_RELAY_TX_FEE : MIN_TX_FEE; + // Base fee is either nMinTxFee or nMinRelayTxFee + int64 nBaseFee = (mode == GMF_RELAY) ? nMinRelayTxFee : nMinTxFee; unsigned int nBytes = ::GetSerializeSize(*this, SER_NETWORK, PROTOCOL_VERSION); unsigned int nNewBlockSize = nBlockSize + nBytes; @@ -598,7 +616,7 @@ int64 CTransaction::GetMinFee(unsigned int nBlockSize, bool fAllowFree, } } - // To limit dust spam, require MIN_TX_FEE/MIN_RELAY_TX_FEE if any output is less than 0.01 + // To limit dust spam, require base fee if any output is less than 0.01 if (nMinFee < nBaseFee) { BOOST_FOREACH(const CTxOut& txout, vout) @@ -746,7 +764,7 @@ bool CTxMemPool::accept(CValidationState &state, CTransaction &tx, bool fCheckIn // Continuously rate-limit free transactions // This mitigates 'penny-flooding' -- sending thousands of free transactions just to // be annoying or make others' transactions take longer to confirm. - if (fLimitFree && nFees < MIN_RELAY_TX_FEE) + if (fLimitFree && nFees < CTransaction::nMinRelayTxFee) { static double dFreeCount; static int64 nLastTime; @@ -4187,15 +4205,6 @@ CBlockTemplate* CreateNewBlock(CReserveKey& reservekey) unsigned int nBlockMinSize = GetArg("-blockminsize", 0); nBlockMinSize = std::min(nBlockMaxSize, nBlockMinSize); - // Fee-per-kilobyte amount considered the same as "free" - // Be careful setting this: if you set it to zero then - // a transaction spammer can cheaply fill blocks using - // 1-satoshi-fee transactions. It should be set above the real - // cost to you of processing a transaction. - int64 nMinTxFee = MIN_TX_FEE; - if (mapArgs.count("-mintxfee")) - ParseMoney(mapArgs["-mintxfee"], nMinTxFee); - // Collect memory pool transactions into the block int64 nFees = 0; { @@ -4313,7 +4322,7 @@ CBlockTemplate* CreateNewBlock(CReserveKey& reservekey) continue; // Skip free transactions if we're past the minimum block size: - if (fSortedByFee && (dFeePerKb < nMinTxFee) && (nBlockSize + nTxSize >= nBlockMinSize)) + if (fSortedByFee && (dFeePerKb < CTransaction::nMinTxFee) && (nBlockSize + nTxSize >= nBlockMinSize)) continue; // Prioritize by fee once past the priority size or we run out of high-priority diff --git a/src/main.h b/src/main.h index e02edbc52..cba8421c8 100644 --- a/src/main.h +++ b/src/main.h @@ -44,10 +44,6 @@ static const unsigned int BLOCKFILE_CHUNK_SIZE = 0x1000000; // 16 MiB static const unsigned int UNDOFILE_CHUNK_SIZE = 0x100000; // 1 MiB /** Fake height value used in CCoins to signify they are only in the memory pool (since 0.8) */ static const unsigned int MEMPOOL_HEIGHT = 0x7FFFFFFF; -/** Fees smaller than this (in satoshi) are considered zero fee (for transaction creation) */ -static const int64 MIN_TX_FEE = 10000; -/** Fees smaller than this (in satoshi) are considered zero fee (for relaying) */ -static const int64 MIN_RELAY_TX_FEE = 10000; /** No amount larger than this (in satoshi) is valid */ static const int64 MAX_MONEY = 21000000 * COIN; inline bool MoneyRange(int64 nValue) { return (nValue >= 0 && nValue <= MAX_MONEY); } @@ -439,6 +435,8 @@ public: return !(a == b); } + bool IsDust() const; + std::string ToString() const { if (scriptPubKey.size() < 6) @@ -467,6 +465,8 @@ enum GetMinFee_mode class CTransaction { public: + static int64 nMinTxFee; + static int64 nMinRelayTxFee; static const int CURRENT_VERSION=1; int nVersion; std::vector vin; diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 2d8870061..f525d1bb3 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -73,7 +73,7 @@ static bool ThreadSafeAskFee(int64 nFeeRequired) { if(!guiref) return false; - if(nFeeRequired < MIN_TX_FEE || nFeeRequired <= nTransactionFee || fDaemon) + if(nFeeRequired < CTransaction::nMinTxFee || nFeeRequired <= nTransactionFee || fDaemon) return true; bool payFee = false; diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 20535a451..fb3ffc5c9 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -181,7 +181,8 @@ WalletModel::SendCoinsReturn WalletModel::sendCoins(const QListCreateTransaction(vecSend, wtx, keyChange, nFeeRequired); + std::string strFailReason; + bool fCreated = wallet->CreateTransaction(vecSend, wtx, keyChange, nFeeRequired, strFailReason); if(!fCreated) { @@ -189,6 +190,8 @@ WalletModel::SendCoinsReturn WalletModel::sendCoins(const QListCreateTransaction(vecSend, wtx, keyChange, nFeeRequired); + string strFailReason; + bool fCreated = pwalletMain->CreateTransaction(vecSend, wtx, keyChange, nFeeRequired, strFailReason); if (!fCreated) - { - if (totalAmount + nFeeRequired > pwalletMain->GetBalance()) - throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Insufficient funds"); - throw JSONRPCError(RPC_WALLET_ERROR, "Transaction creation failed"); - } + throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason); if (!pwalletMain->CommitTransaction(wtx, keyChange)) throw JSONRPCError(RPC_WALLET_ERROR, "Transaction commit failed"); diff --git a/src/test/script_P2SH_tests.cpp b/src/test/script_P2SH_tests.cpp index 3444726ca..ee0d25a2f 100644 --- a/src/test/script_P2SH_tests.cpp +++ b/src/test/script_P2SH_tests.cpp @@ -78,7 +78,9 @@ BOOST_AUTO_TEST_CASE(sign) for (int i = 0; i < 4; i++) { txFrom.vout[i].scriptPubKey = evalScripts[i]; + txFrom.vout[i].nValue = COIN; txFrom.vout[i+4].scriptPubKey = standardScripts[i]; + txFrom.vout[i+4].nValue = COIN; } BOOST_CHECK(txFrom.IsStandard()); @@ -169,6 +171,7 @@ BOOST_AUTO_TEST_CASE(set) for (int i = 0; i < 4; i++) { txFrom.vout[i].scriptPubKey = outer[i]; + txFrom.vout[i].nValue = CENT; } BOOST_CHECK(txFrom.IsStandard()); @@ -179,7 +182,7 @@ BOOST_AUTO_TEST_CASE(set) txTo[i].vout.resize(1); txTo[i].vin[0].prevout.n = i; txTo[i].vin[0].prevout.hash = txFrom.GetHash(); - txTo[i].vout[0].nValue = 1; + txTo[i].vout[0].nValue = 1*CENT; txTo[i].vout[0].scriptPubKey = inner[i]; BOOST_CHECK_MESSAGE(IsMine(keystore, txFrom.vout[i].scriptPubKey), strprintf("IsMine %d", i)); } diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index f44d46fdb..ddff2acd4 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -242,24 +242,34 @@ BOOST_AUTO_TEST_CASE(test_Get) BOOST_CHECK(!t1.AreInputsStandard(coins)); } -BOOST_AUTO_TEST_CASE(test_GetThrow) +BOOST_AUTO_TEST_CASE(test_IsStandard) { CBasicKeyStore keystore; CCoinsView coinsDummy; CCoinsViewCache coins(coinsDummy); std::vector dummyTransactions = SetupDummyInputs(keystore, coins); - CTransaction t1; - t1.vin.resize(3); - t1.vin[0].prevout.hash = dummyTransactions[0].GetHash(); - t1.vin[0].prevout.n = 0; - t1.vin[1].prevout.hash = dummyTransactions[1].GetHash();; - t1.vin[1].prevout.n = 0; - t1.vin[2].prevout.hash = dummyTransactions[1].GetHash();; - t1.vin[2].prevout.n = 1; - t1.vout.resize(2); - t1.vout[0].nValue = 90*CENT; - t1.vout[0].scriptPubKey << OP_1; + CTransaction t; + t.vin.resize(1); + t.vin[0].prevout.hash = dummyTransactions[0].GetHash(); + t.vin[0].prevout.n = 1; + t.vin[0].scriptSig << std::vector(65, 0); + t.vout.resize(1); + t.vout[0].nValue = 90*CENT; + CKey key; + key.MakeNewKey(true); + t.vout[0].scriptPubKey.SetDestination(key.GetPubKey().GetID()); + + BOOST_CHECK(t.IsStandard()); + + t.vout[0].nValue = 5011; // dust + BOOST_CHECK(!t.IsStandard()); + + t.vout[0].nValue = 6011; // not dust + BOOST_CHECK(t.IsStandard()); + + t.vout[0].scriptPubKey = CScript() << OP_1; + BOOST_CHECK(!t.IsStandard()); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/wallet_tests.cpp b/src/test/wallet_tests.cpp index a4cbfaee4..a14f6b2b7 100644 --- a/src/test/wallet_tests.cpp +++ b/src/test/wallet_tests.cpp @@ -21,13 +21,12 @@ static vector vCoins; static void add_coin(int64 nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0) { - static int i; - CTransaction* tx = new CTransaction; - tx->nLockTime = i++; // so all transactions get different hashes - tx->vout.resize(nInput+1); - tx->vout[nInput].nValue = nValue; - CWalletTx* wtx = new CWalletTx(&wallet, *tx); - delete tx; + static int nextLockTime = 0; + CTransaction tx; + tx.nLockTime = nextLockTime++; // so all transactions get different hashes + tx.vout.resize(nInput+1); + tx.vout[nInput].nValue = nValue; + CWalletTx* wtx = new CWalletTx(&wallet, tx); if (fIsFromMe) { // IsFromMe() returns (GetDebit() > 0), and GetDebit() is 0 if vin.empty(), @@ -55,8 +54,8 @@ static bool equal_sets(CoinSet a, CoinSet b) BOOST_AUTO_TEST_CASE(coin_selection_tests) { - static CoinSet setCoinsRet, setCoinsRet2; - static int64 nValueRet; + CoinSet setCoinsRet, setCoinsRet2; + int64 nValueRet; // test multiple times to allow for differences in the shuffle order for (int i = 0; i < RUN_TESTS; i++) diff --git a/src/wallet.cpp b/src/wallet.cpp index a56d53e7e..c70ea20e8 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -1141,17 +1141,24 @@ bool CWallet::SelectCoins(int64 nTargetValue, set >& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, int64& nFeeRet) +bool CWallet::CreateTransaction(const vector >& vecSend, + CWalletTx& wtxNew, CReserveKey& reservekey, int64& nFeeRet, std::string& strFailReason) { int64 nValue = 0; BOOST_FOREACH (const PAIRTYPE(CScript, int64)& s, vecSend) { if (nValue < 0) + { + strFailReason = _("Transaction amounts must be positive"); return false; + } nValue += s.second; } if (vecSend.empty() || nValue < 0) + { + strFailReason = _("Transaction amounts must be positive"); return false; + } wtxNew.BindWallet(this); @@ -1169,13 +1176,24 @@ bool CWallet::CreateTransaction(const vector >& vecSend, CW double dPriority = 0; // vouts to the payees BOOST_FOREACH (const PAIRTYPE(CScript, int64)& s, vecSend) - wtxNew.vout.push_back(CTxOut(s.second, s.first)); + { + CTxOut txout(s.second, s.first); + if (txout.IsDust()) + { + strFailReason = _("Transaction amount too small"); + return false; + } + wtxNew.vout.push_back(txout); + } // Choose coins to use set > setCoins; int64 nValueIn = 0; if (!SelectCoins(nTotalValue, setCoins, nValueIn)) + { + strFailReason = _("Insufficient funds"); return false; + } BOOST_FOREACH(PAIRTYPE(const CWalletTx*, unsigned int) pcoin, setCoins) { int64 nCredit = pcoin.first->vout[pcoin.second].nValue; @@ -1186,12 +1204,12 @@ bool CWallet::CreateTransaction(const vector >& vecSend, CW } int64 nChange = nValueIn - nValue - nFeeRet; - // if sub-cent change is required, the fee must be raised to at least MIN_TX_FEE + // if sub-cent change is required, the fee must be raised to at least nMinTxFee // or until nChange becomes zero // NOTE: this depends on the exact behaviour of GetMinFee - if (nFeeRet < MIN_TX_FEE && nChange > 0 && nChange < CENT) + if (nFeeRet < CTransaction::nMinTxFee && nChange > 0 && nChange < CENT) { - int64 nMoveToFee = min(nChange, MIN_TX_FEE - nFeeRet); + int64 nMoveToFee = min(nChange, CTransaction::nMinTxFee - nFeeRet); nChange -= nMoveToFee; nFeeRet += nMoveToFee; } @@ -1215,9 +1233,21 @@ bool CWallet::CreateTransaction(const vector >& vecSend, CW CScript scriptChange; scriptChange.SetDestination(vchPubKey.GetID()); - // Insert change txn at random position: - vector::iterator position = wtxNew.vout.begin()+GetRandInt(wtxNew.vout.size()+1); - wtxNew.vout.insert(position, CTxOut(nChange, scriptChange)); + CTxOut newTxOut(nChange, scriptChange); + + // Never create dust outputs; if we would, just + // add the dust to the fee. + if (newTxOut.IsDust()) + { + nFeeRet += nChange; + reservekey.ReturnKey(); + } + else + { + // Insert change txn at random position: + vector::iterator position = wtxNew.vout.begin()+GetRandInt(wtxNew.vout.size()+1); + wtxNew.vout.insert(position, newTxOut); + } } else reservekey.ReturnKey(); @@ -1230,12 +1260,18 @@ bool CWallet::CreateTransaction(const vector >& vecSend, CW int nIn = 0; BOOST_FOREACH(const PAIRTYPE(const CWalletTx*,unsigned int)& coin, setCoins) if (!SignSignature(*this, *coin.first, wtxNew, nIn++)) + { + strFailReason = _("Signing transaction failed"); return false; + } // Limit size unsigned int nBytes = ::GetSerializeSize(*(CTransaction*)&wtxNew, SER_NETWORK, PROTOCOL_VERSION); if (nBytes >= MAX_STANDARD_TX_SIZE) + { + strFailReason = _("Transaction too large"); return false; + } dPriority /= nBytes; // Check that enough fee is included @@ -1259,11 +1295,12 @@ bool CWallet::CreateTransaction(const vector >& vecSend, CW return true; } -bool CWallet::CreateTransaction(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew, CReserveKey& reservekey, int64& nFeeRet) +bool CWallet::CreateTransaction(CScript scriptPubKey, int64 nValue, + CWalletTx& wtxNew, CReserveKey& reservekey, int64& nFeeRet, std::string& strFailReason) { vector< pair > vecSend; vecSend.push_back(make_pair(scriptPubKey, nValue)); - return CreateTransaction(vecSend, wtxNew, reservekey, nFeeRet); + return CreateTransaction(vecSend, wtxNew, reservekey, nFeeRet, strFailReason); } // Call after CreateTransaction unless you want to abort @@ -1329,14 +1366,12 @@ string CWallet::SendMoney(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew, printf("SendMoney() : %s", strError.c_str()); return strError; } - if (!CreateTransaction(scriptPubKey, nValue, wtxNew, reservekey, nFeeRequired)) + string strError; + if (!CreateTransaction(scriptPubKey, nValue, wtxNew, reservekey, nFeeRequired, strError)) { - string strError; if (nValue + nFeeRequired > GetBalance()) strError = strprintf(_("Error: This transaction requires a transaction fee of at least %s because of its amount, complexity, or use of recently received funds!"), FormatMoney(nFeeRequired).c_str()); - else - strError = _("Error: Transaction creation failed!"); - printf("SendMoney() : %s", strError.c_str()); + printf("SendMoney() : %s\n", strError.c_str()); return strError; } diff --git a/src/wallet.h b/src/wallet.h index 19f50e7e6..dcba4675e 100644 --- a/src/wallet.h +++ b/src/wallet.h @@ -178,8 +178,10 @@ public: int64 GetBalance() const; int64 GetUnconfirmedBalance() const; int64 GetImmatureBalance() const; - bool CreateTransaction(const std::vector >& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, int64& nFeeRet); - bool CreateTransaction(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew, CReserveKey& reservekey, int64& nFeeRet); + bool CreateTransaction(const std::vector >& vecSend, + CWalletTx& wtxNew, CReserveKey& reservekey, int64& nFeeRet, std::string& strFailReason); + bool CreateTransaction(CScript scriptPubKey, int64 nValue, + CWalletTx& wtxNew, CReserveKey& reservekey, int64& nFeeRet, std::string& strFailReason); bool CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey); std::string SendMoney(CScript scriptPubKey, int64 nValue, CWalletTx& wtxNew, bool fAskFee=false); std::string SendMoneyToDestination(const CTxDestination &address, int64 nValue, CWalletTx& wtxNew, bool fAskFee=false);