staking: validator state transition fix

penumbra-zone · Jan 23, 2024 · f1d07db · f1d07db
1 parent 23b4dbb
commit f1d07db
Show file tree

Hide file tree

Showing 4 changed files with 116 additions and 62 deletions.
diff --git a/crates/core/component/stake/src/action_handler/undelegate_claim.rs b/crates/core/component/stake/src/action_handler/undelegate_claim.rs
@@ -31,9 +31,7 @@ impl ActionHandler for UndelegateClaim {
 
     async fn check_stateful<S: StateRead + 'static>(&self, state: Arc<S>) -> Result<()> {
         // We need to check two things:
-
         // 1. That we're past the specified unbonding end epoch.
-
         let current_epoch = state.epoch().await?;
         let end_epoch_index = state
             .unbonding_end_epoch_for(&self.body.validator_identity, self.body.start_epoch_index)

diff --git a/crates/core/component/stake/src/action_handler/validator_definition.rs b/crates/core/component/stake/src/action_handler/validator_definition.rs
@@ -39,18 +39,16 @@ impl ActionHandler for validator::Definition {
             .verify(&definition_bytes, &self.auth_sig)
             .context("validator definition signature failed to verify")?;
 
-        // TODO(hdevalence) -- is this duplicated by the check during parsing?
-        // Check that the funding streams do not exceed 100% commission (10000bps)
         let total_funding_bps = self
             .validator
             .funding_streams
             .iter()
             .map(|fs| fs.rate_bps() as u64)
             .sum::<u64>();
 
-        if total_funding_bps > 10000 {
+        if total_funding_bps > 10_000 {
             anyhow::bail!(
-                "validator defined {} bps of funding streams, greater than 10000bps = 100%",
+                "validator defined {} bps of funding streams, greater than 10000bps (= 100%)",
                 total_funding_bps
             );
         }
@@ -81,14 +79,11 @@ impl ActionHandler for validator::Definition {
             .await?
         {
             if v.validator.identity_key != existing_v.identity_key {
-                // This is a new validator definition, but the consensus
-                // key it declares is already in use by another validator.
+                // This is a new validator definition, but the consensus it declares
+                // is used by another validator. We MUST reject this definition:
                 //
-                // Rejecting this is important for two reasons:
-                //
-                // 1. It prevents someone from declaring an (app-level)
-                // validator that "piggybacks" on the actual behavior of someone
-                // else's validator.
+                // 1. It prevents someone from declaring an (app-level) validator that
+                // "piggybacks" on the actual behavior of someone else's validator.
                 //
                 // 2. If we submit a validator update to Tendermint that
                 // includes duplicate consensus keys, Tendermint gets confused