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: auction end-all, withdraw-all #4385

Merged
merged 4 commits into from
May 28, 2024
Merged

pcli: auction end-all, withdraw-all #4385

merged 4 commits into from
May 28, 2024

Conversation

hdevalence
Copy link
Member

Describe your changes

Changes the pcli tx auction commands to support "end all", "withdraw all" rather than requiring specific auction IDs

Checklist 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:

    Only client-side changes

@hdevalence
Copy link
Member Author

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)

@hdevalence
Copy link
Member Author

Suggestion: add a local_seq field to https://buf.build/penumbra-zone/penumbra/docs/main:penumbra.view.v1#penumbra.view.v1.AuctionsResponse that gives the local sequence number

@erwanor erwanor self-assigned this May 19, 2024
@erwanor erwanor added A-client Area: Design and implementation for client functionality A-auction Area: Relates to the auction component labels May 19, 2024
hdevalence pushed a commit that referenced this pull request May 24, 2024
#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.
@erwanor erwanor marked this pull request as ready for review May 28, 2024 11:46
Copy link
Member

@erwanor erwanor left a 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.

@erwanor erwanor added this to the Sprint 7 milestone May 28, 2024
Copy link
Contributor

@cratelyn cratelyn left a 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!

crates/view/src/planner.rs Outdated Show resolved Hide resolved
Signed-off-by: Erwan Or <erwan.ounn.84@gmail.com>
Copy link
Contributor

@cratelyn cratelyn left a comment

Choose a reason for hiding this comment

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

very nice!

@erwanor erwanor merged commit 3a8f005 into main May 28, 2024
13 checks passed
@erwanor erwanor deleted the pcli-auction-ux-tweaks branch May 28, 2024 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-auction Area: Relates to the auction component A-client Area: Design and implementation for client functionality
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants