Merge branch 'brent/fix-unjail-bug' (#1793)

* origin/brent/fix-unjail-bug:
  [ci] wasm checksums update
  changelog: add #1793
  pos/tests: unit test for unjailing
  pos/lib: fix bug in error-catching

.changelog/unreleased/bug-fixes/1793-fix-unjail-bug.md

@@ -0,0 +1,4 @@
+- Fixes buggy error handling in pos unjail_validator. Now properly enforces that
+  if an unjail tx is submitted when the validator state is something other than
+  Jailed in any of the current or future epochs, the tx will error out and fail.
+  ([\#1793](https://github.com/anoma/namada/pull/1793))

proof_of_stake/src/lib.rs

@@ -3927,11 +3927,8 @@ where
 
     // Check that the validator is jailed up to the pipeline epoch
     for epoch in current_epoch.iter_range(params.pipeline_len + 1) {
-        let state = validator_state_handle(validator).get(
-            storage,
-            current_epoch,
-            &params,
-        )?;
+        let state =
+            validator_state_handle(validator).get(storage, epoch, &params)?;
         if let Some(state) = state {
             if state != ValidatorState::Jailed {
                 return Err(UnjailValidatorError::NotJailed(

proof_of_stake/src/tests.rs

@@ -51,9 +51,10 @@ use crate::{
     staking_token_address, store_total_consensus_stake, total_deltas_handle,
     unbond_handle, unbond_tokens, unjail_validator, update_validator_deltas,
     update_validator_set, validator_consensus_key_handle,
-    validator_set_update_tendermint, validator_slashes_handle,
-    validator_state_handle, withdraw_tokens, write_validator_address_raw_hash,
-    BecomeValidator, STORE_VALIDATOR_SETS_LEN,
+    validator_set_positions_handle, validator_set_update_tendermint,
+    validator_slashes_handle, validator_state_handle, withdraw_tokens,
+    write_validator_address_raw_hash, BecomeValidator,
+    STORE_VALIDATOR_SETS_LEN,
 };
 
 proptest! {
@@ -124,6 +125,22 @@ proptest! {
     }
 }
 
+proptest! {
+    // Generate arb valid input for `test_unjail_validator_aux`
+    #![proptest_config(Config {
+        cases: 5,
+        .. Config::default()
+    })]
+    #[test]
+    fn test_unjail_validator(
+        (pos_params, genesis_validators)
+            in arb_params_and_genesis_validators(Some(4),6..9)
+    ) {
+        test_unjail_validator_aux(pos_params,
+            genesis_validators)
+    }
+}
+
 fn arb_params_and_genesis_validators(
     num_max_validator_slots: Option<u64>,
     val_size: Range<usize>,
@@ -2112,3 +2129,127 @@ fn arb_genesis_validators(
             },
         )
 }
+
+fn test_unjail_validator_aux(
+    params: PosParams,
+    mut validators: Vec<GenesisValidator>,
+) {
+    println!("\nTest inputs: {params:?}, genesis validators: {validators:#?}");
+    let mut s = TestWlStorage::default();
+
+    // Find the validator with the most stake and 100x his stake to keep the
+    // cubic slash rate small
+    let num_vals = validators.len();
+    validators.sort_by_key(|a| a.tokens);
+    validators[num_vals - 1].tokens = 100 * validators[num_vals - 1].tokens;
+
+    // Get second highest stake validator tomisbehave
+    let val_addr = &validators[num_vals - 2].address;
+    let val_tokens = validators[num_vals - 2].tokens;
+    println!(
+        "Validator that will misbehave addr {val_addr}, tokens {}",
+        val_tokens.to_string_native()
+    );
+
+    // Genesis
+    let mut current_epoch = s.storage.block.epoch;
+    init_genesis(
+        &mut s,
+        &params,
+        validators.clone().into_iter(),
+        current_epoch,
+    )
+    .unwrap();
+    s.commit_block().unwrap();
+
+    current_epoch = advance_epoch(&mut s, &params);
+    super::process_slashes(&mut s, current_epoch).unwrap();
+
+    // Discover first slash
+    let slash_0_evidence_epoch = current_epoch;
+    let evidence_block_height = BlockHeight(0); // doesn't matter for slashing logic
+    let slash_0_type = SlashType::DuplicateVote;
+    slash(
+        &mut s,
+        &params,
+        current_epoch,
+        slash_0_evidence_epoch,
+        evidence_block_height,
+        slash_0_type,
+        val_addr,
+        current_epoch.next(),
+    )
+    .unwrap();
+
+    assert_eq!(
+        validator_state_handle(val_addr)
+            .get(&s, current_epoch, &params)
+            .unwrap(),
+        Some(ValidatorState::Consensus)
+    );
+
+    for epoch in Epoch::iter_bounds_inclusive(
+        current_epoch.next(),
+        current_epoch + params.pipeline_len,
+    ) {
+        // Check the validator state
+        assert_eq!(
+            validator_state_handle(val_addr)
+                .get(&s, epoch, &params)
+                .unwrap(),
+            Some(ValidatorState::Jailed)
+        );
+        // Check the validator set positions
+        assert!(
+            validator_set_positions_handle()
+                .at(&epoch)
+                .get(&s, val_addr)
+                .unwrap()
+                .is_none(),
+        );
+    }
+
+    // Advance past an epoch in which we can unbond
+    let unfreeze_epoch =
+        slash_0_evidence_epoch + params.slash_processing_epoch_offset();
+    while current_epoch < unfreeze_epoch + 4u64 {
+        current_epoch = advance_epoch(&mut s, &params);
+        super::process_slashes(&mut s, current_epoch).unwrap();
+    }
+
+    // Unjail the validator
+    unjail_validator(&mut s, val_addr, current_epoch).unwrap();
+
+    // Check the validator state
+    for epoch in
+        Epoch::iter_bounds_inclusive(current_epoch, current_epoch.next())
+    {
+        assert_eq!(
+            validator_state_handle(val_addr)
+                .get(&s, epoch, &params)
+                .unwrap(),
+            Some(ValidatorState::Jailed)
+        );
+    }
+
+    assert_eq!(
+        validator_state_handle(val_addr)
+            .get(&s, current_epoch + params.pipeline_len, &params)
+            .unwrap(),
+        Some(ValidatorState::Consensus)
+    );
+    assert!(
+        validator_set_positions_handle()
+            .at(&(current_epoch + params.pipeline_len))
+            .get(&s, val_addr)
+            .unwrap()
+            .is_some(),
+    );
+
+    // Advance another epoch
+    current_epoch = advance_epoch(&mut s, &params);
+    super::process_slashes(&mut s, current_epoch).unwrap();
+
+    let second_att = unjail_validator(&mut s, val_addr, current_epoch);
+    assert!(second_att.is_err());
+}

wasm/checksums.json

@@ -1,21 +1,21 @@
 {
-    "tx_bond.wasm": "tx_bond.c607ecb20564c3fdbfa58249c1d6756440062e9474c93211e95752c32f2e4ec6.wasm",
-    "tx_bridge_pool.wasm": "tx_bridge_pool.beaae3a3f27e9190520e04a3490bf8b47f7bd60abbde13a19d10806f590b4f80.wasm",
-    "tx_change_validator_commission.wasm": "tx_change_validator_commission.aeedaa1eaf08e71316f6744e74dad9876dd6ba18814b76e65c52a0822e0eebae.wasm",
-    "tx_ibc.wasm": "tx_ibc.06f4415ea39f0a466c2eea320a678424e2833cb55784f9ae5eb317d220c804b1.wasm",
-    "tx_init_account.wasm": "tx_init_account.b56627e404986f56f5a6bee0cd969ecc139d3d264db6fa26fb53161bf6563d61.wasm",
-    "tx_init_proposal.wasm": "tx_init_proposal.f7547deabae6d16019a98ba1c1223900403257a6996775569fa5de2e0e85f27b.wasm",
-    "tx_init_validator.wasm": "tx_init_validator.26c215e40817393c2301fbcda34a658c6e1e8b3236eb4a12c22bd579b9e4f493.wasm",
-    "tx_reveal_pk.wasm": "tx_reveal_pk.5a7a1bff6a4389b23f494fa990f7f41ff5d8472762dcbb00ab87abf8cdebe8a6.wasm",
-    "tx_transfer.wasm": "tx_transfer.4aad4d35cf388cbf406ce1ebeb28fdd8003dbadfb56f3731be229b8eb3678a73.wasm",
-    "tx_unbond.wasm": "tx_unbond.c004374e239c0210120e67b7792d2acb43fdc9229de321df86e3bb3a21ca1ff7.wasm",
-    "tx_unjail_validator.wasm": "tx_unjail_validator.a4cea3d31aca79252016227c7d0bf22d775e344d0ee728e12689b504d58ca807.wasm",
-    "tx_update_account.wasm": "tx_update_account.b6577aaa999305b009845f1d949d29512c98d87fae242a51185bb23dc0d3c4de.wasm",
-    "tx_vote_proposal.wasm": "tx_vote_proposal.aed9cd152496bd5f0b03cfdc2f8e9512bfcc0d2e2f8cccefb68c83074f40177a.wasm",
-    "tx_withdraw.wasm": "tx_withdraw.ce5d8d8bc36dd81ef48fd0fe94f7487173f79f12b602a4df09b3369031f88eb6.wasm",
-    "vp_implicit.wasm": "vp_implicit.f1a1577f6f0177ce942c4452a07217904fefdedb401bcf5bebbd935ea8e560bf.wasm",
-    "vp_masp.wasm": "vp_masp.d4fd32cab1d356bceea09b051c8343ec00c84aef1af948bc233673d7f0a65375.wasm",
-    "vp_testnet_faucet.wasm": "vp_testnet_faucet.b7a815722cbfa1aa97a2bc2bb1c3f123d57415fbeb5fdb7e001b09de0fa20207.wasm",
-    "vp_user.wasm": "vp_user.6a230b5fe03a5f66cf925ee3c9c536fa6b81c4d78dd5f3f6b5a99efe8a79efcb.wasm",
-    "vp_validator.wasm": "vp_validator.15f6ddc67a785c36e305c1687879db7bb848b8ad32b9357f7c5e9ff0cc6f7e1b.wasm"
+    "tx_bond.wasm": "tx_bond.08cedac1d825cd8ee9b04ef059bc68fee04f82fec57e0f6bc38123de8fb35b85.wasm",
+    "tx_bridge_pool.wasm": "tx_bridge_pool.c9ef2bdb19910e12eadabc7f0552b45c937bcf1f47ca09759e52112b68d94114.wasm",
+    "tx_change_validator_commission.wasm": "tx_change_validator_commission.2be60082c6b891cb437b4392e1b2570f5d92fed35dd624cd079ea48c4bc7902c.wasm",
+    "tx_ibc.wasm": "tx_ibc.f8056f465d17043d2864309d1c50a897a028b6ebf9702e5c403e1bf8c670f6e6.wasm",
+    "tx_init_account.wasm": "tx_init_account.e91ab25cebd15af9a958eca4a34ba287804b1ff6ba4c82fc941f7f50816e9c4d.wasm",
+    "tx_init_proposal.wasm": "tx_init_proposal.1683eb9a2b2616af21f4f55457f7e06e0ae0591f460ede412a548ea9b1ab694c.wasm",
+    "tx_init_validator.wasm": "tx_init_validator.67574f70c1f76c2c68498d17323793366c13d119480b6787c8c5ede312e51da0.wasm",
+    "tx_reveal_pk.wasm": "tx_reveal_pk.e6375abba3f750700c06fbdeb2d5edec22b6a382116a67a7f9d7f7ba49f134e1.wasm",
+    "tx_transfer.wasm": "tx_transfer.e65b47bc94c5a09e3edc68298ad0e5e35041b91bc4d679bf5b7f433dffdce58e.wasm",
+    "tx_unbond.wasm": "tx_unbond.4fd70d297ccedb369bf88d8a8459a47ea733d329410387a6f80a0e026c6e480d.wasm",
+    "tx_unjail_validator.wasm": "tx_unjail_validator.28082f1002a97f06b52bc7a9267d1e5675a5241c5d37f7d0c58257f9fa2cc9b2.wasm",
+    "tx_update_vp.wasm": "tx_update_vp.65c5ca3e48fdef70e696460eca7540cf6196511d76fb2465133b145409329b3e.wasm",
+    "tx_vote_proposal.wasm": "tx_vote_proposal.e0a003d922230d32b741b57ca18913cbc4d5d2290f02cb37dfdaa7f27cebb486.wasm",
+    "tx_withdraw.wasm": "tx_withdraw.40499cb0e268d3cc3d9bca5dacca05d8df35f5b90adf4087a5171505472d921a.wasm",
+    "vp_implicit.wasm": "vp_implicit.57af3b7d2ee9e2c9d7ef1e33a85646abeea7ea02dad19b7e0b20b289de5a7ae9.wasm",
+    "vp_masp.wasm": "vp_masp.4d656f775b7462095e7d9ff533e346829fad02568b1cf92fa2f99cc0677720ce.wasm",
+    "vp_testnet_faucet.wasm": "vp_testnet_faucet.b0c855f0356a468db1d26baebc988508d4824836b86adb94ac432368feb03ac4.wasm",
+    "vp_user.wasm": "vp_user.31967a7411caf61729e46840eec90d2db386cf4745f49d59086a43cc3c3e2d39.wasm",
+    "vp_validator.wasm": "vp_validator.0bca8e34f5d51c74b2861d588e3dd6a8e61de7c63d356af63e2182e030ac42ee.wasm"
 }