[PoC/Concept review] add 2nd wallet "corewallet" #6008
Closed
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
force-pushed
the
2015/04/parallel_wallet2
branch
5 times, most recently
from
7cdade5 to
49ea27f
Compare
jonasschnelli
force-pushed
the
2015/04/parallel_wallet2
branch
from
49ea27f to
9933ba2
Compare
CTransAction::IsEquivalentTo was introduced in bitcoin#5881. This functionality is only useful to the wallet, and should never have been added to the primitive transaction type.
Fixes wrong scriptPubkey problem, which caused the transaction to not actually be signed.
Reduced wallet/core coupling. - hides CWalletDB behind CWallet - reduces ENABLE_WALLET ifdefs - remove some whitespace
CExtPubKey should be serializable like CPubKey.
Conflicts: src/makefile.unix
…et manager signeton - this is required to have a clean CValidationInterface listener who can support multiple wallets
- create transaction logic is more or less copied from the current wallet - refactoring of main/mempool interaction from corewallet
jonasschnelli
force-pushed
the
2015/04/parallel_wallet2
branch
from
fd6900c to
50a8c57
Compare
Contributor
|
Strong supporter of this work - concept ACK - though I tend to prefer closing this as matching the "likely to be a very long lived PR" template. |
Contributor
Author
|
Agreed. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
Includes #6006 and #5990
Supersedes #5686 and #5744
PR is mainly for discussion and a try to get a direction where to go.
This will demonstrate a design of a possible wallet modularity and should allow one to get "the big picture". The new wallet (currently called "corewallet") is completely decoupled from the core and the current wallet. It can run in parallel with the current wallet.
Modularization
CoreWallet
RPC
/wallet.["hex": "ea6", "flag": true]) to avoid parameter orderingBitcoin-Cli
[not implemented yet]
bitcoin-cli -wshould access the new endpoint and parameters will be converted to a json property list (bitcoin-cli -w walletid=default flag=true anotherflag=1 <command>will be converted to["walletid":"default", "flag":true, anotherflag:1]).Code Guidelines
GUI
Step-by-step approach
--enable-corewallet=yes), default isnoso a merging into master could be seen as safe.How to continue
The groundwork PRs are already open (#6006, #5990, etc.). More small and reviewable PRs will follow.
A good way would be to merge the modularity groundwork as well as a very basic "corewallet" implementation. As long as corewallet will only compile with an additional flag, this should be safe and others could also start contributing to the new wallet (add implementation, help reviewing, etc.).
Critics, conceptual reviews and ideas are highly welcome.