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.

Sign up for GitHub

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

Use single instance of CWalletDB in CSparkWallet #1239

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
+422 −225
Open

Use single instance of CWalletDB in CSparkWallet #1239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
a30481b
Use single instance of CWalletDB in CSparkWallet
psolstice May 13, 2023
4e1b204
wallet: Move nAccountingEntryNumber from static/global to CWallet
luke-jr Mar 9, 2017
c747a7a
Bugfix: ForceSetArg should replace entr(ies) in mapMultiArgs, not append
luke-jr Jan 3, 2017
e1a67d8
CWalletDB: Store the update counter per wallet
luke-jr Mar 9, 2017
f481533
Wallet: Replace pwalletMain with a vector of wallet pointers
luke-jr Sep 9, 2016
c153a3e
Wallet: Support loading multiple wallets if -wallet used more than once
luke-jr Sep 9, 2016
4498cbe
Wallet: Move multiwallet sanity checks to CWallet::Verify, and do oth…
luke-jr Mar 9, 2017
3f98994
Bugfix: wallet: Fix warningStr, errorStr argument order
luke-jr Jun 5, 2017
6412711
wallet: Include actual backup filename in recovery warning message
luke-jr Jun 5, 2017
387f5d7
wallet: Base backup filenames on original wallet filename
luke-jr Jun 5, 2017
19fa389
wallet: Forbid -salvagewallet, -zapwallettxes, and -upgradewallet wit…
luke-jr Jun 5, 2017
e4c0237
wallet: Update formatting
luke-jr Jun 5, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions src/init.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,9 @@ void Shutdown()
BatchProofContainer::get_instance()->verify();

#ifdef ENABLE_WALLET
if (pwalletMain)
pwalletMain->Flush(false);
for (CWalletRef pwallet : vpwallets) {
pwallet->Flush(false);
}
#endif
GenerateBitcoins(false, 0, Params());
MapPort(false);
Expand Down Expand Up @@ -311,8 +312,9 @@ void Shutdown()
#endif

#ifdef ENABLE_WALLET
if (pwalletMain)
pwalletMain->Flush(true);
for (CWalletRef pwallet : vpwallets) {
pwallet->Flush(true);
}
#endif

#if ENABLE_ZMQ
Expand Down Expand Up @@ -345,8 +347,10 @@ void Shutdown()
#endif
UnregisterAllValidationInterfaces();
#ifdef ENABLE_WALLET
delete pwalletMain;
pwalletMain = NULL;
for (CWalletRef pwallet : vpwallets) {
delete pwallet;
}
vpwallets.clear();
#endif
globalVerifyHandle.reset();
ECC_Stop();
Expand Down Expand Up @@ -2170,8 +2174,9 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
uiInterface.InitMessage(_("Done loading"));

#ifdef ENABLE_WALLET
if (pwalletMain)
pwalletMain->postInitProcess(threadGroup);
for (CWalletRef pwallet : vpwallets) {
pwallet->postInitProcess(scheduler);
}
#endif

return !fRequestShutdown;
Expand Down
5 changes: 3 additions & 2 deletions src/qt/bitcoin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,9 +480,10 @@ void BitcoinApplication::initializeResult(int retval)
window->setClientModel(clientModel);

