-
Notifications
You must be signed in to change notification settings - Fork 50
Wallet Class Structure Changes
This describes the wallet class structure changes that were discussed at CoreDev Amsterdam. A transcript of most of this discussion can be found here.
The New Structure (:construction: Work in Progress in https://github.com/bitcoin/bitcoin/pull/16341)
The wallet is currently structured as a monolith. The CWallet class contains basically everything in the wallet - address generation, key generation, signing, transaction tracking, determining whether a transaction belongs to it, etc. The goal of this change will be to break out the address generation, key generation, signing, and ismine parts into a separate "box" within the wallet. This "box" will provide a standard interface for address generation, key fetching (for signing) and determining ismine and CWallet can have one or more of these boxes for the various address types (i.e. a default for legacy, one for bech32, and one to query for p2sh-segwit). Each box's internal implementation can vary which allows us to create one for the current wallet behavior and one for native descriptor wallets without requiring other significant changes to CWallet. This will also allow for future expandability
Within every CWallet, there will be one or more boxes, also called ScriptPubKeyManager. ScriptPubKeyManager will provide a GetSigningProvider() function which will return the SigningProvider for a given script to be used during signing. There will be additional functions added to it that allow for address fetching (e.g. GetNewAddress()). Each ScriptPubKeyManager will also have it's own IsMine() function to determine whether a CTxOut belongs to that ScriptPubKeyManager (and thus the wallet itself). Each ScriptPubKeyManager maintains which address type(s) (legacy/bech32/p2sh-wrapped) it can return and will respond accordingly to GetNewAddress() calls.
CWallet will no longer handle keys, addresses, and IsMine directly. Instead it will contain one or more ScriptPubKeyManagers with a map of address types - every combination of legacy/p2sh-segwit/bech32 and change/receive - to ScriptPubKeyManager. So there will be 6 ScriptPubKeyManager objects tracked in the wallet for legacy-receive, legacy-change, p2sh-receive, p2sh-change, bech32-receive, and bech32-change. These may all be the same ScriptPubKeyManager or different. When a new address is being requested, the default ScriptPubKeyManager for that type (legacy/p2sh/bech32, change/receive) is retrieved and a new address fetched from it. The type is usually decided based on the default for either change or non-change in the wallet, but can sometimes be specified explicitly (e.g. in the getnewaddress RPC). IsMine will be moved to be part of ScriptPubKeyManager so wallets will call IsMine for each of its ScriptPubKeyManagers to determine whether a transaction belongs to it. For signing, ScriptPubKeyManager will be passed into ProduceSignature as the SigningProvider to be used there. In order to determine which ScriptPubkeyManger to use when signing, the wallet will go through each of the ScriptPubKeyManagers it has and find the one which returns IsMine = True for a scriptPubKey. The ScriptPubKeyManager that does will be the one used for signing.
IsMine (:heavy_check_mark: done in https://github.com/bitcoin/bitcoin/pull/16226)
Before this new class structure can be implemented, some things need to be moved around and changed. IsMine is currently a standalone module because it is used in a couple of non-wallet tests. However it needs to be moved to be part of the wallet module first, possibly as a member function of CWallet. Once ScriptPubKeyManger is introduced, IsMine will become a member function if that.
CWallet Subclass Stack (:heavy_check_mark: Done in https://github.com/bitcoin/bitcoin/pull/16227)
The inheritance chain from SigningProvider should be condensed and the scopes slightly changed. The chain currently is:
SigningProvider -> CKeyStore -> CBasicKeyStore -> CCryptoKeyStore -> CWallet
Instead, CWallet will need to be standalone and ScriptPubKeyManager will be a SigningProvider; there is no need for CWallet to be a SigningProvider. Additionally CKeyStore, CBasicKeyStore, and CCryptoKeyStore are largely unnecessary. CKeyStore can be removed and it's functionality split into SigningProvider and CBasicKeyStore. The Add* functions can go up to CBasicKeyStore while the Have* go down to SigningProvider. The watch only related functions should go to CWallet and CBasicKeyStore can be renamed to another type of SigningProvider. Lastly, CCryptoKeyStore should be entirely combined with CWallet. At the end of this refactoring, the stack will be:
SigningProvider -> CBasicKeyStore (renamed) -> CWallet
Combined with the addition of ScriptPubKeyManager, CWallet will be standalone and ScriptPubKeyManager will extend CBasicKeyStore (renamed).
Give out Destinations from the wallet instead of keys (:heavy_check_mark: Done in https://github.com/bitcoin/bitcoin/pull/16237)
Since ScriptPubKeyManager moves the wallet to an addresses and scripts model rather than keys, we need to give out the addresses (aka Destinations) from the wallet instead of keys. So RPCs like getnewaddress will fetch a Destinaion from the wallet rather than fetching and converting a key to a Destination. This will allow us to have different ScriptPubKeyManagers later that can have arbitrary scripts. To make the transition easier, everything that was getting keys from the wallet needs to be getting Destinations and the wallet needs to give out Destinations instead of keys.
Wallet and Segwit: https://gist.github.com/sipa/125cfa1615946d0c3f3eec2ad7f250a2