Remove usage of "unwrap" in the dex component (#2995)

crates/core/component/dex/src/batch_swap_output_data.rs

@@ -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)
     }
@@ -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 {

crates/core/component/dex/src/component/dex.rs

@@ -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() {
@@ -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(),
                 ],
             )
@@ -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");
         }

crates/core/component/dex/src/component/router/fill_route.rs

@@ -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();
 
@@ -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
@@ -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"),
         )
     }
 
@@ -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
@@ -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

crates/core/component/dex/src/component/router/params.rs

@@ -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,

crates/core/component/dex/src/component/router/path.rs

@@ -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 {

crates/core/component/dex/src/lib.rs

@@ -1,3 +1,4 @@
+#![deny(clippy::unwrap_used)]
 #![cfg_attr(docsrs, feature(doc_cfg))]
 
 #[cfg_attr(docsrs, doc(cfg(feature = "component")))]

crates/core/component/dex/src/lp/trading_function.rs

@@ -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,
@@ -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:

crates/core/component/dex/src/swap/proof.rs

@@ -1,3 +1,4 @@
+use anyhow::Context;
 use ark_ff::ToConstraintField;
 use ark_groth16::{
     r1cs_to_qap::LibsnarkReduction, Groth16, PreparedVerifyingKey, Proof, ProvingKey,
@@ -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;
@@ -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,
@@ -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();