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

view: add support for auctions #4252

Merged
merged 20 commits into from
Apr 27, 2024
Merged

view: add support for auctions #4252

merged 20 commits into from
Apr 27, 2024

Conversation

TalDerei
Copy link
Collaborator

Describe your changes

View server implementation for auction.

Issue ticket number and link

References #4251

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:

@TalDerei TalDerei self-assigned this Apr 22, 2024
@cratelyn cratelyn added the A-auction Area: Relates to the auction component label Apr 22, 2024
@cratelyn cratelyn added this to the Sprint 5 milestone Apr 22, 2024
@erwanor erwanor self-requested a review April 22, 2024 21:52
conorsch added a commit that referenced this pull request Apr 23, 2024
Prepared during pairing session with @TalDerei, regarding #4252.
erwanor added a commit that referenced this pull request Apr 23, 2024
## Describe your changes

This PR adds:
- auction genesis content to the application state
- auction parameters to the application state (and governance update
logic)
- connects the app ABCI implementation to the auction's component
interface

## Issue ticket number and link

#4228 and #4252 

## 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 is consensus breaking
@erwanor erwanor force-pushed the auction-view-server branch from 2418bba to 648e590 Compare April 23, 2024 16:41
@erwanor erwanor changed the title view server auction stubs view: add support for auctions Apr 23, 2024
@erwanor erwanor force-pushed the auction-view-server branch from 79c817c to 9ed7319 Compare April 24, 2024 17:48
erwanor added a commit that referenced this pull request Apr 24, 2024
## Describe your changes

This PR adds:
- auction genesis content to the application state
- auction parameters to the application state (and governance update
logic)
- connects the app ABCI implementation to the auction's component
interface

## Issue ticket number and link

#4228 and #4252 

## 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 is consensus breaking
@@ -303,21 +303,72 @@ impl Worker {
// Update the position record
self.storage.update_position(position_id, state).await?;
}
penumbra_transaction::Action::ActionDutchAuctionSchedule(
Copy link
Collaborator Author

@TalDerei TalDerei Apr 25, 2024

Choose a reason for hiding this comment

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

what's the utility of these action checks?

Copy link
Member

Choose a reason for hiding this comment

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

On advice, we scan transactions and their actions, and record auction metadata by their asset id. This is useful to be able to display relevant asset metadata when presented with an asset id (among other things).

Copy link
Member

Choose a reason for hiding this comment

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

For a little bit more context: Values in the shielded pool are an Amount and an AssetId. The asset ID (an $\mathbb F_q$ value) is the currently always a hash of a denom string, and client tooling looks up asset.Metadata via the asset ID in an asset registry.

The chain maintains one minimal asset registry (just containing denoms), but this registry does not include entries for all of the LPNFTs (and now ANFTs), because this would bloat the chain state. Instead, the client's view service is responsible for locally deriving and caching the metadata of the bearer tokens it creates. This saves data for all the other clients, who don't want to have to maintain knowledge of everyone else's position NFTs.

@TalDerei
Copy link
Collaborator Author

I reviewed this earlier this morning on an outdated change-set, but haven't dived deeply into the storage.rs module.

@erwanor erwanor marked this pull request as ready for review April 25, 2024 22:03
@erwanor
Copy link
Member

erwanor commented Apr 26, 2024

I think that this is ready for review, the discoverability / rendering of the commands needs to improve, but seem good enough for an mvp.

A few lines about the state modeling in the view server, this PR:

  1. Adds an auctions table in the rust view server:
-- This table records the user's own auction state, using the
-- auction id as a primary key. An extra-column is available
-- to cross-reference note commitments that is associated with
-- the entry.
CREATE TABLE auctions (
     auction_id             BLOB PRIMARY KEY NOT NULL,
     auction_state          BIGINT NOT NULL,
     note_commitment        BLOB
);
  1. The sync worker will record auction metadata as part of its block scanning routine (itself contingent on detecting activity)

  2. As part of recording SNR asset data, it will update the auctions view state with an associated note commitment

  3. This note commitment is used to cross reference spendable notes and cleartext notes from the spendable_notes and notes tables

@erwanor
Copy link
Member

erwanor commented Apr 26, 2024

I have tested that I am able to:

  • query my auction state
  • go through the lifecycle of a DA: scheduling, terminating, and withdrawing

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.

Optimistically merging this, async reviews welcome

@erwanor erwanor force-pushed the auction-view-server branch from 463ea98 to 541ac45 Compare April 27, 2024 00:24
@erwanor erwanor merged commit 6d44baa into main Apr 27, 2024
8 checks passed
@erwanor erwanor deleted the auction-view-server branch April 27, 2024 00:53
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
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants