Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pcli: proposal submission should parse denoms #3455

Open
conorsch opened this issue Nov 30, 2023 · 1 comment
Open

pcli: proposal submission should parse denoms #3455

conorsch opened this issue Nov 30, 2023 · 1 comment
Labels
A-client Area: Design and implementation for client functionality

Comments

@conorsch
Copy link
Contributor

When submitting a governance proposal, there's a mandatory argument --deposit-amount, that expects an amount of upenumbra:

❯ pcli --home ~/.local/share/pcli-devnet tx proposal submit --file proposal.toml --deposit-amount 100000
Scanning blocks from last sync height 263 to latest height 263
[0s] ██████████████████████████████████████████████████       0/0       0/s ETA: 0s
building transaction...
finished proving in 0.543 seconds [4 actions, 3 proofs, 2466 bytes]
broadcasting transaction and awaiting confirmation...
Error: status: Internal, message: "could not broadcast transaction: Error submitting transaction: code 1, log: submitted proposal deposit of 100000upenumbra does not match required proposal deposit of 10000000upenumbra", details: [], metadata: MetadataMap { headers: {"content-type": "application/grpc"} }

It's fine that we require the token to be the network token. However, this is the only command throughout the CLI experience that doesn't accept Amounts such as 1penumbra. Instead, users must count a lot of zeroes in the error message and then type that many zeroes on the CLI. We can do better than that.

Let's update the arg parsing to accept 1penumbra format, erroring on any asset that isn't penumbra, and also if the number is too low.

@cratelyn cratelyn added the A-client Area: Design and implementation for client functionality label May 6, 2024
@conorsch
Copy link
Contributor Author

Mostly resolved by #4319! See a successful example submission here:

❯ pcli tx proposal submit --file proposal.toml --deposit-amount 10penumbra
Scanning blocks from last sync height 93574 to latest height 93574
[0s] ██████████████████████████████████████████████████       0/0       0/s ETA: 0s
building transaction [4 actions, 3 proofs]...
finished proving in 0.551 seconds [4 actions, 3 proofs, 2576 bytes]
broadcasting transaction and awaiting confirmation...
transaction broadcast successfully: 9efbe2bc1fe6e61d2649873148876b944c985dd43cc443499f23b3cb01845a63
transaction confirmed and detected: 9efbe2bc1fe6e61d2649873148876b944c985dd43cc443499f23b3cb01845a63 @ height 93575

However there's still a hiccup in the default arg handling—if you don't explicitly declare a deposit amount, the cli gets confused:

❯ pcli tx proposal submit --file proposal.toml
Scanning blocks from last sync height 93535 to latest height 93535
[0s] ██████████████████████████████████████████████████       0/0       0/s ETA: 0s
Error: could not parse  as a value; provide both a numeric value and denomination, e.g. 1penumbra

Should we look up the minimum from the chain params and set that as a the default? Probably, yeah.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-client Area: Design and implementation for client functionality
Projects
No open projects
Status: No status
Development

No branches or pull requests

2 participants