aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
Diffstat (limited to 'src')
-rw-r--r--src/bench/verify_script.cpp2
-rw-r--r--src/coins.cpp7
-rw-r--r--src/dbwrapper.h12
-rw-r--r--src/streams.h6
-rw-r--r--src/test/coins_tests.cpp28
-rw-r--r--src/wallet/db.cpp8
-rw-r--r--src/wallet/db.h12
-rw-r--r--src/wallet/wallet.cpp31
-rw-r--r--src/wallet/wallet.h4
9 files changed, 76 insertions, 34 deletions
diff --git a/src/bench/verify_script.cpp b/src/bench/verify_script.cpp
index bf04f90e0e..23bbadc88d 100644
--- a/src/bench/verify_script.cpp
+++ b/src/bench/verify_script.cpp
@@ -93,7 +93,7 @@ static void VerifyScriptBench(benchmark::State& state)
txCredit.vout[0].scriptPubKey.data(),
txCredit.vout[0].scriptPubKey.size(),
txCredit.vout[0].nValue,
- (const unsigned char*)&stream[0], stream.size(), 0, flags, nullptr);
+ (const unsigned char*)stream.data(), stream.size(), 0, flags, nullptr);
assert(csuccess == 1);
#endif
}
diff --git a/src/coins.cpp b/src/coins.cpp
index 68f32a9da4..4d0e4bc0ad 100644
--- a/src/coins.cpp
+++ b/src/coins.cpp
@@ -207,6 +207,13 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn
entry.flags |= CCoinsCacheEntry::FRESH;
}
} else {
+ // Assert that the child cache entry was not marked FRESH if the
+ // parent cache entry has unspent outputs. If this ever happens,
+ // it means the FRESH flag was misapplied and there is a logic
+ // error in the calling code.
+ if ((it->second.flags & CCoinsCacheEntry::FRESH) && !itUs->second.coins.IsPruned())
+ throw std::logic_error("FRESH flag misapplied to cache entry for base transaction with spendable outputs");
+
// Found the entry in the parent cache
if ((itUs->second.flags & CCoinsCacheEntry::FRESH) && it->second.coins.IsPruned()) {
// The grandparent does not have an entry, and the child is
diff --git a/src/dbwrapper.h b/src/dbwrapper.h
index 787e2608db..dd59cc00ff 100644
--- a/src/dbwrapper.h
+++ b/src/dbwrapper.h
@@ -67,12 +67,12 @@ public:
{
ssKey.reserve(DBWRAPPER_PREALLOC_KEY_SIZE);
ssKey << key;
- leveldb::Slice slKey(&ssKey[0], ssKey.size());
+ leveldb::Slice slKey(ssKey.data(), ssKey.size());
ssValue.reserve(DBWRAPPER_PREALLOC_VALUE_SIZE);
ssValue << value;
ssValue.Xor(dbwrapper_private::GetObfuscateKey(parent));
- leveldb::Slice slValue(&ssValue[0], ssValue.size());
+ leveldb::Slice slValue(ssValue.data(), ssValue.size());
batch.Put(slKey, slValue);
ssKey.clear();
@@ -84,7 +84,7 @@ public:
{
ssKey.reserve(DBWRAPPER_PREALLOC_KEY_SIZE);
ssKey << key;
- leveldb::Slice slKey(&ssKey[0], ssKey.size());
+ leveldb::Slice slKey(ssKey.data(), ssKey.size());
batch.Delete(slKey);
ssKey.clear();
@@ -115,7 +115,7 @@ public:
CDataStream ssKey(SER_DISK, CLIENT_VERSION);
ssKey.reserve(DBWRAPPER_PREALLOC_KEY_SIZE);
ssKey << key;
- leveldb::Slice slKey(&ssKey[0], ssKey.size());
+ leveldb::Slice slKey(ssKey.data(), ssKey.size());
piter->Seek(slKey);
}
@@ -208,7 +208,7 @@ public:
CDataStream ssKey(SER_DISK, CLIENT_VERSION);
ssKey.reserve(DBWRAPPER_PREALLOC_KEY_SIZE);
ssKey << key;
- leveldb::Slice slKey(&ssKey[0], ssKey.size());
+ leveldb::Slice slKey(ssKey.data(), ssKey.size());
std::string strValue;
leveldb::Status status = pdb->Get(readoptions, slKey, &strValue);
@@ -242,7 +242,7 @@ public:
CDataStream ssKey(SER_DISK, CLIENT_VERSION);
ssKey.reserve(DBWRAPPER_PREALLOC_KEY_SIZE);
ssKey << key;
- leveldb::Slice slKey(&ssKey[0], ssKey.size());
+ leveldb::Slice slKey(ssKey.data(), ssKey.size());
std::string strValue;
leveldb::Status status = pdb->Get(readoptions, slKey, &strValue);
diff --git a/src/streams.h b/src/streams.h
index 471602227a..1d3b55c91e 100644
--- a/src/streams.h
+++ b/src/streams.h
@@ -174,12 +174,10 @@ public:
Init(nTypeIn, nVersionIn);
}
-#if !defined(_MSC_VER) || _MSC_VER >= 1300
CDataStream(const char* pbegin, const char* pend, int nTypeIn, int nVersionIn) : vch(pbegin, pend)
{
Init(nTypeIn, nVersionIn);
}
-#endif
CDataStream(const vector_type& vchIn, int nTypeIn, int nVersionIn) : vch(vchIn.begin(), vchIn.end())
{
@@ -245,6 +243,8 @@ public:
void clear() { vch.clear(); nReadPos = 0; }
iterator insert(iterator it, const char& x=char()) { return vch.insert(it, x); }
void insert(iterator it, size_type n, const char& x) { vch.insert(it, n, x); }
+ value_type* data() { return vch.data() + nReadPos; }
+ const value_type* data() const { return vch.data() + nReadPos; }
void insert(iterator it, std::vector<char>::const_iterator first, std::vector<char>::const_iterator last)
{
@@ -259,7 +259,6 @@ public:
vch.insert(it, first, last);
}
-#if !defined(_MSC_VER) || _MSC_VER >= 1300
void insert(iterator it, const char* first, const char* last)
{
assert(last - first >= 0);
@@ -272,7 +271,6 @@ public:
else
vch.insert(it, first, last);
}
-#endif
iterator erase(iterator it)
{
diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp
index 70b958c296..b25c7ccc51 100644
--- a/src/test/coins_tests.cpp
+++ b/src/test/coins_tests.cpp
@@ -791,12 +791,18 @@ BOOST_AUTO_TEST_CASE(ccoins_modify_new)
void CheckWriteCoins(CAmount parent_value, CAmount child_value, CAmount expected_value, char parent_flags, char child_flags, char expected_flags)
{
SingleEntryCacheTest test(ABSENT, parent_value, parent_flags);
- WriteCoinsViewEntry(test.cache, child_value, child_flags);
- test.cache.SelfTest();
CAmount result_value;
char result_flags;
- GetCoinsMapEntry(test.cache.map(), result_value, result_flags);
+ try {
+ WriteCoinsViewEntry(test.cache, child_value, child_flags);
+ test.cache.SelfTest();
+ GetCoinsMapEntry(test.cache.map(), result_value, result_flags);
+ } catch (std::logic_error& e) {
+ result_value = FAIL;
+ result_flags = NO_ENTRY;
+ }
+
BOOST_CHECK_EQUAL(result_value, expected_value);
BOOST_CHECK_EQUAL(result_flags, expected_flags);
}
@@ -840,21 +846,21 @@ BOOST_AUTO_TEST_CASE(ccoins_write)
CheckWriteCoins(VALUE1, ABSENT, VALUE1, DIRTY , NO_ENTRY , DIRTY );
CheckWriteCoins(VALUE1, ABSENT, VALUE1, DIRTY|FRESH, NO_ENTRY , DIRTY|FRESH);
CheckWriteCoins(VALUE1, PRUNED, PRUNED, 0 , DIRTY , DIRTY );
- CheckWriteCoins(VALUE1, PRUNED, PRUNED, 0 , DIRTY|FRESH, DIRTY );
+ CheckWriteCoins(VALUE1, PRUNED, FAIL , 0 , DIRTY|FRESH, NO_ENTRY );
CheckWriteCoins(VALUE1, PRUNED, ABSENT, FRESH , DIRTY , NO_ENTRY );
- CheckWriteCoins(VALUE1, PRUNED, ABSENT, FRESH , DIRTY|FRESH, NO_ENTRY );
+ CheckWriteCoins(VALUE1, PRUNED, FAIL , FRESH , DIRTY|FRESH, NO_ENTRY );
CheckWriteCoins(VALUE1, PRUNED, PRUNED, DIRTY , DIRTY , DIRTY );
- CheckWriteCoins(VALUE1, PRUNED, PRUNED, DIRTY , DIRTY|FRESH, DIRTY );
+ CheckWriteCoins(VALUE1, PRUNED, FAIL , DIRTY , DIRTY|FRESH, NO_ENTRY );
CheckWriteCoins(VALUE1, PRUNED, ABSENT, DIRTY|FRESH, DIRTY , NO_ENTRY );
- CheckWriteCoins(VALUE1, PRUNED, ABSENT, DIRTY|FRESH, DIRTY|FRESH, NO_ENTRY );
+ CheckWriteCoins(VALUE1, PRUNED, FAIL , DIRTY|FRESH, DIRTY|FRESH, NO_ENTRY );
CheckWriteCoins(VALUE1, VALUE2, VALUE2, 0 , DIRTY , DIRTY );
- CheckWriteCoins(VALUE1, VALUE2, VALUE2, 0 , DIRTY|FRESH, DIRTY );
+ CheckWriteCoins(VALUE1, VALUE2, FAIL , 0 , DIRTY|FRESH, NO_ENTRY );
CheckWriteCoins(VALUE1, VALUE2, VALUE2, FRESH , DIRTY , DIRTY|FRESH);
- CheckWriteCoins(VALUE1, VALUE2, VALUE2, FRESH , DIRTY|FRESH, DIRTY|FRESH);
+ CheckWriteCoins(VALUE1, VALUE2, FAIL , FRESH , DIRTY|FRESH, NO_ENTRY );
CheckWriteCoins(VALUE1, VALUE2, VALUE2, DIRTY , DIRTY , DIRTY );
- CheckWriteCoins(VALUE1, VALUE2, VALUE2, DIRTY , DIRTY|FRESH, DIRTY );
+ CheckWriteCoins(VALUE1, VALUE2, FAIL , DIRTY , DIRTY|FRESH, NO_ENTRY );
CheckWriteCoins(VALUE1, VALUE2, VALUE2, DIRTY|FRESH, DIRTY , DIRTY|FRESH);
- CheckWriteCoins(VALUE1, VALUE2, VALUE2, DIRTY|FRESH, DIRTY|FRESH, DIRTY|FRESH);
+ CheckWriteCoins(VALUE1, VALUE2, FAIL , DIRTY|FRESH, DIRTY|FRESH, NO_ENTRY );
// The checks above omit cases where the child flags are not DIRTY, since
// they would be too repetitive (the parent cache is never updated in these
diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp
index 4ae5e80236..800e1f4e29 100644
--- a/src/wallet/db.cpp
+++ b/src/wallet/db.cpp
@@ -397,15 +397,15 @@ bool CDB::Rewrite(const string& strFile, const char* pszSkip)
break;
}
if (pszSkip &&
- strncmp(&ssKey[0], pszSkip, std::min(ssKey.size(), strlen(pszSkip))) == 0)
+ strncmp(ssKey.data(), pszSkip, std::min(ssKey.size(), strlen(pszSkip))) == 0)
continue;
- if (strncmp(&ssKey[0], "\x07version", 8) == 0) {
+ if (strncmp(ssKey.data(), "\x07version", 8) == 0) {
// Update version:
ssValue.clear();
ssValue << CLIENT_VERSION;
}
- Dbt datKey(&ssKey[0], ssKey.size());
- Dbt datValue(&ssValue[0], ssValue.size());
+ Dbt datKey(ssKey.data(), ssKey.size());
+ Dbt datValue(ssValue.data(), ssValue.size());
int ret2 = pdbCopy->put(NULL, &datKey, &datValue, DB_NOOVERWRITE);
if (ret2 > 0)
fSuccess = false;
diff --git a/src/wallet/db.h b/src/wallet/db.h
index b78f0f092a..bc15f2147f 100644
--- a/src/wallet/db.h
+++ b/src/wallet/db.h
@@ -122,7 +122,7 @@ protected:
CDataStream ssKey(SER_DISK, CLIENT_VERSION);
ssKey.reserve(1000);
ssKey << key;
- Dbt datKey(&ssKey[0], ssKey.size());
+ Dbt datKey(ssKey.data(), ssKey.size());
// Read
Dbt datValue;
@@ -158,13 +158,13 @@ protected:
CDataStream ssKey(SER_DISK, CLIENT_VERSION);
ssKey.reserve(1000);
ssKey << key;
- Dbt datKey(&ssKey[0], ssKey.size());
+ Dbt datKey(ssKey.data(), ssKey.size());
// Value
CDataStream ssValue(SER_DISK, CLIENT_VERSION);
ssValue.reserve(10000);
ssValue << value;
- Dbt datValue(&ssValue[0], ssValue.size());
+ Dbt datValue(ssValue.data(), ssValue.size());
// Write
int ret = pdb->put(activeTxn, &datKey, &datValue, (fOverwrite ? 0 : DB_NOOVERWRITE));
@@ -187,7 +187,7 @@ protected:
CDataStream ssKey(SER_DISK, CLIENT_VERSION);
ssKey.reserve(1000);
ssKey << key;
- Dbt datKey(&ssKey[0], ssKey.size());
+ Dbt datKey(ssKey.data(), ssKey.size());
// Erase
int ret = pdb->del(activeTxn, &datKey, 0);
@@ -207,7 +207,7 @@ protected:
CDataStream ssKey(SER_DISK, CLIENT_VERSION);
ssKey.reserve(1000);
ssKey << key;
- Dbt datKey(&ssKey[0], ssKey.size());
+ Dbt datKey(ssKey.data(), ssKey.size());
// Exists
int ret = pdb->exists(activeTxn, &datKey, 0);
@@ -234,7 +234,7 @@ protected:
Dbt datKey;
unsigned int fFlags = DB_NEXT;
if (setRange) {
- datKey.set_data(&ssKey[0]);
+ datKey.set_data(ssKey.data());
datKey.set_size(ssKey.size());
fFlags = DB_SET_RANGE;
}
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index ff7a03bc55..2775f4def3 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2534,8 +2534,37 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
return false;
}
- if (nFeeRet >= nFeeNeeded)
+ if (nFeeRet >= nFeeNeeded) {
+ // Reduce fee to only the needed amount if we have change
+ // output to increase. This prevents potential overpayment
+ // in fees if the coins selected to meet nFeeNeeded result
+ // in a transaction that requires less fee than the prior
+ // iteration.
+ // TODO: The case where nSubtractFeeFromAmount > 0 remains
+ // to be addressed because it requires returning the fee to
+ // the payees and not the change output.
+ // TODO: The case where there is no change output remains
+ // to be addressed so we avoid creating too small an output.
+ if (nFeeRet > nFeeNeeded && nChangePosInOut != -1 && nSubtractFeeFromAmount == 0) {
+ CAmount extraFeePaid = nFeeRet - nFeeNeeded;
+ vector<CTxOut>::iterator change_position = txNew.vout.begin()+nChangePosInOut;
+ change_position->nValue += extraFeePaid;
+ nFeeRet -= extraFeePaid;
+ }
break; // Done, enough fee included.
+ }
+
+ // Try to reduce change to include necessary fee
+ if (nChangePosInOut != -1 && nSubtractFeeFromAmount == 0) {
+ CAmount additionalFeeNeeded = nFeeNeeded - nFeeRet;
+ vector<CTxOut>::iterator change_position = txNew.vout.begin()+nChangePosInOut;
+ // Only reduce change if remaining amount is still a large enough output.
+ if (change_position->nValue >= MIN_FINAL_CHANGE + additionalFeeNeeded) {
+ change_position->nValue -= additionalFeeNeeded;
+ nFeeRet += additionalFeeNeeded;
+ break; // Done, able to increase fee from change
+ }
+ }
// Include more fee and try again.
nFeeRet = nFeeNeeded;
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 1d1f84ebb9..b9fa6bb24b 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -48,8 +48,10 @@ static const CAmount DEFAULT_TRANSACTION_FEE = 0;
static const CAmount DEFAULT_FALLBACK_FEE = 20000;
//! -mintxfee default
static const CAmount DEFAULT_TRANSACTION_MINFEE = 1000;
-//! minimum change amount
+//! target minimum change amount
static const CAmount MIN_CHANGE = CENT;
+//! final minimum change amount after paying for fees
+static const CAmount MIN_FINAL_CHANGE = MIN_CHANGE/2;
//! Default for -spendzeroconfchange
static const bool DEFAULT_SPEND_ZEROCONF_CHANGE = true;
//! Default for -sendfreetransactions