这是indexloc提供的服务,不要输入任何密码
Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Conversation

@t-nelson
Copy link
Contributor

@t-nelson t-nelson commented Feb 4, 2020

Problem

The pay subcommand is overloaded with BudgetProgram features, which are disabled for SLP1/2

Summary of Changes

Factor out the system transfer portion of pay to a new transfer subcommand (with offline/nonce support)

Requires #8009 and #8107

@t-nelson t-nelson added the v0.23 label Feb 4, 2020
@t-nelson t-nelson changed the title Cli transfer command CLI: Implement transfer command Feb 4, 2020
@t-nelson t-nelson force-pushed the cli-transfer-command branch from 3ce07c2 to fe8f65a Compare February 5, 2020 18:14
@t-nelson t-nelson force-pushed the cli-transfer-command branch from fe8f65a to 13f5935 Compare February 7, 2020 16:16
@t-nelson t-nelson marked this pull request as ready for review February 7, 2020 16:20
mvines
mvines previously approved these changes Feb 7, 2020
Copy link
Contributor

@mvines mvines left a comment

Choose a reason for hiding this comment

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

lgtm

Comment on lines +404 to +409
sign_only: bool,
signers: Option<Vec<(Pubkey, Signature)>>,
blockhash_query: BlockhashQuery,
nonce_account: Option<Pubkey>,
nonce_authority: Option<SigningAuthority>,
fee_payer: Option<SigningAuthority>,
Copy link
Contributor

Choose a reason for hiding this comment

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

All this boilerplate across various cli commands is getting a little much (seems ripe for future cleanup, not here!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

💯
A refactor has been rattling around in my brain throughout this whole offline fixup session. I've been jotting down notes as I transition back to snapshot verification and will surface the gems as issues

@mergify mergify bot dismissed mvines’s stale review February 7, 2020 17:25

Pull request has been modified.

@codecov
Copy link

codecov bot commented Feb 7, 2020

Codecov Report

Merging #8108 into master will decrease coverage by <.1%.
The diff coverage is 68.4%.

@@           Coverage Diff            @@
##           master   #8108     +/-   ##
========================================
- Coverage    81.9%   81.8%   -0.1%     
========================================
  Files         245     248      +3     
  Lines       53691   53936    +245     
========================================
+ Hits        43977   44152    +175     
- Misses       9714    9784     +70

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.

lgtm too

@t-nelson t-nelson merged commit 87c6508 into solana-labs:master Feb 7, 2020
@t-nelson t-nelson deleted the cli-transfer-command branch February 7, 2020 19:16
mergify bot pushed a commit that referenced this pull request Feb 7, 2020
* CLI: Add transfer subcommand

* Add tests

* checks

(cherry picked from commit 87c6508)
solana-grimes pushed a commit that referenced this pull request Feb 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.

3 participants