Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Comments

Introduce sign_with method in keystore#4925

Merged
bkchr merged 73 commits intoparitytech:masterfrom
rakanalh:sign_with
Mar 30, 2020
Merged

Introduce sign_with method in keystore#4925
bkchr merged 73 commits intoparitytech:masterfrom
rakanalh:sign_with

Conversation

@rakanalh
Copy link
Contributor

This PR is part of the effort on implementing Remote signing.

The change introduces sign_with method where the caller passes the public key and the key type ID. The pair matching against those parameters is done within the store rather than outside of it so that the call site isn't responsible about signing implementation details.

@rakanalh rakanalh added the A0-please_review Pull request needs code review. label Feb 14, 2020
@rakanalh rakanalh requested a review from gnunicorn February 14, 2020 10:45
@rakanalh rakanalh requested a review from pepyakin as a code owner February 14, 2020 10:45
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is a good start. However I don't understand the reason for SupportedPublicKey. We decided against using an enum for the crypto types and added explicit functions. I would like to see that we continue with that.

sign_with should return a Future.

This change is being introduced for the purpose of identifying a public
key with it's identifier and algorithm "kind".
@gnunicorn gnunicorn added A3-in_progress Pull request is in progress. No review needed at this stage. and removed A0-please_review Pull request needs code review. labels Feb 18, 2020
@rakanalh rakanalh requested a review from mxinden as a code owner February 19, 2020 16:09
Copy link
Contributor

@gnunicorn gnunicorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting there!

@bkchr bkchr added the A1-onice label Mar 23, 2020
@bkchr
Copy link
Member

bkchr commented Mar 23, 2020

Before merging, I would like to test this on one of our Kusama validators.

@rakanalh
Copy link
Contributor Author

Opps, screwed something up... sorry about that and i'll fix it

@ddorgan
Copy link
Member

ddorgan commented Mar 25, 2020

Deployed to one validator.

@bkchr bkchr merged commit 7e7d3e0 into paritytech:master Mar 30, 2020
@rakanalh rakanalh deleted the sign_with branch March 30, 2020 11:54
This was referenced May 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants