diff --git a/crates/apps_lib/src/client/rpc.rs b/crates/apps_lib/src/client/rpc.rs index bb44f82881..d29c0c03fd 100644 --- a/crates/apps_lib/src/client/rpc.rs +++ b/crates/apps_lib/src/client/rpc.rs @@ -830,6 +830,7 @@ pub async fn query_protocol_parameters( liveness_threshold, rewards_gain_p, rewards_gain_d, + min_commission_rate, }, max_proposal_period: _, } = query_pos_parameters(context.client()).await; @@ -929,6 +930,12 @@ pub async fn query_protocol_parameters( "", tm_votes_per_token ); + display_line!( + context.io(), + "{:4}Minimum allowable validator commission rate: {}", + "", + min_commission_rate + ); } pub async fn query_bond( diff --git a/crates/apps_lib/src/config/genesis/chain.rs b/crates/apps_lib/src/config/genesis/chain.rs index ac4ad408c4..628b887a3d 100644 --- a/crates/apps_lib/src/config/genesis/chain.rs +++ b/crates/apps_lib/src/config/genesis/chain.rs @@ -400,6 +400,7 @@ impl Finalized { liveness_threshold, rewards_gain_p, rewards_gain_d, + min_commission_rate, } = self.parameters.pos_params.clone(); namada_sdk::proof_of_stake::parameters::PosParams { @@ -420,6 +421,7 @@ impl Finalized { liveness_threshold, rewards_gain_p, rewards_gain_d, + min_commission_rate, }, max_proposal_period: self.parameters.gov_params.max_proposal_period, } diff --git a/crates/apps_lib/src/config/genesis/templates.rs b/crates/apps_lib/src/config/genesis/templates.rs index 5e3aaf881e..7170bac033 100644 --- a/crates/apps_lib/src/config/genesis/templates.rs +++ b/crates/apps_lib/src/config/genesis/templates.rs @@ -415,6 +415,8 @@ pub struct PosParams { pub rewards_gain_p: Dec, /// PoS gain d (read only) pub rewards_gain_d: Dec, + /// Minimum validator commission rate + pub min_commission_rate: Dec, } #[derive( diff --git a/crates/node/src/shell/init_chain.rs b/crates/node/src/shell/init_chain.rs index 05624f1871..6a5bcb5cbc 100644 --- a/crates/node/src/shell/init_chain.rs +++ b/crates/node/src/shell/init_chain.rs @@ -635,7 +635,10 @@ where eth_cold_key: ð_cold_key.pk.raw, eth_hot_key: ð_hot_key.pk.raw, current_epoch, - commission_rate: *commission_rate, + commission_rate: std::cmp::max( + *commission_rate, + params.min_commission_rate, + ), max_commission_rate_change: *max_commission_rate_change, metadata: metadata.clone(), diff --git a/crates/proof_of_stake/src/error.rs b/crates/proof_of_stake/src/error.rs index 40b1b46592..fa6cfbf499 100644 --- a/crates/proof_of_stake/src/error.rs +++ b/crates/proof_of_stake/src/error.rs @@ -91,6 +91,11 @@ pub enum CommissionRateChangeError { LargerThanOne(Dec, Address), #[error("Rate change of {0} is too large for validator {1}")] RateChangeTooLarge(Dec, Address), + #[error( + "Invalid commission rate of {0}; the minimum allowed by the protocol \ + is {1}" + )] + RateBelowMin(Dec, Dec), #[error( "There is no maximum rate change written in storage for validator {0}" )] diff --git a/crates/proof_of_stake/src/lib.rs b/crates/proof_of_stake/src/lib.rs index c93126c44a..10a0948040 100644 --- a/crates/proof_of_stake/src/lib.rs +++ b/crates/proof_of_stake/src/lib.rs @@ -1667,6 +1667,7 @@ where S: StorageRead + StorageWrite, Gov: governance::Read, { + // Check that the rate is a number between 0.0 and 1.0 if new_rate.is_negative() { return Err(CommissionRateChangeError::NegativeRate( new_rate, @@ -1674,7 +1675,6 @@ where ) .into()); } - if new_rate > Dec::one() { return Err(CommissionRateChangeError::LargerThanOne( new_rate, @@ -1690,6 +1690,16 @@ where })?; let params = read_pos_params::(storage)?; + + // Check that the new rate is allowed by the min commission rate + if new_rate < params.min_commission_rate { + return Err(CommissionRateChangeError::RateBelowMin( + new_rate, + params.min_commission_rate, + ) + .into()); + } + let commission_handle = validator_commission_rate_handle(validator); let pipeline_epoch = checked!(current_epoch + params.pipeline_len)?; diff --git a/crates/proof_of_stake/src/parameters.rs b/crates/proof_of_stake/src/parameters.rs index 1e1032dbe5..88286b77eb 100644 --- a/crates/proof_of_stake/src/parameters.rs +++ b/crates/proof_of_stake/src/parameters.rs @@ -80,6 +80,8 @@ pub struct OwnedPosParams { pub rewards_gain_p: Dec, /// PoS gain d (read only) pub rewards_gain_d: Dec, + /// Minimum validator commission rate + pub min_commission_rate: Dec, } impl Default for OwnedPosParams { @@ -108,6 +110,7 @@ impl Default for OwnedPosParams { liveness_threshold: Dec::new(9, 1).expect("Test failed"), rewards_gain_p: Dec::from_str("0.25").expect("Test failed"), rewards_gain_d: Dec::from_str("0.25").expect("Test failed"), + min_commission_rate: Dec::from_str("0.05").expect("Test failed"), } } } diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 9e74fd3cc5..fd98b5fe5c 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -636,6 +636,18 @@ pub async fn build_validator_commission_change( *rate, ))); } + if *rate < params.min_commission_rate { + edisplay_line!( + context.io(), + "New rate is below the minimum allowed by the protocol: {}", + params.min_commission_rate + ); + if !tx_args.force { + return Err(Error::from(TxSubmitError::InvalidCommissionRate( + *rate, + ))); + } + } let pipeline_epoch_minus_one = epoch.unchecked_add(params.pipeline_len - 1); @@ -873,6 +885,19 @@ pub async fn build_validator_metadata_change( ))); } } + if *rate < params.min_commission_rate { + edisplay_line!( + context.io(), + "New rate is below the minimum allowed by the protocol: {}", + params.min_commission_rate + ); + if !tx_args.force { + return Err(Error::from(TxSubmitError::InvalidCommissionRate( + *rate, + ))); + } + } + let pipeline_epoch_minus_one = epoch.unchecked_add(params.pipeline_len - 1); diff --git a/crates/tests/src/e2e/ledger_tests.rs b/crates/tests/src/e2e/ledger_tests.rs index dceed2b2ac..af8bb62972 100644 --- a/crates/tests/src/e2e/ledger_tests.rs +++ b/crates/tests/src/e2e/ledger_tests.rs @@ -29,9 +29,11 @@ use namada_core::chain::ChainId; use namada_core::token::NATIVE_MAX_DECIMAL_PLACES; use namada_sdk::address::Address; use namada_sdk::chain::Epoch; +use namada_sdk::dec::Dec; use namada_sdk::time::DateTimeUtc; use namada_sdk::token; use namada_test_utils::TestWasms; +use regex::Regex; use serde::Serialize; use serde_json::json; use setup::constants::*; @@ -503,10 +505,13 @@ fn stop_ledger_at_height() -> Result<()> { fn pos_bonds() -> Result<()> { let pipeline_len = 2; let unbonding_len = 4; + let min_commission_rate = Dec::from_str("0.1").unwrap(); let test = setup::network( |mut genesis, base_dir: &_| { genesis.parameters.pos_params.pipeline_len = pipeline_len; genesis.parameters.pos_params.unbonding_len = unbonding_len; + genesis.parameters.pos_params.min_commission_rate = + min_commission_rate; genesis.parameters.parameters.min_num_of_blocks = 6; genesis.parameters.parameters.epochs_per_year = 31_536_000; let mut genesis = setup::set_validators( @@ -576,7 +581,51 @@ fn pos_bonds() -> Result<()> { let rpc = get_actor_rpc(&test, Who::Validator(0)); wait_for_block_height(&test, &rpc, 2, 30)?; + // 1.1 Query the validator commission rate (should be the min parameter) let validator_0_rpc = get_actor_rpc(&test, Who::Validator(0)); + let tx_args = vec![ + "commission-rate", + "--validator", + "validator-0-validator", + "--node", + &validator_0_rpc, + ]; + let mut client = + run_as!(test, Who::Validator(0), Bin::Client, tx_args, Some(40))?; + + let str1 = client.exp_regex(r".* commission rate: [0-1]\.[0-9]*,.*")?.1; + client.assert_success(); + + let rate1 = Regex::new(r"commission rate: ([0-1]\.[0-9]+),")? + .captures(&str1) + .unwrap() + .get(1) + .unwrap() + .as_str(); + let rate1 = Dec::from_str(rate1).unwrap(); + assert_eq!(rate1, min_commission_rate); + + let tx_args = vec![ + "commission-rate", + "--validator", + "validator-1-validator", + "--node", + &validator_0_rpc, + ]; + let mut client = + run_as!(test, Who::Validator(1), Bin::Client, tx_args, Some(40))?; + + let str2 = client.exp_regex(r".* commission rate: [0-1]\.[0-9]*,.*")?.1; + client.assert_success(); + + let rate2 = Regex::new(r"commission rate: ([0-1]\.[0-9]+),")? + .captures(&str2) + .unwrap() + .get(1) + .unwrap() + .as_str(); + let rate2 = Dec::from_str(rate2).unwrap(); + assert_eq!(rate1, rate2); // 2. Submit a self-bond for the first genesis validator let tx_args = vec![ diff --git a/crates/tests/src/integration/ledger_tests.rs b/crates/tests/src/integration/ledger_tests.rs index 7d6322d448..cf1146df9e 100644 --- a/crates/tests/src/integration/ledger_tests.rs +++ b/crates/tests/src/integration/ledger_tests.rs @@ -1666,6 +1666,131 @@ fn change_validator_metadata() -> Result<()> { assert_matches!(captured.result, Ok(_)); assert!(captured.contains(TX_APPLIED_SUCCESS)); + // 6. Query the metadata to see that the validator website is removed + let captured = + CapturedOutput::of(|| run(&node, Bin::Client, metadata_query_args)); + assert_matches!(captured.result, Ok(_)); + assert!(captured.contains("Email: theokayestvalidator@namada.net")); + assert!(captured.contains( + "Description: We are just an okay validator node trying to get by" + )); + assert!(captured.contains("No website")); + assert!(captured.contains("No discord handle")); + assert!(captured.contains("commission rate:")); + assert!(captured.contains("max change per epoch:")); + + // 6. Try to change the validator commission below the minimum + let commission_change_args = apply_use_device(vec![ + "change-commission-rate", + "--validator", + "validator-0-validator", + "--commission-rate", + "0.01", + "--node", + &validator_one_rpc, + ]); + let captured = + CapturedOutput::of(|| run(&node, Bin::Client, commission_change_args)); + assert_matches!(captured.result, Err(_)); + + // 7. Query to ensure it hasn't changed + let commission_change_args = apply_use_device(vec![ + "commission-rate", + "--validator", + "validator-0-validator", + "--node", + &validator_one_rpc, + ]); + let captured = + CapturedOutput::of(|| run(&node, Bin::Client, commission_change_args)); + assert_matches!(captured.result, Ok(_)); + assert!( + captured.contains("commission rate: 0.05, max change per epoch: 0.01") + ); + + Ok(()) +} + +/// Change validator metadata +#[test] +fn check_validator_commission() -> Result<()> { + // This address doesn't matter for tests. But an argument is required. + let validator_one_rpc = "http://127.0.0.1:26567"; + // 1. start the ledger node + let (node, _services) = setup::setup()?; + + // 2. Query the validator metadata loaded from genesis + let metadata_query_args = vec![ + "validator-metadata", + "--validator", + "validator-0-validator", + "--node", + &validator_one_rpc, + ]; + let captured = CapturedOutput::of(|| { + run(&node, Bin::Client, metadata_query_args.clone()) + }); + assert_matches!(captured.result, Ok(_)); + assert!(captured.contains("No validator name")); + assert!(captured.contains("Email:")); + assert!(captured.contains("No description")); + assert!(captured.contains("No website")); + assert!(captured.contains("No discord handle")); + assert!(captured.contains("commission rate:")); + assert!(captured.contains("max change per epoch:")); + + // 3. Add some metadata to the validator + let metadata_change_args = apply_use_device(vec![ + "change-metadata", + "--validator", + "validator-0-validator", + "--name", + "theokayestvalidator", + "--email", + "theokayestvalidator@namada.net", + "--description", + "We are just an okay validator node trying to get by", + "--website", + "theokayestvalidator.com", + "--node", + &validator_one_rpc, + ]); + + let captured = + CapturedOutput::of(|| run(&node, Bin::Client, metadata_change_args)); + assert_matches!(captured.result, Ok(_)); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + + // 4. Query the metadata after the change + let captured = CapturedOutput::of(|| { + run(&node, Bin::Client, metadata_query_args.clone()) + }); + assert_matches!(captured.result, Ok(_)); + assert!(captured.contains("Validator name: theokayestvalidator")); + assert!(captured.contains("Email: theokayestvalidator@namada.net")); + assert!(captured.contains( + "Description: We are just an okay validator node trying to get by" + )); + assert!(captured.contains("Website: theokayestvalidator.com")); + assert!(captured.contains("No discord handle")); + assert!(captured.contains("commission rate:")); + assert!(captured.contains("max change per epoch:")); + + // 5. Remove the validator website + let metadata_change_args = apply_use_device(vec![ + "change-metadata", + "--validator", + "validator-0-validator", + "--website", + "", + "--node", + &validator_one_rpc, + ]); + let captured = + CapturedOutput::of(|| run(&node, Bin::Client, metadata_change_args)); + assert_matches!(captured.result, Ok(_)); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + // 6. Query the metadata to see that the validator website is removed let captured = CapturedOutput::of(|| run(&node, Bin::Client, metadata_query_args)); diff --git a/genesis/hardware/parameters.toml b/genesis/hardware/parameters.toml index cda15160aa..ffd3abdad5 100644 --- a/genesis/hardware/parameters.toml +++ b/genesis/hardware/parameters.toml @@ -71,6 +71,8 @@ liveness_threshold = "0.9" rewards_gain_p = "0.25" # The D gain factor in the Proof of Stake rewards controller rewards_gain_d = "0.25" +# Minimum allowable validator commission rate +min_commission_rate = "0.05" # Governance parameters. [gov_params] diff --git a/genesis/localnet/parameters.toml b/genesis/localnet/parameters.toml index 161e4da0ab..f843bdd17f 100644 --- a/genesis/localnet/parameters.toml +++ b/genesis/localnet/parameters.toml @@ -71,6 +71,8 @@ liveness_threshold = "0.9" rewards_gain_p = "0.25" # The D gain factor in the Proof of Stake rewards controller rewards_gain_d = "0.25" +# Minimum allowable validator commission rate +min_commission_rate = "0.05" # Governance parameters. [gov_params] diff --git a/genesis/starter/parameters.toml b/genesis/starter/parameters.toml index 79b53b6b1c..1a09c1c8cc 100644 --- a/genesis/starter/parameters.toml +++ b/genesis/starter/parameters.toml @@ -71,6 +71,8 @@ liveness_threshold = "0.9" rewards_gain_p = "0.25" # The D gain factor in the Proof of Stake rewards controller rewards_gain_d = "0.25" +# Minimum allowable validator commission rate +min_commission_rate = "0.05" # Governance parameters. [gov_params] diff --git a/wasm/tx_change_validator_commission/src/lib.rs b/wasm/tx_change_validator_commission/src/lib.rs index bc30b89ebd..5dd14e0c24 100644 --- a/wasm/tx_change_validator_commission/src/lib.rs +++ b/wasm/tx_change_validator_commission/src/lib.rs @@ -114,7 +114,11 @@ mod tests { assert_eq!(commission_rates_pre[0], Some(initial_rate)); - apply_tx(ctx(), signed_tx.batch_first_tx())?; + let res = apply_tx(ctx(), signed_tx.batch_first_tx()); + if commission_change.new_rate < pos_params.min_commission_rate { + assert!(res.is_err()); + return Ok(()); + } // Read the data after the tx is executed