Replace keys API with Signer API to support hardware wallets eventually #404
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
TheBlueMatt
changed the title
TheBlueMatt
force-pushed
the
2019-11-signer-api
branch
from
71a0d39 to
4b35f86
Compare
TheBlueMatt
force-pushed
the
2019-11-signer-api
branch
from
4b35f86 to
e15ab02
Compare
|
Process note: I'd like to land #347 before this and cut a release so that we don't have a partial API in a release that is needed to get important protocol features that other clients are requiring now. |
TheBlueMatt
force-pushed
the
2019-11-signer-api
branch
from
e15ab02 to
d010039
Compare
|
Amended to use the new remote transaction signing API in the initial funding_signed signature generation. |
TheBlueMatt
force-pushed
the
2019-11-signer-api
branch
4 times, most recently
from
3bbaec3 to
42e1d49
Compare
ariard
reviewed
Dec 10, 2019
There was a problem hiding this comment.
I don't see this breaking anything for the new ChannelKeys::sign_remote_commitment method. The rest of the change are just templating.
I've taken my nits + better documentation of current assumptions of KeysInterface in commit ariard@95f666d
I think that's a good head start in making hardware-wallet support a first-class concern of RL.
Instead of having in-memory access to the list of private keys associated with a channel, we should have a generic API which allows us to request signing, allowing the user to store private keys any way they like. The first step is the (rather mechanical) process of templating the entire tree of ChannelManager -> Channel impls by the key-providing type. In a later commit we should expose only public keys where possible.
This adds a new fn to ChannelKeys which is called when we generte a new remote commitment transaction for signing. While it may be theoretically possible to unwind state updates by disconnecting and reconnecting as well as making appropriate state machine changes, the effort required to get it correct likely outweighs the UX cost of "preflighting" the requests to hardwre wallets.
Improve some comments of interface methods.
TheBlueMatt
force-pushed
the
2019-11-signer-api
branch
from
42e1d49 to
35814b6
Compare
|
Took your commit. Will merge after the 0.0.10 bump is done. |
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.
Built on #403 cause I'm lazy (as always), so just the top two commits.
Its obviously insufficient, but its a decent size diff so figured should get the ball rolling by just getting it upstream.