#ifdef ENABLE_WALLET
if(pwalletMain)
// TODO: Expose secondary wallets
if (!vpwallets.empty())
{
walletModel = new WalletModel(platformStyle, pwalletMain, optionsModel);
walletModel = new WalletModel(platformStyle, vpwallets[0], optionsModel);

window->addWallet(BitcoinGUI::DEFAULT_WALLET, walletModel);
window->setCurrentWallet(BitcoinGUI::DEFAULT_WALLET);
Expand Down
48 changes: 18 additions & 30 deletions src/spark/sparkwallet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@

const uint32_t DEFAULT_SPARK_NCOUNT = 1;

CSparkWallet::CSparkWallet(const std::string& strWalletFile) {

CWalletDB walletdb(strWalletFile);
this->strWalletFile = strWalletFile;

CSparkWallet::CSparkWallet(const std::string &strWalletFile) : walletdb(strWalletFile) {
Copy link

Choose a reason for hiding this comment

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

Consider handling the case where walletdb initialization fails.

const spark::Params* params = spark::Params::get_default();

fullViewKey = spark::FullViewKey(params);
Expand Down Expand Up @@ -66,11 +62,11 @@ CSparkWallet::~CSparkWallet() {
delete (ParallelOpThreadPool<void>*)threadPool;
}

void CSparkWallet::resetDiversifierFromDB(CWalletDB& walletdb) {
void CSparkWallet::resetDiversifierFromDB() {
walletdb.readDiversifier(lastDiversifier);
}

void CSparkWallet::updatetDiversifierInDB(CWalletDB& walletdb) {
void CSparkWallet::updatetDiversifierInDB() {
Copy link

Choose a reason for hiding this comment

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

Fix the typo in the method name updatetDiversifierInDB to updateDiversifierInDB.

- void CSparkWallet::updatetDiversifierInDB() {
+ void CSparkWallet::updateDiversifierInDB() {
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
void CSparkWallet::updatetDiversifierInDB() {
void CSparkWallet::updateDiversifierInDB() {

walletdb.writeDiversifier(lastDiversifier);
}

Expand Down Expand Up @@ -173,8 +169,7 @@ spark::Address CSparkWallet::generateNewAddress() {
spark::Address address(viewKey, lastDiversifier);

addresses[lastDiversifier] = address;
CWalletDB walletdb(strWalletFile);
updatetDiversifierInDB(walletdb);
updatetDiversifierInDB();
return address;
}

Expand Down Expand Up @@ -281,9 +276,8 @@ std::vector<CSparkMintMeta> CSparkWallet::ListSparkMints(bool fUnusedOnly, bool
return setMints;
}

std::list<CSparkSpendEntry> CSparkWallet::ListSparkSpends() const {
std::list<CSparkSpendEntry> CSparkWallet::ListSparkSpends() {
std::list<CSparkSpendEntry> result;
CWalletDB walletdb(strWalletFile);
walletdb.ListSparkSpends(result);
return result;
}
Expand Down Expand Up @@ -316,7 +310,7 @@ spark::Coin CSparkWallet::getCoinFromLTag(const GroupElement& lTag) const {
}


void CSparkWallet::clearAllMints(CWalletDB& walletdb) {
void CSparkWallet::clearAllMints() {
LOCK(cs_spark_wallet);
for (auto& itr : coinMeta) {
walletdb.EraseSparkMint(itr.first);
Expand All @@ -327,13 +321,13 @@ void CSparkWallet::clearAllMints(CWalletDB& walletdb) {
walletdb.writeDiversifier(lastDiversifier);
}

void CSparkWallet::eraseMint(const uint256& hash, CWalletDB& walletdb) {
void CSparkWallet::eraseMint(const uint256& hash) {
LOCK(cs_spark_wallet);
walletdb.EraseSparkMint(hash);
coinMeta.erase(hash);
}

void CSparkWallet::addOrUpdateMint(const CSparkMintMeta& mint, const uint256& lTagHash, CWalletDB& walletdb) {
void CSparkWallet::addOrUpdateMint(const CSparkMintMeta& mint, const uint256& lTagHash) {
LOCK(cs_spark_wallet);

if (mint.i > lastDiversifier) {
Expand All @@ -344,24 +338,23 @@ void CSparkWallet::addOrUpdateMint(const CSparkMintMeta& mint, const uint256& lT
walletdb.WriteSparkMint(lTagHash, mint);
}

void CSparkWallet::updateMint(const CSparkMintMeta& mint, CWalletDB& walletdb) {
void CSparkWallet::updateMint(const CSparkMintMeta& mint) {
LOCK(cs_spark_wallet);
for (const auto& coin : coinMeta) {
if (mint == coin.second) {
addOrUpdateMint(mint, coin.first, walletdb);
addOrUpdateMint(mint, coin.first);
}
}
}

void CSparkWallet::setCoinUnused(const GroupElement& lTag) {
LOCK(cs_spark_wallet);
CWalletDB walletdb(strWalletFile);
uint256 lTagHash = primitives::GetLTagHash(lTag);
CSparkMintMeta coinMeta = getMintMeta(lTagHash);

if (coinMeta != CSparkMintMeta()) {
coinMeta.isUsed = false;
updateMint(coinMeta, walletdb);
updateMint(coinMeta);
}
}

Expand Down Expand Up @@ -402,12 +395,11 @@ void CSparkWallet::UpdateSpendState(const GroupElement& lTag, const uint256& lTa
spendEntry.hashTx = txHash;
spendEntry.amount = mintMeta.v;

CWalletDB walletdb(strWalletFile);
walletdb.WriteSparkSpendEntry(spendEntry);

if (fUpdateMint) {
mintMeta.isUsed = true;
addOrUpdateMint(mintMeta, lTagHash, walletdb);
addOrUpdateMint(mintMeta, lTagHash);
}

// pwalletMain->NotifyZerocoinChanged(
Expand Down Expand Up @@ -502,7 +494,7 @@ CAmount CSparkWallet::getMySpendAmount(const std::vector<GroupElement>& lTags) c
return result;
}

void CSparkWallet::UpdateMintState(const std::vector<spark::Coin>& coins, const uint256& txHash, CWalletDB& walletdb) {
void CSparkWallet::UpdateMintState(const std::vector<spark::Coin>& coins, const uint256& txHash) {
Copy link

Choose a reason for hiding this comment

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

Consider refactoring CreateSparkSpendTransaction to improve readability and maintainability. The method is complex and handles multiple responsibilities, which could be separated into smaller, more focused functions.

spark::CSparkState *sparkState = spark::CSparkState::GetState();
for (auto coin : coins) {
try {
Expand All @@ -528,7 +520,7 @@ void CSparkWallet::UpdateMintState(const std::vector<spark::Coin>& coins, const
}

uint256 lTagHash = primitives::GetLTagHash(recoveredCoinData.T);
addOrUpdateMint(mintMeta, lTagHash, walletdb);
addOrUpdateMint(mintMeta, lTagHash);

if (mintMeta.isUsed) {
uint256 spendTxHash;
Expand All @@ -553,8 +545,7 @@ void CSparkWallet::UpdateMintState(const std::vector<spark::Coin>& coins, const
void CSparkWallet::UpdateMintStateFromMempool(const std::vector<spark::Coin>& coins, const uint256& txHash) {
((ParallelOpThreadPool<void>*)threadPool)->PostTask([=]() mutable {
LOCK(cs_spark_wallet);
CWalletDB walletdb(strWalletFile);
UpdateMintState(coins, txHash, walletdb);
UpdateMintState(coins, txHash);
});
}

Expand All @@ -563,12 +554,11 @@ void CSparkWallet::UpdateMintStateFromBlock(const CBlock& block) {

((ParallelOpThreadPool<void>*)threadPool)->PostTask([=] () mutable {
LOCK(cs_spark_wallet);
CWalletDB walletdb(strWalletFile);
for (const auto& tx : transactions) {
if (tx->IsSparkTransaction()) {
auto coins = spark::GetSparkMintCoins(*tx);
uint256 txHash = tx->GetHash();
UpdateMintState(coins, txHash, walletdb);
UpdateMintState(coins, txHash);
}
}
});
Expand All @@ -580,10 +570,9 @@ void CSparkWallet::RemoveSparkMints(const std::vector<spark::Coin>& mints) {
spark::IdentifiedCoinData identifiedCoinData = coin.identify(this->viewKey);
spark::RecoveredCoinData recoveredCoinData = coin.recover(this->fullViewKey, identifiedCoinData);

CWalletDB walletdb(strWalletFile);
uint256 lTagHash = primitives::GetLTagHash(recoveredCoinData.T);

eraseMint(lTagHash, walletdb);
eraseMint(lTagHash);
} catch (const std::runtime_error &e) {
continue;
}
Expand All @@ -598,8 +587,7 @@ void CSparkWallet::RemoveSparkSpends(const std::unordered_map<GroupElement, int>
if (coinMeta.count(lTagHash)) {
auto mintMeta = coinMeta[lTagHash];
mintMeta.isUsed = false;
CWalletDB walletdb(strWalletFile);
addOrUpdateMint(mintMeta, lTagHash, walletdb);
addOrUpdateMint(mintMeta, lTagHash);
walletdb.EraseSparkSpendEntry(spend.first);
}
}
Expand Down
20 changes: 10 additions & 10 deletions src/spark/sparkwallet.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,16 @@ const uint32_t BIP44_SPARK_INDEX = 0x6;

class CSparkWallet {
public:
CSparkWallet(const std::string& strWalletFile);
CSparkWallet(const std::string &strWalletFile);
~CSparkWallet();
// increment diversifier and generate address for that
spark::Address generateNextAddress();
spark::Address generateNewAddress();
spark::Address getDefaultAddress();
// assign difersifier to the value from db
void resetDiversifierFromDB(CWalletDB& walletdb);
void resetDiversifierFromDB();
// assign diversifier in to to current value
void updatetDiversifierInDB(CWalletDB& walletdb);
void updatetDiversifierInDB();

// functions for key set generation
spark::SpendKey generateSpendKey(const spark::Params* params);
Expand All @@ -45,7 +45,7 @@ class CSparkWallet {

// list spark mint, mint metadata in memory and in db should be the same at this moment, so get from memory
std::vector<CSparkMintMeta> ListSparkMints(bool fUnusedOnly = false, bool fMatureOnly = false) const;
std::list<CSparkSpendEntry> ListSparkSpends() const;
std::list<CSparkSpendEntry> ListSparkSpends();
std::unordered_map<uint256, CSparkMintMeta> getMintMap() const;
// generate spark Coin from meta data
spark::Coin getCoinFromMeta(const CSparkMintMeta& meta) const;
Expand All @@ -62,12 +62,12 @@ class CSparkWallet {
CAmount getAddressUnconfirmedBalance(const spark::Address& address);

// function to be used for zap wallet
void clearAllMints(CWalletDB& walletdb);
void clearAllMints();
// erase mint meta data from memory and from db
void eraseMint(const uint256& hash, CWalletDB& walletdb);
void eraseMint(const uint256& hash);
// add mint meta data to memory and to db
void addOrUpdateMint(const CSparkMintMeta& mint, const uint256& lTagHash, CWalletDB& walletdb);
void updateMint(const CSparkMintMeta& mint, CWalletDB& walletdb);
void addOrUpdateMint(const CSparkMintMeta& mint, const uint256& lTagHash);
void updateMint(const CSparkMintMeta& mint);

void setCoinUnused(const GroupElement& lTag);

Expand All @@ -87,7 +87,7 @@ class CSparkWallet {
void UpdateSpendState(const GroupElement& lTag, const uint256& txHash, bool fUpdateMint = true);
void UpdateSpendStateFromMempool(const std::vector<GroupElement>& lTags, const uint256& txHash, bool fUpdateMint = true);
void UpdateSpendStateFromBlock(const CBlock& block);
void UpdateMintState(const std::vector<spark::Coin>& coins, const uint256& txHash, CWalletDB& walletdb);
void UpdateMintState(const std::vector<spark::Coin>& coins, const uint256& txHash);
void UpdateMintStateFromMempool(const std::vector<spark::Coin>& coins, const uint256& txHash);
void UpdateMintStateFromBlock(const CBlock& block);
void RemoveSparkMints(const std::vector<spark::Coin>& mints);
Expand Down Expand Up @@ -136,7 +136,7 @@ class CSparkWallet {
mutable CCriticalSection cs_spark_wallet;

private:
std::string strWalletFile;
CWalletDB walletdb;
// this is latest used diversifier
int32_t lastDiversifier;

Expand Down
2 changes: 2 additions & 0 deletions src/util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,8 @@ void ForceSetArg(const std::string& strArg, const std::string& strValue)
{
LOCK(cs_args);
mapArgs[strArg] = strValue;
mapMultiArgs[strArg].clear();
mapMultiArgs[strArg].push_back(strValue);
Comment on lines +468 to +469
Copy link

Choose a reason for hiding this comment

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

Consider using mapMultiArgs[strArg] = {strValue}; for clarity and efficiency.

-    mapMultiArgs[strArg].clear();
-    mapMultiArgs[strArg].push_back(strValue);
+    mapMultiArgs[strArg] = {strValue};

This refactoring simplifies the code by directly setting the vector to contain the single strValue, which is the intended outcome of clearing and then pushing back the value. It's more concise and potentially more efficient by avoiding the separate clear and push operations.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
mapMultiArgs[strArg].clear();
mapMultiArgs[strArg].push_back(strValue);
mapMultiArgs[strArg] = {strValue};

}


Expand Down
Loading