Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minimum validator commission rate param #3831

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions crates/apps_lib/src/client/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<C: Client + Sync>(
Expand Down
2 changes: 2 additions & 0 deletions crates/apps_lib/src/config/genesis/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
}
Expand Down
2 changes: 2 additions & 0 deletions crates/apps_lib/src/config/genesis/templates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
5 changes: 4 additions & 1 deletion crates/node/src/shell/init_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,10 @@ where
eth_cold_key: &eth_cold_key.pk.raw,
eth_hot_key: &eth_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(),
Expand Down
5 changes: 5 additions & 0 deletions crates/proof_of_stake/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
)]
Expand Down
12 changes: 11 additions & 1 deletion crates/proof_of_stake/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1667,14 +1667,14 @@ where
S: StorageRead + StorageWrite,
Gov: governance::Read<S>,
{
// Check that the rate is a number between 0.0 and 1.0
if new_rate.is_negative() {
return Err(CommissionRateChangeError::NegativeRate(
new_rate,
validator.clone(),
)
.into());
}

if new_rate > Dec::one() {
return Err(CommissionRateChangeError::LargerThanOne(
new_rate,
Expand All @@ -1690,6 +1690,16 @@ where
})?;

let params = read_pos_params::<S, Gov>(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)?;

Expand Down
3 changes: 3 additions & 0 deletions crates/proof_of_stake/src/parameters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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"),
}
}
}
Expand Down
25 changes: 25 additions & 0 deletions crates/sdk/src/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down
49 changes: 49 additions & 0 deletions crates/tests/src/e2e/ledger_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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![
Expand Down
125 changes: 125 additions & 0 deletions crates/tests/src/integration/ledger_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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: [email protected]"));
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",
"[email protected]",
"--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: [email protected]"));
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));
Expand Down
2 changes: 2 additions & 0 deletions genesis/hardware/parameters.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
2 changes: 2 additions & 0 deletions genesis/localnet/parameters.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
2 changes: 2 additions & 0 deletions genesis/starter/parameters.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Loading
Loading