diff --git a/.changelog/unreleased/bug-fixes/1793-fix-unjail-bug.md b/.changelog/unreleased/bug-fixes/1793-fix-unjail-bug.md new file mode 100644 index 00000000000..11a2b1e5459 --- /dev/null +++ b/.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)) \ No newline at end of file diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs index 4c087dbbd63..d84c538571f 100644 --- a/proof_of_stake/src/lib.rs +++ b/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, - ¶ms, - )?; + let state = + validator_state_handle(validator).get(storage, epoch, ¶ms)?; if let Some(state) = state { if state != ValidatorState::Jailed { return Err(UnjailValidatorError::NotJailed( diff --git a/proof_of_stake/src/tests.rs b/proof_of_stake/src/tests.rs index 64768274171..b7463c8ea58 100644 --- a/proof_of_stake/src/tests.rs +++ b/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, val_size: Range, @@ -2112,3 +2129,127 @@ fn arb_genesis_validators( }, ) } + +fn test_unjail_validator_aux( + params: PosParams, + mut validators: Vec, +) { + 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, + ¶ms, + validators.clone().into_iter(), + current_epoch, + ) + .unwrap(); + s.commit_block().unwrap(); + + current_epoch = advance_epoch(&mut s, ¶ms); + 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, + ¶ms, + 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, ¶ms) + .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, ¶ms) + .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, ¶ms); + 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, ¶ms) + .unwrap(), + Some(ValidatorState::Jailed) + ); + } + + assert_eq!( + validator_state_handle(val_addr) + .get(&s, current_epoch + params.pipeline_len, ¶ms) + .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, ¶ms); + super::process_slashes(&mut s, current_epoch).unwrap(); + + let second_att = unjail_validator(&mut s, val_addr, current_epoch); + assert!(second_att.is_err()); +} diff --git a/wasm/checksums.json b/wasm/checksums.json index c2d5cc099b2..29b6d2ea6f3 100644 --- a/wasm/checksums.json +++ b/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" } \ No newline at end of file