[PoC/Concept review] add 2nd wallet "corewallet"

jonasschnelli commented at 5:18 pm on April 13, 2015: contributor

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

The current wallet is functional untouched, but there are serval modularity changes (mainly #5990).

CoreWallet

Dummy wallet (can only generate addresses)
logdb: checksummed append only file format
basic multiwallet support
No selected-wallet state (no “active wallet”), every RPC call should have a “walletid” arg (it would be possible to keep this empty if only one wallet is present)
Very basic Bip32 implementation (I’m currently working on that)

RPC

RPC endpoint for CoreWallet is /wallet.
RPC parameters should always come through a property list (json object like ["hex": "ea6", "flag": true]) to avoid parameter ordering

Bitcoin-Cli

[not implemented yet]

bitcoin-cli -w should 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

CoreWallet is completely held in a namespace to avoid interfering with CWallet and co.
CoreWallet should not use any exiting namespaces like std, boost or json_spirit

GUI

If CoreWallet gets stable, it would be possible to switch the GUI implementation to the new wallet
Currently the GUI is tied to the current wallet. Supporting multiple wallet implementations on GUI level makes no sense IMO.

Step-by-step approach

corewallet must be enabled during configuring (--enable-corewallet=yes), default is no so 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.

jonasschnelli force-pushed on Apr 14, 2015

jonasschnelli force-pushed on Apr 16, 2015

jonasschnelli force-pushed on May 2, 2015

laanwj added the label Wallet on May 6, 2015

Move recently introduced CTransAction::IsEquivalentTo to CWalletTx

CTransAction::IsEquivalentTo was introduced in #5881.
This functionality is only useful to the wallet, and should never have
been added to the primitive transaction type.

c9e5cf9688

tests: Fix bitcoin-tx signing testcase

Fixes wrong scriptPubkey problem, which caused the transaction to
not actually be signed.

c64ac71cf5

[bitcoin-cli] improve error output eb69554aa2

[Wallet] refactor and optimize init.cpp/CWallet interaction

Reduced wallet/core coupling.

- hides CWalletDB behind CWallet
- reduces ENABLE_WALLET ifdefs
- remove some whitespace

de52ba4c23

decouple wallet from core, allow multiple wallets through signaling fa6a5b28fc

[RPC] add possibility to extend the JSONRPC URI schemas 4c956eebd6

add bip32 pub key serialization

CExtPubKey should be serializable like CPubKey.

8bab0c341c

logdb: an safe append-only key-value store

Conflicts:
	src/makefile.unix

251ff6d0a5

improve sipas logdb 03c52a2da9

[corewallet] add basic structur of a 2nd wallet implementation 6fec1c9906

[corewallet] add basic multiwallet support f42e8e79f2

[corewallet] add very basic bip32 support 288ca00817

[corewallet] remove most of the global corewallet functions, add wallet manager signeton

- this is required to have a clean CValidationInterface listener who can support multiple wallets

0b8ef46baa

extend bitcoin-cli to support different endpoints and flexible parameter lists 02364e20b9

[corewallet] improve bip32 support 4f804b038e

[corewallet] Adapt UniValue for corewallet 6668911803

[corewallet] add hdkeystore# f29c400eed

[corewallet] add multiwallet map lock a43c52031c

[corewallet] adapt main wallet HD PR to corewallet b7ff3f90f6

[corewallet] extend receiving, try to refactor WalletTX/Core interaction 6a5952114c

[corewallet] basic sending and receiving done

- create transaction logic is more or less copied from the current wallet
- refactoring of main/mempool interaction from corewallet

433e58d97a

[corewallet] GetDebit refactoring, cleanup and commenting f21c70f306

[corewallet] refactor rpc help/error handling b951a040f6

[corewallet] add getter for HDPubKey in hdkeystore dfec9c42da

[corewallet] rpc cleanup, add verifying of params, better help 50a8c57652

jonasschnelli force-pushed on Jul 8, 2015

jgarzik commented at 6:26 pm on September 15, 2015: 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.

jonasschnelli commented at 6:28 pm on September 15, 2015: contributor

Agreed. Currently this was getting to big for a PR and therefor continued in a standalone fork https://github.com/jonasschnelli/bitcoin

jonasschnelli closed this on Sep 15, 2015

DrahtBot locked this on Sep 8, 2021

[PoC/Concept review] add 2nd wallet “corewallet” #6008