Wallet refactoring leading up to multiwallet #8776
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonasschnelli
approved these changes
Sep 22, 2016
| { | ||
| std::string walletFile = GetArg("-wallet", DEFAULT_WALLET_DAT); | ||
|
|
||
| CWallet * const pwallet = CreateWalletFromFile(walletFile); |
There was a problem hiding this comment.
nit: const CWallet *pwallet =
|
Needs rebase. |
|
Rebased |
682936c to
24539dc
Compare
luke-jr
added a commit
to bitcoinknots/bitcoin
that referenced
this pull request
Oct 20, 2016
Github-Pull: bitcoin#8776 Rebased-From: 4a8788f
luke-jr
added a commit
to bitcoinknots/bitcoin
that referenced
this pull request
Oct 20, 2016
Github-Pull: bitcoin#8776 Rebased-From: 24539dc
|
utACK 24539dc |
maflcko
reviewed
Nov 2, 2016
| @@ -1527,7 +1527,7 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) | |||
| #ifdef ENABLE_WALLET | |||
| if (pwalletMain) { | |||
| // Run a thread to flush wallet periodically | |||
| threadGroup.create_thread(boost::bind(&ThreadFlushWalletDB, boost::ref(pwalletMain->strWalletFile))); | |||
| threadGroup.create_thread(ThreadFlushWalletDB); | |||
There was a problem hiding this comment.
Needs merge conflict solved here
|
@luke-jr Mind to solve the merge conflict? Also, assigned 0.14 milestone, I think this is relatively trivial/uncontroversial refactoring. |
24539dc to
5394b39
Compare
|
Done |
|
utACK 5394b39 |
| @@ -810,6 +810,7 @@ void ThreadFlushWalletDB(const string& strFile) | |||
| if (nRefCount == 0) | |||
| { | |||
| boost::this_thread::interruption_point(); | |||
| const std::string& strFile = pwalletMain->strWalletFile; | |||
There was a problem hiding this comment.
In theory, this should only be done when holding cs_wallet. I guess changing CWallet::strWalletFile to a const std::string is not possible with the current concept of setting the filename in a instance method.
But maybe I'm wrong.
There was a problem hiding this comment.
wallet.h says "This lock protects all the fields added by CWallet except for: ... strWalletFile (immutable after instantiation)"
| @@ -914,6 +917,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface | |||
| static std::string GetWalletHelpString(bool showDebug); | |||
|
|
|||
| /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */ | |||
There was a problem hiding this comment.
Heh. This (unchanged) comment was wrong before and correct after this PR. :)
| @@ -914,6 +917,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface | |||
| static std::string GetWalletHelpString(bool showDebug); | |||
|
|
|||
| /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */ | |||
| static CWallet* CreateWalletFromFile(const std::string walletFile); | |||
There was a problem hiding this comment.
Maybe declare CreateWalletFromFile() private to avoid layer confusion?
There was a problem hiding this comment.
It's not really intended to be private...
|
@jonasschnelli Are your comments addressed by the feedback? |
|
@jonasschnelli Are your comments addressed by the feedback? Yes. |
|
Code Review ACK. |
|
utACK |
|
utACK 5394b39 |
sipa
added a commit
that referenced
this pull request
Jan 3, 2017
andvgal
pushed a commit
to energicryptocurrency/gen2-energi
that referenced
this pull request
Jan 6, 2019
CryptoCentric
pushed a commit
to absolute-community/absolute
that referenced
this pull request
Feb 26, 2019
random-zebra
added a commit
to PIVX-Project/PIVX
that referenced
this pull request
Aug 25, 2020
7e71759 Wallet: Split main logic from InitLoadWallet into CreateWalletFromFile (random-zebra) e585dad [Wallet] refactor wallet/init interaction (random-zebra) 49646b2 [Refactor] Nuke zwalletMain global object (random-zebra) f5f9df9 Wallet: Let the interval-flushing thread figure out the filename (Luke Dashjr) 74445e4 [wallet] Move hardcoded file name out of log messages (random-zebra) 3f1838d [Wallet] optimize return value of InitLoadWallet() (random-zebra) 539dea4 [Wallet] move "load wallet phase" to CWallet (random-zebra) 7644318 [Wallet] move wallet help string creation to CWallet (random-zebra) a9d69b8 [trivial] Reuse translation and cleanup DEFAULT_* values (random-zebra) cfd007a [Bug] Omit wallet-related options from -help when wallet not supported (random-zebra) da642e6 [Refactor] More constant default values cleanup (random-zebra) a45275a [Refactor] Implement CBaseChainParams::BaseParams(Network) (random-zebra) 09abb98 Constrain constant values to a single location in code (random-zebra) Pull request description: Adapts the following refactoring PRs: - bitcoin#6961 Constrain constant values to a single location in code - bitcoin#7576 [Wallet] move wallet help string creation to CWallet - bitcoin#7577 move "load wallet phase" to CWallet - bitcoin#7608 Move hardcoded file name out of log messages - bitcoin#7691 refactor wallet/init interaction - bitcoin#8776 Wallet refactoring leading up to multiwallet ACKs for top commit: furszy: re ACK 7e71759 Fuzzbawls: ACK 7e71759 Tree-SHA512: 5fad328b9ddf8187af97d3d5fb285d0b67e73d51ac1bc44a3d57d0af86bce8b34efaab539b7bdbc4f9a2fa575a936f83788cffcc9c6d6d04cd3e63b19d399400
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of the refactorings needed for basic multiwallet (#8694)