Merge branch 'tomas/checked-arith' (#3074)

* origin/tomas/checked-arith:
  changelog: add #3074
  namada/pos: re-use into_tm_voting_power from pos crate
  wasm: update to non-panicking core API
  benches: update to non-panicking core API
  apps: update to non-panicking core API
  namada: update to non-panicking core API
  sdk: update to non-panicking core API
  eth_bridge: update to non-panicking core API
  ibc: update to non-panicking core API
  pos: update to non-panicking core API
  gov: update to non-panicking core API
  state: update to non-panicking core API
  shielded_token: update to non-panicking core API
  trans_token: update to non-panicking core API
  vote_ext: update to non-panicking core API
  controller: replacing panicking code
  storage/collections/lazy_map: add try_update
  storage: add conv from core::arith::Error for convenience
  test/core: fixes for checked arith
  core: clippy fixes
  core/dec: sanitize panicking code
  core/storage: sanitize panicking code
  core/token: sanitize panicking code
  core/uint: sanitize panicking code
  core: add `assert_matches` dev-dep
  core/voting_power: sanitize panicking code
  core: strict clippy arith
  deps: add smooth-operator and re-export from core/arith
  core: add arith mod with a custom `trait CheckedAdd`

Author: brentstone

.changelog/unreleased/improvements/3074-checked-arith.md

@@ -0,0 +1,2 @@
+- Prohibit unchecked arithmetics and conversions in the core crate.
+  ([\#3074](https://github.com/anoma/namada/pull/3074))

Cargo.lock

@@ -157,6 +157,7 @@ sha2 = "0.9.3"
 sha2-const = "0.1.2"
 signal-hook = "0.3.9"
 slip10_ed25519 = "0.1.3"
+smooth-operator = {git = "https://github.com/heliaxdev/smooth-operator", tag = "v0.6.0"}
 # sysinfo with disabled multithread feature
 sysinfo = {version = "0.27.8", default-features = false}
 tar = "0.4.37"

Cargo.toml

@@ -59,7 +59,7 @@ pub fn main() -> Result<()> {
                     let wasm_dir = chain_ctx.wasm_dir();
                     chain_ctx.config.ledger.shell.action_at_height =
                         Some(ActionAtHeight {
-                            height: args.last_height + 2,
+                            height: args.last_height.checked_add(2).unwrap(),
                             action: Action::Halt,
                         });
                     std::env::set_var(

crates/apps/src/bin/namada-node/cli.rs

@@ -257,8 +257,8 @@ impl Default for BenchShell {
                 author: defaults::albert_address(),
                 r#type: ProposalType::Default,
                 voting_start_epoch,
-                voting_end_epoch: voting_start_epoch + 3_u64,
-                activation_epoch: voting_start_epoch + 9_u64,
+                voting_end_epoch: voting_start_epoch.unchecked_add(3_u64),
+                activation_epoch: voting_start_epoch.unchecked_add(9_u64),
             },
             None,
             Some(vec![content_section]),
@@ -417,7 +417,7 @@ impl BenchShell {
             &mut self.state,
             &params,
             current_epoch,
-            current_epoch + params.pipeline_len,
+            current_epoch.unchecked_add(params.pipeline_len),
         )
         .unwrap();
 
@@ -573,7 +573,7 @@ impl BenchShell {
         self.inner
             .state
             .in_mem_mut()
-            .begin_block(last_height + 1)
+            .begin_block(last_height.next_height())
             .unwrap();
 
         self.inner.commit();

crates/apps/src/lib/bench_utils.rs

@@ -859,11 +859,13 @@ pub async fn query_and_print_unbonds(
     let mut not_yet_withdrawable = HashMap::<Epoch, token::Amount>::new();
     for ((_start_epoch, withdraw_epoch), amount) in unbonds.into_iter() {
         if withdraw_epoch <= current_epoch {
-            total_withdrawable += amount;
+            total_withdrawable =
+                total_withdrawable.checked_add(amount).unwrap();
         } else {
             let withdrawable_amount =
                 not_yet_withdrawable.entry(withdraw_epoch).or_default();
-            *withdrawable_amount += amount;
+            *withdrawable_amount =
+                withdrawable_amount.checked_add(amount).unwrap();
         }
     }
     if !total_withdrawable.is_zero() {
@@ -948,7 +950,7 @@ pub async fn query_bonds(
                 context.io(),
                 &mut w;
                 "Active (slashable) bonds total: {}",
-                details.bonds_total_active().to_string_native()
+                details.bonds_total_active().unwrap().to_string_native()
             )?;
         }
         display_line!(context.io(), &mut w; "Bonds total: {}", details.bonds_total.to_string_native())?;
@@ -992,7 +994,7 @@ pub async fn query_bonds(
             context.io(),
             &mut w;
             "All bonds total active: {}",
-            bonds_and_unbonds.bonds_total_active().to_string_native()
+            bonds_and_unbonds.bonds_total_active().unwrap().to_string_native()
         )?;
     }
     display_line!(
@@ -1015,7 +1017,7 @@ pub async fn query_bonds(
             context.io(),
             &mut w;
             "All unbonds total active: {}",
-            bonds_and_unbonds.unbonds_total_active().to_string_native()
+            bonds_and_unbonds.unbonds_total_active().unwrap().to_string_native()
         )?;
     }
     display_line!(

crates/apps/src/lib/client/rpc.rs

@@ -430,7 +430,10 @@ pub fn init_network(
             if tm_votes_per_token > Dec::from(1) {
                 Uint::one()
             } else {
-                (Dec::from(1) / tm_votes_per_token).ceil().abs()
+                (Dec::from(1).checked_div(tm_votes_per_token).unwrap())
+                    .ceil()
+                    .unwrap()
+                    .abs()
             },
             token::NATIVE_MAX_DECIMAL_PLACES,
         )

crates/apps/src/lib/client/utils.rs

@@ -543,7 +543,9 @@ impl Transactions<Validated> {
                     BTreeMap::new();
                 for tx in txs {
                     let entry = stakes.entry(&tx.validator).or_default();
-                    *entry += tx.amount.amount();
+                    *entry = entry
+                        .checked_add(tx.amount.amount())
+                        .expect("Validator total stake must not overflow");
                 }
 
                 stakes.into_values().any(|stake| {

crates/apps/src/lib/config/genesis/transactions.rs

@@ -452,7 +452,7 @@ async fn run_oracle_aux<C: RpcClient>(mut oracle: Oracle<C>) {
         if !config.active {
             config = oracle.wait_on_reactivation().await;
         }
-        next_block_to_process += 1.into();
+        next_block_to_process = next_block_to_process.next();
     }
 }
 
@@ -481,8 +481,9 @@ async fn process_events_in_block<C: RpcClient>(
         SyncStatus::Syncing => return Err(Error::FallenBehind),
     }
     .into();
-    let minimum_latest_block =
-        block_to_process.clone() + config.min_confirmations.into();
+    let minimum_latest_block = block_to_process.clone().unchecked_add(
+        ethereum_structs::BlockHeight::from(config.min_confirmations),
+    );
     if minimum_latest_block > latest_block {
         tracing::debug!(
             ?block_to_process,

crates/apps/src/lib/node/ledger/ethereum_oracle/mod.rs

@@ -545,7 +545,7 @@ where
     /// if necessary. Returns a bool indicating if a new epoch began and the
     /// height of the new block.
     fn update_state(&mut self, header: Header) -> (BlockHeight, bool) {
-        let height = self.state.in_mem().get_last_block_height() + 1;
+        let height = self.state.in_mem().get_last_block_height().next_height();
 
         self.state
             .in_mem_mut()
@@ -597,7 +597,9 @@ where
         current_epoch: Epoch,
         events: &mut impl EmitEvents,
     ) -> Result<()> {
-        let last_epoch = current_epoch.prev();
+        let last_epoch = current_epoch
+            .prev()
+            .expect("Must have a prev epoch when applying inflation");
 
         // Get the number of blocks in the last epoch
         let first_block_of_last_epoch =
@@ -622,11 +624,9 @@ where
 
         // Take events that may be emitted from PGF
         for event in self.state.write_log_mut().take_events() {
-            events.emit(
-                event.with(Height(
-                    self.state.in_mem().get_last_block_height() + 1,
-                )),
-            );
+            events.emit(event.with(Height(
+                self.state.in_mem().get_last_block_height().next_height(),
+            )));
         }
 
         Ok(())
@@ -1923,7 +1923,8 @@ mod test_finalize_block {
         // The total rewards claimed should be approximately equal to the total
         // minted inflation, minus (unbond_amount / initial_stake) * rewards
         // from the unbond epoch and the following epoch (the missed_rewards)
-        let ratio = Dec::from(unbond_amount) / Dec::from(init_stake);
+        let ratio = Dec::try_from(unbond_amount).unwrap()
+            / Dec::try_from(init_stake).unwrap();
         let lost_rewards = ratio * missed_rewards;
         let uncertainty = Dec::from_str("0.07").unwrap();
         let token_uncertainty = uncertainty * lost_rewards;
@@ -2087,8 +2088,8 @@ mod test_finalize_block {
         ));
 
         // Check that the commission earned is expected
-        let del_stake = Dec::from(del_amount);
-        let tot_stake = Dec::from(init_stake + del_amount);
+        let del_stake = Dec::try_from(del_amount).unwrap();
+        let tot_stake = Dec::try_from(init_stake + del_amount).unwrap();
         let stake_ratio = del_stake / tot_stake;
         let del_rewards_no_commission = stake_ratio * inflation_3;
         let commission = commission_rate * del_rewards_no_commission;
@@ -3592,7 +3593,7 @@ mod test_finalize_block {
         let total_stake =
             read_total_stake(&shell.state, &params, pipeline_epoch)?;
 
-        let expected_slashed = initial_stake.mul_ceil(cubic_rate);
+        let expected_slashed = initial_stake.mul_ceil(cubic_rate).unwrap();
 
         println!(
             "Initial stake = {}\nCubic rate = {}\nExpected slashed = {}\n",
@@ -4115,8 +4116,10 @@ mod test_finalize_block {
         )
         .unwrap();
 
-        let vp_frac_3 = Dec::from(val_stake_3) / Dec::from(tot_stake_3);
-        let vp_frac_4 = Dec::from(val_stake_4) / Dec::from(tot_stake_4);
+        let vp_frac_3 = Dec::try_from(val_stake_3).unwrap()
+            / Dec::try_from(tot_stake_3).unwrap();
+        let vp_frac_4 = Dec::try_from(val_stake_4).unwrap()
+            / Dec::try_from(tot_stake_4).unwrap();
         let tot_frac = Dec::two() * vp_frac_3 + vp_frac_4;
         let cubic_rate = std::cmp::min(
             Dec::one(),
@@ -4126,7 +4129,7 @@ mod test_finalize_block {
 
         let equal_enough = |rate1: Dec, rate2: Dec| -> bool {
             let tolerance = Dec::new(1, 9).unwrap();
-            rate1.abs_diff(&rate2) < tolerance
+            rate1.abs_diff(rate2).unwrap() < tolerance
         };
 
         // There should be 2 slashes processed for the validator, each with rate
@@ -4162,7 +4165,8 @@ mod test_finalize_block {
             - del_unbond_1_amount
             + self_bond_1_amount
             - self_unbond_2_amount)
-            .mul_ceil(slash_rate_3);
+            .mul_ceil(slash_rate_3)
+            .unwrap();
         assert!(
             ((pre_stake_10 - post_stake_10).change()
                 - exp_slashed_during_processing_9.change())
@@ -4176,22 +4180,27 @@ mod test_finalize_block {
         // Check that we can compute the stake at the pipeline epoch
         // NOTE: may be off. by 1 namnam due to rounding;
         let exp_pipeline_stake = (Dec::one() - slash_rate_3)
-            * Dec::from(
+            * Dec::try_from(
                 initial_stake + del_1_amount
                     - self_unbond_1_amount
                     - del_unbond_1_amount
                     + self_bond_1_amount
                     - self_unbond_2_amount,
             )
-            + Dec::from(del_2_amount);
+            .unwrap()
+            + Dec::try_from(del_2_amount).unwrap();
 
         assert!(
-            exp_pipeline_stake.abs_diff(&Dec::from(post_stake_10))
+            exp_pipeline_stake
+                .abs_diff(Dec::try_from(post_stake_10).unwrap())
+                .unwrap()
                 <= Dec::new(2, NATIVE_MAX_DECIMAL_PLACES).unwrap(),
             "Expected {}, got {} (with less than 2 err), diff {}",
             exp_pipeline_stake,
             post_stake_10.to_string_native(),
-            exp_pipeline_stake.abs_diff(&Dec::from(post_stake_10)),
+            exp_pipeline_stake
+                .abs_diff(Dec::try_from(post_stake_10).unwrap())
+                .unwrap(),
         );
 
         // Check the balance of the Slash Pool
@@ -4418,6 +4427,7 @@ mod test_finalize_block {
             (self_details.unbonds[1].slashed_amount.unwrap().change()
                 - (self_unbond_2_amount - self_bond_1_amount)
                     .mul_ceil(rate)
+                    .unwrap()
                     .change())
             .abs()
                 <= Uint::from(1)
@@ -4438,7 +4448,7 @@ mod test_finalize_block {
         .unwrap();
 
         let exp_del_withdraw_slashed_amount =
-            del_unbond_1_amount.mul_ceil(slash_rate_3);
+            del_unbond_1_amount.mul_ceil(slash_rate_3).unwrap();
         assert!(
             (del_withdraw
                 - (del_unbond_1_amount - exp_del_withdraw_slashed_amount))