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

Conversation

@joncinque
Copy link
Contributor

Problem

#6550 fixed the token-cli for offline signing, but still defaults to using simulated compute units, while #6499 in the stake-pool-cli forces people to specify that they want to use SIMULATED compute units, and otherwise adds nothing. This is inconsistent.

Solution

Make the stake pool CLI consistent with the token CLI by defaulting to simulated compute units. On the other hand, this PR allows people to specify that they want DEFAULT compute units.

Let me know what you think!

@joncinque joncinque requested a review from 2501babe April 25, 2024 23:07
@2501babe
Copy link
Contributor

i like the latest ux the most! unfortunately i ran into two issues:

  • now, proving no compute flags at all introduces a simulated compute budget; i believe this should be default ie no instruction
  • there is no way to use a simulated budget with a price because the budget flag is required and SIMULATE is no longer a valid option

this is what i believe the grammar should look like:

  • no price flag, no budget flag -> no instructions
  • no price flag, budget flag with default -> no instructions
  • no price flag, budget flag with number -> fixed budged instruction
  • price flag, no budget flag -> price instruction, simulated budget instruction
  • price flag, budget flag with default -> price instruction, no budget instruction
  • price flag, budget flag with number -> price instruction, fixed budget instruction

budget options remain number or DEFAULT. remove the requirement for a budget flag if price flag is given

ive pushed a commit that implements this logic, lmk what you think!

@joncinque
Copy link
Contributor Author

That makes sense to me, thanks for doing the work! I'll do the same thing with the monrepo CLI

@joncinque joncinque merged commit 4297ae6 into solana-labs:master Apr 30, 2024
@joncinque joncinque deleted the spclidef branch April 30, 2024 10:33
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.

2 participants