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.

Sign up for GitHub

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

Jump to bottom

auction: TxV/TxP support to populate the views #4219

Open
Tracked by #4196
erwanor opened this issue Apr 16, 2024 · 3 comments
Open
Tracked by #4196

auction: TxV/TxP support to populate the views #4219

erwanor opened this issue Apr 16, 2024 · 3 comments
Labels
A-auction Area: Relates to the auction component _P-medium Medium priority protobuf-changes Makes changes to the protobuf definitions.

Comments

@erwanor
Copy link
Member

erwanor commented Apr 16, 2024 •
edited
Loading

As of v0.75.0, there's not enough data in the current TransactionPerspectives to generate ActionDutchAuctionWithdrawViews that are useful to clients. The view exists but its content is hardcoded to have zero reserves and no positions. Instead, we should record extended metadata during scanning so that it's possible to cross-reference openings to note commitments with auction reserves. This also offers an opportunity to lay the groundwork so that we can add position views.
@erwanor erwanor mentioned this issue Apr 16, 2024
Open
50 tasks
@github-project-automation github-project-automation bot moved this to Backlog in Penumbra Apr 16, 2024
@github-actions github-actions bot added the needs-refinement unclear, incomplete, or stub issue that needs work label Apr 16, 2024
@zbuc zbuc mentioned this issue Apr 16, 2024
Closed
1 task
@erwanor erwanor mentioned this issue Apr 18, 2024
Merged
1 task
@erwanor erwanor assigned erwanor and unassigned zbuc Apr 18, 2024
@erwanor erwanor added _P-V1 Priority: slated for V1 release _P-high High priority A-auction Area: Relates to the auction component and removed needs-refinement unclear, incomplete, or stub issue that needs work labels Apr 18, 2024
@erwanor erwanor added this to the Sprint 4 milestone Apr 18, 2024
@erwanor erwanor moved this from Backlog to In progress in Penumbra Apr 18, 2024
@erwanor erwanor added the protobuf-changes Makes changes to the protobuf definitions. label Apr 18, 2024
erwanor added a commit that referenced this issue Apr 19, 2024
@erwanor
transaction: finalize auction integration (#4237)
971006e 
## Describe your changes

This PR:
- adds an action plan for withdrawing DA auctions
- plugs in `GasCost` and `IsAction` impls
- add basic TxV/TxP support/definitions

## Issue ticket number and link
Part of #4212 + #4219

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

  > Consensus breaking
@erwanor
Copy link
Member Author

erwanor commented Apr 20, 2024

All the scaffolding exists to make this happen, we just need to connect do the plumbing so that the views get populated. This is somewhat lower priority, but if anyone wants to pick it up feel free to.

@cratelyn cratelyn modified the milestones: Sprint 4, Sprint 5 Apr 22, 2024
This was referenced May 1, 2024
Merged
Merged
erwanor added a commit that referenced this issue May 1, 2024
@erwanor
auction: add views to the ActionView definition (#4295)
a500f2c 
## Describe your changes

This PR adds `ActionDutchAuctionScheduleView` and
`ActionDutchAuctionWithdrawView` to the `ActionView` message, it also
replaces the `IsAction` perspective impls with concrete implementation.

Populating a view for the withdraw action without extending the `TxP`
structure has proven squirrelly, and it will require a follow-up,
although the data is "here" in the view server so I don't think it will
be too complicated.

## Issue ticket number and link

Part of #4219 

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

> Breaking changes are made to `ActionView`: the
`action_dutch_auction_schedule` and `action_dutch_auction_withdraw`
fields have had their types changes from `ActionDutchAuctionSchedule`
and `ActionDutchAuctionWithdraw` to `ActionDutchAuctionScheduleView` and
`ActionDutchAuctionWithdrawView`, respectively.
@erwanor
Copy link
Member Author

erwanor commented May 1, 2024

We stubbed the withdraw view, this needs some addition to transaction perspectives, so leaving this open, until I replace it with a more detailed plan of action.

@aubrika aubrika added the friction something made this fall into the following milestone & the reason should be noted in a comment label May 6, 2024
@aubrika aubrika modified the milestones: Sprint 5, Sprint 6 May 6, 2024
@cratelyn cratelyn removed the friction something made this fall into the following milestone & the reason should be noted in a comment label May 6, 2024
@erwanor erwanor moved this from In progress to Todo in Penumbra May 8, 2024
@jessepinho jessepinho mentioned this issue May 9, 2024
Merged
@cratelyn cratelyn modified the milestones: Sprint 6, Sprint 7 May 21, 2024
@aubrika aubrika modified the milestones: Sprint 7, Sprint 8 Jun 3, 2024
@cronokirby cronokirby moved this from Todo to In progress in Penumbra Jun 11, 2024
@cronokirby cronokirby assigned cronokirby and unassigned cronokirby Jun 11, 2024
@cronokirby cronokirby moved this from In progress to Backlog in Penumbra Jun 13, 2024
@cronokirby cronokirby added _P-medium Medium priority and removed _P-V1 Priority: slated for V1 release _P-high High priority labels Jun 13, 2024
@cronokirby
Copy link
Contributor

Shelved for now, this turns out to be a bit more subtle than we realized, and also requires a bit of a different processing scheme than other views.

Basically, unlike other views, we need access to the historical value of the auction reserves at the time the withdrawal happens, which is unlike how the rest of the view services are designed.

We could add this in the future with the addition of an event for withdrawing actions, and then indexing those events to populate the view of withdrawal actions appropriately.

@cronokirby cronokirby mentioned this issue Jun 13, 2024
Merged
1 task
cronokirby added a commit that referenced this issue Jun 13, 2024
@cronokirby
Add an event for auction withdrawals (#4612)
5793168 
This will make it easy to fill in withdrawal views later, since we'll
have an event with the exact state of the auction at that time.

## Describe your changes

This adds a new proto event for when an auction is withdrawn, including
the state of the auction at that time.

## Issue ticket number and link

This can be seen as laying the groundwork for #4219 once we're able to
link events in the view services we implement.

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

  > This just adds a new event, without changing existing protos.
@aubrika aubrika removed this from the Sprint 8 milestone Jun 14, 2024
@TalDerei TalDerei added the friction something made this fall into the following milestone & the reason should be noted in a comment label Jun 15, 2024
@erwanor erwanor removed the friction something made this fall into the following milestone & the reason should be noted in a comment label Jun 17, 2024
@TalDerei TalDerei mentioned this issue Jun 18, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-auction Area: Relates to the auction component _P-medium Medium priority protobuf-changes Makes changes to the protobuf definitions.
Projects
Penumbra
Status: Backlog
Milestone
No milestone
Development

No branches or pull requests
6 participants
@zbuc @erwanor @cronokirby @cratelyn @TalDerei @aubrika