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

Add create-stake command to solana-tokens CLI #17550

Merged
merged 2 commits into from
Jun 1, 2021

Conversation

garious
Copy link
Contributor

@garious garious commented May 27, 2021

Problem

When we created the solana-tokens CLI, we only had locked up tokens and so the distribute-stake command requires a source stake account to split up and a lockup authority signer to change the target accounts' lockup dates. Now that we have unlocked tokens, it is tedious to manually move tokens into stake accounts just so solana-tokens can split them. Instead, it can skip the split and just create the stake accounts directly.

Summary of Changes

Add create-stake command to solana-tokens CLI. It uses the --from System account to both send the recipient SOL for fees and to fund the new stake accounts. Unlike the distribute-stake stake command, it does not require a source stake account or a lockup authority signer.

tokens/src/commands.rs Outdated Show resolved Hide resolved
@garious
Copy link
Contributor Author

garious commented May 27, 2021

Hold off on reviewing. The implementation can be simpler and I'll play with integrating this functionality into one of the existing commands.

@CriesofCarrots
Copy link
Contributor

Hold off on reviewing. The implementation can be simpler and I'll play with integrating this functionality into one of the existing commands.

ack

@codecov
Copy link

codecov bot commented May 27, 2021

Codecov Report

Merging #17550 (039e93f) into master (a0d721c) will decrease coverage by 0.1%.
The diff coverage is 74.6%.

❗ Current head 039e93f differs from pull request most recent head 6352744. Consider uploading reports for the commit 6352744 to get more accurate results

@@            Coverage Diff            @@
##           master   #17550     +/-   ##
=========================================
- Coverage    82.7%    82.6%   -0.2%     
=========================================
  Files         428      427      -1     
  Lines      119963   119345    -618     
=========================================
- Hits        99324    98638    -686     
- Misses      20639    20707     +68     

@garious
Copy link
Contributor Author

garious commented May 27, 2021

@CriesofCarrots @danpaul000, ready for review. Something to consider: we could merge the distribute-tokens and create-stake commands by making the rule that if a lockup date is in the input CSV file, then it should create a stake account. That'd also mean that the output CSV file would contain a blank column of new_stake_account_address when that feature is not used, which might seem weird. For now, I'm thinking we should just let separate things be separate.

@garious
Copy link
Contributor Author

garious commented Jun 1, 2021

@CriesofCarrots @danpaul000, wdyt?

Copy link
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

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

Generally looks good, but one missing cli Arg.

For now, I'm thinking we should just let separate things be separate.

Agreed, I like the commands being explicit, although I do think distribute-stake is a little ambiguous now. Fine for now, though.

tokens/src/arg_parser.rs Show resolved Hide resolved
@garious garious added the automerge Merge this Pull Request automatically once CI passes label Jun 1, 2021
@mergify mergify bot removed the automerge Merge this Pull Request automatically once CI passes label Jun 1, 2021
@mergify
Copy link
Contributor

mergify bot commented Jun 1, 2021

automerge label removed due to a CI failure

@CriesofCarrots CriesofCarrots added the automerge Merge this Pull Request automatically once CI passes label Jun 1, 2021
@mergify mergify bot merged commit 1b7f877 into solana-labs:master Jun 1, 2021
mergify bot pushed a commit that referenced this pull request Jun 1, 2021
* Add create-stake command to solana-tokens CLI

* Add --unlocked-sol arg

Thanks @CriesofCarrots!

(cherry picked from commit 1b7f877)
mergify bot added a commit that referenced this pull request Jun 2, 2021
* Add create-stake command to solana-tokens CLI

* Add --unlocked-sol arg

Thanks @CriesofCarrots!

(cherry picked from commit 1b7f877)

Co-authored-by: Greg Fitzgerald <greg@solana.com>
@brooksprumo brooksprumo mentioned this pull request Aug 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants