Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

CLI: Support offline and nonced stake subcommands#7831

Merged
t-nelson merged 3 commits into
solana-labs:masterfrom
t-nelson:cli-offline-and-nonce-stake-cmds
Jan 17, 2020
Merged

CLI: Support offline and nonced stake subcommands#7831
t-nelson merged 3 commits into
solana-labs:masterfrom
t-nelson:cli-offline-and-nonce-stake-cmds

Conversation

@t-nelson
Copy link
Copy Markdown
Contributor

Problem

CLI's stake management subcommands do not support offline or nonced transactions

Summary of Changes

Plumb offline signing and durable nonces throughout

@t-nelson t-nelson added the v0.22 label Jan 15, 2020
@t-nelson t-nelson changed the title CLI: Suport offline and nonced stake subcommands CLI: Support offline and nonced stake subcommands Jan 15, 2020
@jackcmay
Copy link
Copy Markdown
Contributor

lgtm

Comment thread cli/tests/stake.rs
stake_account_pubkey,
new_authorized_pubkey: nonced_authority_pubkey,
stake_authorize: StakeAuthorize::Staker,
// We need to be able to specify the authority by pubkey/sig pair here
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@rob-solana I'm thinking the best option here is to build a wrapper implementing KeypairUtil that resolves the command line arg to either a Keypair, or maps from a supplied Pubkey supplied by --*-authority to a (Pubkey, Signature) tuple supplied with the --signer arg. sign_message() would then do a normal signing in the Keypair case and in the (Pubkey, Signature) case verify+return the signature. Caveat being that sign_message() is currently infallible, so would probably need to panic in case of verify failing.

Something like

enum SigningAuthority {
    Online(Keypair),
    Offline(Pubkey, Signature),
}

impl KeypairUtil for SigningAuthority {
    ...
    fn sign_message(&self, msg: &[u8]) {
        match self {
            SigningAuthority::Online(keypair) => keypair.sign_message(msg),
            SigningAuthority::Offline(pubkey, signature) => {
                if signature.verify(pubkey, msg) {
                     signature
                } else {
                     panic!("Signature verification failed!");
                }
            }
        }
    }
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

neat!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Only thing: last time I tried to make a slice of KeypairUtils from 2 instances of different types, I failed (Rust newb).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fun! I'll definitely be prototyping outside the codebase. It's food for another PR in any case

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Only thing: last time I tried to make a slice of KeypairUtils from 2 instances of different types, I failed (Rust newb).

Checkout #8318 😉

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 15, 2020

Codecov Report

Merging #7831 into master will increase coverage by <.1%.
The diff coverage is 77.8%.

@@           Coverage Diff            @@
##           master   #7831     +/-   ##
========================================
+ Coverage    81.9%   81.9%   +<.1%     
========================================
  Files         242     239      -3     
  Lines       51401   51569    +168     
========================================
+ Hits        42114   42253    +139     
- Misses       9287    9316     +29

@t-nelson
Copy link
Copy Markdown
Contributor Author

Anything holding us up here?

Copy link
Copy Markdown
Contributor

@rob-solana rob-solana left a comment

Choose a reason for hiding this comment

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

lgtm

@t-nelson t-nelson merged commit 0de35fd into solana-labs:master Jan 17, 2020
@t-nelson t-nelson deleted the cli-offline-and-nonce-stake-cmds branch January 17, 2020 17:31
mergify Bot pushed a commit that referenced this pull request Jan 17, 2020
* Support durable nonce for staker-authorize-*

* CLI: Factor out sign-only reply parsing to helper

* Support offline signing for staker-authorize-*

(cherry picked from commit 0de35fd)
t-nelson added a commit that referenced this pull request Jan 17, 2020
* Support durable nonce for staker-authorize-*

* CLI: Factor out sign-only reply parsing to helper

* Support offline signing for staker-authorize-*

(cherry picked from commit 0de35fd)
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.

3 participants