Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move CWalletDB::ReorderTransactions to CWallet #8828

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
73 changes: 72 additions & 1 deletion src/wallet/wallet.cpp
Expand Up @@ -658,8 +658,79 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase)

DBErrors CWallet::ReorderTransactions()
{
LOCK(cs_wallet);
CWalletDB walletdb(strWalletFile);
return walletdb.ReorderTransactions(this);

// Old wallets didn't have any defined order for transactions
// Probably a bad idea to change the output of this

// First: get all CWalletTx and CAccountingEntry into a sorted-by-time multimap.
typedef pair<CWalletTx*, CAccountingEntry*> TxPair;
typedef multimap<int64_t, TxPair > TxItems;
TxItems txByTime;

for (map<uint256, CWalletTx>::iterator it = mapWallet.begin(); it != mapWallet.end(); ++it)
{
CWalletTx* wtx = &((*it).second);
txByTime.insert(make_pair(wtx->nTimeReceived, TxPair(wtx, (CAccountingEntry*)0)));
}
list<CAccountingEntry> acentries;
walletdb.ListAccountCreditDebit("", acentries);
BOOST_FOREACH(CAccountingEntry& entry, acentries)
{
txByTime.insert(make_pair(entry.nTime, TxPair((CWalletTx*)0, &entry)));
}

nOrderPosNext = 0;
std::vector<int64_t> nOrderPosOffsets;
for (TxItems::iterator it = txByTime.begin(); it != txByTime.end(); ++it)
{
CWalletTx *const pwtx = (*it).second.first;
CAccountingEntry *const pacentry = (*it).second.second;
int64_t& nOrderPos = (pwtx != 0) ? pwtx->nOrderPos : pacentry->nOrderPos;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+wallet/wallet.cpp:684:14: warning: declaration shadows a field of 'CWallet' [-Wshadow]
+    int64_t& nOrderPosNext = nOrderPosNext;
+             ^
+./wallet/wallet.h:659:13: note: previous declaration is here
+    int64_t nOrderPosNext;
+            ^
+wallet/wallet.cpp:684:30: warning: reference 'nOrderPosNext' is not yet bound to a value when used within its own initialization [-Wuninitialized]
+    int64_t& nOrderPosNext = nOrderPosNext;
+             ~~~~~~~~~~~~~   ^~~~~~~~~~~~~
+2 warnings generated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed that is the issue


if (nOrderPos == -1)
{
nOrderPos = nOrderPosNext++;
nOrderPosOffsets.push_back(nOrderPos);

if (pwtx)
{
if (!walletdb.WriteTx(*pwtx))
return DB_LOAD_FAIL;
}
else
if (!walletdb.WriteAccountingEntry(pacentry->nEntryNo, *pacentry))
return DB_LOAD_FAIL;
}
else
{
int64_t nOrderPosOff = 0;
BOOST_FOREACH(const int64_t& nOffsetStart, nOrderPosOffsets)
{
if (nOrderPos >= nOffsetStart)
++nOrderPosOff;
}
nOrderPos += nOrderPosOff;
nOrderPosNext = std::max(nOrderPosNext, nOrderPos + 1);

if (!nOrderPosOff)
continue;

// Since we're changing the order, write it back
if (pwtx)
{
if (!walletdb.WriteTx(*pwtx))
return DB_LOAD_FAIL;
}
else
if (!walletdb.WriteAccountingEntry(pacentry->nEntryNo, *pacentry))
return DB_LOAD_FAIL;
}
}
walletdb.WriteOrderPosNext(nOrderPosNext);

return DB_LOAD_OK;
}

int64_t CWallet::IncOrderPosNext(CWalletDB *pwalletdb)
Expand Down
78 changes: 1 addition & 77 deletions src/wallet/walletdb.cpp
Expand Up @@ -251,82 +251,6 @@ void CWalletDB::ListAccountCreditDebit(const string& strAccount, list<CAccountin
pcursor->close();
}

DBErrors CWalletDB::ReorderTransactions(CWallet* pwallet)
{
LOCK(pwallet->cs_wallet);
// Old wallets didn't have any defined order for transactions
// Probably a bad idea to change the output of this

// First: get all CWalletTx and CAccountingEntry into a sorted-by-time multimap.
typedef pair<CWalletTx*, CAccountingEntry*> TxPair;
typedef multimap<int64_t, TxPair > TxItems;
TxItems txByTime;

for (map<uint256, CWalletTx>::iterator it = pwallet->mapWallet.begin(); it != pwallet->mapWallet.end(); ++it)
{
CWalletTx* wtx = &((*it).second);
txByTime.insert(make_pair(wtx->nTimeReceived, TxPair(wtx, (CAccountingEntry*)0)));
}
list<CAccountingEntry> acentries;
ListAccountCreditDebit("", acentries);
BOOST_FOREACH(CAccountingEntry& entry, acentries)
{
txByTime.insert(make_pair(entry.nTime, TxPair((CWalletTx*)0, &entry)));
}

int64_t& nOrderPosNext = pwallet->nOrderPosNext;
nOrderPosNext = 0;
std::vector<int64_t> nOrderPosOffsets;
for (TxItems::iterator it = txByTime.begin(); it != txByTime.end(); ++it)
{
CWalletTx *const pwtx = (*it).second.first;
CAccountingEntry *const pacentry = (*it).second.second;
int64_t& nOrderPos = (pwtx != 0) ? pwtx->nOrderPos : pacentry->nOrderPos;

if (nOrderPos == -1)
{
nOrderPos = nOrderPosNext++;
nOrderPosOffsets.push_back(nOrderPos);

if (pwtx)
{
if (!WriteTx(*pwtx))
return DB_LOAD_FAIL;
}
else
if (!WriteAccountingEntry(pacentry->nEntryNo, *pacentry))
return DB_LOAD_FAIL;
}
else
{
int64_t nOrderPosOff = 0;
BOOST_FOREACH(const int64_t& nOffsetStart, nOrderPosOffsets)
{
if (nOrderPos >= nOffsetStart)
++nOrderPosOff;
}
nOrderPos += nOrderPosOff;
nOrderPosNext = std::max(nOrderPosNext, nOrderPos + 1);

if (!nOrderPosOff)
continue;

// Since we're changing the order, write it back
if (pwtx)
{
if (!WriteTx(*pwtx))
return DB_LOAD_FAIL;
}
else
if (!WriteAccountingEntry(pacentry->nEntryNo, *pacentry))
return DB_LOAD_FAIL;
}
}
WriteOrderPosNext(nOrderPosNext);

return DB_LOAD_OK;
}

class CWalletScanState {
public:
unsigned int nKeys;
Expand Down Expand Up @@ -711,7 +635,7 @@ DBErrors CWalletDB::LoadWallet(CWallet* pwallet)
WriteVersion(CLIENT_VERSION);

if (wss.fAnyUnordered)
result = ReorderTransactions(pwallet);
result = pwallet->ReorderTransactions();

pwallet->laccentries.clear();
ListAccountCreditDebit("*", pwallet->laccentries);
Expand Down
3 changes: 1 addition & 2 deletions src/wallet/walletdb.h
Expand Up @@ -153,6 +153,7 @@ class CWalletDB : public CDB

/// This writes directly to the database, and will not update the CWallet's cached accounting entries!
/// Use wallet.AddAccountingEntry instead, to write *and* update its caches.
bool WriteAccountingEntry(const uint64_t nAccEntryNum, const CAccountingEntry& acentry);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this a line down because of the comment?

bool WriteAccountingEntry_Backend(const CAccountingEntry& acentry);
bool ReadAccount(const std::string& strAccount, CAccount& account);
bool WriteAccount(const std::string& strAccount, const CAccount& account);
Expand All @@ -165,7 +166,6 @@ class CWalletDB : public CDB
CAmount GetAccountCreditDebit(const std::string& strAccount);
void ListAccountCreditDebit(const std::string& strAccount, std::list<CAccountingEntry>& acentries);

DBErrors ReorderTransactions(CWallet* pwallet);
DBErrors LoadWallet(CWallet* pwallet);
DBErrors FindWalletTx(CWallet* pwallet, std::vector<uint256>& vTxHash, std::vector<CWalletTx>& vWtx);
DBErrors ZapWalletTx(CWallet* pwallet, std::vector<CWalletTx>& vWtx);
Expand All @@ -180,7 +180,6 @@ class CWalletDB : public CDB
CWalletDB(const CWalletDB&);
void operator=(const CWalletDB&);

bool WriteAccountingEntry(const uint64_t nAccEntryNum, const CAccountingEntry& acentry);
};

void ThreadFlushWalletDB(const std::string& strFile);
Expand Down