From 5b0f79de6cfee3c27e3e5bc450363fdb160d3134 Mon Sep 17 00:00:00 2001 From: Henry de Valence Date: Mon, 27 May 2024 22:59:42 -0400 Subject: [PATCH 1/4] pcli: auction end-all, withdraw-all --- .../bin/pcli/src/command/tx/auction/dutch.rs | 187 ++++++++++++------ crates/view/src/planner.rs | 30 ++- 2 files changed, 155 insertions(+), 62 deletions(-) diff --git a/crates/bin/pcli/src/command/tx/auction/dutch.rs b/crates/bin/pcli/src/command/tx/auction/dutch.rs index 1dc57cb14f..e9cb7f43bd 100644 --- a/crates/bin/pcli/src/command/tx/auction/dutch.rs +++ b/crates/bin/pcli/src/command/tx/auction/dutch.rs @@ -2,18 +2,16 @@ use std::path::Path; use crate::command::tx::FeeTier; use crate::App; +use anyhow::Result; use anyhow::{anyhow, bail, Context}; use clap::Subcommand; use comfy_table::presets; use dialoguer::Confirm; use penumbra_asset::{asset::Cache, Value}; -use penumbra_auction::auction::dutch::actions::ActionDutchAuctionWithdrawPlan; use penumbra_auction::auction::{dutch::DutchAuction, dutch::DutchAuctionDescription, AuctionId}; -use penumbra_dex::lp::position::Position; use penumbra_keys::keys::AddressIndex; use penumbra_num::Amount; -use penumbra_proto::{view::v1::GasPricesRequest, DomainType, Name}; -use penumbra_view::SpendableNoteRecord; +use penumbra_proto::{view::v1::GasPricesRequest, DomainType}; use penumbra_view::ViewClient; use penumbra_wallet::plan::Planner; use rand::RngCore; @@ -103,9 +101,12 @@ pub enum DutchCmd { /// Source account terminating the auction. #[clap(long, display_order = 100, default_value = "0")] source: u32, - /// Identifier of the auction. - #[clap(long, display_order = 200)] - auction_id: String, + /// If set, ends all auctions owned by the specified account. + #[clap(long, display_order = 150)] + all: bool, + /// Identifier of the auction to end, if `--all` is not set. + #[clap(display_order = 200)] + auction_id: Option, /// The selected fee tier to multiply the fee amount by. #[clap(short, long, value_enum, default_value_t, display_order = 300)] fee_tier: FeeTier, @@ -114,11 +115,14 @@ pub enum DutchCmd { #[clap(display_order = 200, name = "withdraw")] DutchAuctionWithdraw { /// Source account withdrawing from the auction. - #[clap(long, display_order = 100)] + #[clap(long, display_order = 100, default_value = "0")] source: u32, - /// The auction to withdraw funds from. - #[clap(long, display_order = 200)] - auction_id: String, + /// If set, withdraws all auctions owned by the specified account. + #[clap(long, display_order = 150)] + all: bool, + /// Identifier of the auction to withdraw from, if `--all` is not set. + #[clap(display_order = 200)] + auction_id: Option, /// The selected fee tier to multiply the fee amount by. #[clap(short, long, value_enum, default_value_t, display_order = 600)] fee_tier: FeeTier, @@ -178,21 +182,38 @@ impl DutchCmd { AddressIndex::new(*source), ) .await - .context("can't build send transaction")?; + .context("can't build auction schedule transaction")?; app.build_and_submit_transaction(plan).await?; Ok(()) } DutchCmd::DutchAuctionEnd { + all, auction_id, source, fee_tier, } => { - let auction_id = auction_id.parse::()?; + let auction_ids = match (all, auction_id) { + (true, _) => auctions_to_end(app.view(), *source).await?, + (false, Some(auction_id)) => { + let auction_id = auction_id.parse::()?; + vec![auction_id] + } + (false, None) => { + bail!("auction_id is required when --all is not set") + } + }; - let plan = Planner::new(OsRng) + let mut planner = Planner::new(OsRng); + + planner .set_gas_prices(gas_prices) - .set_fee_tier((*fee_tier).into()) - .dutch_auction_end(auction_id) + .set_fee_tier((*fee_tier).into()); + + for auction_id in auction_ids { + planner.dutch_auction_end(auction_id); + } + + let plan = planner .plan( app.view .as_mut() @@ -200,62 +221,46 @@ impl DutchCmd { AddressIndex::new(*source), ) .await - .context("can't build send transaction")?; + .context("can't build auction end transaction")?; app.build_and_submit_transaction(plan).await?; Ok(()) } DutchCmd::DutchAuctionWithdraw { + all, source, auction_id, fee_tier, } => { - let auction_id = auction_id.parse::()?; - - use pbjson_types::Any; - let view_client = app.view(); - let (auction_id, _, auction_raw, _): ( - AuctionId, - SpendableNoteRecord, - Option, - Vec, - ) = view_client - .auctions(None, true, true) - .await? - .into_iter() - .find(|(id, _, _, _)| &auction_id == id) - .ok_or_else(|| anyhow!("the auction id is unknown from the view service!"))?; - - let Some(raw_da_state) = auction_raw else { - bail!("auction state is missing from view server response") + let auctions = match (all, auction_id) { + (true, _) => auctions_to_withdraw(app.view(), *source).await?, + (false, Some(auction_id)) => { + let auction_id = auction_id.parse::()?; + + // TODO: better way to do this? + let all = auctions_to_withdraw(app.view(), *source).await?; + vec![all + .into_iter() + .find(|a| a.description.id() == auction_id) + .ok_or_else(|| { + anyhow!("the auction id is unknown from the view service!") + })?] + } + (false, None) => { + bail!("auction_id is required when --all is not set") + } }; - use penumbra_proto::core::component::auction::v1 as pb_auction; - // We're processing a Dutch auction: - assert_eq!(raw_da_state.type_url, pb_auction::DutchAuction::type_url()); - - let dutch_auction = DutchAuction::decode(raw_da_state.value)?; + let mut planner = Planner::new(OsRng); - let reserves_input = Value { - amount: dutch_auction.state.input_reserves, - asset_id: dutch_auction.description.input.asset_id, - }; - let reserves_output = Value { - amount: dutch_auction.state.output_reserves, - asset_id: dutch_auction.description.output_id, - }; - let seq = dutch_auction.state.sequence + 1; + planner + .set_gas_prices(gas_prices) + .set_fee_tier((*fee_tier).into()); - let mut planner = Planner::new(OsRng); + for auction in &auctions { + planner.dutch_auction_withdraw(auction); + } let plan = planner - .set_gas_prices(gas_prices) - .set_fee_tier((*fee_tier).into()) - .dutch_auction_withdraw(ActionDutchAuctionWithdrawPlan { - auction_id, - seq, - reserves_input, - reserves_output, - }) .plan( app.view .as_mut() @@ -263,7 +268,7 @@ impl DutchCmd { AddressIndex::new(*source), ) .await - .context("can't build send transaction")?; + .context("can't build auction withdrawal transaction")?; app.build_and_submit_transaction(plan).await?; Ok(()) } @@ -366,6 +371,70 @@ impl DutchCmd { } } +async fn dutch_auction_states( + view_client: &mut impl ViewClient, + source: impl Into, +) -> Result> { + let auctions = view_client + .auctions(Some(source.into()), false, true) + .await? + .into_iter() + .filter_map(|(id, _, state, _)| { + if let Some(state) = state { + if let Ok(da) = DutchAuction::decode(state.value) { + Some((id, da)) + } else { + None + } + } else { + None + } + }) + .collect(); + Ok(auctions) +} + +async fn auctions_to_end(view_client: &mut impl ViewClient, source: u32) -> Result> { + let auctions = dutch_auction_states(view_client, source).await?; + + let auction_ids = auctions + .into_iter() + .filter_map(|(id, auction)| { + // Ending an auction changes sequence from 0 => 1 + // so if the sequence is 0, it's open and we should end it. + if auction.state.sequence == 0 { + Some(id) + } else { + None + } + }) + .collect(); + + Ok(auction_ids) +} + +async fn auctions_to_withdraw( + view_client: &mut impl ViewClient, + source: u32, +) -> Result> { + let auctions = dutch_auction_states(view_client, source).await?; + + let auction_ids = auctions + .into_iter() + .filter_map(|(_id, auction)| { + // Withdrawing an auction changes sequence from 1 => 2 + // so we want to withdraw if the sequence is 1. + if auction.state.sequence == 1 { + Some(auction) + } else { + None + } + }) + .collect(); + + Ok(auction_ids) +} + fn display_auction_description(asset_cache: &Cache, auctions: Vec) { let mut tally_max_output = Amount::zero(); let mut tally_min_output = Amount::zero(); diff --git a/crates/view/src/planner.rs b/crates/view/src/planner.rs index 1bbf40a1b3..755ad3daef 100644 --- a/crates/view/src/planner.rs +++ b/crates/view/src/planner.rs @@ -13,8 +13,8 @@ use tracing::instrument; use crate::{SpendableNoteRecord, ViewClient}; use anyhow::anyhow; use penumbra_asset::{asset, Value}; -use penumbra_auction::auction::dutch::actions::ActionDutchAuctionWithdrawPlan; use penumbra_auction::auction::dutch::DutchAuctionDescription; +use penumbra_auction::auction::dutch::{actions::ActionDutchAuctionWithdrawPlan, DutchAuction}; use penumbra_auction::auction::{ dutch::actions::{ActionDutchAuctionEnd, ActionDutchAuctionSchedule}, AuctionId, @@ -222,9 +222,33 @@ impl Planner { } /// Withdraws the reserves of the Dutch auction. - // TODO: nicer api? what do we get by passing fields individually rather than the plan? + /// + /// Uses the provided auction state to automatically end the auction + /// if necessary. #[instrument(skip(self))] - pub fn dutch_auction_withdraw(&mut self, plan: ActionDutchAuctionWithdrawPlan) -> &mut Self { + pub fn dutch_auction_withdraw(&mut self, auction: &DutchAuction) -> &mut Self { + let auction_id = auction.description.id(); + // Check if the auction needs to be ended + if auction.state.sequence == 0 { + self.dutch_auction_end(auction_id); + } + + let reserves_input = Value { + amount: auction.state.input_reserves, + asset_id: auction.description.input.asset_id, + }; + let reserves_output = Value { + amount: auction.state.output_reserves, + asset_id: auction.description.output_id, + }; + + let plan = ActionDutchAuctionWithdrawPlan { + auction_id, + seq: 2, + reserves_input, + reserves_output, + }; + self.action_list.push(plan); self } From b8354fbe701e65ed9f1560bfe9418849a1760008 Mon Sep 17 00:00:00 2001 From: Erwan Or Date: Mon, 27 May 2024 22:59:42 -0400 Subject: [PATCH 2/4] view: inform clients about auction state "lag" --- crates/view/src/client.rs | 83 +++++++++++++++++++++++++------------- crates/view/src/service.rs | 11 ++--- crates/view/src/storage.rs | 9 +++-- 3 files changed, 65 insertions(+), 38 deletions(-) diff --git a/crates/view/src/client.rs b/crates/view/src/client.rs index 769213cdc0..be7edbccd8 100644 --- a/crates/view/src/client.rs +++ b/crates/view/src/client.rs @@ -60,7 +60,13 @@ pub trait ViewClient { Box< dyn Future< Output = Result< - Vec<(AuctionId, SpendableNoteRecord, Option, Vec)>, + Vec<( + AuctionId, + SpendableNoteRecord, + u64, + Option, + Vec, + )>, >, > + Send + 'static, @@ -971,7 +977,13 @@ where Box< dyn Future< Output = Result< - Vec<(AuctionId, SpendableNoteRecord, Option, Vec)>, + Vec<( + AuctionId, + SpendableNoteRecord, + u64, + Option, + Vec, + )>, >, > + Send + 'static, @@ -993,33 +1005,46 @@ where .try_collect() .await?; - let resp: Vec<(AuctionId, SpendableNoteRecord, Option, Vec)> = - auctions - .into_iter() - .map(|auction_rsp| { - let pb_id = auction_rsp - .id - .ok_or_else(|| anyhow::anyhow!("missing auction id!!"))?; - let auction_id: AuctionId = pb_id.try_into()?; - let snr: SpendableNoteRecord = auction_rsp - .note_record - .ok_or_else(|| anyhow::anyhow!("mission SNR from auction response"))? - .try_into()?; - - let auction = auction_rsp.auction; - let lps: Vec = auction_rsp - .positions - .into_iter() - .map(TryInto::try_into) - .collect::>>()?; - - Ok::< - (AuctionId, SpendableNoteRecord, Option, Vec), - anyhow::Error, - >((auction_id, snr, auction, lps)) - }) - .filter_map(|res| res.ok()) // TODO: scrap this later. - .collect(); + let resp: Vec<( + AuctionId, + SpendableNoteRecord, + u64, + Option, + Vec, + )> = auctions + .into_iter() + .map(|auction_rsp| { + let pb_id = auction_rsp + .id + .ok_or_else(|| anyhow::anyhow!("missing auction id"))?; + let auction_id: AuctionId = pb_id.try_into()?; + let snr: SpendableNoteRecord = auction_rsp + .note_record + .ok_or_else(|| anyhow::anyhow!("missing SNR from auction response"))? + .try_into()?; + + let local_seq = auction_rsp.local_seq; + + let auction = auction_rsp.auction; + let lps: Vec = auction_rsp + .positions + .into_iter() + .map(TryInto::try_into) + .collect::>>()?; + + Ok::< + ( + AuctionId, + SpendableNoteRecord, + u64, /* the local sequence number */ + Option, /* the auction state if it was requested */ + Vec, /* associated liquidity positions if we queried the latest state */ + ), + anyhow::Error, + >((auction_id, snr, local_seq, auction, lps)) + }) + .filter_map(|res| res.ok()) // TODO: scrap this later. + .collect(); Ok(resp) } diff --git a/crates/view/src/service.rs b/crates/view/src/service.rs index 9219f075aa..8a311ae758 100644 --- a/crates/view/src/service.rs +++ b/crates/view/src/service.rs @@ -436,8 +436,8 @@ impl ViewService for ViewServer { None }; - let responses = - futures::future::join_all(all_auctions.into_iter().map(|(auction_id, note_record)| { + let responses = futures::future::join_all(all_auctions.into_iter().map( + |(auction_id, note_record, local_seq)| { let maybe_client = client.clone(); async move { let (any_state, positions) = if let Some(mut client2) = maybe_client { @@ -458,11 +458,12 @@ impl ViewService for ViewServer { note_record: Some(note_record.into()), auction: any_state, positions, - local_seq: 0, // TODO: implement with real values + local_seq, }) } - })) - .await; + }, + )) + .await; let stream = stream::iter(responses) .map_err(|e| tonic::Status::internal(format!("error getting auction: {e}"))) diff --git a/crates/view/src/storage.rs b/crates/view/src/storage.rs index e7acd104f5..d4c953f007 100644 --- a/crates/view/src/storage.rs +++ b/crates/view/src/storage.rs @@ -1094,7 +1094,7 @@ impl Storage { &self, account_filter: Option, include_inactive: bool, - ) -> anyhow::Result> { + ) -> anyhow::Result> { let account_clause = account_filter .map(|idx| { format!( @@ -1111,7 +1111,7 @@ impl Storage { }; let query = format!( - "SELECT auctions.auction_id, spendable_notes.*, notes.* + "SELECT auctions.auction_id, spendable_notes.*, notes.*, auctions.auction_state FROM auctions JOIN spendable_notes ON auctions.note_commitment = spendable_notes.note_commitment JOIN notes ON auctions.note_commitment = notes.note_commitment @@ -1128,7 +1128,7 @@ impl Storage { let mut conn = pool.get()?; let tx = conn.transaction()?; - let spendable_note_records: Vec<(AuctionId, SpendableNoteRecord)> = tx + let spendable_note_records: Vec<(AuctionId, SpendableNoteRecord, u64)> = tx .prepare(&query)? .query_and_then((), |row| { let raw_auction_id: Vec = row.get("auction_id")?; @@ -1137,7 +1137,8 @@ impl Storage { .map_err(|_| anyhow!("auction id must be 32 bytes"))?; let auction_id = AuctionId(array_auction_id); let spendable_note_record: SpendableNoteRecord = row.try_into()?; - Ok((auction_id, spendable_note_record)) + let local_seq: u64 = row.get("auction_state")?; + Ok((auction_id, spendable_note_record, local_seq)) })? .collect::>>()?; From 77e0fb020afdee8ea687c8c91653064b371485f3 Mon Sep 17 00:00:00 2001 From: Erwan Or Date: Mon, 27 May 2024 22:59:42 -0400 Subject: [PATCH 3/4] pcli: use `local_seq` to impl close/with-all --- crates/bin/pcli/src/command/query/auction.rs | 17 +++++-- .../bin/pcli/src/command/tx/auction/dutch.rs | 48 ++++++++++++------- crates/bin/pcli/src/command/view/auction.rs | 14 ++++-- 3 files changed, 53 insertions(+), 26 deletions(-) diff --git a/crates/bin/pcli/src/command/query/auction.rs b/crates/bin/pcli/src/command/query/auction.rs index 7d7b0ef7a4..d75d1536a6 100644 --- a/crates/bin/pcli/src/command/query/auction.rs +++ b/crates/bin/pcli/src/command/query/auction.rs @@ -66,7 +66,7 @@ impl AuctionCmd { let asset_cache = app.view().assets().await?; - render_dutch_auction(&asset_cache, &dutch_auction, position).await?; + render_dutch_auction(&asset_cache, &dutch_auction, None, position).await?; } else { unimplemented!("only supporting dutch auctions at the moment, come back later"); } @@ -79,10 +79,11 @@ impl AuctionCmd { pub async fn render_dutch_auction( asset_cache: &Cache, dutch_auction: &DutchAuction, + local_view: Option, position: Option, ) -> anyhow::Result<()> { let auction_id = dutch_auction.description.id(); - println!("dutch auction with id {auction_id:?}"); + println!("dutch auction with id {auction_id:?}:"); let initial_input = dutch_auction.description.input; let input_id = initial_input.asset_id; @@ -137,7 +138,7 @@ pub async fn render_dutch_auction( .set_content_arrangement(ContentArrangement::DynamicFullWidth) .add_row(vec![ Cell::new(truncate_auction_id(&auction_id)).set_delimiter('.'), - Cell::new(render_sequence(dutch_auction.state.sequence)), + Cell::new(render_sequence(dutch_auction.state.sequence, local_view)), Cell::new(format!("{start_height} -> {end_height}")), Cell::new(dutch_auction.description.step_count.to_string()), Cell::new(format!("{}", start_price)), @@ -163,13 +164,19 @@ pub async fn render_dutch_auction( Ok(()) } -fn render_sequence(state: u64) -> String { - if state == 0 { +fn render_sequence(state: u64, local_seq: Option) -> String { + let main = if state == 0 { format!("Opened") } else if state == 1 { format!("Closed") } else { format!("Withdrawn (seq={state})") + }; + + if let Some(local_seq) = local_seq { + format!("{main} (local_seq={local_seq})") + } else { + main } } diff --git a/crates/bin/pcli/src/command/tx/auction/dutch.rs b/crates/bin/pcli/src/command/tx/auction/dutch.rs index e9cb7f43bd..12c560a7ce 100644 --- a/crates/bin/pcli/src/command/tx/auction/dutch.rs +++ b/crates/bin/pcli/src/command/tx/auction/dutch.rs @@ -236,7 +236,6 @@ impl DutchCmd { (false, Some(auction_id)) => { let auction_id = auction_id.parse::()?; - // TODO: better way to do this? let all = auctions_to_withdraw(app.view(), *source).await?; vec![all .into_iter() @@ -371,18 +370,33 @@ impl DutchCmd { } } -async fn dutch_auction_states( +async fn all_dutch_auction_states( view_client: &mut impl ViewClient, source: impl Into, -) -> Result> { +) -> Result> { + fetch_dutch_auction_states(view_client, source, true).await +} + +async fn active_dutch_auction_states( + view_client: &mut impl ViewClient, + source: impl Into, +) -> Result> { + fetch_dutch_auction_states(view_client, source, false).await +} + +async fn fetch_dutch_auction_states( + view_client: &mut impl ViewClient, + source: impl Into, + include_inactive: bool, +) -> Result> { let auctions = view_client - .auctions(Some(source.into()), false, true) + .auctions(Some(source.into()), include_inactive, true) .await? .into_iter() - .filter_map(|(id, _, state, _)| { + .filter_map(|(id, _, local_seq, state, _)| { if let Some(state) = state { if let Ok(da) = DutchAuction::decode(state.value) { - Some((id, da)) + Some((id, da, local_seq)) } else { None } @@ -393,16 +407,16 @@ async fn dutch_auction_states( .collect(); Ok(auctions) } - +/// Return all the auctions that need to be ended, based on our local view of the chain state. async fn auctions_to_end(view_client: &mut impl ViewClient, source: u32) -> Result> { - let auctions = dutch_auction_states(view_client, source).await?; + let auctions = active_dutch_auction_states(view_client, source).await?; let auction_ids = auctions .into_iter() - .filter_map(|(id, auction)| { - // Ending an auction changes sequence from 0 => 1 - // so if the sequence is 0, it's open and we should end it. - if auction.state.sequence == 0 { + .filter_map(|(id, _auction, local_seq)| { + // We want to end auctions that we track as "opened" (local_seq == 0) + // so that we can close them, or catch-up with the chain state if they are already closed. + if local_seq == 0 { Some(id) } else { None @@ -417,14 +431,14 @@ async fn auctions_to_withdraw( view_client: &mut impl ViewClient, source: u32, ) -> Result> { - let auctions = dutch_auction_states(view_client, source).await?; + let auctions = all_dutch_auction_states(view_client, source).await?; let auction_ids = auctions .into_iter() - .filter_map(|(_id, auction)| { - // Withdrawing an auction changes sequence from 1 => 2 - // so we want to withdraw if the sequence is 1. - if auction.state.sequence == 1 { + .filter_map(|(_, auction, local_seq)| { + // We want to end auctions that we track as "closed" (local_seq == 1) + // so that we can close them, or catch-up with the chain state if they are already closed. + if local_seq == 1 { Some(auction) } else { None diff --git a/crates/bin/pcli/src/command/view/auction.rs b/crates/bin/pcli/src/command/view/auction.rs index c73e33ab8a..087a62de0b 100644 --- a/crates/bin/pcli/src/command/view/auction.rs +++ b/crates/bin/pcli/src/command/view/auction.rs @@ -30,21 +30,27 @@ impl AuctionCmd { let auctions: Vec<( penumbra_auction::auction::AuctionId, penumbra_view::SpendableNoteRecord, + u64, Option, Vec, )> = view_client .auctions(None, self.include_inactive, self.query_latest_state) .await?; - for (auction_id, _, maybe_auction_state, positions) in auctions.into_iter() { + for (auction_id, _, local_seq, maybe_auction_state, positions) in auctions.into_iter() { if let Some(pb_auction_state) = maybe_auction_state { if pb_auction_state.type_url == pb_auction::DutchAuction::type_url() { let dutch_auction = DutchAuction::decode(pb_auction_state.value) .expect("no deserialization error"); let asset_cache = view_client.assets().await?; - render_dutch_auction(&asset_cache, &dutch_auction, positions.get(0).cloned()) - .await - .expect("no rendering errors"); + render_dutch_auction( + &asset_cache, + &dutch_auction, + Some(local_seq), + positions.get(0).cloned(), + ) + .await + .expect("no rendering errors"); } else { unimplemented!("only supporting dutch auctions at the moment, come back later"); } From 0f7250f405cd1560d88cabf1bc1539f1ee8e7079 Mon Sep 17 00:00:00 2001 From: Erwan Or Date: Tue, 28 May 2024 14:03:26 -0400 Subject: [PATCH 4/4] view(auction): explain hardcoded seqnum Signed-off-by: Erwan Or --- crates/view/src/planner.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/view/src/planner.rs b/crates/view/src/planner.rs index 755ad3daef..1c19f2d81e 100644 --- a/crates/view/src/planner.rs +++ b/crates/view/src/planner.rs @@ -244,7 +244,7 @@ impl Planner { let plan = ActionDutchAuctionWithdrawPlan { auction_id, - seq: 2, + seq: 2, // 1 (closed) -> 2 (withdrawn) reserves_input, reserves_output, };