-
Notifications
You must be signed in to change notification settings - Fork 305
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: auction end-all, withdraw-all #4385
Conversation
This doesn't work because there's not a way to track the "local" state of the auction NFT, which may lag the on-chain state. So once a GDA is completed, close-all doesn't work (because all the auctions are believed to be closed), but withdraw-all doesn't work (because the auction NFTs are not closed) |
Suggestion: add a |
#4472) ## Describe your changes This PR makes two changes: 1. Add an `auction_ids_filter` to `AuctionsRequest`, so that clients can make requests (e.g., with `queryLatestState`) for just specific auctions rather than all auctions at once. 2. Add a `local_seq` property to `AuctionsResponse` to indicate the local view service's own knowledge of the sequence number for an auction. (See #4385 (comment).) Relevant changes are to the protobuf [here](https://github.com/penumbra-zone/penumbra/pull/4472/files#diff-03b7341d5bf81ab9c8d8542d220d5ba4ae122bd3837531309742b685d7ec1619), plus a couple TODO items [here](https://github.com/penumbra-zone/penumbra/pull/4472/files#diff-9f4ca4cefeac93c9275a2f9d962d6670f6ab1c1381fd3d7c806f9316e3779ccbR986) and [here](https://github.com/penumbra-zone/penumbra/pull/4472/files#diff-36188e6ab5083e8be9d29039370cc523cfcfede683b223327b2d90e3f75d40c7R461). ## Issue ticket number and link penumbra-zone/web#1059 and #4385 (comment) ## Checklist before requesting a review - [x] If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason: > Only touches Auctions RPC methods.
b0a9c4d
to
77e0fb0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tested it with my local wallet, but it would be cool to have someone else take it for a spin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i have one question about a hard-coded sequence number, but this looks good!
Signed-off-by: Erwan Or <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very nice!
Describe your changes
Changes the
pcli tx auction
commands to support "end all", "withdraw all" rather than requiring specific auction IDsChecklist before requesting a review
If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason: