Skip to content

Commit

Permalink
Remove usage of "unwrap" in the dex component
Browse files Browse the repository at this point in the history
  • Loading branch information
zbuc committed Sep 11, 2023
1 parent e3ed6cc commit 72fece0
Show file tree
Hide file tree
Showing 10 changed files with 168 additions and 56 deletions.
54 changes: 43 additions & 11 deletions crates/core/component/dex/src/batch_swap_output_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,18 +88,50 @@ impl ToConstraintField<Fq> for BatchSwapOutputData {
fn to_field_elements(&self) -> Option<Vec<Fq>> {
let mut public_inputs = Vec::new();
let delta_1 = U128x128::from(self.delta_1);
public_inputs.extend(delta_1.to_field_elements().unwrap());
public_inputs.extend(U128x128::from(self.delta_2).to_field_elements().unwrap());
public_inputs.extend(U128x128::from(self.lambda_1).to_field_elements().unwrap());
public_inputs.extend(U128x128::from(self.lambda_2).to_field_elements().unwrap());
public_inputs.extend(U128x128::from(self.unfilled_1).to_field_elements().unwrap());
public_inputs.extend(U128x128::from(self.unfilled_2).to_field_elements().unwrap());
public_inputs.extend(Fq::from(self.height).to_field_elements().unwrap());
public_inputs.extend(self.trading_pair.to_field_elements().unwrap());
public_inputs.extend(
delta_1
.to_field_elements()
.expect("delta_1 is a Bls12-377 field member"),
);
public_inputs.extend(
U128x128::from(self.delta_2)
.to_field_elements()
.expect("U128x128 types are Bls12-377 field members"),
);
public_inputs.extend(
U128x128::from(self.lambda_1)
.to_field_elements()
.expect("U128x128 types are Bls12-377 field members"),
);
public_inputs.extend(
U128x128::from(self.lambda_2)
.to_field_elements()
.expect("U128x128 types are Bls12-377 field members"),
);
public_inputs.extend(
U128x128::from(self.unfilled_1)
.to_field_elements()
.expect("U128x128 types are Bls12-377 field members"),
);
public_inputs.extend(
U128x128::from(self.unfilled_2)
.to_field_elements()
.expect("U128x128 types are Bls12-377 field members"),
);
public_inputs.extend(
Fq::from(self.height)
.to_field_elements()
.expect("Fq types are Bls12-377 field members"),
);
public_inputs.extend(
self.trading_pair
.to_field_elements()
.expect("trading_pair is a Bls12-377 field member"),
);
public_inputs.extend(
Fq::from(self.epoch_starting_height)
.to_field_elements()
.unwrap(),
.expect("Fq types are Bls12-377 field members"),
);
Some(public_inputs)
}
Expand Down Expand Up @@ -376,11 +408,11 @@ mod tests {
let trading_pair = TradingPair {
asset_1: asset::Cache::with_known_assets()
.get_unit("upenumbra")
.unwrap()
.expect("upenumbra denom should always be known by the asset registry")
.id(),
asset_2: asset::Cache::with_known_assets()
.get_unit("nala")
.unwrap()
.expect("nala denom should always be known by the asset registry")
.id(),
};
Self {
Expand Down
16 changes: 8 additions & 8 deletions crates/core/component/dex/src/component/dex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ impl Component for Dex {
state: &mut Arc<S>,
end_block: &abci::request::EndBlock,
) {
let current_epoch = state.epoch().await.unwrap();
let current_epoch = state.epoch().await.expect("epoch is set");

// For each batch swap during the block, calculate clearing prices and set in the JMT.
for (trading_pair, swap_flows) in state.swap_flows() {
Expand Down Expand Up @@ -77,27 +77,27 @@ impl Component for Dex {
*STAKING_TOKEN_ASSET_ID,
asset::Cache::with_known_assets()
.get_unit("gm")
.unwrap()
.expect("gm is a known asset")
.id(),
asset::Cache::with_known_assets()
.get_unit("gn")
.unwrap()
.expect("gn is a known asset")
.id(),
asset::Cache::with_known_assets()
.get_unit("test_usd")
.unwrap()
.expect("test_usd is a known asset")
.id(),
asset::Cache::with_known_assets()
.get_unit("test_btc")
.unwrap()
.expect("test_btc is a known asset")
.id(),
asset::Cache::with_known_assets()
.get_unit("test_atom")
.unwrap()
.expect("test_atom is a known asset")
.id(),
asset::Cache::with_known_assets()
.get_unit("test_osmo")
.unwrap()
.expect("test_osmo is a known asset")
.id(),
],
)
Expand All @@ -108,7 +108,7 @@ impl Component for Dex {
// TODO: hack to avoid needing an asset cache for nice debug output
let unit = asset::Cache::with_known_assets()
.get_unit("penumbra")
.unwrap();
.expect("penumbra is a known asset");
let burn = format!("{}{}", unit.format_value(arb_burn.amount), unit);
tracing::info!(%burn, "executed arbitrage opportunity");
}
Expand Down
28 changes: 21 additions & 7 deletions crates/core/component/dex/src/component/router/fill_route.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,13 +320,13 @@ impl FrontierTx {
let input: U128x128 = self
.trace
.first()
.unwrap()
.expect("input amount is set in a complete trace")
.expect("input amount is set in a complete trace")
.into();
let output: U128x128 = self
.trace
.last()
.unwrap()
.expect("output amount is set in a complete trace")
.expect("output amount is set in a complete trace")
.into();

Expand Down Expand Up @@ -364,7 +364,7 @@ impl<S: StateRead + StateWrite> Frontier<S> {
'next_position: loop {
let id = positions_by_price
.get_mut(pair)
.unwrap()
.expect("positions_by_price should have an entry for each pair")
.as_mut()
.next()
.await
Expand Down Expand Up @@ -435,8 +435,16 @@ impl<S: StateRead + StateWrite> Frontier<S> {
self.trace.push(trace);

(
changes.trace.first().unwrap().unwrap(),
changes.trace.last().unwrap().unwrap(),
changes
.trace
.first()
.expect("first should be set for a trace")
.expect("input amount should be set for a trace"),
changes
.trace
.last()
.expect("last should be set for a trace")
.expect("output amount should be set for a trace"),
)
}

Expand Down Expand Up @@ -481,7 +489,7 @@ impl<S: StateRead + StateWrite> Frontier<S> {
let next_position_id = match self
.positions_by_price
.get_mut(pair)
.unwrap()
.expect("positions_by_price should have an entry for each pair")
.as_mut()
.next()
.await
Expand Down Expand Up @@ -585,7 +593,13 @@ impl<S: StateRead + StateWrite> Frontier<S> {

#[instrument(skip(self, input), fields(input = ?input.amount))]
fn fill_unconstrained(&self, input: Value) -> FrontierTx {
assert_eq!(input.asset_id, self.pairs.first().unwrap().start);
assert_eq!(
input.asset_id,
self.pairs
.first()
.expect("first should be set for a trace")
.start
);

let mut tx = FrontierTx::new(&self);
// We have to manually update the trace here, because fill_forward
Expand Down
14 changes: 7 additions & 7 deletions crates/core/component/dex/src/component/router/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,31 +17,31 @@ impl Default for RoutingParams {
fixed_candidates: Arc::new(vec![
asset::Cache::with_known_assets()
.get_unit("test_usd")
.unwrap()
.expect("hardcoded \"test_usd\" denom should be known")
.id(),
asset::Cache::with_known_assets()
.get_unit("penumbra")
.unwrap()
.expect("hardcoded \"penumbra\" denom should be known")
.id(),
asset::Cache::with_known_assets()
.get_unit("gm")
.unwrap()
.expect("hardcoded \"gm\" denom should be known")
.id(),
asset::Cache::with_known_assets()
.get_unit("gn")
.unwrap()
.expect("hardcoded \"gn\" denom should be known")
.id(),
asset::Cache::with_known_assets()
.get_unit("test_atom")
.unwrap()
.expect("hardcoded \"test_atom\" denom should be known")
.id(),
asset::Cache::with_known_assets()
.get_unit("test_osmo")
.unwrap()
.expect("hardcoded \"test_osmo\" denom should be known")
.id(),
asset::Cache::with_known_assets()
.get_unit("test_btc")
.unwrap()
.expect("hardcoded \"test_btc\" denom should be known")
.id(),
]),
max_hops: 4,
Expand Down
2 changes: 1 addition & 1 deletion crates/core/component/dex/src/component/router/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ impl<S: StateRead + 'static> Path<S> {
let hop_price = best_price_position
.phi
.orient_end(new_end)
.unwrap()
.expect("position should be contain the end asset")
.effective_price();

match self.price * hop_price {
Expand Down
1 change: 1 addition & 0 deletions crates/core/component/dex/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![deny(clippy::unwrap_used)]
#![cfg_attr(docsrs, feature(doc_cfg))]

#[cfg_attr(docsrs, doc(cfg(feature = "component")))]
Expand Down
7 changes: 5 additions & 2 deletions crates/core/component/dex/src/lp/trading_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,10 @@ impl BareTradingFunction {
// Observe that for the case when `tentative_lambda_2` equals
// `reserves.r1`, rounding it down does not change anything since
// `reserves.r1` is integral. Therefore `reserves.r1 - lambda_2 >= 0`.
let lambda_2: Amount = tentative_lambda_2.round_down().try_into().unwrap();
let lambda_2: Amount = tentative_lambda_2
.round_down()
.try_into()
.expect("lambda_2 fits in an Amount");
let new_reserves = Reserves {
r1: reserves.r1 + delta_1,
r2: reserves.r2 - lambda_2,
Expand Down Expand Up @@ -362,7 +365,7 @@ impl BareTradingFunction {
.round_up()
.expect("no overflow")
.try_into()
.unwrap();
.expect("fillable_delta_1 fits in an Amount");

// How to show that: `unfilled_amount >= 0`:
// In this branch, we have:
Expand Down
30 changes: 24 additions & 6 deletions crates/core/component/dex/src/swap/proof.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use anyhow::Context;
use ark_ff::ToConstraintField;
use ark_groth16::{
r1cs_to_qap::LibsnarkReduction, Groth16, PreparedVerifyingKey, Proof, ProvingKey,
Expand Down Expand Up @@ -105,8 +106,10 @@ impl DummyWitness for SwapCircuit {
fn with_dummy_witness() -> Self {
let a = asset::Cache::with_known_assets()
.get_unit("upenumbra")
.unwrap();
let b = asset::Cache::with_known_assets().get_unit("nala").unwrap();
.expect("upenumbra asset exists");
let b = asset::Cache::with_known_assets()
.get_unit("nala")
.expect("nala asset exists");
let trading_pair = TradingPair::new(a.id(), b.id());
let diversifier_bytes = [1u8; 16];
let pk_d_bytes = decaf377::basepoint().vartime_compress().0;
Expand All @@ -126,7 +129,7 @@ impl DummyWitness for SwapCircuit {
amount: 3u64.into(),
asset_id: asset::Cache::with_known_assets()
.get_unit("upenumbra")
.unwrap()
.expect("upenumbra asset exists")
.id(),
}),
claim_address: address,
Expand Down Expand Up @@ -196,9 +199,24 @@ impl SwapProof {
Proof::deserialize_compressed_unchecked(&self.0[..]).map_err(|e| anyhow::anyhow!(e))?;

let mut public_inputs = Vec::new();
public_inputs.extend(balance_commitment.0.to_field_elements().unwrap());
public_inputs.extend(swap_commitment.0.to_field_elements().unwrap());
public_inputs.extend(fee_commitment.0.to_field_elements().unwrap());
public_inputs.extend(
balance_commitment
.0
.to_field_elements()
.context("balance_commitment should be a Bls12-377 field member")?,
);
public_inputs.extend(
swap_commitment
.0
.to_field_elements()
.context("swap_commitment should be a Bls12-377 field member")?,
);
public_inputs.extend(
fee_commitment
.0
.to_field_elements()
.context("fee_commitment should be a Bls12-377 field member")?,
);

tracing::trace!(?public_inputs);
let start = std::time::Instant::now();
Expand Down
Loading

0 comments on commit 72fece0

Please sign in to comment.