Refactor the lnwallet package to directly use the WalletController interface #17
Conversation
This file contains 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
Roasbeef
force-pushed
the
wallet-interface-refactor
branch
from
efb5813 to
ddc5925
Compare
|
The final diff count on this PR is a bit off due to this commit e07d1fa. I renamed |
Roasbeef
force-pushed
the
wallet-interface-refactor
branch
4 times, most recently
from
c4417bc to
5d90321
Compare
|
I've modified the stream of commits. The final diff now accurately reflects the changes to the master branch by this PR. |
Roasbeef
force-pushed
the
wallet-interface-refactor
branch
from
5d90321 to
127b796
Compare
|
LGTM!
|
Roasbeef
force-pushed
the
wallet-interface-refactor
branch
from
3b545bf to
bbbd6e4
Compare
This commit removes the EncryptorDecryptor interface, and all related usage within channeldb. This interface is no longer needed as wallet specific secrets such as private keys are no longer stored within the database.
This commit removes the storage+encryption of private keys within channeldb. We no longer need to encrypt these secrets before storing as the base wallet is now expected to retain full control of these secrets rather than the database. As a result, we now only store public keys within the database.
…f's btcwallet fork
This commit removes the wrapper functions used to rely on the coinset package for coin selection. In a future commit the prior behavior will be replaced by a custom coin selection implementation which estimates the size of the funding transaction.
This commit revamps the previous WalletController interface, edging it closer to a more complete version. Additionally, this commit also introduces two new interfaces: BlockchainIO, and Singer along with a new factor driver struct, the WalletDriver. This BlockChainIO abstracts read-only access to the blockchain, while the Singer interface abstracts the signing of inputs from the base wallet paving the way to hardware wallets, air-gapped signing, etc. Finally, in order to provide an easy method for selecting a particular concrete implementation of a WalletController interface, the concept of registering “WalletDriver”s has been introduced. A wallet driver is essentially the encapsulation of a factory function capable of create a new instance of a Wallet Controller.
This commit adds the first concrete implementation of the WalletController interface: BtcWallet. This implementation is simply a series of wrapper functions are the base btcwallet struct. Additionally, for ease of use both the BlockChain IO and Signer interface are also implemented by BtcWallet. Finally a new WalletDriver implementation has been implemented, and will be register by the init() method within this new package.
This file is no longer needed as each implementation of the WalletController is expected to handle its own set up via an instance of the WalletDriver factory struct.
This commit modifies the elkrem root derivation for each newly created channel. First a master elkrem root is derived from the rood HD seed generated from private wallet data. Next, a HKDF is used with the secret being the master elkrem root.
…ntroller. This commit performs a major refactor of the current wallet, reservation, and channel code in order to call into a WalletController implementation rather than directly into btcwallet. The current set of wallets tests have been modified in order to test against *all* registered WalletController implementations rather than only btcwallet. As a result, all future WalletControllers primary need to ensure that their implementation passes the current set of tests (which will be expanded into the future), providing an easy path of integration assurance. Rather than directly holding the private keys throughout funding and channel creation, the burden of securing keys has been shifted to the specified WalletController and Signer interfaces. All signing is done via the Signer interface rather than directly, increasing flexibility dramatically. During channel funding, rather than creating a txscript.Engine to verify commitment signatures, regular ECDSA sig verification is now used instead. This is faster and more efficient. Finally certain fields/methods within ChannelReservation and LightningChannel have been exposed publicly in order to restrict the amount of modifications the prior tests needed to undergo in order to support testing directly agains the WalletController interface.
Roasbeef
force-pushed
the
wallet-interface-refactor
branch
from
bbbd6e4 to
bccb1b9
Compare
joostjager
pushed a commit
to joostjager/lnd
that referenced
this pull request
Nov 14, 2019
…rialization Obfuscator Serialization
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR implements a major refactor of the
lnwalletpackage in order to provide a more streamlined path of the integration of new internal/external wallets intolnd. Previously theLightningWalletstruct directly interacted with an instance ofbtcwallet.Walletto handle duties such as signing, coin selection, address derivation, funding channels, etc. With this PR, rather than directly referencing a concrete wallet implementation, theWalletControllerinterface is now used throughout the codebase. This will allow future wallets to drop in easily into the daemon, de-coupling the daemon from the underlying wallet implementation.Along the way, two new interfaces have been introduced:
BlockChainIO, andSigner.The
BlockChainIOinterface currently serves as read-only access to the most current blockchain state. In a future PR, this new interface may be merged with theChainNotifierinterface as the abstractions are similar, and may be implemented by the same object behind the scenes.The
Signerinterface abstracts away all signing related functionality within the wallet. Rather than holding private keys directly in memory, we now defer all signing to theSignerinterface. The interface is capable of generating both raw signatures, and full input scripts (including nested p2sh spends). This interface further decouples signing within the daemon and paves the way for more secure methods such as hardware wallet integration, HSM's, hardware tokens, etc. Currently theBtcWalletimplementation of theWalletControllerinterface is used by default throughout the codebase, however new signers can easily be dropped.The following is an overview of the changes within this PR:
channeldb. This practice has been obviated by the usage of theSignerinterface throughout the daemon. As a result, we no longer need theEncryptorDecryptorinterface, so it's been scrapped.glidebuild files have been updated to include a modification required to my fork of `btcwallet.btcutil.coinsetpackage, we now perform manual coin selection. This change allows our soin selection to more accurately estimate the size of the resulting transaction, thereby accounting for fees defined to a certain fee rate (in sat/byte).lnwallet/wallet_test.gofile has been renamed tolnwallet/interface_test.go. This new file tests against all registeredWalletControllerinterfaces rather than directly against a concrete implementation.WalletDriverhas been introduced, whcih is essentially a "factory" struct capable of creating a concreteWalletControllerinterface from a variable set of parameters.init()method of all create concrete implementations.WalletController,BtcWallethas been created. To go along with it a driver has been created and will be registered upon import of the new sub-package .LightningWalletreservation workflow has been *significantly refactored to directly use theWalletControllerrather thanbtcwallet. This has had the major benefiting of allowing the removal of several hundred lines of code, simplifying the reservation workflow code.