From 22d845bd33aef987538016831195057cddfffc33 Mon Sep 17 00:00:00 2001 From: brentstone Date: Tue, 30 Aug 2022 20:29:47 +0300 Subject: [PATCH 01/38] replace floating point arithm from token module with rust_decimal --- proof_of_stake/src/parameters.rs | 5 ++++- shared/src/types/token.rs | 29 ++++++++++++----------------- tests/src/native_vp/pos.rs | 18 +++++++++++------- 3 files changed, 27 insertions(+), 25 deletions(-) diff --git a/proof_of_stake/src/parameters.rs b/proof_of_stake/src/parameters.rs index 7ee0abdf98..1c265bf1a8 100644 --- a/proof_of_stake/src/parameters.rs +++ b/proof_of_stake/src/parameters.rs @@ -71,11 +71,14 @@ pub enum ValidationError { UnbondingLenTooShort(u64, u64), } +/// The number of fundamental units per whole token of the native staking token +pub const TOKENS_PER_NAM: u64 = 1_000_000; + /// From Tendermint: const MAX_TOTAL_VOTING_POWER: i64 = i64::MAX / 8; /// Assuming token amount is `u64` in micro units. -const TOKEN_MAX_AMOUNT: u64 = u64::MAX / 1_000_000; +const TOKEN_MAX_AMOUNT: u64 = u64::MAX / TOKENS_PER_NAM; impl PosParams { /// Validate PoS parameters values. Returns an empty list if the values are diff --git a/shared/src/types/token.rs b/shared/src/types/token.rs index 787a8855dc..b19642b85a 100644 --- a/shared/src/types/token.rs +++ b/shared/src/types/token.rs @@ -6,6 +6,7 @@ use std::ops::{Add, AddAssign, Mul, Sub, SubAssign}; use std::str::FromStr; use borsh::{BorshDeserialize, BorshSchema, BorshSerialize}; +use rust_decimal::prelude::{Decimal, ToPrimitive}; use serde::{Deserialize, Serialize}; use thiserror::Error; @@ -37,7 +38,6 @@ pub struct Amount { pub const MAX_DECIMAL_PLACES: u32 = 6; /// Decimal scale of token [`Amount`] and [`Change`]. pub const SCALE: u64 = 1_000_000; -const SCALE_F64: f64 = SCALE as f64; /// A change in tokens amount pub type Change = i128; @@ -109,21 +109,16 @@ impl<'de> serde::Deserialize<'de> for Amount { } } -impl From for f64 { - /// Warning: `f64` loses precision and it should not be used when exact - /// values are required. +impl From for Decimal { fn from(amount: Amount) -> Self { - amount.micro as f64 / SCALE_F64 + Into::::into(amount.micro) / Into::::into(SCALE) } } -impl From for Amount { - /// Warning: `f64` loses precision and it should not be used when exact - /// values are required. - fn from(micro: f64) -> Self { - Self { - micro: (micro * SCALE_F64).round() as u64, - } +impl From for Amount { + fn from(micro: Decimal) -> Self { + let res = (micro * Into::::into(SCALE)).to_u64().unwrap(); + Self { micro: res } } } @@ -205,7 +200,7 @@ impl FromStr for Amount { match rust_decimal::Decimal::from_str(s) { Ok(decimal) => { let scale = decimal.scale(); - if scale > 6 { + if scale > MAX_DECIMAL_PLACES { return Err(AmountParseError::ScaleTooLarge(scale)); } let whole = @@ -440,11 +435,11 @@ mod tests { /// The upper limit is set to `2^51`, because then the float is /// starting to lose precision. #[test] - fn test_token_amount_f64_conversion(raw_amount in 0..2_u64.pow(51)) { + fn test_token_amount_decimal_conversion(raw_amount in 0..2_u64.pow(51)) { let amount = Amount::from(raw_amount); - // A round-trip conversion to and from f64 should be an identity - let float = f64::from(amount); - let identity = Amount::from(float); + // A round-trip conversion to and from Decimal should be an identity + let decimal = Decimal::from(amount); + let identity = Amount::from(decimal); assert_eq!(amount, identity); } } diff --git a/tests/src/native_vp/pos.rs b/tests/src/native_vp/pos.rs index ec6f75a15f..61b859a7d6 100644 --- a/tests/src/native_vp/pos.rs +++ b/tests/src/native_vp/pos.rs @@ -594,6 +594,10 @@ pub mod testing { use crate::tx::{self, tx_host_env}; + const TOKENS_PER_NAM: i128 = + namada::ledger::pos::namada_proof_of_stake::parameters::TOKENS_PER_NAM + as i128; + #[derive(Clone, Debug, Default)] pub struct TestValidator { pub address: Option
, @@ -940,9 +944,9 @@ pub mod testing { // We convert the tokens from micro units to whole tokens // with division by 10^6 let vp_before = - params.votes_per_token * ((total_delta) / 1_000_000); + params.votes_per_token * (total_delta / TOKENS_PER_NAM); let vp_after = params.votes_per_token - * ((total_delta + token_delta) / 1_000_000); + * ((total_delta + token_delta) / TOKENS_PER_NAM); // voting power delta let vp_delta = vp_after - vp_before; @@ -1001,12 +1005,12 @@ pub mod testing { let total_delta = validator_total_deltas .get(epoch) .unwrap_or_default(); - // We convert the tokens from micro units to whole + // We convert the tokens from micro units to whole // tokens with division by 10^6 let vp_before = params.votes_per_token - * ((total_delta) / 1_000_000); + * (total_delta / TOKENS_PER_NAM); let vp_after = params.votes_per_token - * ((total_delta + token_delta) / 1_000_000); + * ((total_delta + token_delta) / TOKENS_PER_NAM); // voting power delta let vp_delta_at_unbonding = vp_after - vp_before - vp_delta - total_vp_delta; @@ -1080,9 +1084,9 @@ pub mod testing { // We convert the tokens from micro units to whole tokens // with division by 10^6 let vp_before = params.votes_per_token - * ((total_delta_cur) / 1_000_000); + * (total_delta_cur / TOKENS_PER_NAM); let vp_after = params.votes_per_token - * ((total_delta_cur + token_delta) / 1_000_000); + * ((total_delta_cur + token_delta) / TOKENS_PER_NAM); // voting power delta let vp_delta = vp_after - vp_before; From 97523f47f0394af9eebb0ec420beb74466be8ca5 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 26 Oct 2022 09:22:00 +0000 Subject: [PATCH 02/38] [ci] wasm checksums update --- wasm/checksums.json | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/wasm/checksums.json b/wasm/checksums.json index 496d1c7a0f..22dc699c30 100644 --- a/wasm/checksums.json +++ b/wasm/checksums.json @@ -1,15 +1,18 @@ { - "tx_bond.wasm": "tx_bond.04d6847800dad11990b42e8f2981a4a79d06d6d0c981c3d70c929e5b6a4f348b.wasm", - "tx_ibc.wasm": "tx_ibc.6ab530398ed8e276a8af7f231edbfae984b7e84eeb854714ba9339c5bed9d330.wasm", - "tx_init_account.wasm": "tx_init_account.578d987351e6ae42baa7849ae167e3ba33f3a62dba51cd47b0fa6d3ea6e4f128.wasm", - "tx_init_proposal.wasm": "tx_init_proposal.71e27610210622fa53c3de58351761cca839681a4f450d4eff6b46bde3ae85a5.wasm", - "tx_init_validator.wasm": "tx_init_validator.269f065ff683782db2fdcac6e2485e80cbebb98929671a42eeb01703e0bbd8f5.wasm", - "tx_transfer.wasm": "tx_transfer.784325cf7763faf8d75797960cda6fbabbd343f3c6f7e6785f60f5e0911a6bb5.wasm", - "tx_unbond.wasm": "tx_unbond.ed13fa636d138ac4e35f2b4f31a6b4d3bed67e6b998dc6325f90711a2aca3704.wasm", - "tx_update_vp.wasm": "tx_update_vp.c4050e597116203eba5afde946a014afb067bdeaaae417377214a80c38a3786b.wasm", - "tx_vote_proposal.wasm": "tx_vote_proposal.ece325881aad1c8a29f715c2f435c3335e08e51eed837c00ce0f7bbaddbefe50.wasm", - "tx_withdraw.wasm": "tx_withdraw.408fc10b3744c398258124e5e48e3449f6baf82a263df26911586a3382fbceb9.wasm", - "vp_testnet_faucet.wasm": "vp_testnet_faucet.ae9a681dc2c1bd244b0575474fa4a364af56fa75833950693ca52ab25018c97d.wasm", - "vp_token.wasm": "vp_token.468de153dc5ce3af208bd762de3e85be48bc631012ec5f0947af95168da6cb93.wasm", - "vp_user.wasm": "vp_user.c101016a85a72f40da7f33e5d9061cfd2e3274eaac75a71c59c9ab4ed9896ffd.wasm" -} \ No newline at end of file + "tx_bond.wasm": "tx_bond.059b1256240d15a64cf419f3a2d24af1496211c386d85acc3733095bc2e5da9b.wasm", + "tx_ibc.wasm": "tx_ibc.4eeb30bc1e6a32b8efe8958eab568082a238db01eb98340f28a9fa41371a3753.wasm", + "tx_init_account.wasm": "tx_init_account.85d017ac76e51f359fa07e753c0e6fcbd3341e7661492cbf2801cf3c41480dd4.wasm", + "tx_init_nft.wasm": "tx_init_nft.fbeb1687a364b2c249c9fd69588ff0985bd9c1f8f4c93e5328de1e9ba527d991.wasm", + "tx_init_proposal.wasm": "tx_init_proposal.43b52414649bb0fec86dfb35d001656c1825bc5906e450e8b0c3a60aaa5f3d45.wasm", + "tx_init_validator.wasm": "tx_init_validator.6ccb7fcf246cb7a2f97a5dfdcefe16ee1add72a832081c2572adc2d7a355cf56.wasm", + "tx_mint_nft.wasm": "tx_mint_nft.f2ed21521c676f04be4c6278cc60bec83265c3750437c87d9ea681b830767d71.wasm", + "tx_transfer.wasm": "tx_transfer.b6fc342f4a76918874e6d037a3864e4369dbba7cd7d558622e7a723e3d854da3.wasm", + "tx_unbond.wasm": "tx_unbond.6e7316d08bf8ab9a6fb1889f64a5a2265ee0399661dbb48e33555170545d1c7c.wasm", + "tx_update_vp.wasm": "tx_update_vp.6d291dadb43545a809ba33fe26582b7984c67c65f05e363a93dbc62e06a33484.wasm", + "tx_vote_proposal.wasm": "tx_vote_proposal.d0a87d58f64f46586ae3d83852deee269e22520f4780875c05aaf1731044c356.wasm", + "tx_withdraw.wasm": "tx_withdraw.e5dcc5ef2362018c1fa5ea02912528ee929aa7b6fefcf06f4ccf7509bfa52283.wasm", + "vp_nft.wasm": "vp_nft.9be5a821bc7b3075b917e8ead45893502d82cc7417e6af50dfd3f6baf36243e0.wasm", + "vp_testnet_faucet.wasm": "vp_testnet_faucet.8e2e45ff165d40dc8249188aca108e5cba86ac5dd1cd989b0237cadd4b66bfdf.wasm", + "vp_token.wasm": "vp_token.4a0446f20e7436de1e889c640a11644d1a1295c4d29e45b24582df2b9ed3176e.wasm", + "vp_user.wasm": "vp_user.eb1d6f1f524c28571ad0f21f75371aa635257313cea2702b9a70e5022fe6c3ef.wasm" +} From d08da6ecb02dff4894599ccf6b2f1e43ae939b8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Wed, 26 Oct 2022 17:07:19 +0200 Subject: [PATCH 03/38] changelog: #436 --- .changelog/unreleased/improvements/436-remove-f64.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/436-remove-f64.md diff --git a/.changelog/unreleased/improvements/436-remove-f64.md b/.changelog/unreleased/improvements/436-remove-f64.md new file mode 100644 index 0000000000..e55af7ee8f --- /dev/null +++ b/.changelog/unreleased/improvements/436-remove-f64.md @@ -0,0 +1,2 @@ +- Refactored token decimal formatting. + ([#436](https://github.com/anoma/namada/pull/436)) \ No newline at end of file From 29eb68f24c3426850a3e42cc5755ff95e1525a7a Mon Sep 17 00:00:00 2001 From: brentstone Date: Tue, 25 Oct 2022 19:57:07 -0400 Subject: [PATCH 04/38] remove staking reward address from all code --- apps/src/lib/cli.rs | 17 ------- apps/src/lib/client/tx.rs | 32 ------------ apps/src/lib/client/utils.rs | 27 ----------- apps/src/lib/config/genesis.rs | 52 +------------------- proof_of_stake/src/lib.rs | 59 ----------------------- proof_of_stake/src/types.rs | 5 -- proof_of_stake/src/validation.rs | 57 ++-------------------- shared/src/ledger/pos/mod.rs | 11 ----- shared/src/ledger/pos/storage.rs | 52 -------------------- shared/src/ledger/pos/vp.rs | 21 +------- shared/src/types/transaction/mod.rs | 6 --- tests/src/e2e/ledger_tests.rs | 1 - tests/src/native_vp/pos.rs | 23 +-------- tx_prelude/src/proof_of_stake.rs | 25 ++-------- wasm/wasm_source/src/tx_bond.rs | 4 -- wasm/wasm_source/src/tx_init_validator.rs | 8 +-- wasm/wasm_source/src/tx_unbond.rs | 4 -- wasm/wasm_source/src/tx_withdraw.rs | 4 -- 18 files changed, 13 insertions(+), 395 deletions(-) diff --git a/apps/src/lib/cli.rs b/apps/src/lib/cli.rs index a3d39ed73e..093759bb96 100644 --- a/apps/src/lib/cli.rs +++ b/apps/src/lib/cli.rs @@ -1289,8 +1289,6 @@ pub mod args { const RAW_ADDRESS: Arg
= arg("address"); const RAW_ADDRESS_OPT: ArgOpt
= RAW_ADDRESS.opt(); const RAW_PUBLIC_KEY_OPT: ArgOpt = arg_opt("public-key"); - const REWARDS_CODE_PATH: ArgOpt = arg_opt("rewards-code-path"); - const REWARDS_KEY: ArgOpt = arg_opt("rewards-key"); const SCHEME: ArgDefault = arg_default("scheme", DefaultFn(|| SchemeType::Ed25519)); const SIGNER: ArgOpt = arg_opt("signer"); @@ -1528,10 +1526,8 @@ pub mod args { pub scheme: SchemeType, pub account_key: Option, pub consensus_key: Option, - pub rewards_account_key: Option, pub protocol_key: Option, pub validator_vp_code_path: Option, - pub rewards_vp_code_path: Option, pub unsafe_dont_encrypt: bool, } @@ -1542,10 +1538,8 @@ pub mod args { let scheme = SCHEME.parse(matches); let account_key = VALIDATOR_ACCOUNT_KEY.parse(matches); let consensus_key = VALIDATOR_CONSENSUS_KEY.parse(matches); - let rewards_account_key = REWARDS_KEY.parse(matches); let protocol_key = PROTOCOL_KEY.parse(matches); let validator_vp_code_path = VALIDATOR_CODE_PATH.parse(matches); - let rewards_vp_code_path = REWARDS_CODE_PATH.parse(matches); let unsafe_dont_encrypt = UNSAFE_DONT_ENCRYPT.parse(matches); Self { tx, @@ -1553,10 +1547,8 @@ pub mod args { scheme, account_key, consensus_key, - rewards_account_key, protocol_key, validator_vp_code_path, - rewards_vp_code_path, unsafe_dont_encrypt, } } @@ -1578,10 +1570,6 @@ pub mod args { "A consensus key for the validator account. A new one \ will be generated if none given.", )) - .arg(REWARDS_KEY.def().about( - "A public key for the staking reward account. A new one \ - will be generated if none given.", - )) .arg(PROTOCOL_KEY.def().about( "A public key for signing protocol transactions. A new \ one will be generated if none given.", @@ -1591,11 +1579,6 @@ pub mod args { for the validator account. Uses the default validator VP \ if none specified.", )) - .arg(REWARDS_CODE_PATH.def().about( - "The path to the validity predicate WASM code to be used \ - for the staking reward account. Uses the default staking \ - reward VP if none specified.", - )) .arg(UNSAFE_DONT_ENCRYPT.def().about( "UNSAFE: Do not encrypt the generated keypairs. Do not \ use this for keys used in a live network.", diff --git a/apps/src/lib/client/tx.rs b/apps/src/lib/client/tx.rs index 0d369ba6b7..c193810fdd 100644 --- a/apps/src/lib/client/tx.rs +++ b/apps/src/lib/client/tx.rs @@ -154,10 +154,8 @@ pub async fn submit_init_validator( scheme, account_key, consensus_key, - rewards_account_key, protocol_key, validator_vp_code_path, - rewards_vp_code_path, unsafe_dont_encrypt, }: args::TxInitValidator, ) { @@ -169,7 +167,6 @@ pub async fn submit_init_validator( let validator_key_alias = format!("{}-key", alias); let consensus_key_alias = format!("{}-consensus-key", alias); - let rewards_key_alias = format!("{}-rewards-key", alias); let account_key = ctx.get_opt_cached(&account_key).unwrap_or_else(|| { println!("Generating validator account key..."); ctx.wallet @@ -203,18 +200,6 @@ pub async fn submit_init_validator( .1 }); - let rewards_account_key = - ctx.get_opt_cached(&rewards_account_key).unwrap_or_else(|| { - println!("Generating staking reward account key..."); - ctx.wallet - .gen_key( - scheme, - Some(rewards_key_alias.clone()), - unsafe_dont_encrypt, - ) - .1 - .ref_to() - }); let protocol_key = ctx.get_opt_cached(&protocol_key); if protocol_key.is_none() { @@ -245,30 +230,14 @@ pub async fn submit_init_validator( safe_exit(1) } } - let rewards_vp_code = rewards_vp_code_path - .map(|path| ctx.read_wasm(path)) - .unwrap_or_else(|| ctx.read_wasm(VP_USER_WASM)); - // Validate the rewards VP code - if let Err(err) = vm::validate_untrusted_wasm(&rewards_vp_code) { - eprintln!( - "Staking reward account validity predicate code validation failed \ - with {}", - err - ); - if !tx_args.force { - safe_exit(1) - } - } let tx_code = ctx.read_wasm(TX_INIT_VALIDATOR_WASM); let data = InitValidator { account_key, consensus_key: consensus_key.ref_to(), - rewards_account_key, protocol_key, dkg_key, validator_vp_code, - rewards_vp_code, }; let data = data.try_to_vec().expect("Encoding tx data shouldn't fail"); let tx = Tx::new(tx_code, Some(data)); @@ -364,7 +333,6 @@ pub async fn submit_init_validator( println!(" Staking reward address \"{}\"", rewards_address_alias); println!(" Validator account key \"{}\"", validator_key_alias); println!(" Consensus key \"{}\"", consensus_key_alias); - println!(" Staking reward key \"{}\"", rewards_key_alias); println!( "The ledger node has been setup to use this validator's address \ and consensus key." diff --git a/apps/src/lib/client/utils.rs b/apps/src/lib/client/utils.rs index 8848726792..2c0f54dd91 100644 --- a/apps/src/lib/client/utils.rs +++ b/apps/src/lib/client/utils.rs @@ -475,10 +475,7 @@ pub fn init_network( // Generate account and reward addresses let address = address::gen_established_address("validator account"); - let reward_address = - address::gen_established_address("validator reward account"); config.address = Some(address.to_string()); - config.staking_reward_address = Some(reward_address.to_string()); // Generate the consensus, account and reward keys, unless they're // pre-defined. @@ -518,24 +515,6 @@ pub fn init_network( keypair.ref_to() }); - let staking_reward_pk = try_parse_public_key( - format!("validator {name} staking reward key"), - &config.staking_reward_public_key, - ) - .unwrap_or_else(|| { - let alias = format!("{}-reward-key", name); - println!( - "Generating validator {} staking reward account key...", - name - ); - let (_alias, keypair) = wallet.gen_key( - SchemeType::Ed25519, - Some(alias), - unsafe_dont_encrypt, - ); - keypair.ref_to() - }); - let protocol_pk = try_parse_public_key( format!("validator {name} protocol key"), &config.protocol_public_key, @@ -583,8 +562,6 @@ pub fn init_network( Some(genesis_config::HexString(consensus_pk.to_string())); config.account_public_key = Some(genesis_config::HexString(account_pk.to_string())); - config.staking_reward_public_key = - Some(genesis_config::HexString(staking_reward_pk.to_string())); config.protocol_public_key = Some(genesis_config::HexString(protocol_pk.to_string())); @@ -593,7 +570,6 @@ pub fn init_network( // Write keypairs to wallet wallet.add_address(name.clone(), address); - wallet.add_address(format!("{}-reward", &name), reward_address); wallet.save().unwrap(); }); @@ -940,9 +916,6 @@ pub fn init_genesis_validator( account_public_key: Some(HexString( pre_genesis.account_key.ref_to().to_string(), )), - staking_reward_public_key: Some(HexString( - pre_genesis.rewards_key.ref_to().to_string(), - )), protocol_public_key: Some(HexString( pre_genesis .store diff --git a/apps/src/lib/config/genesis.rs b/apps/src/lib/config/genesis.rs index 9425e3b019..66be875ff4 100644 --- a/apps/src/lib/config/genesis.rs +++ b/apps/src/lib/config/genesis.rs @@ -159,8 +159,6 @@ pub mod genesis_config { pub consensus_public_key: Option, // Public key for validator account. (default: generate) pub account_public_key: Option, - // Public key for staking reward account. (default: generate) - pub staking_reward_public_key: Option, // Public protocol signing key for validator account. (default: // generate) pub protocol_public_key: Option, @@ -168,8 +166,6 @@ pub mod genesis_config { pub dkg_public_key: Option, // Validator address (default: generate). pub address: Option, - // Staking reward account address (default: generate). - pub staking_reward_address: Option, // Total number of tokens held at genesis. // XXX: u64 doesn't work with toml-rs! pub tokens: Option, @@ -178,8 +174,6 @@ pub mod genesis_config { pub non_staked_balance: Option, // Filename of validator VP. (default: default validator VP) pub validator_vp: Option, - // Filename of staking reward account VP. (default: user VP) - pub staking_reward_vp: Option, // IP:port of the validator. (used in generation only) pub net_address: Option, /// Tendermint node key is used to derive Tendermint node ID for node @@ -277,17 +271,11 @@ pub mod genesis_config { ) -> Validator { let validator_vp_name = config.validator_vp.as_ref().unwrap(); let validator_vp_config = wasm.get(validator_vp_name).unwrap(); - let reward_vp_name = config.staking_reward_vp.as_ref().unwrap(); - let reward_vp_config = wasm.get(reward_vp_name).unwrap(); Validator { pos_data: GenesisValidator { address: Address::decode(&config.address.as_ref().unwrap()) .unwrap(), - staking_reward_address: Address::decode( - &config.staking_reward_address.as_ref().unwrap(), - ) - .unwrap(), tokens: token::Amount::whole(config.tokens.unwrap_or_default()), consensus_key: config .consensus_public_key @@ -295,12 +283,6 @@ pub mod genesis_config { .unwrap() .to_public_key() .unwrap(), - staking_reward_key: config - .staking_reward_public_key - .as_ref() - .unwrap() - .to_public_key() - .unwrap(), }, account_key: config .account_public_key @@ -330,16 +312,6 @@ pub mod genesis_config { .unwrap() .to_sha256_bytes() .unwrap(), - reward_vp_code_path: reward_vp_config.filename.to_owned(), - reward_vp_sha256: reward_vp_config - .sha256 - .clone() - .unwrap_or_else(|| { - eprintln!("Unknown validator VP WASM sha256"); - cli::safe_exit(1); - }) - .to_sha256_bytes() - .unwrap(), } } @@ -658,10 +630,6 @@ pub struct Validator { pub validator_vp_code_path: String, /// Expected SHA-256 hash of the validator VP pub validator_vp_sha256: [u8; 32], - /// Staking reward account code WASM - pub reward_vp_code_path: String, - /// Expected SHA-256 hash of the staking reward VP - pub reward_vp_sha256: [u8; 32], } #[derive( @@ -736,23 +704,13 @@ pub fn genesis() -> Genesis { // `tests::gen_genesis_validator` below. let consensus_keypair = wallet::defaults::validator_keypair(); let account_keypair = wallet::defaults::validator_keypair(); - let ed_staking_reward_keypair = ed25519::SecretKey::try_from_slice(&[ - 61, 198, 87, 204, 44, 94, 234, 228, 217, 72, 245, 27, 40, 2, 151, 174, - 24, 247, 69, 6, 9, 30, 44, 16, 88, 238, 77, 162, 243, 125, 240, 206, - ]) - .unwrap(); - let staking_reward_keypair = - common::SecretKey::try_from_sk(&ed_staking_reward_keypair).unwrap(); let address = wallet::defaults::validator_address(); - let staking_reward_address = Address::decode("atest1v4ehgw36xcersvee8qerxd35x9prsw2xg5erxv6pxfpygd2x89z5xsf5xvmnysejgv6rwd2rnj2avt").unwrap(); let (protocol_keypair, dkg_keypair) = wallet::defaults::validator_keys(); let validator = Validator { pos_data: GenesisValidator { address, - staking_reward_address, tokens: token::Amount::whole(200_000), consensus_key: consensus_keypair.ref_to(), - staking_reward_key: staking_reward_keypair.ref_to(), }, account_key: account_keypair.ref_to(), protocol_key: protocol_keypair.ref_to(), @@ -761,8 +719,6 @@ pub fn genesis() -> Genesis { // TODO replace with https://github.com/anoma/anoma/issues/25) validator_vp_code_path: vp_user_path.into(), validator_vp_sha256: Default::default(), - reward_vp_code_path: vp_user_path.into(), - reward_vp_sha256: Default::default(), }; let parameters = Parameters { epoch_duration: EpochDuration { @@ -853,24 +809,18 @@ pub mod tests { use crate::wallet; /// Run `cargo test gen_genesis_validator -- --nocapture` to generate a - /// new genesis validator address, staking reward address and keypair. + /// new genesis validator address and keypair. #[test] fn gen_genesis_validator() { let address = gen_established_address(); - let staking_reward_address = gen_established_address(); let mut rng: ThreadRng = thread_rng(); let keypair: common::SecretKey = ed25519::SigScheme::generate(&mut rng).try_to_sk().unwrap(); let kp_arr = keypair.try_to_vec().unwrap(); - let staking_reward_keypair: common::SecretKey = - ed25519::SigScheme::generate(&mut rng).try_to_sk().unwrap(); - let srkp_arr = staking_reward_keypair.try_to_vec().unwrap(); let (protocol_keypair, dkg_keypair) = wallet::defaults::validator_keys(); println!("address: {}", address); - println!("staking_reward_address: {}", staking_reward_address); println!("keypair: {:?}", kp_arr); - println!("staking_reward_keypair: {:?}", srkp_arr); println!("protocol_keypair: {:?}", protocol_keypair); println!("dkg_keypair: {:?}", dkg_keypair.try_to_vec().unwrap()); } diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs index 6e5f4e2196..aab015a5a8 100644 --- a/proof_of_stake/src/lib.rs +++ b/proof_of_stake/src/lib.rs @@ -107,11 +107,6 @@ pub trait PosReadOnly { /// Read PoS parameters. fn read_pos_params(&self) -> Result; - /// Read PoS validator's staking reward address. - fn read_validator_staking_reward_address( - &self, - key: &Self::Address, - ) -> Result, Self::Error>; /// Read PoS validator's consensus key (used for signing block votes). fn read_validator_consensus_key( &self, @@ -186,13 +181,6 @@ pub trait PosActions: PosReadOnly { address: &Self::Address, consensus_key: &Self::PublicKey, ) -> Result<(), Self::Error>; - /// Write PoS validator's staking reward address, into which staking rewards - /// will be credited. - fn write_validator_staking_reward_address( - &mut self, - key: &Self::Address, - value: Self::Address, - ) -> Result<(), Self::Error>; /// Write PoS validator's consensus key (used for signing block votes). fn write_validator_consensus_key( &mut self, @@ -267,7 +255,6 @@ pub trait PosActions: PosReadOnly { fn become_validator( &mut self, address: &Self::Address, - staking_reward_address: &Self::Address, consensus_key: &Self::PublicKey, current_epoch: impl Into, ) -> Result<(), Self::BecomeValidatorError> { @@ -277,13 +264,6 @@ pub trait PosActions: PosReadOnly { if self.is_validator(address)? { Err(BecomeValidatorError::AlreadyValidator(address.clone()))?; } - if address == staking_reward_address { - Err( - BecomeValidatorError::StakingRewardAddressEqValidatorAddress( - address.clone(), - ), - )?; - } let consensus_key_clone = consensus_key.clone(); let BecomeValidatorData { consensus_key, @@ -297,10 +277,6 @@ pub trait PosActions: PosReadOnly { &mut validator_set, current_epoch, ); - self.write_validator_staking_reward_address( - address, - staking_reward_address.clone(), - )?; self.write_validator_consensus_key(address, consensus_key)?; self.write_validator_state(address, state)?; self.write_validator_set(validator_set)?; @@ -626,13 +602,6 @@ pub trait PosBase { address: &Self::Address, consensus_key: &Self::PublicKey, ); - /// Write PoS validator's staking reward address, into which staking rewards - /// will be credited. - fn write_validator_staking_reward_address( - &mut self, - key: &Self::Address, - value: &Self::Address, - ); /// Write PoS validator's consensus key (used for signing block votes). fn write_validator_consensus_key( &mut self, @@ -674,12 +643,6 @@ pub trait PosBase { fn write_validator_set(&mut self, value: &ValidatorSets); /// Read PoS total voting power of all validators (active and inactive). fn write_total_voting_power(&mut self, value: &TotalVotingPowers); - /// Initialize staking reward account with the given public key. - fn init_staking_reward_account( - &mut self, - address: &Self::Address, - pk: &Self::PublicKey, - ); /// Credit tokens to the `target` account. This should only be used at /// genesis. fn credit_tokens( @@ -727,9 +690,7 @@ pub trait PosBase { for res in validators { let GenesisValidatorData { ref address, - staking_reward_address, consensus_key, - staking_reward_key, state, total_deltas, voting_power, @@ -741,19 +702,11 @@ pub trait PosBase { .get(current_epoch) .expect("Consensus key must be set"), ); - self.write_validator_staking_reward_address( - address, - &staking_reward_address, - ); self.write_validator_consensus_key(address, &consensus_key); self.write_validator_state(address, &state); self.write_validator_total_deltas(address, &total_deltas); self.write_validator_voting_power(address, &voting_power); self.write_bond(&bond_id, &bond); - self.init_staking_reward_account( - &staking_reward_address, - &staking_reward_key, - ); } self.write_validator_set(&validator_set); self.write_total_voting_power(&total_voting_power); @@ -955,11 +908,6 @@ pub enum GenesisError { pub enum BecomeValidatorError { #[error("The given address {0} is already a validator")] AlreadyValidator(Address), - #[error( - "The staking reward address must be different from the validator's \ - address {0}" - )] - StakingRewardAddressEqValidatorAddress(Address), } #[allow(missing_docs)] @@ -1104,9 +1052,7 @@ where PK: Debug + Clone + BorshDeserialize + BorshSerialize + BorshSchema, { address: Address, - staking_reward_address: Address, consensus_key: ValidatorConsensusKeys, - staking_reward_key: PK, state: ValidatorStates, total_deltas: ValidatorTotalDeltas, voting_power: ValidatorVotingPowers, @@ -1205,11 +1151,8 @@ where let validators = validators.map( move |GenesisValidator { address, - staking_reward_address, - tokens, consensus_key, - staking_reward_key, }| { let consensus_key = Epoched::init_at_genesis(consensus_key.clone(), current_epoch); @@ -1240,9 +1183,7 @@ where ); Ok(GenesisValidatorData { address: address.clone(), - staking_reward_address: staking_reward_address.clone(), consensus_key, - staking_reward_key: staking_reward_key.clone(), state, total_deltas, voting_power, diff --git a/proof_of_stake/src/types.rs b/proof_of_stake/src/types.rs index 5f5ac6846d..783c5855aa 100644 --- a/proof_of_stake/src/types.rs +++ b/proof_of_stake/src/types.rs @@ -108,15 +108,10 @@ pub struct VotingPowerDelta(i64); pub struct GenesisValidator { /// Validator's address pub address: Address, - /// An address to which any staking rewards will be credited, must be - /// different from the `address` - pub staking_reward_address: Address, /// Staked tokens are put into a self-bond pub tokens: Token, /// A public key used for signing validator's consensus actions pub consensus_key: PK, - /// An public key associated with the staking reward address - pub staking_reward_key: PK, } /// An update of the active and inactive validator set. diff --git a/proof_of_stake/src/validation.rs b/proof_of_stake/src/validation.rs index faef13f457..13356cbb55 100644 --- a/proof_of_stake/src/validation.rs +++ b/proof_of_stake/src/validation.rs @@ -50,13 +50,6 @@ where MissingNewValidatorConsensusKey(u64), #[error("Invalid validator consensus key update in epoch {0}")] InvalidValidatorConsensusKeyUpdate(u64), - #[error("Validator staking reward address is required for validator {0}")] - StakingRewardAddressIsRequired(Address), - #[error( - "Staking reward address must be different from the validator's \ - address {0}" - )] - StakingRewardAddressEqValidator(Address), #[error("Unexpectedly missing total deltas value for validator {0}")] MissingValidatorTotalDeltas(Address), #[error("The sum of total deltas for validator {0} are negative")] @@ -245,7 +238,7 @@ where /// Validator's address address: Address, /// Validator's data update - update: ValidatorUpdate, + update: ValidatorUpdate, }, /// Validator set update ValidatorSet(Data>), @@ -262,9 +255,8 @@ where /// An update of a validator's data. #[derive(Clone, Debug)] -pub enum ValidatorUpdate +pub enum ValidatorUpdate where - Address: Clone + Debug, TokenChange: Display + Debug + Default @@ -283,8 +275,6 @@ where State(Data), /// Consensus key update ConsensusKey(Data>), - /// Staking reward address update - StakingRewardAddress(Data
), /// Total deltas update TotalDeltas(Data>), /// Voting power update @@ -313,7 +303,6 @@ pub struct NewValidator { has_consensus_key: Option, has_total_deltas: bool, has_voting_power: bool, - has_staking_reward_address: bool, has_address_raw_hash: Option, voting_power: VotingPower, } @@ -806,16 +795,11 @@ where has_consensus_key, has_total_deltas, has_voting_power, - has_staking_reward_address, has_address_raw_hash, voting_power, } = &new_validator; // The new validator must have set all the required fields - if !(*has_state - && *has_total_deltas - && *has_voting_power - && *has_staking_reward_address) - { + if !(*has_state && *has_total_deltas && *has_voting_power) { errors.push(Error::InvalidNewValidator( address.clone(), new_validator.clone(), @@ -1129,15 +1113,6 @@ where address, data, ), - StakingRewardAddress(data) => { - Self::validator_staking_reward_address( - errors, - new_validators, - address, - data, - ) - } - TotalDeltas(data) => Self::validator_total_deltas( constants, errors, @@ -1327,32 +1302,6 @@ where } } - fn validator_staking_reward_address( - errors: &mut Vec>, - new_validators: &mut HashMap>, - address: Address, - data: Data
, - ) { - match (data.pre, data.post) { - (Some(_), Some(post)) => { - if post == address { - errors - .push(Error::StakingRewardAddressEqValidator(address)); - } - } - (None, Some(post)) => { - if post == address { - errors.push(Error::StakingRewardAddressEqValidator( - address.clone(), - )); - } - let validator = new_validators.entry(address).or_default(); - validator.has_staking_reward_address = true; - } - _ => errors.push(Error::StakingRewardAddressIsRequired(address)), - } - } - fn validator_total_deltas( constants: &Constants, errors: &mut Vec>, diff --git a/shared/src/ledger/pos/mod.rs b/shared/src/ledger/pos/mod.rs index 0b1617c7c3..011a96a6ba 100644 --- a/shared/src/ledger/pos/mod.rs +++ b/shared/src/ledger/pos/mod.rs @@ -166,17 +166,6 @@ mod macros { Ok($crate::ledger::storage::types::decode(value).unwrap()) } - fn read_validator_staking_reward_address( - &self, - key: &Self::Address, - ) -> std::result::Result, Self::Error> { - let value = $crate::ledger::storage_api::StorageRead::read_bytes( - self, - &validator_staking_reward_address_key(key), - )?; - Ok(value.map(|value| $crate::ledger::storage::types::decode(value).unwrap())) - } - fn read_validator_consensus_key( &self, key: &Self::Address, diff --git a/shared/src/ledger/pos/storage.rs b/shared/src/ledger/pos/storage.rs index 366ce489b5..7afe1a3b4f 100644 --- a/shared/src/ledger/pos/storage.rs +++ b/shared/src/ledger/pos/storage.rs @@ -19,8 +19,6 @@ use crate::types::{key, token}; const PARAMS_STORAGE_KEY: &str = "params"; const VALIDATOR_STORAGE_PREFIX: &str = "validator"; const VALIDATOR_ADDRESS_RAW_HASH: &str = "address_raw_hash"; -const VALIDATOR_STAKING_REWARD_ADDRESS_STORAGE_KEY: &str = - "staking_reward_address"; const VALIDATOR_CONSENSUS_KEY_STORAGE_KEY: &str = "consensus_key"; const VALIDATOR_STATE_STORAGE_KEY: &str = "state"; const VALIDATOR_TOTAL_DELTAS_STORAGE_KEY: &str = "total_deltas"; @@ -92,31 +90,6 @@ pub fn is_validator_address_raw_hash_key(key: &Key) -> Option<&str> { } } -/// Storage key for validator's staking reward address. -pub fn validator_staking_reward_address_key(validator: &Address) -> Key { - validator_prefix(validator) - .push(&VALIDATOR_STAKING_REWARD_ADDRESS_STORAGE_KEY.to_owned()) - .expect("Cannot obtain a storage key") -} - -/// Is storage key for validator's staking reward address? -pub fn is_validator_staking_reward_address_key(key: &Key) -> Option<&Address> { - match &key.segments[..] { - [ - DbKeySeg::AddressSeg(addr), - DbKeySeg::StringSeg(prefix), - DbKeySeg::AddressSeg(validator), - DbKeySeg::StringSeg(key), - ] if addr == &ADDRESS - && prefix == VALIDATOR_STORAGE_PREFIX - && key == VALIDATOR_STAKING_REWARD_ADDRESS_STORAGE_KEY => - { - Some(validator) - } - _ => None, - } -} - /// Storage key for validator's consensus key. pub fn validator_consensus_key_key(validator: &Address) -> Key { validator_prefix(validator) @@ -464,15 +437,6 @@ where .unwrap(); } - fn write_validator_staking_reward_address( - &mut self, - key: &Self::Address, - value: &Self::Address, - ) { - self.write(&validator_staking_reward_address_key(key), encode(value)) - .unwrap(); - } - fn write_validator_consensus_key( &mut self, key: &Self::Address, @@ -533,22 +497,6 @@ where .unwrap(); } - fn init_staking_reward_account( - &mut self, - address: &Self::Address, - pk: &Self::PublicKey, - ) { - // let user_vp = - // std::fs::read("wasm/vp_user.wasm").expect("cannot load user VP"); - // // The staking reward accounts are setup with a user VP - // self.write(&Key::validity_predicate(address), user_vp.to_vec()) - // .unwrap(); - - // Write the public key - let pk_key = key::pk_key(address); - self.write(&pk_key, encode(pk)).unwrap(); - } - fn credit_tokens( &mut self, token: &Self::Address, diff --git a/shared/src/ledger/pos/vp.rs b/shared/src/ledger/pos/vp.rs index 60264e4926..058acac262 100644 --- a/shared/src/ledger/pos/vp.rs +++ b/shared/src/ledger/pos/vp.rs @@ -17,12 +17,10 @@ use thiserror::Error; use super::{ bond_key, is_bond_key, is_params_key, is_total_voting_power_key, - is_unbond_key, is_validator_set_key, - is_validator_staking_reward_address_key, is_validator_total_deltas_key, + is_unbond_key, is_validator_set_key, is_validator_total_deltas_key, is_validator_voting_power_key, params_key, staking_token_address, total_voting_power_key, unbond_key, validator_consensus_key_key, - validator_set_key, validator_slashes_key, - validator_staking_reward_address_key, validator_state_key, + validator_set_key, validator_slashes_key, validator_state_key, validator_total_deltas_key, validator_voting_power_key, BondId, Bonds, Unbonds, ValidatorConsensusKeys, ValidatorSets, ValidatorTotalDeltas, }; @@ -149,21 +147,6 @@ where address: validator.clone(), update: State(Data { pre, post }), }); - } else if let Some(validator) = - is_validator_staking_reward_address_key(key) - { - let pre = - self.ctx.pre().read_bytes(key)?.and_then(|bytes| { - Address::try_from_slice(&bytes[..]).ok() - }); - let post = - self.ctx.post().read_bytes(key)?.and_then(|bytes| { - Address::try_from_slice(&bytes[..]).ok() - }); - changes.push(Validator { - address: validator.clone(), - update: StakingRewardAddress(Data { pre, post }), - }); } else if let Some(validator) = is_validator_consensus_key_key(key) { let pre = self.ctx.pre().read_bytes(key)?.and_then(|bytes| { diff --git a/shared/src/types/transaction/mod.rs b/shared/src/types/transaction/mod.rs index 071c3df2a5..7d15fa0d2c 100644 --- a/shared/src/types/transaction/mod.rs +++ b/shared/src/types/transaction/mod.rs @@ -185,18 +185,12 @@ pub struct InitValidator { pub account_key: common::PublicKey, /// A key to be used for signing blocks and votes on blocks. pub consensus_key: common::PublicKey, - /// Public key to be written into the staking reward account's storage. - /// This can be used for signature verification of transactions for the - /// newly created account. - pub rewards_account_key: common::PublicKey, /// Public key used to sign protocol transactions pub protocol_key: common::PublicKey, /// Serialization of the public session key used in the DKG pub dkg_key: DkgPublicKey, /// The VP code for validator account pub validator_vp_code: Vec, - /// The VP code for validator's staking reward account - pub rewards_vp_code: Vec, } /// Module that includes helper functions for classifying diff --git a/tests/src/e2e/ledger_tests.rs b/tests/src/e2e/ledger_tests.rs index 8a7427a9fd..11bf1228c9 100644 --- a/tests/src/e2e/ledger_tests.rs +++ b/tests/src/e2e/ledger_tests.rs @@ -1674,7 +1674,6 @@ fn test_genesis_validators() -> Result<()> { config.tokens = Some(200000); config.non_staked_balance = Some(1000000000000); config.validator_vp = Some("vp_user".into()); - config.staking_reward_vp = Some("vp_user".into()); // Setup the validator ports same as what // `setup::add_validators` would do let mut net_address = net_address_0; diff --git a/tests/src/native_vp/pos.rs b/tests/src/native_vp/pos.rs index 61b859a7d6..2a748f0259 100644 --- a/tests/src/native_vp/pos.rs +++ b/tests/src/native_vp/pos.rs @@ -71,7 +71,6 @@ //! address in Tendermint) //! - `#{PoS}/validator_set` //! - `#{PoS}/validator/#{validator}/consensus_key` -//! - `#{PoS}/validator/#{validator}/staking_reward_address` //! - `#{PoS}/validator/#{validator}/state` //! - `#{PoS}/validator/#{validator}/total_deltas` //! - `#{PoS}/validator/#{validator}/voting_power` @@ -121,10 +120,7 @@ pub fn init_pos( // addresses exist tx_env.spawn_accounts([&staking_token_address()]); for validator in genesis_validators { - tx_env.spawn_accounts([ - &validator.address, - &validator.staking_reward_address, - ]); + tx_env.spawn_accounts([&validator.address]); } tx_env.storage.block.epoch = start_epoch; // Initialize PoS storage @@ -601,7 +597,6 @@ pub mod testing { #[derive(Clone, Debug, Default)] pub struct TestValidator { pub address: Option
, - pub staking_reward_address: Option
, pub stake: Option, /// Balance is a pair of token address and its amount pub unstaked_balances: Vec<(Address, token::Amount)>, @@ -683,10 +678,6 @@ pub mod testing { #[derivative(Debug = "ignore")] pk: PublicKey, }, - ValidatorStakingRewardsAddress { - validator: Address, - address: Address, - }, ValidatorTotalDeltas { validator: Address, delta: i128, @@ -897,10 +888,6 @@ pub mod testing { validator: address.clone(), pk: consensus_key, }, - PosStorageChange::ValidatorStakingRewardsAddress { - validator: address.clone(), - address: address::testing::established_address_1(), - }, PosStorageChange::ValidatorState { validator: address.clone(), state: ValidatorState::Pending, @@ -1389,14 +1376,6 @@ pub mod testing { .write_validator_consensus_key(&validator, consensus_key) .unwrap(); } - PosStorageChange::ValidatorStakingRewardsAddress { - validator, - address, - } => { - tx::ctx() - .write_validator_staking_reward_address(&validator, address) - .unwrap(); - } PosStorageChange::ValidatorTotalDeltas { validator, delta, diff --git a/tx_prelude/src/proof_of_stake.rs b/tx_prelude/src/proof_of_stake.rs index c11b035495..ee50943336 100644 --- a/tx_prelude/src/proof_of_stake.rs +++ b/tx_prelude/src/proof_of_stake.rs @@ -4,8 +4,7 @@ pub use namada::ledger::pos::*; use namada::ledger::pos::{ bond_key, namada_proof_of_stake, params_key, total_voting_power_key, unbond_key, validator_address_raw_hash_key, validator_consensus_key_key, - validator_set_key, validator_slashes_key, - validator_staking_reward_address_key, validator_state_key, + validator_set_key, validator_slashes_key, validator_state_key, validator_total_deltas_key, validator_voting_power_key, }; use namada::types::address::Address; @@ -74,19 +73,17 @@ impl Ctx { } /// Attempt to initialize a validator account. On success, returns the - /// initialized validator account's address and its staking reward address. + /// initialized validator account's address. pub fn init_validator( &mut self, InitValidator { account_key, consensus_key, - rewards_account_key, protocol_key, dkg_key, validator_vp_code, - rewards_vp_code, }: InitValidator, - ) -> EnvResult<(Address, Address)> { + ) -> EnvResult
{ let current_epoch = self.get_block_epoch()?; // Init validator account let validator_address = self.init_account(&validator_vp_code)?; @@ -97,19 +94,13 @@ impl Ctx { let dkg_pk_key = key::dkg_session_keys::dkg_pk_key(&validator_address); self.write(&dkg_pk_key, &dkg_key)?; - // Init staking reward account - let rewards_address = self.init_account(&rewards_vp_code)?; - let pk_key = key::pk_key(&rewards_address); - self.write(&pk_key, &rewards_account_key)?; - self.become_validator( &validator_address, - &rewards_address, &consensus_key, current_epoch, )?; - Ok((validator_address, rewards_address)) + Ok(validator_address) } } @@ -140,14 +131,6 @@ impl namada_proof_of_stake::PosActions for Ctx { self.write(&validator_address_raw_hash_key(raw_hash), address) } - fn write_validator_staking_reward_address( - &mut self, - key: &Self::Address, - value: Self::Address, - ) -> Result<(), Self::Error> { - self.write(&validator_staking_reward_address_key(key), &value) - } - fn write_validator_consensus_key( &mut self, key: &Self::Address, diff --git a/wasm/wasm_source/src/tx_bond.rs b/wasm/wasm_source/src/tx_bond.rs index 6718988657..bda4818722 100644 --- a/wasm/wasm_source/src/tx_bond.rs +++ b/wasm/wasm_source/src/tx_bond.rs @@ -68,16 +68,12 @@ mod tests { ) -> TxResult { let is_delegation = matches!( &bond.source, Some(source) if *source != bond.validator); - let staking_reward_address = address::testing::established_address_1(); let consensus_key = key::testing::keypair_1().ref_to(); - let staking_reward_key = key::testing::keypair_2().ref_to(); let genesis_validators = [GenesisValidator { address: bond.validator.clone(), - staking_reward_address, tokens: initial_stake, consensus_key, - staking_reward_key, }]; init_pos(&genesis_validators[..], &pos_params, Epoch(0)); diff --git a/wasm/wasm_source/src/tx_init_validator.rs b/wasm/wasm_source/src/tx_init_validator.rs index 2d5f1a6256..a99bb8cde9 100644 --- a/wasm/wasm_source/src/tx_init_validator.rs +++ b/wasm/wasm_source/src/tx_init_validator.rs @@ -15,12 +15,8 @@ fn apply_tx(ctx: &mut Ctx, tx_data: Vec) -> TxResult { // Register the validator in PoS match ctx.init_validator(init_validator) { - Ok((validator_address, staking_reward_address)) => { - debug_log!( - "Created validator {} and staking reward account {}", - validator_address.encode(), - staking_reward_address.encode() - ) + Ok(validator_address) => { + debug_log!("Created validator {}", validator_address.encode(),) } Err(err) => { debug_log!("Validator creation failed with: {}", err); diff --git a/wasm/wasm_source/src/tx_unbond.rs b/wasm/wasm_source/src/tx_unbond.rs index 6199393fb1..df4299deab 100644 --- a/wasm/wasm_source/src/tx_unbond.rs +++ b/wasm/wasm_source/src/tx_unbond.rs @@ -67,13 +67,10 @@ mod tests { ) -> TxResult { let is_delegation = matches!( &unbond.source, Some(source) if *source != unbond.validator); - let staking_reward_address = address::testing::established_address_1(); let consensus_key = key::testing::keypair_1().ref_to(); - let staking_reward_key = key::testing::keypair_2().ref_to(); let genesis_validators = [GenesisValidator { address: unbond.validator.clone(), - staking_reward_address, tokens: if is_delegation { // If we're unbonding a delegation, we'll give the initial stake // to the delegation instead of the validator @@ -82,7 +79,6 @@ mod tests { initial_stake }, consensus_key, - staking_reward_key, }]; init_pos(&genesis_validators[..], &pos_params, Epoch(0)); diff --git a/wasm/wasm_source/src/tx_withdraw.rs b/wasm/wasm_source/src/tx_withdraw.rs index 8add20a78d..4cc0e35691 100644 --- a/wasm/wasm_source/src/tx_withdraw.rs +++ b/wasm/wasm_source/src/tx_withdraw.rs @@ -73,13 +73,10 @@ mod tests { ) -> TxResult { let is_delegation = matches!( &withdraw.source, Some(source) if *source != withdraw.validator); - let staking_reward_address = address::testing::established_address_1(); let consensus_key = key::testing::keypair_1().ref_to(); - let staking_reward_key = key::testing::keypair_2().ref_to(); let genesis_validators = [GenesisValidator { address: withdraw.validator.clone(), - staking_reward_address, tokens: if is_delegation { // If we're withdrawing a delegation, we'll give the initial // stake to the delegation instead of the @@ -89,7 +86,6 @@ mod tests { initial_stake }, consensus_key, - staking_reward_key, }]; init_pos(&genesis_validators[..], &pos_params, Epoch(0)); From 020f6bf308f89073ae92bc516e0201bc21340965 Mon Sep 17 00:00:00 2001 From: brentstone Date: Tue, 25 Oct 2022 20:30:45 -0400 Subject: [PATCH 05/38] remove staking reward address from genesis toml files --- genesis/dev.toml | 6 ------ genesis/e2e-tests-single-node.toml | 2 -- 2 files changed, 8 deletions(-) diff --git a/genesis/dev.toml b/genesis/dev.toml index fc95244e14..3c4bc101b7 100644 --- a/genesis/dev.toml +++ b/genesis/dev.toml @@ -7,20 +7,14 @@ genesis_time = "2021-09-30:10:00.00Z" consensus_public_key = "5e704c4e46265e1ccc87505149f79b9d2e414d01a4e3806dfc65f0a73901c1d0" # Public key of the validator's Anoma account. account_public_key = "5e704c4e46265e1ccc87505149f79b9d2e414d01a4e3806dfc65f0a73901c1d0" -# Public key of the Anoma account for this validator's staking rewards. -staking_reward_public_key = "6f5c421769d321ec05d01158b170649a01848f43a27988f71443041be23f2f39" # Address of the validator. address = "a1qq5qqqqqgfqnsd6pxse5zdj9g5crzsf5x4zyzv6yxerr2d2rxpryzwp5g5m5zvfjxv6ygsekjmraj0" -# Staking reward address of the validator. -staking_reward_address = "a1qq5qqqqqxaz5vven8yu5gdpng9zrys6ygvurwv3sgsmrvd6xgdzrys6yg4pnwd6z89rrqv2xvjcy9t" # Validator's token balance at genesis. tokens = 200000 # Amount of the validator's genesis token balance which is not staked. non_staked_balance = 100000 # VP for the validator account validator_vp = "vp_user" -# VP for the staking reward account -staking_reward_vp = "vp_user" # Public IP:port address net_address = "127.0.0.1:26656" diff --git a/genesis/e2e-tests-single-node.toml b/genesis/e2e-tests-single-node.toml index 95e51173c6..d4934d78db 100644 --- a/genesis/e2e-tests-single-node.toml +++ b/genesis/e2e-tests-single-node.toml @@ -11,8 +11,6 @@ tokens = 200000 non_staked_balance = 1000000000000 # VP for the validator account validator_vp = "vp_user" -# VP for the staking reward account -staking_reward_vp = "vp_user" # Public IP:port address. # We set the port to be the default+1000, so that if a local node was running at # the same time as the E2E tests, it wouldn't affect them. From 3eb21f72ef66383e04153fb5123212e4a07cde4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Wed, 26 Oct 2022 11:29:06 +0200 Subject: [PATCH 06/38] client: remove staking rewards address from init-validator result --- apps/src/lib/client/tx.rs | 36 ++++-------------------------------- 1 file changed, 4 insertions(+), 32 deletions(-) diff --git a/apps/src/lib/client/tx.rs b/apps/src/lib/client/tx.rs index c193810fdd..d558c44de4 100644 --- a/apps/src/lib/client/tx.rs +++ b/apps/src/lib/client/tx.rs @@ -244,21 +244,10 @@ pub async fn submit_init_validator( let (mut ctx, initialized_accounts) = process_tx(ctx, &tx_args, tx, Some(&source)).await; if !tx_args.dry_run { - let (validator_address_alias, validator_address, rewards_address_alias) = + let (validator_address_alias, validator_address) = match &initialized_accounts[..] { - // There should be 2 accounts, one for the validator itself, one - // for its staking reward address. - [account_1, account_2] => { - // We need to find out which address is which - let (validator_address, rewards_address) = - if rpc::is_validator(account_1, tx_args.ledger_address) - .await - { - (account_1, account_2) - } else { - (account_2, account_1) - }; - + // There should be 1 account for the validator itself + [validator_address] => { let validator_address_alias = match tx_args .initialized_account_alias { @@ -293,23 +282,7 @@ pub async fn submit_init_validator( validator_address.encode() ); } - let rewards_address_alias = - format!("{}-rewards", validator_address_alias); - if let Some(new_alias) = ctx.wallet.add_address( - rewards_address_alias.clone(), - rewards_address.clone(), - ) { - println!( - "Added alias {} for address {}.", - new_alias, - rewards_address.encode() - ); - } - ( - validator_address_alias, - validator_address.clone(), - rewards_address_alias, - ) + (validator_address_alias, validator_address.clone()) } _ => { eprintln!("Expected two accounts to be created"); @@ -330,7 +303,6 @@ pub async fn submit_init_validator( "The validator's addresses and keys were stored in the wallet:" ); println!(" Validator address \"{}\"", validator_address_alias); - println!(" Staking reward address \"{}\"", rewards_address_alias); println!(" Validator account key \"{}\"", validator_key_alias); println!(" Consensus key \"{}\"", consensus_key_alias); println!( From b54b5c04fbd7275f703f7341f7c404324717e55d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Wed, 26 Oct 2022 11:29:45 +0200 Subject: [PATCH 07/38] wallet: remove validator rewards key --- apps/src/lib/wallet/alias.rs | 5 ----- apps/src/lib/wallet/pre_genesis.rs | 11 ----------- apps/src/lib/wallet/store.rs | 5 ----- 3 files changed, 21 deletions(-) diff --git a/apps/src/lib/wallet/alias.rs b/apps/src/lib/wallet/alias.rs index 25fcf03d11..13d977b852 100644 --- a/apps/src/lib/wallet/alias.rs +++ b/apps/src/lib/wallet/alias.rs @@ -97,11 +97,6 @@ pub fn validator_consensus_key(validator_alias: &Alias) -> Alias { format!("{validator_alias}-consensus-key").into() } -/// Default alias of a validator's staking rewards key -pub fn validator_rewards_key(validator_alias: &Alias) -> Alias { - format!("{validator_alias}-rewards-key").into() -} - /// Default alias of a validator's Tendermint node key pub fn validator_tendermint_node_key(validator_alias: &Alias) -> Alias { format!("{validator_alias}-tendermint-node-key").into() diff --git a/apps/src/lib/wallet/pre_genesis.rs b/apps/src/lib/wallet/pre_genesis.rs index f28be00d1b..fb47fb9f88 100644 --- a/apps/src/lib/wallet/pre_genesis.rs +++ b/apps/src/lib/wallet/pre_genesis.rs @@ -40,8 +40,6 @@ pub struct ValidatorWallet { pub account_key: Rc, /// Cryptographic keypair for consensus key pub consensus_key: Rc, - /// Cryptographic keypair for rewards key - pub rewards_key: Rc, /// Cryptographic keypair for Tendermint node key pub tendermint_node_key: Rc, } @@ -54,8 +52,6 @@ pub struct ValidatorStore { pub account_key: wallet::StoredKeypair, /// Cryptographic keypair for consensus key pub consensus_key: wallet::StoredKeypair, - /// Cryptographic keypair for rewards key - pub rewards_key: wallet::StoredKeypair, /// Cryptographic keypair for Tendermint node key pub tendermint_node_key: wallet::StoredKeypair, /// Special validator keys @@ -107,7 +103,6 @@ impl ValidatorWallet { let password = if store.account_key.is_encrypted() || store.consensus_key.is_encrypted() - || store.rewards_key.is_encrypted() || store.account_key.is_encrypted() { Some(wallet::read_password("Enter decryption password: ")) @@ -119,8 +114,6 @@ impl ValidatorWallet { store.account_key.get(true, password.clone())?; let consensus_key = store.consensus_key.get(true, password.clone())?; - let rewards_key = - store.rewards_key.get(true, password.clone())?; let tendermint_node_key = store.tendermint_node_key.get(true, password)?; @@ -128,7 +121,6 @@ impl ValidatorWallet { store, account_key, consensus_key, - rewards_key, tendermint_node_key, }) } @@ -149,7 +141,6 @@ impl ValidatorWallet { SchemeType::Ed25519, &password, ); - let (rewards_key, rewards_sk) = gen_key_to_store(scheme, &password); let (tendermint_node_key, tendermint_node_sk) = gen_key_to_store( // Note that TM only allows ed25519 for node IDs SchemeType::Ed25519, @@ -159,7 +150,6 @@ impl ValidatorWallet { let store = ValidatorStore { account_key, consensus_key, - rewards_key, tendermint_node_key, validator_keys, }; @@ -167,7 +157,6 @@ impl ValidatorWallet { store, account_key: account_sk, consensus_key: consensus_sk, - rewards_key: rewards_sk, tendermint_node_key: tendermint_node_sk, } } diff --git a/apps/src/lib/wallet/store.rs b/apps/src/lib/wallet/store.rs index e189255355..8668b6ed1b 100644 --- a/apps/src/lib/wallet/store.rs +++ b/apps/src/lib/wallet/store.rs @@ -392,7 +392,6 @@ impl Store { other: pre_genesis::ValidatorWallet, ) { let account_key_alias = alias::validator_key(&validator_alias); - let rewards_key_alias = alias::validator_rewards_key(&validator_alias); let consensus_key_alias = alias::validator_consensus_key(&validator_alias); let tendermint_node_key_alias = @@ -400,7 +399,6 @@ impl Store { let keys = [ (account_key_alias.clone(), other.store.account_key), - (rewards_key_alias.clone(), other.store.rewards_key), (consensus_key_alias.clone(), other.store.consensus_key), ( tendermint_node_key_alias.clone(), @@ -410,12 +408,10 @@ impl Store { self.keys.extend(keys.into_iter()); let account_pk = other.account_key.ref_to(); - let rewards_pk = other.rewards_key.ref_to(); let consensus_pk = other.consensus_key.ref_to(); let tendermint_node_pk = other.tendermint_node_key.ref_to(); let addresses = [ (account_key_alias.clone(), (&account_pk).into()), - (rewards_key_alias.clone(), (&rewards_pk).into()), (consensus_key_alias.clone(), (&consensus_pk).into()), ( tendermint_node_key_alias.clone(), @@ -426,7 +422,6 @@ impl Store { let pkhs = [ ((&account_pk).into(), account_key_alias), - ((&rewards_pk).into(), rewards_key_alias), ((&consensus_pk).into(), consensus_key_alias), ((&tendermint_node_pk).into(), tendermint_node_key_alias), ]; From 9852d28bb93d10ebfb01916802858a5a4ddb3212 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Wed, 26 Oct 2022 11:30:45 +0200 Subject: [PATCH 08/38] remove staking rewards address from cli strings and docs strings --- apps/src/lib/cli.rs | 10 +++++----- apps/src/lib/client/utils.rs | 5 ++--- shared/src/types/transaction/mod.rs | 3 +-- wasm/wasm_source/src/tx_init_validator.rs | 4 ++-- 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/apps/src/lib/cli.rs b/apps/src/lib/cli.rs index 093759bb96..3d51d53695 100644 --- a/apps/src/lib/cli.rs +++ b/apps/src/lib/cli.rs @@ -840,8 +840,8 @@ pub mod cmds { fn def() -> App { App::new(Self::CMD) .about( - "Send a signed transaction to create a new validator and \ - its staking reward account.", + "Send a signed transaction to create a new validator \ + account.", ) .add_args::() } @@ -1194,9 +1194,9 @@ pub mod cmds { fn def() -> App { App::new(Self::CMD) .about( - "Initialize genesis validator's address, staking reward \ - address, consensus key, validator account key and \ - staking rewards key and use it in the ledger's node.", + "Initialize genesis validator's address, consensus key \ + and validator account key and use it in the ledger's \ + node.", ) .add_args::() } diff --git a/apps/src/lib/client/utils.rs b/apps/src/lib/client/utils.rs index 2c0f54dd91..d3a4f31fd7 100644 --- a/apps/src/lib/client/utils.rs +++ b/apps/src/lib/client/utils.rs @@ -874,9 +874,8 @@ fn init_established_account( } } -/// Initialize genesis validator's address, staking reward address, -/// consensus key, validator account key and staking rewards key and use -/// it in the ledger's node. +/// Initialize genesis validator's address, consensus key and validator account +/// key and use it in the ledger's node. pub fn init_genesis_validator( global_args: args::Global, args::InitGenesisValidator { diff --git a/shared/src/types/transaction/mod.rs b/shared/src/types/transaction/mod.rs index 7d15fa0d2c..08ac8ceb09 100644 --- a/shared/src/types/transaction/mod.rs +++ b/shared/src/types/transaction/mod.rs @@ -166,8 +166,7 @@ pub struct InitAccount { pub vp_code: Vec, } -/// A tx data type to initialize a new validator account and its staking reward -/// account. +/// A tx data type to initialize a new validator account. #[derive( Debug, Clone, diff --git a/wasm/wasm_source/src/tx_init_validator.rs b/wasm/wasm_source/src/tx_init_validator.rs index a99bb8cde9..6a823faf3f 100644 --- a/wasm/wasm_source/src/tx_init_validator.rs +++ b/wasm/wasm_source/src/tx_init_validator.rs @@ -1,5 +1,5 @@ -//! A tx to initialize a new validator account and staking reward account with a -//! given public keys and a validity predicates. +//! A tx to initialize a new validator account with a given public keys and a +//! validity predicates. use namada_tx_prelude::transaction::InitValidator; use namada_tx_prelude::*; From 70fa58b95c2d9a822aaf4ebca0ba59b60bb87e48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Wed, 26 Oct 2022 17:14:28 +0200 Subject: [PATCH 09/38] changelog: #687 --- .changelog/unreleased/features/687-remove-staking-address.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/features/687-remove-staking-address.md diff --git a/.changelog/unreleased/features/687-remove-staking-address.md b/.changelog/unreleased/features/687-remove-staking-address.md new file mode 100644 index 0000000000..39d4def2aa --- /dev/null +++ b/.changelog/unreleased/features/687-remove-staking-address.md @@ -0,0 +1,2 @@ +- PoS: Removed staking reward addresses in preparation of auto-staked rewards + system. ([#687](https://github.com/anoma/namada/pull/687)) \ No newline at end of file From da7bffc86f41991e5b20fae6a7677b8ed7a06c38 Mon Sep 17 00:00:00 2001 From: brentstone Date: Wed, 21 Sep 2022 17:13:47 -0400 Subject: [PATCH 10/38] introduce validator commission rate and changes --- apps/src/lib/config/genesis.rs | 30 ++++++++++ proof_of_stake/src/lib.rs | 27 +++++++++ proof_of_stake/src/types.rs | 4 ++ shared/src/ledger/pos/storage.rs | 94 +++++++++++++++++++++++++++++++- 4 files changed, 153 insertions(+), 2 deletions(-) diff --git a/apps/src/lib/config/genesis.rs b/apps/src/lib/config/genesis.rs index 66be875ff4..c0135708bd 100644 --- a/apps/src/lib/config/genesis.rs +++ b/apps/src/lib/config/genesis.rs @@ -179,6 +179,11 @@ pub mod genesis_config { /// Tendermint node key is used to derive Tendermint node ID for node /// authentication pub tendermint_node_key: Option, + /// Commission rate charged on rewards for delegators (bounded inside + /// 0-1) + pub commission_rate: Option, + /// Maximum change in commission rate permitted per epoch + pub max_commission_rate_change: Option, } #[derive(Clone, Debug, Deserialize, Serialize)] @@ -283,6 +288,29 @@ pub mod genesis_config { .unwrap() .to_public_key() .unwrap(), + commission_rate: config + .commission_rate + .and_then(|rate| { + if rate >= Decimal::ZERO && rate <= Decimal::ONE { + Some(rate) + } else { + None + } + }) + .expect("Commission rate must be between 0.0 and 1.0"), + max_commission_rate_change: config + .max_commission_rate_change + .and_then(|rate| { + if rate >= Decimal::ZERO && rate <= Decimal::ONE { + Some(rate) + } else { + None + } + }) + .expect( + "Max commission rate change must be between 0.0 and \ + 1.0", + ), }, account_key: config .account_public_key @@ -711,6 +739,8 @@ pub fn genesis() -> Genesis { address, tokens: token::Amount::whole(200_000), consensus_key: consensus_keypair.ref_to(), + commission_rate: dec!(0.05), + max_commission_rate_change: dec!(0.01), }, account_key: account_keypair.ref_to(), protocol_key: protocol_keypair.ref_to(), diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs index aab015a5a8..51d64f26ca 100644 --- a/proof_of_stake/src/lib.rs +++ b/proof_of_stake/src/lib.rs @@ -589,6 +589,13 @@ pub trait PosBase { ) -> Option; /// Read PoS slashes applied to a validator. fn read_validator_slashes(&self, key: &Self::Address) -> Slashes; + /// Read PoS validator's commission rate + fn read_validator_commission_rate(&self, key: &Self::Address) -> Decimal; + /// Read PoS validator's maximum commission rate change per epoch + fn read_validator_max_commission_rate_change( + &self, + key: &Self::Address, + ) -> Decimal; /// Read PoS validator set (active and inactive). fn read_validator_set(&self) -> ValidatorSets; /// Read PoS total voting power of all validators (active and inactive). @@ -627,6 +634,18 @@ pub trait PosBase { key: &Self::Address, value: &ValidatorVotingPowers, ); + /// Write PoS validator's commission rate. + fn write_validator_commission_rate( + &mut self, + key: &Self::Address, + value: &Decimal, + ); + /// Write PoS validator's commission rate. + fn write_validator_max_commission_rate_change( + &mut self, + key: &Self::Address, + value: &Decimal, + ); /// Write (append) PoS slash applied to a validator. fn write_validator_slash( &mut self, @@ -691,6 +710,8 @@ pub trait PosBase { let GenesisValidatorData { ref address, consensus_key, + commission_rate, + max_commission_rate_change, state, total_deltas, voting_power, @@ -1053,6 +1074,8 @@ where { address: Address, consensus_key: ValidatorConsensusKeys, + commission_rate: Decimal, + max_commission_rate_change: Decimal, state: ValidatorStates, total_deltas: ValidatorTotalDeltas, voting_power: ValidatorVotingPowers, @@ -1153,6 +1176,8 @@ where address, tokens, consensus_key, + commission_rate, + max_commission_rate_change, }| { let consensus_key = Epoched::init_at_genesis(consensus_key.clone(), current_epoch); @@ -1184,6 +1209,8 @@ where Ok(GenesisValidatorData { address: address.clone(), consensus_key, + commission_rate: commission_rate.clone(), + max_commission_rate_change: max_commission_rate_change.clone(), state, total_deltas, voting_power, diff --git a/proof_of_stake/src/types.rs b/proof_of_stake/src/types.rs index 783c5855aa..7b9f809e38 100644 --- a/proof_of_stake/src/types.rs +++ b/proof_of_stake/src/types.rs @@ -112,6 +112,10 @@ pub struct GenesisValidator { pub tokens: Token, /// A public key used for signing validator's consensus actions pub consensus_key: PK, + /// Commission rate charged on rewards for delegators (bounded inside 0-1) + pub commission_rate: Decimal, + /// Maximum change in commission rate permitted per epoch + pub max_commission_rate_change: Decimal, } /// An update of the active and inactive validator set. diff --git a/shared/src/ledger/pos/storage.rs b/shared/src/ledger/pos/storage.rs index 7afe1a3b4f..ac4b007b63 100644 --- a/shared/src/ledger/pos/storage.rs +++ b/shared/src/ledger/pos/storage.rs @@ -23,6 +23,9 @@ const VALIDATOR_CONSENSUS_KEY_STORAGE_KEY: &str = "consensus_key"; const VALIDATOR_STATE_STORAGE_KEY: &str = "state"; const VALIDATOR_TOTAL_DELTAS_STORAGE_KEY: &str = "total_deltas"; const VALIDATOR_VOTING_POWER_STORAGE_KEY: &str = "voting_power"; +const VALIDATOR_COMMISSION_RATE_STORAGE_KEY: &str = "commission_rate"; +const VALIDATOR_MAX_COMMISSION_CHANGE_STORAGE_KEY: &str = + "max_commission_rate_change"; const SLASHES_PREFIX: &str = "slash"; const BOND_STORAGE_KEY: &str = "bond"; const UNBOND_STORAGE_KEY: &str = "unbond"; @@ -115,8 +118,60 @@ pub fn is_validator_consensus_key_key(key: &Key) -> Option<&Address> { } } -/// Storage key for validator's state. -pub fn validator_state_key(validator: &Address) -> Key { +/// Storage key for validator's commission rate. +pub fn validator_commission_rate_key(validator: &Address) -> Key { + validator_prefix(validator) + .push(&VALIDATOR_COMMISSION_RATE_STORAGE_KEY.to_owned()) + .expect("Cannot obtain a storage key") +} + +/// Is storage key for validator's commissionr ate? +pub fn is_validator_commission_rate_key(key: &Key) -> Option<&Address> { + match &key.segments[..] { + [ + DbKeySeg::AddressSeg(addr), + DbKeySeg::StringSeg(prefix), + DbKeySeg::AddressSeg(validator), + DbKeySeg::StringSeg(key), + ] if addr == &ADDRESS + && prefix == VALIDATOR_STORAGE_PREFIX + && key == VALIDATOR_COMMISSION_RATE_STORAGE_KEY => + { + Some(validator) + } + _ => None, + } +} + +/// Storage key for validator's maximum commission rate change per epoch. +pub fn validator_max_commission_rate_change_key(validator: &Address) -> Key { + validator_prefix(validator) + .push(&VALIDATOR_MAX_COMMISSION_CHANGE_STORAGE_KEY.to_owned()) + .expect("Cannot obtain a storage key") +} + +/// Is storage key for validator's maximum commission rate change per epoch? +pub fn is_validator_max_commission_rate_change_key( + key: &Key, +) -> Option<&Address> { + match &key.segments[..] { + [ + DbKeySeg::AddressSeg(addr), + DbKeySeg::StringSeg(prefix), + DbKeySeg::AddressSeg(validator), + DbKeySeg::StringSeg(key), + ] if addr == &ADDRESS + && prefix == VALIDATOR_STORAGE_PREFIX + && key == VALIDATOR_MAX_COMMISSION_CHANGE_STORAGE_KEY => + { + Some(validator) + } + _ => None, + } +} + +/// Storage key for validator's consensus key. +pub fn validator_consensus_key_key(validator: &Address) -> Key { validator_prefix(validator) .push(&VALIDATOR_STATE_STORAGE_KEY.to_owned()) .expect("Cannot obtain a storage key") @@ -413,6 +468,23 @@ where .unwrap_or_default() } + fn read_validator_commission_rate( + &self, + key: &Self::Address, + ) -> rust_decimal::Decimal { + let (value, _gas) = + self.read(&validator_commission_rate_key(key)).unwrap(); + value.map(|value| decode(value).unwrap()).unwrap() + } + + fn read_validator_max_commission_rate_change( + &self, + key: &Self::Address, + ) -> rust_decimal::Decimal { + let (value, _gas) = + self.read(&validator_commission_rate_key(key)).unwrap(); + value.map(|value| decode(value).unwrap()).unwrap() + } fn read_validator_set(&self) -> ValidatorSets { let (value, _gas) = self.read(&validator_set_key()).unwrap(); decode(value.unwrap()).unwrap() @@ -437,6 +509,24 @@ where .unwrap(); } + fn write_validator_commission_rate( + &mut self, + key: &Self::Address, + value: &rust_decimal::Decimal, + ) { + self.write(&validator_commission_rate_key(key), encode(value)) + .unwrap(); + } + + fn write_validator_max_commission_rate_change( + &mut self, + key: &Self::Address, + value: &rust_decimal::Decimal, + ) { + self.write(&validator_max_commission_rate_change_key(key), encode(value)) + .unwrap(); +} + fn write_validator_consensus_key( &mut self, key: &Self::Address, From 3fbc30eea3d6c610747d98e80dabbbdf1eea0649 Mon Sep 17 00:00:00 2001 From: brentstone Date: Thu, 22 Sep 2022 20:40:22 -0400 Subject: [PATCH 11/38] require commission rate input data for new validators --- Cargo.lock | 1 + apps/src/lib/cli.rs | 30 +++++++++++++++++++++++++++++ apps/src/lib/client/tx.rs | 17 ++++++++++++++++ apps/src/lib/client/utils.rs | 4 ++++ apps/src/lib/config/genesis.rs | 10 +++++----- genesis/dev.toml | 4 ++++ genesis/e2e-tests-single-node.toml | 4 ++++ proof_of_stake/src/lib.rs | 28 +++++++++++++++++++++++++++ shared/src/types/transaction/mod.rs | 5 +++++ tx_prelude/Cargo.toml | 1 + tx_prelude/src/proof_of_stake.rs | 24 +++++++++++++++++++++++ 11 files changed, 123 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3d7588b5b7..60fe270c7b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3130,6 +3130,7 @@ dependencies = [ "namada", "namada_macros", "namada_vm_env", + "rust_decimal", "sha2 0.10.6", "thiserror", ] diff --git a/apps/src/lib/cli.rs b/apps/src/lib/cli.rs index 3d51d53695..9c7366faa4 100644 --- a/apps/src/lib/cli.rs +++ b/apps/src/lib/cli.rs @@ -1217,6 +1217,10 @@ pub mod args { use namada::types::storage::{self, Epoch}; use namada::types::token; use namada::types::transaction::GasLimit; + use rust_decimal::Decimal; + use serde::Deserialize; + use tendermint::Timeout; + use tendermint_config::net::Address as TendermintAddress; use super::context::{WalletAddress, WalletKeypair, WalletPublicKey}; use super::utils::*; @@ -1245,6 +1249,7 @@ pub mod args { const CHAIN_ID_PREFIX: Arg = arg("chain-prefix"); const CODE_PATH: Arg = arg("code-path"); const CODE_PATH_OPT: ArgOpt = CODE_PATH.opt(); + const COMMISSION_RATE: Arg = arg("commission-rate"); const CONSENSUS_TIMEOUT_COMMIT: ArgDefault = arg_default( "consensus-timeout-commit", DefaultFn(|| Timeout::from_str("1s").unwrap()), @@ -1276,6 +1281,7 @@ pub mod args { const LEDGER_ADDRESS: Arg = arg("ledger-address"); const LOCALHOST: ArgFlag = flag("localhost"); + const MAX_COMMISSION_RATE_CHANGE: Arg = arg("max-commission-rate-change"); const MODE: ArgOpt = arg_opt("mode"); const NET_ADDRESS: Arg = arg("net-address"); const OWNER: ArgOpt = arg_opt("owner"); @@ -1527,6 +1533,8 @@ pub mod args { pub account_key: Option, pub consensus_key: Option, pub protocol_key: Option, + pub commission_rate: Decimal, + pub max_commission_rate_change: Decimal, pub validator_vp_code_path: Option, pub unsafe_dont_encrypt: bool, } @@ -1539,6 +1547,8 @@ pub mod args { let account_key = VALIDATOR_ACCOUNT_KEY.parse(matches); let consensus_key = VALIDATOR_CONSENSUS_KEY.parse(matches); let protocol_key = PROTOCOL_KEY.parse(matches); + let commission_rate = COMMISSION_RATE.parse(matches); + let max_commission_rate_change = MAX_COMMISSION_RATE_CHANGE.parse(matches); let validator_vp_code_path = VALIDATOR_CODE_PATH.parse(matches); let unsafe_dont_encrypt = UNSAFE_DONT_ENCRYPT.parse(matches); Self { @@ -1548,6 +1558,8 @@ pub mod args { account_key, consensus_key, protocol_key, + commission_rate, + max_commission_rate_change, validator_vp_code_path, unsafe_dont_encrypt, } @@ -1574,6 +1586,12 @@ pub mod args { "A public key for signing protocol transactions. A new \ one will be generated if none given.", )) + .arg(COMMISSION_RATE.def().about( + "The commission rate charged by the validator for delegation rewards. This is a required parameter.", + )) + .arg(MAX_COMMISSION_RATE_CHANGE.def().about( + "The maximum change per epoch in the commission rate charged by the validator for delegation rewards. This is a required parameter.", + )) .arg(VALIDATOR_CODE_PATH.def().about( "The path to the validity predicate WASM code to be used \ for the validator account. Uses the default validator VP \ @@ -2552,6 +2570,8 @@ pub mod args { #[derive(Clone, Debug)] pub struct InitGenesisValidator { pub alias: String, + pub commission_rate: Decimal, + pub max_commission_rate_change: Decimal, pub net_address: SocketAddr, pub unsafe_dont_encrypt: bool, pub key_scheme: SchemeType, @@ -2560,6 +2580,8 @@ pub mod args { impl Args for InitGenesisValidator { fn parse(matches: &ArgMatches) -> Self { let alias = ALIAS.parse(matches); + let commission_rate = COMMISSION_RATE.parse(matches); + let max_commission_rate_change = MAX_COMMISSION_RATE_CHANGE.parse(matches); let net_address = NET_ADDRESS.parse(matches); let unsafe_dont_encrypt = UNSAFE_DONT_ENCRYPT.parse(matches); let key_scheme = SCHEME.parse(matches); @@ -2568,6 +2590,8 @@ pub mod args { net_address, unsafe_dont_encrypt, key_scheme, + commission_rate, + max_commission_rate_change } } @@ -2578,6 +2602,12 @@ pub mod args { Anoma uses port `26656` for P2P connections by default, \ but you can configure a different value.", )) + .arg(COMMISSION_RATE.def().about( + "The commission rate charged by the validator for delegation rewards. This is a required parameter.", + )) + .arg(MAX_COMMISSION_RATE_CHANGE.def().about( + "The maximum change per epoch in the commission rate charged by the validator for delegation rewards. This is a required parameter.", + )) .arg(UNSAFE_DONT_ENCRYPT.def().about( "UNSAFE: Do not encrypt the generated keypairs. Do not \ use this for keys used in a live network.", diff --git a/apps/src/lib/client/tx.rs b/apps/src/lib/client/tx.rs index d558c44de4..e2c7a79942 100644 --- a/apps/src/lib/client/tx.rs +++ b/apps/src/lib/client/tx.rs @@ -23,6 +23,11 @@ use namada::types::transaction::governance::{ use namada::types::transaction::{pos, InitAccount, InitValidator, UpdateVp}; use namada::types::{address, storage, token}; use namada::{ledger, vm}; +use rust_decimal::Decimal; +use tendermint_config::net::Address as TendermintAddress; +use tendermint_rpc::endpoint::broadcast::tx_sync::Response; +use tendermint_rpc::query::{EventType, Query}; +use tendermint_rpc::{Client, HttpClient}; use super::rpc; use crate::cli::context::WalletAddress; @@ -155,6 +160,8 @@ pub async fn submit_init_validator( account_key, consensus_key, protocol_key, + commission_rate, + max_commission_rate_change, validator_vp_code_path, unsafe_dont_encrypt, }: args::TxInitValidator, @@ -220,6 +227,14 @@ pub async fn submit_init_validator( let validator_vp_code = validator_vp_code_path .map(|path| ctx.read_wasm(path)) .unwrap_or_else(|| ctx.read_wasm(VP_USER_WASM)); + + // Validate the commission rate data + if commission_rate > Decimal::ONE || commission_rate < Decimal::ZERO { + eprintln!("The validator commission rate must not exceed 1.0 or 100%, and it must be 0 or positive"); + } + if max_commission_rate_change > Decimal::ONE || max_commission_rate_change < Decimal::ZERO { + eprintln!("The validator maximum change in commission rate per epoch must not exceed 1.0 or 100%"); + } // Validate the validator VP code if let Err(err) = vm::validate_untrusted_wasm(&validator_vp_code) { eprintln!( @@ -237,6 +252,8 @@ pub async fn submit_init_validator( consensus_key: consensus_key.ref_to(), protocol_key, dkg_key, + commission_rate, + max_commission_rate_change, validator_vp_code, }; let data = data.try_to_vec().expect("Encoding tx data shouldn't fail"); diff --git a/apps/src/lib/client/utils.rs b/apps/src/lib/client/utils.rs index d3a4f31fd7..d892601ac6 100644 --- a/apps/src/lib/client/utils.rs +++ b/apps/src/lib/client/utils.rs @@ -880,6 +880,8 @@ pub fn init_genesis_validator( global_args: args::Global, args::InitGenesisValidator { alias, + commission_rate, + max_commission_rate_change, net_address, unsafe_dont_encrypt, key_scheme, @@ -933,6 +935,8 @@ pub fn init_genesis_validator( .public() .to_string(), )), + commission_rate: Some(commission_rate), + max_commission_rate_change: Some(max_commission_rate_change), tendermint_node_key: Some(HexString( pre_genesis.tendermint_node_key.ref_to().to_string(), )), diff --git a/apps/src/lib/config/genesis.rs b/apps/src/lib/config/genesis.rs index c0135708bd..89b07b14af 100644 --- a/apps/src/lib/config/genesis.rs +++ b/apps/src/lib/config/genesis.rs @@ -172,6 +172,11 @@ pub mod genesis_config { // Unstaked balance at genesis. // XXX: u64 doesn't work with toml-rs! pub non_staked_balance: Option, + /// Commission rate charged on rewards for delegators (bounded inside + /// 0-1) + pub commission_rate: Option, + /// Maximum change in commission rate permitted per epoch + pub max_commission_rate_change: Option, // Filename of validator VP. (default: default validator VP) pub validator_vp: Option, // IP:port of the validator. (used in generation only) @@ -179,11 +184,6 @@ pub mod genesis_config { /// Tendermint node key is used to derive Tendermint node ID for node /// authentication pub tendermint_node_key: Option, - /// Commission rate charged on rewards for delegators (bounded inside - /// 0-1) - pub commission_rate: Option, - /// Maximum change in commission rate permitted per epoch - pub max_commission_rate_change: Option, } #[derive(Clone, Debug, Deserialize, Serialize)] diff --git a/genesis/dev.toml b/genesis/dev.toml index 3c4bc101b7..2baa503dfe 100644 --- a/genesis/dev.toml +++ b/genesis/dev.toml @@ -15,6 +15,10 @@ tokens = 200000 non_staked_balance = 100000 # VP for the validator account validator_vp = "vp_user" +# Commission rate for rewards +commission_rate = 0.05 +# Maximum change per epoch in the commission rate +max_commission_rate_change = 0.01 # Public IP:port address net_address = "127.0.0.1:26656" diff --git a/genesis/e2e-tests-single-node.toml b/genesis/e2e-tests-single-node.toml index d4934d78db..88634561c3 100644 --- a/genesis/e2e-tests-single-node.toml +++ b/genesis/e2e-tests-single-node.toml @@ -11,6 +11,10 @@ tokens = 200000 non_staked_balance = 1000000000000 # VP for the validator account validator_vp = "vp_user" +# Commission rate for rewards +commission_rate = 0.05 +# Maximum change per epoch in the commission rate +max_commission_rate_change = 0.01 # Public IP:port address. # We set the port to be the default+1000, so that if a local node was running at # the same time as the E2E tests, it wouldn't affect them. diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs index 51d64f26ca..bd2e3260fe 100644 --- a/proof_of_stake/src/lib.rs +++ b/proof_of_stake/src/lib.rs @@ -193,6 +193,18 @@ pub trait PosActions: PosReadOnly { key: &Self::Address, value: ValidatorStates, ) -> Result<(), Self::Error>; + /// Write PoS validator's commission rate for delegator rewards + fn write_validator_commission_rate( + &mut self, + key: &Self::Address, + value: Decimal, + ) -> Result<(), Self::Error>; + /// Write PoS validator's maximum change in the commission rate per epoch + fn write_validator_max_commission_rate_change( + &mut self, + key: &Self::Address, + value: Decimal, + ) -> Result<(), Self::Error>; /// Write PoS validator's total deltas of their bonds (validator self-bonds /// and delegations). fn write_validator_total_deltas( @@ -257,6 +269,8 @@ pub trait PosActions: PosReadOnly { address: &Self::Address, consensus_key: &Self::PublicKey, current_epoch: impl Into, + commission_rate: Decimal, + max_commission_rate_change: Decimal ) -> Result<(), Self::BecomeValidatorError> { let current_epoch = current_epoch.into(); let params = self.read_pos_params()?; @@ -270,12 +284,16 @@ pub trait PosActions: PosReadOnly { state, total_deltas, voting_power, + commission_rate, + max_commission_rate_change } = become_validator( ¶ms, address, consensus_key, &mut validator_set, current_epoch, + commission_rate, + max_commission_rate_change ); self.write_validator_consensus_key(address, consensus_key)?; self.write_validator_state(address, state)?; @@ -283,6 +301,10 @@ pub trait PosActions: PosReadOnly { self.write_validator_address_raw_hash(address, &consensus_key_clone)?; self.write_validator_total_deltas(address, total_deltas)?; self.write_validator_voting_power(address, voting_power)?; + self.write_validator_commission_rate(address, commission_rate)?; + self.write_validator_max_commission_rate_change(address, max_commission_rate_change)?; + + // Do we need to write the total deltas of all validators? Ok(()) } @@ -1328,6 +1350,8 @@ where state: ValidatorStates, total_deltas: ValidatorTotalDeltas, voting_power: ValidatorVotingPowers, + commission_rate: Decimal, + max_commission_rate_change: Decimal, } /// A function that initialized data for a new validator. @@ -1337,6 +1361,8 @@ fn become_validator( consensus_key: &PK, validator_set: &mut ValidatorSets
, current_epoch: Epoch, + commission_rate: Decimal, + max_commission_rate_change: Decimal, ) -> BecomeValidatorData where Address: Debug @@ -1399,6 +1425,8 @@ where state, total_deltas, voting_power, + commission_rate, + max_commission_rate_change } } diff --git a/shared/src/types/transaction/mod.rs b/shared/src/types/transaction/mod.rs index 08ac8ceb09..8a391d858f 100644 --- a/shared/src/types/transaction/mod.rs +++ b/shared/src/types/transaction/mod.rs @@ -21,6 +21,7 @@ pub use decrypted::*; #[cfg(feature = "ferveo-tpke")] pub use encrypted::EncryptionKey; pub use protocol::UpdateDkgSessionKey; +use rust_decimal::Decimal; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; pub use wrapper::*; @@ -188,6 +189,10 @@ pub struct InitValidator { pub protocol_key: common::PublicKey, /// Serialization of the public session key used in the DKG pub dkg_key: DkgPublicKey, + /// The initial commission rate charged for delegation rewards + pub commission_rate: Decimal, + /// The maximum change allowed per epoch to the commission rate. This is immutable once set here. + pub max_commission_rate_change: Decimal, /// The VP code for validator account pub validator_vp_code: Vec, } diff --git a/tx_prelude/Cargo.toml b/tx_prelude/Cargo.toml index 11449b9460..b5b8df7f51 100644 --- a/tx_prelude/Cargo.toml +++ b/tx_prelude/Cargo.toml @@ -16,3 +16,4 @@ namada_macros = {path = "../macros"} borsh = "0.9.0" sha2 = "0.10.1" thiserror = "1.0.30" +rust_decimal = "1.26.1" diff --git a/tx_prelude/src/proof_of_stake.rs b/tx_prelude/src/proof_of_stake.rs index ee50943336..d7ad7099fd 100644 --- a/tx_prelude/src/proof_of_stake.rs +++ b/tx_prelude/src/proof_of_stake.rs @@ -6,6 +6,7 @@ use namada::ledger::pos::{ unbond_key, validator_address_raw_hash_key, validator_consensus_key_key, validator_set_key, validator_slashes_key, validator_state_key, validator_total_deltas_key, validator_voting_power_key, + validator_commission_rate_key, validator_max_commission_rate_change_key }; use namada::types::address::Address; use namada::types::transaction::InitValidator; @@ -14,6 +15,7 @@ pub use namada_proof_of_stake::{ epoched, parameters, types, PosActions as PosWrite, PosReadOnly as PosRead, }; +use rust_decimal::Decimal; use super::*; impl Ctx { @@ -81,6 +83,8 @@ impl Ctx { consensus_key, protocol_key, dkg_key, + commission_rate, + max_commission_rate_change, validator_vp_code, }: InitValidator, ) -> EnvResult
{ @@ -98,6 +102,8 @@ impl Ctx { &validator_address, &consensus_key, current_epoch, + commission_rate, + max_commission_rate_change )?; Ok(validator_address) @@ -147,6 +153,24 @@ impl namada_proof_of_stake::PosActions for Ctx { self.write(&validator_state_key(key), &value) } + fn write_validator_commission_rate( + &mut self, + key: &Self::Address, + value: Decimal, + ) -> Result<(), Self::Error> { + self.write(&validator_commission_rate_key(key), &value) + .into_env_result() + } + + fn write_validator_max_commission_rate_change( + &mut self, + key: &Self::Address, + value: Decimal, + ) -> Result<(), Self::Error> { + self.write(&validator_max_commission_rate_change_key(key), &value) + .into_env_result() + } + fn write_validator_total_deltas( &mut self, key: &Self::Address, From 4b88573791b3d19fe15b9c347c6adf81fc472d37 Mon Sep 17 00:00:00 2001 From: brentstone Date: Tue, 27 Sep 2022 11:01:00 -0400 Subject: [PATCH 12/38] epoched commission rate and tx for validator to change their rate --- proof_of_stake/src/lib.rs | 130 +++++++++++++++++++++++++++++-- proof_of_stake/src/types.rs | 2 + shared/src/ledger/pos/mod.rs | 21 +++++ shared/src/ledger/pos/storage.rs | 12 +-- shared/src/ledger/pos/vp.rs | 3 +- tx_prelude/src/proof_of_stake.rs | 8 +- wasm/wasm_source/src/tx_bond.rs | 4 + 7 files changed, 161 insertions(+), 19 deletions(-) diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs index bd2e3260fe..f218fcd265 100644 --- a/proof_of_stake/src/lib.rs +++ b/proof_of_stake/src/lib.rs @@ -33,7 +33,7 @@ use epoched::{ use parameters::PosParams; use thiserror::Error; use types::{ - ActiveValidator, Bonds, Epoch, GenesisValidator, Slash, SlashType, Slashes, + ActiveValidator, Bonds, CommissionRates, Epoch, GenesisValidator, Slash, SlashType, Slashes, TotalVotingPowers, Unbond, Unbonds, ValidatorConsensusKeys, ValidatorSet, ValidatorSetUpdate, ValidatorSets, ValidatorState, ValidatorStates, ValidatorTotalDeltas, ValidatorVotingPowers, VotingPower, VotingPowerDelta, @@ -133,6 +133,17 @@ pub trait PosReadOnly { &self, key: &Self::Address, ) -> Result, Self::Error>; + /// Read PoS validator's commission rate for delegation rewards + fn read_validator_commission_rate( + &self, + key: &Self::Address, + ) -> Result, Self::Error>; + /// Read PoS validator's maximum change in the commission rate for + /// delegation rewards + fn read_validator_max_commission_rate_change( + &self, + key: &Self::Address, + ) -> Result, Self::Error>; /// Read PoS bond (validator self-bond or a delegation). fn read_bond( &self, @@ -197,7 +208,7 @@ pub trait PosActions: PosReadOnly { fn write_validator_commission_rate( &mut self, key: &Self::Address, - value: Decimal, + value: CommissionRates, ) -> Result<(), Self::Error>; /// Write PoS validator's maximum change in the commission rate per epoch fn write_validator_max_commission_rate_change( @@ -301,8 +312,14 @@ pub trait PosActions: PosReadOnly { self.write_validator_address_raw_hash(address, &consensus_key_clone)?; self.write_validator_total_deltas(address, total_deltas)?; self.write_validator_voting_power(address, voting_power)?; - self.write_validator_commission_rate(address, commission_rate)?; - self.write_validator_max_commission_rate_change(address, max_commission_rate_change)?; + self.write_validator_max_commission_rate_change( + address, + max_commission_rate_change, + )?; + + let commission_rates = + Epoched::init(commission_rate, current_epoch, ¶ms); + self.write_validator_commission_rate(address, commission_rates)?; // Do we need to write the total deltas of all validators? Ok(()) @@ -511,6 +528,76 @@ pub trait PosActions: PosReadOnly { Ok(slashed) } + + /// Change the commission rate of a validator + fn change_validator_commission_rate( + &mut self, + params: &PosParams, + validator: &Self::Address, + change: Decimal, + current_epoch: impl Into, + ) -> Result<(), CommissionRateChangeError> { + let current_epoch = current_epoch.into(); + let max_change = self + .read_validator_max_commission_rate_change(validator) + .map_err(|_| { + CommissionRateChangeError::NoMaxSetInStorage(validator) + }) + .unwrap() + .unwrap(); + + if change == Decimal::ZERO { + return Err(CommissionRateChangeError::ChangeIsZero( + change, + validator.clone(), + )); + } else if change.abs() > max_change { + return Err(CommissionRateChangeError::RateChangeTooLarge( + change, + validator.clone(), + )); + } else { + let mut commission_rates = + match self.read_validator_commission_rate(validator) { + Ok(Some(rates)) => rates, + _ => { + return Err(CommissionRateChangeError::ChangeIsZero( + change, + validator.clone(), + )); + } + }; + let commission_rate = *commission_rates + .get_at_offset( + current_epoch, + DynEpochOffset::PipelineLen, + params, + ) + .expect("Could not find a rate in given epoch"); + if commission_rate + change < Decimal::ZERO { + return Err(CommissionRateChangeError::NegativeRate( + change, + validator.clone(), + )); + } else { + commission_rates.update_from_offset( + |val, _epoch| { + *val += commission_rate; + }, + current_epoch, + DynEpochOffset::PipelineLen, + params, + ); + self.write_validator_commission_rate( + validator, + commission_rates, + ) + .map_err(|_| CommissionRateChangeError::CannotWrite(validator)) + .unwrap(); + } + } + Ok(()) + } } /// PoS system base trait for system initialization on genesis block, updating @@ -612,7 +699,10 @@ pub trait PosBase { /// Read PoS slashes applied to a validator. fn read_validator_slashes(&self, key: &Self::Address) -> Slashes; /// Read PoS validator's commission rate - fn read_validator_commission_rate(&self, key: &Self::Address) -> Decimal; + fn read_validator_commission_rate( + &self, + key: &Self::Address, + ) -> CommissionRates; /// Read PoS validator's maximum commission rate change per epoch fn read_validator_max_commission_rate_change( &self, @@ -660,7 +750,7 @@ pub trait PosBase { fn write_validator_commission_rate( &mut self, key: &Self::Address, - value: &Decimal, + value: &CommissionRates, ); /// Write PoS validator's commission rate. fn write_validator_max_commission_rate_change( @@ -1028,6 +1118,26 @@ where NegativeStake(i128, Address), } +#[allow(missing_docs)] +#[derive(Error, Debug)] +pub enum CommissionRateChangeError
+where + Address: Display + Debug + Clone + PartialOrd + Ord + Hash, +{ + #[error("Unexpected negative commission rate {0} for validator {1}")] + NegativeRate(Decimal, Address), + #[error("Rate change of {0} is too large for validator {1}")] + RateChangeTooLarge(Decimal, Address), + #[error("The rate change is {0} for validator {1}")] + ChangeIsZero(Decimal, Address), + #[error( + "There is no maximum rate change written in storage for validator {0}" + )] + NoMaxSetInStorage(Address), + #[error("Cannot write to storage for validator {0}")] + CannotWrite(Address), +} + struct GenesisData where Validators: Iterator< @@ -1096,7 +1206,7 @@ where { address: Address, consensus_key: ValidatorConsensusKeys, - commission_rate: Decimal, + commission_rate: CommissionRates, max_commission_rate_change: Decimal, state: ValidatorStates, total_deltas: ValidatorTotalDeltas, @@ -1203,6 +1313,10 @@ where }| { let consensus_key = Epoched::init_at_genesis(consensus_key.clone(), current_epoch); + let commission_rate = Epoched::init_at_genesis( + commission_rate.clone(), + current_epoch, + ); let state = Epoched::init_at_genesis( ValidatorState::Candidate, current_epoch, @@ -1231,7 +1345,7 @@ where Ok(GenesisValidatorData { address: address.clone(), consensus_key, - commission_rate: commission_rate.clone(), + commission_rate, max_commission_rate_change: max_commission_rate_change.clone(), state, total_deltas, diff --git a/proof_of_stake/src/types.rs b/proof_of_stake/src/types.rs index 7b9f809e38..4366cf65c9 100644 --- a/proof_of_stake/src/types.rs +++ b/proof_of_stake/src/types.rs @@ -37,6 +37,8 @@ pub type ValidatorSets
= Epoched, OffsetUnbondingLen>; /// Epoched total voting power. pub type TotalVotingPowers = EpochedDelta; +/// Epoched validator commission rate +pub type CommissionRates = Epoched; /// Epoch identifier. Epochs are identified by consecutive natural numbers. /// diff --git a/shared/src/ledger/pos/mod.rs b/shared/src/ledger/pos/mod.rs index 011a96a6ba..62c754a7da 100644 --- a/shared/src/ledger/pos/mod.rs +++ b/shared/src/ledger/pos/mod.rs @@ -75,6 +75,9 @@ pub type GenesisValidator = namada_proof_of_stake::types::GenesisValidator< key::common::PublicKey, >; +/// Alias for a PoS type with the same name with concrete type parameters +pub type CommissionRates = namada_proof_of_stake::types::CommissionRates; + impl From for namada_proof_of_stake::types::Epoch { fn from(epoch: Epoch) -> Self { let epoch: u64 = epoch.into(); @@ -175,6 +178,24 @@ mod macros { Ok(value.map(|value| $crate::ledger::storage::types::decode(value).unwrap())) } + fn read_validator_commission_rate( + &self, + key: &Self::Address, + ) -> std::result::Result, Self::Error> { + let value = + $crate::ledger::storage_api::StorageRead::read_bytes(self, &validator_commission_rate_key(key))?; + Ok(value.map(|value| $crate::ledger::storage::types::decode(value).unwrap())) + } + + fn read_validator_max_commission_rate_change( + &self, + key: &Self::Address, + ) -> std::result::Result, Self::Error> { + let value = + $crate::ledger::storage_api::StorageRead::read_bytes(self, &validator_max_commission_rate_change_key(key))?; + Ok(value.map(|value| $crate::ledger::storage::types::decode(value).unwrap())) + } + fn read_validator_state( &self, key: &Self::Address, diff --git a/shared/src/ledger/pos/storage.rs b/shared/src/ledger/pos/storage.rs index ac4b007b63..a5707073ef 100644 --- a/shared/src/ledger/pos/storage.rs +++ b/shared/src/ledger/pos/storage.rs @@ -8,7 +8,7 @@ use namada_proof_of_stake::{types, PosBase}; use super::{ BondId, Bonds, ValidatorConsensusKeys, ValidatorSets, ValidatorTotalDeltas, - ADDRESS, + ADDRESS, CommissionRates }; use crate::ledger::storage::types::{decode, encode}; use crate::ledger::storage::{self, Storage, StorageHasher}; @@ -471,7 +471,7 @@ where fn read_validator_commission_rate( &self, key: &Self::Address, - ) -> rust_decimal::Decimal { + ) -> CommissionRates { let (value, _gas) = self.read(&validator_commission_rate_key(key)).unwrap(); value.map(|value| decode(value).unwrap()).unwrap() @@ -510,10 +510,10 @@ where } fn write_validator_commission_rate( - &mut self, - key: &Self::Address, - value: &rust_decimal::Decimal, - ) { + &mut self, + key: &Self::Address, + value: &CommissionRates, + ) { self.write(&validator_commission_rate_key(key), encode(value)) .unwrap(); } diff --git a/shared/src/ledger/pos/vp.rs b/shared/src/ledger/pos/vp.rs index 058acac262..191a2c19a2 100644 --- a/shared/src/ledger/pos/vp.rs +++ b/shared/src/ledger/pos/vp.rs @@ -13,6 +13,7 @@ pub use namada_proof_of_stake::types::{ }; use namada_proof_of_stake::validation::validate; use namada_proof_of_stake::{validation, PosReadOnly}; +use rust_decimal::Decimal; use thiserror::Error; use super::{ @@ -22,7 +23,7 @@ use super::{ total_voting_power_key, unbond_key, validator_consensus_key_key, validator_set_key, validator_slashes_key, validator_state_key, validator_total_deltas_key, validator_voting_power_key, BondId, Bonds, - Unbonds, ValidatorConsensusKeys, ValidatorSets, ValidatorTotalDeltas, + Unbonds, ValidatorConsensusKeys, ValidatorSets, ValidatorTotalDeltas, validator_commission_rate_key, max_commission_rate_change_key, }; use crate::impl_pos_read_only; use crate::ledger::governance::vp::is_proposal_accepted; diff --git a/tx_prelude/src/proof_of_stake.rs b/tx_prelude/src/proof_of_stake.rs index d7ad7099fd..e929b0fb57 100644 --- a/tx_prelude/src/proof_of_stake.rs +++ b/tx_prelude/src/proof_of_stake.rs @@ -154,10 +154,10 @@ impl namada_proof_of_stake::PosActions for Ctx { } fn write_validator_commission_rate( - &mut self, - key: &Self::Address, - value: Decimal, - ) -> Result<(), Self::Error> { + &mut self, + key: &Self::Address, + value: CommissionRates, + ) -> Result<(), Self::Error> { self.write(&validator_commission_rate_key(key), &value) .into_env_result() } diff --git a/wasm/wasm_source/src/tx_bond.rs b/wasm/wasm_source/src/tx_bond.rs index bda4818722..fd43138662 100644 --- a/wasm/wasm_source/src/tx_bond.rs +++ b/wasm/wasm_source/src/tx_bond.rs @@ -69,11 +69,15 @@ mod tests { let is_delegation = matches!( &bond.source, Some(source) if *source != bond.validator); let consensus_key = key::testing::keypair_1().ref_to(); + let commission_rate = rust_decimal::Decimal::new(5, 2); + let max_commission_rate_change = rust_decimal::Decimal::new(1, 2); let genesis_validators = [GenesisValidator { address: bond.validator.clone(), tokens: initial_stake, consensus_key, + commission_rate, + max_commission_rate_change, }]; init_pos(&genesis_validators[..], &pos_params, Epoch(0)); From 4ff76d8eb07de3fd37cd237178b51089ae57cf7d Mon Sep 17 00:00:00 2001 From: brentstone Date: Wed, 28 Sep 2022 14:16:35 -0400 Subject: [PATCH 13/38] commission rate: query + refactor validator change tx --- apps/src/bin/anoma-client/cli.rs | 3 + apps/src/lib/cli.rs | 55 +++++++++++++++++ apps/src/lib/client/rpc.rs | 41 +++++++++++++ proof_of_stake/src/lib.rs | 102 +++++++++++++++++-------------- 4 files changed, 154 insertions(+), 47 deletions(-) diff --git a/apps/src/bin/anoma-client/cli.rs b/apps/src/bin/anoma-client/cli.rs index 841e4b56ae..e4d2461525 100644 --- a/apps/src/bin/anoma-client/cli.rs +++ b/apps/src/bin/anoma-client/cli.rs @@ -58,6 +58,9 @@ pub async fn main() -> Result<()> { Sub::QueryVotingPower(QueryVotingPower(args)) => { rpc::query_voting_power(ctx, args).await; } + Sub::QueryCommissionRate(QueryCommissionRate(args)) => { + rpc::query_commission_rate(ctx, args).await; + } Sub::QuerySlashes(QuerySlashes(args)) => { rpc::query_slashes(ctx, args).await; } diff --git a/apps/src/lib/cli.rs b/apps/src/lib/cli.rs index 9c7366faa4..a896dfffd6 100644 --- a/apps/src/lib/cli.rs +++ b/apps/src/lib/cli.rs @@ -273,6 +273,7 @@ pub mod cmds { QueryBalance(QueryBalance), QueryBonds(QueryBonds), QueryVotingPower(QueryVotingPower), + QueryCommissionRate(QueryCommissionRate), QuerySlashes(QuerySlashes), QueryRawBytes(QueryRawBytes), QueryProposal(QueryProposal), @@ -999,6 +1000,25 @@ pub mod cmds { } } + #[derive(Clone, Debug)] + pub struct QueryCommissionRate(pub args::QueryCommissionRate); + + impl SubCmd for QueryCommissionRate { + const CMD: &'static str = "commission-rate"; + + fn parse(matches: &ArgMatches) -> Option { + matches.subcommand_matches(Self::CMD).map(|matches| { + QueryCommissionRate(args::QueryCommissionRate::parse(matches)) + }) + } + + fn def() -> App { + App::new(Self::CMD) + .about("Query commission rate.") + .add_args::() + } + } + #[derive(Clone, Debug)] pub struct QuerySlashes(pub args::QuerySlashes); @@ -2071,6 +2091,41 @@ pub mod args { } } + /// Query PoS commission rate + #[derive(Clone, Debug)] + pub struct QueryCommissionRate { + /// Common query args + pub query: Query, + /// Address of a validator + pub validator: Option, + /// Epoch in which to find commission rate + pub epoch: Option, + } + + impl Args for QueryCommissionRate { + fn parse(matches: &ArgMatches) -> Self { + let query = Query::parse(matches); + let validator = VALIDATOR_OPT.parse(matches); + let epoch = EPOCH.parse(matches); + Self { + query, + validator, + epoch, + } + } + + fn def(app: App) -> App { + app.add_args::() + .arg(VALIDATOR_OPT.def().about( + "The validator's address whose commission rate to query.", + )) + .arg(EPOCH.def().about( + "The epoch at which to query (last committed, if not \ + specified).", + )) + } + } + /// Query PoS slashes #[derive(Clone, Debug)] pub struct QuerySlashes { diff --git a/apps/src/lib/client/rpc.rs b/apps/src/lib/client/rpc.rs index ce6a542897..c8740bce85 100644 --- a/apps/src/lib/client/rpc.rs +++ b/apps/src/lib/client/rpc.rs @@ -975,6 +975,47 @@ pub async fn query_voting_power(ctx: Context, args: args::QueryVotingPower) { println!("Total voting power: {}", total_voting_power); } +/// Query PoS commssion rate +pub async fn query_commission_rate(ctx: Context, args: args::QueryCommissionRate) { + let epoch = match args.epoch { + Some(epoch) => epoch, + None => query_epoch(args.query.clone()).await, + }; + let client = HttpClient::new(args.query.ledger_address).unwrap(); + + match args.validator { + Some(validator) => { + let validator = ctx.get(&validator); + let validator_commission_key = pos::validator_commission_rate_key(&validator); + let commission_rates = query_storage_value::( + &client, + &validator_commission_key, + ) + .await; + let commission_rates = commission_rates + .expect("No commission rate found "); + match commission_rates.get(epoch) { + Some(rate) => { + println!( + "Validator {} commission rate: {}", + validator.encode(), + *rate + ) + } + None => { + println!("No commission rate found for {} in epoch {}", + validator.encode(), + epoch + ) + } + } + } + None => { + println!("No validator found from the args") + } + } +} + /// Query PoS slashes pub async fn query_slashes(ctx: Context, args: args::QuerySlashes) { let client = HttpClient::new(args.query.ledger_address).unwrap(); diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs index f218fcd265..237364775b 100644 --- a/proof_of_stake/src/lib.rs +++ b/proof_of_stake/src/lib.rs @@ -534,9 +534,15 @@ pub trait PosActions: PosReadOnly { &mut self, params: &PosParams, validator: &Self::Address, - change: Decimal, + new_rate: Decimal, current_epoch: impl Into, ) -> Result<(), CommissionRateChangeError> { + if new_rate < Decimal::ZERO { + return Err(CommissionRateChangeError::NegativeRate( + new_rate, + validator.clone(), + )); + } let current_epoch = current_epoch.into(); let max_change = self .read_validator_max_commission_rate_change(validator) @@ -545,57 +551,57 @@ pub trait PosActions: PosReadOnly { }) .unwrap() .unwrap(); - - if change == Decimal::ZERO { + let mut commission_rates = + match self.read_validator_commission_rate(validator) { + Ok(Some(rates)) => rates, + _ => { + return Err(CommissionRateChangeError::CannotRead( + validator.clone(), + )); + } + }; + let rate_at_pipeline = *commission_rates + .get_at_offset( + current_epoch, + DynEpochOffset::PipelineLen, + params, + ) + .expect("Could not find a rate in given epoch"); + if new_rate == rate_at_pipeline { return Err(CommissionRateChangeError::ChangeIsZero( - change, validator.clone(), )); - } else if change.abs() > max_change { + } + + let rate_before_pipeline = *commission_rates + .get_at_offset( + current_epoch-1, + DynEpochOffset::PipelineLen, + params, + ) + .expect("Could not find a rate in given epoch"); + let change_from_prev = new_rate - rate_before_pipeline; + if change_from_prev.abs() > max_change { return Err(CommissionRateChangeError::RateChangeTooLarge( - change, + change_from_prev, validator.clone(), )); - } else { - let mut commission_rates = - match self.read_validator_commission_rate(validator) { - Ok(Some(rates)) => rates, - _ => { - return Err(CommissionRateChangeError::ChangeIsZero( - change, - validator.clone(), - )); - } - }; - let commission_rate = *commission_rates - .get_at_offset( - current_epoch, - DynEpochOffset::PipelineLen, - params, - ) - .expect("Could not find a rate in given epoch"); - if commission_rate + change < Decimal::ZERO { - return Err(CommissionRateChangeError::NegativeRate( - change, - validator.clone(), - )); - } else { - commission_rates.update_from_offset( - |val, _epoch| { - *val += commission_rate; - }, - current_epoch, - DynEpochOffset::PipelineLen, - params, - ); - self.write_validator_commission_rate( - validator, - commission_rates, - ) - .map_err(|_| CommissionRateChangeError::CannotWrite(validator)) - .unwrap(); - } } + commission_rates.update_from_offset( + |val, _epoch| { + *val = new_rate; + }, + current_epoch, + DynEpochOffset::PipelineLen, + params, + ); + self.write_validator_commission_rate( + validator, + commission_rates, + ) + .map_err(|_| CommissionRateChangeError::CannotWrite(validator)) + .unwrap(); + Ok(()) } } @@ -1128,14 +1134,16 @@ where NegativeRate(Decimal, Address), #[error("Rate change of {0} is too large for validator {1}")] RateChangeTooLarge(Decimal, Address), - #[error("The rate change is {0} for validator {1}")] - ChangeIsZero(Decimal, Address), + #[error("The rate change is 0 for validator {0}")] + ChangeIsZero(Address), #[error( "There is no maximum rate change written in storage for validator {0}" )] NoMaxSetInStorage(Address), #[error("Cannot write to storage for validator {0}")] CannotWrite(Address), + #[error("Cannot read storage for validator {0}")] + CannotRead(Address), } struct GenesisData From 77dfff4aea5eb684e24feb399c743434b5eda7d1 Mon Sep 17 00:00:00 2001 From: brentstone Date: Wed, 26 Oct 2022 14:19:06 -0400 Subject: [PATCH 14/38] add missing commission rate-related instances --- proof_of_stake/src/lib.rs | 5 +++++ shared/src/ledger/pos/vp.rs | 6 ++++-- wasm/wasm_source/src/tx_unbond.rs | 4 ++++ wasm/wasm_source/src/tx_withdraw.rs | 4 ++++ 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs index 237364775b..fe0aa2c153 100644 --- a/proof_of_stake/src/lib.rs +++ b/proof_of_stake/src/lib.rs @@ -846,6 +846,11 @@ pub trait PosBase { self.write_validator_total_deltas(address, &total_deltas); self.write_validator_voting_power(address, &voting_power); self.write_bond(&bond_id, &bond); + self.write_validator_commission_rate(address, &commission_rate); + self.write_validator_max_commission_rate_change( + address, + &max_commission_rate_change, + ); } self.write_validator_set(&validator_set); self.write_total_voting_power(&total_voting_power); diff --git a/shared/src/ledger/pos/vp.rs b/shared/src/ledger/pos/vp.rs index 191a2c19a2..185a1bbc4d 100644 --- a/shared/src/ledger/pos/vp.rs +++ b/shared/src/ledger/pos/vp.rs @@ -20,10 +20,12 @@ use super::{ bond_key, is_bond_key, is_params_key, is_total_voting_power_key, is_unbond_key, is_validator_set_key, is_validator_total_deltas_key, is_validator_voting_power_key, params_key, staking_token_address, - total_voting_power_key, unbond_key, validator_consensus_key_key, + total_voting_power_key, unbond_key, validator_commission_rate_key, + validator_consensus_key_key, validator_max_commission_rate_change_key, validator_set_key, validator_slashes_key, validator_state_key, validator_total_deltas_key, validator_voting_power_key, BondId, Bonds, - Unbonds, ValidatorConsensusKeys, ValidatorSets, ValidatorTotalDeltas, validator_commission_rate_key, max_commission_rate_change_key, + CommissionRates, Unbonds, ValidatorConsensusKeys, ValidatorSets, + ValidatorTotalDeltas, }; use crate::impl_pos_read_only; use crate::ledger::governance::vp::is_proposal_accepted; diff --git a/wasm/wasm_source/src/tx_unbond.rs b/wasm/wasm_source/src/tx_unbond.rs index df4299deab..3b9f9bc76e 100644 --- a/wasm/wasm_source/src/tx_unbond.rs +++ b/wasm/wasm_source/src/tx_unbond.rs @@ -68,6 +68,8 @@ mod tests { let is_delegation = matches!( &unbond.source, Some(source) if *source != unbond.validator); let consensus_key = key::testing::keypair_1().ref_to(); + let commission_rate = rust_decimal::Decimal::new(5, 2); + let max_commission_rate_change = rust_decimal::Decimal::new(1, 2); let genesis_validators = [GenesisValidator { address: unbond.validator.clone(), @@ -79,6 +81,8 @@ mod tests { initial_stake }, consensus_key, + commission_rate, + max_commission_rate_change, }]; init_pos(&genesis_validators[..], &pos_params, Epoch(0)); diff --git a/wasm/wasm_source/src/tx_withdraw.rs b/wasm/wasm_source/src/tx_withdraw.rs index 4cc0e35691..3525b7b7cc 100644 --- a/wasm/wasm_source/src/tx_withdraw.rs +++ b/wasm/wasm_source/src/tx_withdraw.rs @@ -74,6 +74,8 @@ mod tests { let is_delegation = matches!( &withdraw.source, Some(source) if *source != withdraw.validator); let consensus_key = key::testing::keypair_1().ref_to(); + let commission_rate = rust_decimal::Decimal::new(5, 2); + let max_commission_rate_change = rust_decimal::Decimal::new(1, 2); let genesis_validators = [GenesisValidator { address: withdraw.validator.clone(), @@ -86,6 +88,8 @@ mod tests { initial_stake }, consensus_key, + commission_rate, + max_commission_rate_change, }]; init_pos(&genesis_validators[..], &pos_params, Epoch(0)); From 5b6565e14a162124037ef67ada4d27a550e09b69 Mon Sep 17 00:00:00 2001 From: brentstone Date: Wed, 26 Oct 2022 14:21:43 -0400 Subject: [PATCH 15/38] include and update `rust_decimal` --- Cargo.lock | 15 +++++++++++++++ apps/Cargo.toml | 2 ++ apps/src/lib/config/genesis.rs | 2 ++ proof_of_stake/Cargo.toml | 2 ++ proof_of_stake/src/lib.rs | 1 + proof_of_stake/src/types.rs | 1 + wasm/Cargo.lock | 14 ++++++++++++++ wasm/wasm_source/Cargo.toml | 3 ++- wasm/wasm_source/src/tx_bond.rs | 1 + wasm_for_tests/wasm_source/Cargo.lock | 14 ++++++++++++++ 10 files changed, 54 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 60fe270c7b..98b3c64417 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3026,6 +3026,8 @@ dependencies = [ "rlimit", "rocksdb", "rpassword", + "rust_decimal", + "rust_decimal_macros", "serde 1.0.145", "serde_bytes", "serde_json", @@ -3085,6 +3087,8 @@ dependencies = [ "borsh", "derivative", "proptest", + "rust_decimal", + "rust_decimal_macros", "thiserror", ] @@ -4333,10 +4337,21 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ee9164faf726e4f3ece4978b25ca877ddc6802fa77f38cdccb32c7f805ecd70c" dependencies = [ "arrayvec 0.7.2", + "borsh", "num-traits 0.2.15", "serde 1.0.145", ] +[[package]] +name = "rust_decimal_macros" +version = "1.26.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4903d8db81d2321699ca8318035d6ff805c548868df435813968795a802171b2" +dependencies = [ + "quote", + "rust_decimal", +] + [[package]] name = "rustc-demangle" version = "0.1.21" diff --git a/apps/Cargo.toml b/apps/Cargo.toml index 87ad660beb..f11e1cd151 100644 --- a/apps/Cargo.toml +++ b/apps/Cargo.toml @@ -142,6 +142,8 @@ tracing-subscriber = {version = "0.3.7", features = ["env-filter"]} websocket = "0.26.2" winapi = "0.3.9" bimap = {version = "0.6.2", features = ["serde"]} +rust_decimal = "1.26.1" +rust_decimal_macros = "1.26.1" [dev-dependencies] namada = {path = "../shared", features = ["testing", "wasm-runtime"]} diff --git a/apps/src/lib/config/genesis.rs b/apps/src/lib/config/genesis.rs index 89b07b14af..03a614abde 100644 --- a/apps/src/lib/config/genesis.rs +++ b/apps/src/lib/config/genesis.rs @@ -36,6 +36,7 @@ pub mod genesis_config { use namada::types::key::*; use namada::types::time::Rfc3339String; use namada::types::{storage, token}; + use rust_decimal::Decimal; use serde::{Deserialize, Serialize}; use thiserror::Error; @@ -721,6 +722,7 @@ pub fn genesis(base_dir: impl AsRef, chain_id: &ChainId) -> Genesis { pub fn genesis() -> Genesis { use namada::ledger::parameters::EpochDuration; use namada::types::address; + use rust_decimal_macros::dec; use crate::wallet; diff --git a/proof_of_stake/Cargo.toml b/proof_of_stake/Cargo.toml index d6ee686121..c680a3229a 100644 --- a/proof_of_stake/Cargo.toml +++ b/proof_of_stake/Cargo.toml @@ -19,5 +19,7 @@ thiserror = "1.0.30" # A fork with state machine testing proptest = {git = "https://github.com/heliaxdev/proptest", branch = "tomas/sm", optional = true} derivative = "2.2.0" +rust_decimal = { version = "1.26.1", features = ["borsh"] } +rust_decimal_macros = "1.26.1" [dev-dependencies] diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs index fe0aa2c153..e0bf2192f6 100644 --- a/proof_of_stake/src/lib.rs +++ b/proof_of_stake/src/lib.rs @@ -31,6 +31,7 @@ use epoched::{ DynEpochOffset, EpochOffset, Epoched, EpochedDelta, OffsetPipelineLen, }; use parameters::PosParams; +use rust_decimal::Decimal; use thiserror::Error; use types::{ ActiveValidator, Bonds, CommissionRates, Epoch, GenesisValidator, Slash, SlashType, Slashes, diff --git a/proof_of_stake/src/types.rs b/proof_of_stake/src/types.rs index 4366cf65c9..a8cfcdbe1c 100644 --- a/proof_of_stake/src/types.rs +++ b/proof_of_stake/src/types.rs @@ -9,6 +9,7 @@ use std::num::TryFromIntError; use std::ops::{Add, AddAssign, Mul, Sub, SubAssign}; use borsh::{BorshDeserialize, BorshSchema, BorshSerialize}; +use rust_decimal::Decimal; use crate::epoched::{ Epoched, EpochedDelta, OffsetPipelineLen, OffsetUnbondingLen, diff --git a/wasm/Cargo.lock b/wasm/Cargo.lock index 9d63d52b84..3345a04b2c 100644 --- a/wasm/Cargo.lock +++ b/wasm/Cargo.lock @@ -1418,6 +1418,8 @@ dependencies = [ "borsh", "derivative", "proptest", + "rust_decimal", + "rust_decimal_macros", "thiserror", ] @@ -1448,6 +1450,7 @@ dependencies = [ "namada", "namada_macros", "namada_vm_env", + "rust_decimal", "sha2 0.10.6", "thiserror", ] @@ -1987,10 +1990,21 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ee9164faf726e4f3ece4978b25ca877ddc6802fa77f38cdccb32c7f805ecd70c" dependencies = [ "arrayvec", + "borsh", "num-traits", "serde", ] +[[package]] +name = "rust_decimal_macros" +version = "1.26.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4903d8db81d2321699ca8318035d6ff805c548868df435813968795a802171b2" +dependencies = [ + "quote", + "rust_decimal", +] + [[package]] name = "rustc-demangle" version = "0.1.21" diff --git a/wasm/wasm_source/Cargo.toml b/wasm/wasm_source/Cargo.toml index f306196c39..5abce7f1e1 100644 --- a/wasm/wasm_source/Cargo.toml +++ b/wasm/wasm_source/Cargo.toml @@ -32,7 +32,7 @@ namada_tx_prelude = {path = "../../tx_prelude", optional = true} namada_vp_prelude = {path = "../../vp_prelude", optional = true} borsh = "0.9.0" once_cell = {version = "1.8.0", optional = true} -rust_decimal = {version = "1.14.3", optional = true} +rust_decimal = {version = "1.26.1", optional = true} wee_alloc = "0.4.5" getrandom = { version = "0.2", features = ["custom"] } @@ -45,3 +45,4 @@ namada_vp_prelude = {path = "../../vp_prelude"} proptest = {git = "https://github.com/heliaxdev/proptest", branch = "tomas/sm"} tracing = "0.1.30" tracing-subscriber = {version = "0.3.7", default-features = false, features = ["env-filter", "fmt"]} +rust_decimal = "1.26.1" \ No newline at end of file diff --git a/wasm/wasm_source/src/tx_bond.rs b/wasm/wasm_source/src/tx_bond.rs index fd43138662..38002d2495 100644 --- a/wasm/wasm_source/src/tx_bond.rs +++ b/wasm/wasm_source/src/tx_bond.rs @@ -39,6 +39,7 @@ mod tests { staking_token_address, BondId, GenesisValidator, PosVP, }; use proptest::prelude::*; + use rust_decimal; use super::*; diff --git a/wasm_for_tests/wasm_source/Cargo.lock b/wasm_for_tests/wasm_source/Cargo.lock index 1cbb2ff069..27dc824168 100644 --- a/wasm_for_tests/wasm_source/Cargo.lock +++ b/wasm_for_tests/wasm_source/Cargo.lock @@ -1418,6 +1418,8 @@ dependencies = [ "borsh", "derivative", "proptest", + "rust_decimal", + "rust_decimal_macros", "thiserror", ] @@ -1448,6 +1450,7 @@ dependencies = [ "namada", "namada_macros", "namada_vm_env", + "rust_decimal", "sha2 0.10.6", "thiserror", ] @@ -1981,10 +1984,21 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ee9164faf726e4f3ece4978b25ca877ddc6802fa77f38cdccb32c7f805ecd70c" dependencies = [ "arrayvec", + "borsh", "num-traits", "serde", ] +[[package]] +name = "rust_decimal_macros" +version = "1.26.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4903d8db81d2321699ca8318035d6ff805c548868df435813968795a802171b2" +dependencies = [ + "quote", + "rust_decimal", +] + [[package]] name = "rustc-demangle" version = "0.1.21" From 288bf334d528184d9eb0e421b8b8b35db3f71f7d Mon Sep 17 00:00:00 2001 From: brentstone Date: Wed, 26 Oct 2022 14:23:29 -0400 Subject: [PATCH 16/38] bug fix from splitting this PR off of #388 --- shared/src/ledger/pos/storage.rs | 12 +++++++----- tx_prelude/src/proof_of_stake.rs | 2 -- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/shared/src/ledger/pos/storage.rs b/shared/src/ledger/pos/storage.rs index a5707073ef..aaa48d164e 100644 --- a/shared/src/ledger/pos/storage.rs +++ b/shared/src/ledger/pos/storage.rs @@ -171,7 +171,7 @@ pub fn is_validator_max_commission_rate_change_key( } /// Storage key for validator's consensus key. -pub fn validator_consensus_key_key(validator: &Address) -> Key { +pub fn validator_state_key(validator: &Address) -> Key { validator_prefix(validator) .push(&VALIDATOR_STATE_STORAGE_KEY.to_owned()) .expect("Cannot obtain a storage key") @@ -474,17 +474,19 @@ where ) -> CommissionRates { let (value, _gas) = self.read(&validator_commission_rate_key(key)).unwrap(); - value.map(|value| decode(value).unwrap()).unwrap() + decode(value.unwrap()).unwrap() } fn read_validator_max_commission_rate_change( &self, key: &Self::Address, ) -> rust_decimal::Decimal { - let (value, _gas) = - self.read(&validator_commission_rate_key(key)).unwrap(); - value.map(|value| decode(value).unwrap()).unwrap() + let (value, _gas) = self + .read(&validator_max_commission_rate_change_key(key)) + .unwrap(); + decode(value.unwrap()).unwrap() } + fn read_validator_set(&self) -> ValidatorSets { let (value, _gas) = self.read(&validator_set_key()).unwrap(); decode(value.unwrap()).unwrap() diff --git a/tx_prelude/src/proof_of_stake.rs b/tx_prelude/src/proof_of_stake.rs index e929b0fb57..70b7f991c5 100644 --- a/tx_prelude/src/proof_of_stake.rs +++ b/tx_prelude/src/proof_of_stake.rs @@ -159,7 +159,6 @@ impl namada_proof_of_stake::PosActions for Ctx { value: CommissionRates, ) -> Result<(), Self::Error> { self.write(&validator_commission_rate_key(key), &value) - .into_env_result() } fn write_validator_max_commission_rate_change( @@ -168,7 +167,6 @@ impl namada_proof_of_stake::PosActions for Ctx { value: Decimal, ) -> Result<(), Self::Error> { self.write(&validator_max_commission_rate_change_key(key), &value) - .into_env_result() } fn write_validator_total_deltas( From 3704ef629fdf5d27014502b8ab5edbd69bc9a72e Mon Sep 17 00:00:00 2001 From: brentstone Date: Wed, 26 Oct 2022 14:28:59 -0400 Subject: [PATCH 17/38] cleaning: incl fmt + clippy --- apps/src/lib/cli.rs | 32 ++++++++++++--------- apps/src/lib/client/rpc.rs | 15 ++++++---- apps/src/lib/client/tx.rs | 18 +++++++----- proof_of_stake/src/lib.rs | 44 ++++++++++++----------------- shared/src/ledger/pos/storage.rs | 11 +++++--- shared/src/types/transaction/mod.rs | 3 +- tx_prelude/src/proof_of_stake.rs | 16 +++++------ 7 files changed, 75 insertions(+), 64 deletions(-) diff --git a/apps/src/lib/cli.rs b/apps/src/lib/cli.rs index a896dfffd6..187f11413a 100644 --- a/apps/src/lib/cli.rs +++ b/apps/src/lib/cli.rs @@ -1009,7 +1009,7 @@ pub mod cmds { fn parse(matches: &ArgMatches) -> Option { matches.subcommand_matches(Self::CMD).map(|matches| { QueryCommissionRate(args::QueryCommissionRate::parse(matches)) - }) + }) } fn def() -> App { @@ -1238,9 +1238,6 @@ pub mod args { use namada::types::token; use namada::types::transaction::GasLimit; use rust_decimal::Decimal; - use serde::Deserialize; - use tendermint::Timeout; - use tendermint_config::net::Address as TendermintAddress; use super::context::{WalletAddress, WalletKeypair, WalletPublicKey}; use super::utils::*; @@ -1301,7 +1298,8 @@ pub mod args { const LEDGER_ADDRESS: Arg = arg("ledger-address"); const LOCALHOST: ArgFlag = flag("localhost"); - const MAX_COMMISSION_RATE_CHANGE: Arg = arg("max-commission-rate-change"); + const MAX_COMMISSION_RATE_CHANGE: Arg = + arg("max-commission-rate-change"); const MODE: ArgOpt = arg_opt("mode"); const NET_ADDRESS: Arg = arg("net-address"); const OWNER: ArgOpt = arg_opt("owner"); @@ -1568,7 +1566,8 @@ pub mod args { let consensus_key = VALIDATOR_CONSENSUS_KEY.parse(matches); let protocol_key = PROTOCOL_KEY.parse(matches); let commission_rate = COMMISSION_RATE.parse(matches); - let max_commission_rate_change = MAX_COMMISSION_RATE_CHANGE.parse(matches); + let max_commission_rate_change = + MAX_COMMISSION_RATE_CHANGE.parse(matches); let validator_vp_code_path = VALIDATOR_CODE_PATH.parse(matches); let unsafe_dont_encrypt = UNSAFE_DONT_ENCRYPT.parse(matches); Self { @@ -1607,10 +1606,13 @@ pub mod args { one will be generated if none given.", )) .arg(COMMISSION_RATE.def().about( - "The commission rate charged by the validator for delegation rewards. This is a required parameter.", + "The commission rate charged by the validator for \ + delegation rewards. This is a required parameter.", )) .arg(MAX_COMMISSION_RATE_CHANGE.def().about( - "The maximum change per epoch in the commission rate charged by the validator for delegation rewards. This is a required parameter.", + "The maximum change per epoch in the commission rate \ + charged by the validator for delegation rewards. This is \ + a required parameter.", )) .arg(VALIDATOR_CODE_PATH.def().about( "The path to the validity predicate WASM code to be used \ @@ -2125,7 +2127,7 @@ pub mod args { )) } } - + /// Query PoS slashes #[derive(Clone, Debug)] pub struct QuerySlashes { @@ -2636,7 +2638,8 @@ pub mod args { fn parse(matches: &ArgMatches) -> Self { let alias = ALIAS.parse(matches); let commission_rate = COMMISSION_RATE.parse(matches); - let max_commission_rate_change = MAX_COMMISSION_RATE_CHANGE.parse(matches); + let max_commission_rate_change = + MAX_COMMISSION_RATE_CHANGE.parse(matches); let net_address = NET_ADDRESS.parse(matches); let unsafe_dont_encrypt = UNSAFE_DONT_ENCRYPT.parse(matches); let key_scheme = SCHEME.parse(matches); @@ -2646,7 +2649,7 @@ pub mod args { unsafe_dont_encrypt, key_scheme, commission_rate, - max_commission_rate_change + max_commission_rate_change, } } @@ -2658,10 +2661,13 @@ pub mod args { but you can configure a different value.", )) .arg(COMMISSION_RATE.def().about( - "The commission rate charged by the validator for delegation rewards. This is a required parameter.", + "The commission rate charged by the validator for \ + delegation rewards. This is a required parameter.", )) .arg(MAX_COMMISSION_RATE_CHANGE.def().about( - "The maximum change per epoch in the commission rate charged by the validator for delegation rewards. This is a required parameter.", + "The maximum change per epoch in the commission rate \ + charged by the validator for delegation rewards. This is \ + a required parameter.", )) .arg(UNSAFE_DONT_ENCRYPT.def().about( "UNSAFE: Do not encrypt the generated keypairs. Do not \ diff --git a/apps/src/lib/client/rpc.rs b/apps/src/lib/client/rpc.rs index c8740bce85..37b8249998 100644 --- a/apps/src/lib/client/rpc.rs +++ b/apps/src/lib/client/rpc.rs @@ -976,7 +976,10 @@ pub async fn query_voting_power(ctx: Context, args: args::QueryVotingPower) { } /// Query PoS commssion rate -pub async fn query_commission_rate(ctx: Context, args: args::QueryCommissionRate) { +pub async fn query_commission_rate( + ctx: Context, + args: args::QueryCommissionRate, +) { let epoch = match args.epoch { Some(epoch) => epoch, None => query_epoch(args.query.clone()).await, @@ -986,14 +989,15 @@ pub async fn query_commission_rate(ctx: Context, args: args::QueryCommissionRate match args.validator { Some(validator) => { let validator = ctx.get(&validator); - let validator_commission_key = pos::validator_commission_rate_key(&validator); + let validator_commission_key = + pos::validator_commission_rate_key(&validator); let commission_rates = query_storage_value::( &client, &validator_commission_key, ) .await; - let commission_rates = commission_rates - .expect("No commission rate found "); + let commission_rates = + commission_rates.expect("No commission rate found "); match commission_rates.get(epoch) { Some(rate) => { println!( @@ -1003,7 +1007,8 @@ pub async fn query_commission_rate(ctx: Context, args: args::QueryCommissionRate ) } None => { - println!("No commission rate found for {} in epoch {}", + println!( + "No commission rate found for {} in epoch {}", validator.encode(), epoch ) diff --git a/apps/src/lib/client/tx.rs b/apps/src/lib/client/tx.rs index e2c7a79942..e2bd9ee848 100644 --- a/apps/src/lib/client/tx.rs +++ b/apps/src/lib/client/tx.rs @@ -24,10 +24,6 @@ use namada::types::transaction::{pos, InitAccount, InitValidator, UpdateVp}; use namada::types::{address, storage, token}; use namada::{ledger, vm}; use rust_decimal::Decimal; -use tendermint_config::net::Address as TendermintAddress; -use tendermint_rpc::endpoint::broadcast::tx_sync::Response; -use tendermint_rpc::query::{EventType, Query}; -use tendermint_rpc::{Client, HttpClient}; use super::rpc; use crate::cli::context::WalletAddress; @@ -230,10 +226,18 @@ pub async fn submit_init_validator( // Validate the commission rate data if commission_rate > Decimal::ONE || commission_rate < Decimal::ZERO { - eprintln!("The validator commission rate must not exceed 1.0 or 100%, and it must be 0 or positive"); + eprintln!( + "The validator commission rate must not exceed 1.0 or 100%, and \ + it must be 0 or positive" + ); } - if max_commission_rate_change > Decimal::ONE || max_commission_rate_change < Decimal::ZERO { - eprintln!("The validator maximum change in commission rate per epoch must not exceed 1.0 or 100%"); + if max_commission_rate_change > Decimal::ONE + || max_commission_rate_change < Decimal::ZERO + { + eprintln!( + "The validator maximum change in commission rate per epoch must \ + not exceed 1.0 or 100%" + ); } // Validate the validator VP code if let Err(err) = vm::validate_untrusted_wasm(&validator_vp_code) { diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs index e0bf2192f6..ff9431bd92 100644 --- a/proof_of_stake/src/lib.rs +++ b/proof_of_stake/src/lib.rs @@ -34,10 +34,11 @@ use parameters::PosParams; use rust_decimal::Decimal; use thiserror::Error; use types::{ - ActiveValidator, Bonds, CommissionRates, Epoch, GenesisValidator, Slash, SlashType, Slashes, - TotalVotingPowers, Unbond, Unbonds, ValidatorConsensusKeys, ValidatorSet, - ValidatorSetUpdate, ValidatorSets, ValidatorState, ValidatorStates, - ValidatorTotalDeltas, ValidatorVotingPowers, VotingPower, VotingPowerDelta, + ActiveValidator, Bonds, CommissionRates, Epoch, GenesisValidator, Slash, + SlashType, Slashes, TotalVotingPowers, Unbond, Unbonds, + ValidatorConsensusKeys, ValidatorSet, ValidatorSetUpdate, ValidatorSets, + ValidatorState, ValidatorStates, ValidatorTotalDeltas, + ValidatorVotingPowers, VotingPower, VotingPowerDelta, }; use crate::btree_set::BTreeSetShims; @@ -282,7 +283,7 @@ pub trait PosActions: PosReadOnly { consensus_key: &Self::PublicKey, current_epoch: impl Into, commission_rate: Decimal, - max_commission_rate_change: Decimal + max_commission_rate_change: Decimal, ) -> Result<(), Self::BecomeValidatorError> { let current_epoch = current_epoch.into(); let params = self.read_pos_params()?; @@ -297,7 +298,7 @@ pub trait PosActions: PosReadOnly { total_deltas, voting_power, commission_rate, - max_commission_rate_change + max_commission_rate_change, } = become_validator( ¶ms, address, @@ -305,7 +306,7 @@ pub trait PosActions: PosReadOnly { &mut validator_set, current_epoch, commission_rate, - max_commission_rate_change + max_commission_rate_change, ); self.write_validator_consensus_key(address, consensus_key)?; self.write_validator_state(address, state)?; @@ -562,11 +563,7 @@ pub trait PosActions: PosReadOnly { } }; let rate_at_pipeline = *commission_rates - .get_at_offset( - current_epoch, - DynEpochOffset::PipelineLen, - params, - ) + .get_at_offset(current_epoch, DynEpochOffset::PipelineLen, params) .expect("Could not find a rate in given epoch"); if new_rate == rate_at_pipeline { return Err(CommissionRateChangeError::ChangeIsZero( @@ -576,7 +573,7 @@ pub trait PosActions: PosReadOnly { let rate_before_pipeline = *commission_rates .get_at_offset( - current_epoch-1, + current_epoch - 1, DynEpochOffset::PipelineLen, params, ) @@ -596,13 +593,10 @@ pub trait PosActions: PosReadOnly { DynEpochOffset::PipelineLen, params, ); - self.write_validator_commission_rate( - validator, - commission_rates, - ) - .map_err(|_| CommissionRateChangeError::CannotWrite(validator)) - .unwrap(); - + self.write_validator_commission_rate(validator, commission_rates) + .map_err(|_| CommissionRateChangeError::CannotWrite(validator)) + .unwrap(); + Ok(()) } } @@ -1327,10 +1321,8 @@ where }| { let consensus_key = Epoched::init_at_genesis(consensus_key.clone(), current_epoch); - let commission_rate = Epoched::init_at_genesis( - commission_rate.clone(), - current_epoch, - ); + let commission_rate = + Epoched::init_at_genesis(*commission_rate, current_epoch); let state = Epoched::init_at_genesis( ValidatorState::Candidate, current_epoch, @@ -1360,7 +1352,7 @@ where address: address.clone(), consensus_key, commission_rate, - max_commission_rate_change: max_commission_rate_change.clone(), + max_commission_rate_change: *max_commission_rate_change, state, total_deltas, voting_power, @@ -1554,7 +1546,7 @@ where total_deltas, voting_power, commission_rate, - max_commission_rate_change + max_commission_rate_change, } } diff --git a/shared/src/ledger/pos/storage.rs b/shared/src/ledger/pos/storage.rs index aaa48d164e..be8709f285 100644 --- a/shared/src/ledger/pos/storage.rs +++ b/shared/src/ledger/pos/storage.rs @@ -7,8 +7,8 @@ use namada_proof_of_stake::types::{ use namada_proof_of_stake::{types, PosBase}; use super::{ - BondId, Bonds, ValidatorConsensusKeys, ValidatorSets, ValidatorTotalDeltas, - ADDRESS, CommissionRates + BondId, Bonds, CommissionRates, ValidatorConsensusKeys, ValidatorSets, + ValidatorTotalDeltas, ADDRESS, }; use crate::ledger::storage::types::{decode, encode}; use crate::ledger::storage::{self, Storage, StorageHasher}; @@ -525,9 +525,12 @@ where key: &Self::Address, value: &rust_decimal::Decimal, ) { - self.write(&validator_max_commission_rate_change_key(key), encode(value)) + self.write( + &validator_max_commission_rate_change_key(key), + encode(value), + ) .unwrap(); -} + } fn write_validator_consensus_key( &mut self, diff --git a/shared/src/types/transaction/mod.rs b/shared/src/types/transaction/mod.rs index 8a391d858f..3ee6ebd218 100644 --- a/shared/src/types/transaction/mod.rs +++ b/shared/src/types/transaction/mod.rs @@ -191,7 +191,8 @@ pub struct InitValidator { pub dkg_key: DkgPublicKey, /// The initial commission rate charged for delegation rewards pub commission_rate: Decimal, - /// The maximum change allowed per epoch to the commission rate. This is immutable once set here. + /// The maximum change allowed per epoch to the commission rate. This is + /// immutable once set here. pub max_commission_rate_change: Decimal, /// The VP code for validator account pub validator_vp_code: Vec, diff --git a/tx_prelude/src/proof_of_stake.rs b/tx_prelude/src/proof_of_stake.rs index 70b7f991c5..e732597461 100644 --- a/tx_prelude/src/proof_of_stake.rs +++ b/tx_prelude/src/proof_of_stake.rs @@ -3,10 +3,10 @@ pub use namada::ledger::pos::*; use namada::ledger::pos::{ bond_key, namada_proof_of_stake, params_key, total_voting_power_key, - unbond_key, validator_address_raw_hash_key, validator_consensus_key_key, + unbond_key, validator_address_raw_hash_key, validator_commission_rate_key, + validator_consensus_key_key, validator_max_commission_rate_change_key, validator_set_key, validator_slashes_key, validator_state_key, validator_total_deltas_key, validator_voting_power_key, - validator_commission_rate_key, validator_max_commission_rate_change_key }; use namada::types::address::Address; use namada::types::transaction::InitValidator; @@ -14,8 +14,8 @@ use namada::types::{key, token}; pub use namada_proof_of_stake::{ epoched, parameters, types, PosActions as PosWrite, PosReadOnly as PosRead, }; - use rust_decimal::Decimal; + use super::*; impl Ctx { @@ -103,7 +103,7 @@ impl Ctx { &consensus_key, current_epoch, commission_rate, - max_commission_rate_change + max_commission_rate_change, )?; Ok(validator_address) @@ -162,10 +162,10 @@ impl namada_proof_of_stake::PosActions for Ctx { } fn write_validator_max_commission_rate_change( - &mut self, - key: &Self::Address, - value: Decimal, - ) -> Result<(), Self::Error> { + &mut self, + key: &Self::Address, + value: Decimal, + ) -> Result<(), Self::Error> { self.write(&validator_max_commission_rate_change_key(key), &value) } From 0e097d8886cc97b252e47db0cb5510616db0827d Mon Sep 17 00:00:00 2001 From: brentstone Date: Wed, 26 Oct 2022 16:06:21 -0400 Subject: [PATCH 18/38] init validator: add commission rate required args for tests --- tests/src/e2e/ledger_tests.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/src/e2e/ledger_tests.rs b/tests/src/e2e/ledger_tests.rs index 11bf1228c9..32ad6650d2 100644 --- a/tests/src/e2e/ledger_tests.rs +++ b/tests/src/e2e/ledger_tests.rs @@ -808,6 +808,10 @@ fn pos_init_validator() -> Result<()> { "0", "--fee-token", NAM, + "--commission-rate", + "0.05", + "--max-commission-rate-change", + "0.01", "--ledger-address", &validator_one_rpc, ]; @@ -1598,6 +1602,10 @@ fn test_genesis_validators() -> Result<()> { validator_0_alias, "--scheme", "ed25519", + "--commission-rate", + "0.05", + "--max-commission-rate-change", + "0.01", "--net-address", &format!("127.0.0.1:{}", get_first_port(0)), ], @@ -1636,6 +1644,10 @@ fn test_genesis_validators() -> Result<()> { validator_1_alias, "--scheme", "secp256k1", + "--commission-rate", + "0.05", + "--max-commission-rate-change", + "0.01", "--net-address", &format!("127.0.0.1:{}", get_first_port(1)), ], From 80283b2ff4ca0e74789d6cb4ae260a16ad5bf4c8 Mon Sep 17 00:00:00 2001 From: brentstone Date: Thu, 27 Oct 2022 20:17:24 -0400 Subject: [PATCH 19/38] fix commission rate validation on validator initialization --- apps/src/lib/client/tx.rs | 6 ++++++ apps/src/lib/client/utils.rs | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/apps/src/lib/client/tx.rs b/apps/src/lib/client/tx.rs index e2bd9ee848..6a07204695 100644 --- a/apps/src/lib/client/tx.rs +++ b/apps/src/lib/client/tx.rs @@ -230,6 +230,9 @@ pub async fn submit_init_validator( "The validator commission rate must not exceed 1.0 or 100%, and \ it must be 0 or positive" ); + if !tx_args.force { + safe_exit(1) + } } if max_commission_rate_change > Decimal::ONE || max_commission_rate_change < Decimal::ZERO @@ -238,6 +241,9 @@ pub async fn submit_init_validator( "The validator maximum change in commission rate per epoch must \ not exceed 1.0 or 100%" ); + if !tx_args.force { + safe_exit(1) + } } // Validate the validator VP code if let Err(err) = vm::validate_untrusted_wasm(&validator_vp_code) { diff --git a/apps/src/lib/client/utils.rs b/apps/src/lib/client/utils.rs index d892601ac6..5f6a81897b 100644 --- a/apps/src/lib/client/utils.rs +++ b/apps/src/lib/client/utils.rs @@ -16,6 +16,7 @@ use namada::types::key::*; use prost::bytes::Bytes; use rand::prelude::ThreadRng; use rand::thread_rng; +use rust_decimal::Decimal; use serde_json::json; use sha2::{Digest, Sha256}; @@ -887,6 +888,23 @@ pub fn init_genesis_validator( key_scheme, }: args::InitGenesisValidator, ) { + // Validate the commission rate data + if commission_rate > Decimal::ONE || commission_rate < Decimal::ZERO { + eprintln!( + "The validator commission rate must not exceed 1.0 or 100%, and \ + it must be 0 or positive" + ); + cli::safe_exit(1) + } + if max_commission_rate_change > Decimal::ONE + || max_commission_rate_change < Decimal::ZERO + { + eprintln!( + "The validator maximum change in commission rate per epoch must \ + not exceed 1.0 or 100%" + ); + cli::safe_exit(1) + } let pre_genesis_dir = validator_pre_genesis_dir(&global_args.base_dir, &alias); println!("Generating validator keys..."); From 9d8a0d087862274aa9d595e80a455e2145080fba Mon Sep 17 00:00:00 2001 From: brentstone Date: Thu, 27 Oct 2022 20:17:44 -0400 Subject: [PATCH 20/38] improve docs --- apps/src/lib/cli.rs | 8 +++++--- proof_of_stake/src/lib.rs | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/apps/src/lib/cli.rs b/apps/src/lib/cli.rs index 187f11413a..5e85efdd5d 100644 --- a/apps/src/lib/cli.rs +++ b/apps/src/lib/cli.rs @@ -1607,12 +1607,14 @@ pub mod args { )) .arg(COMMISSION_RATE.def().about( "The commission rate charged by the validator for \ - delegation rewards. This is a required parameter.", + delegation rewards. Expressed as a decimal between 0 and \ + 1. This is a required parameter.", )) .arg(MAX_COMMISSION_RATE_CHANGE.def().about( "The maximum change per epoch in the commission rate \ - charged by the validator for delegation rewards. This is \ - a required parameter.", + charged by the validator for delegation rewards. \ + Expressed as a decimal between 0 and 1. This is a \ + required parameter.", )) .arg(VALIDATOR_CODE_PATH.def().about( "The path to the validity predicate WASM code to be used \ diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs index ff9431bd92..9f9b86e474 100644 --- a/proof_of_stake/src/lib.rs +++ b/proof_of_stake/src/lib.rs @@ -753,7 +753,7 @@ pub trait PosBase { key: &Self::Address, value: &CommissionRates, ); - /// Write PoS validator's commission rate. + /// Write PoS validator's maximum change in the commission rate. fn write_validator_max_commission_rate_change( &mut self, key: &Self::Address, From 629e1c55afe7ca9282be48a80f6e1ecd947c1960 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 31 Oct 2022 20:04:10 +0000 Subject: [PATCH 21/38] [ci] wasm checksums update --- wasm/checksums.json | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/wasm/checksums.json b/wasm/checksums.json index 22dc699c30..379e4d6c40 100644 --- a/wasm/checksums.json +++ b/wasm/checksums.json @@ -1,18 +1,15 @@ { - "tx_bond.wasm": "tx_bond.059b1256240d15a64cf419f3a2d24af1496211c386d85acc3733095bc2e5da9b.wasm", - "tx_ibc.wasm": "tx_ibc.4eeb30bc1e6a32b8efe8958eab568082a238db01eb98340f28a9fa41371a3753.wasm", - "tx_init_account.wasm": "tx_init_account.85d017ac76e51f359fa07e753c0e6fcbd3341e7661492cbf2801cf3c41480dd4.wasm", - "tx_init_nft.wasm": "tx_init_nft.fbeb1687a364b2c249c9fd69588ff0985bd9c1f8f4c93e5328de1e9ba527d991.wasm", - "tx_init_proposal.wasm": "tx_init_proposal.43b52414649bb0fec86dfb35d001656c1825bc5906e450e8b0c3a60aaa5f3d45.wasm", - "tx_init_validator.wasm": "tx_init_validator.6ccb7fcf246cb7a2f97a5dfdcefe16ee1add72a832081c2572adc2d7a355cf56.wasm", - "tx_mint_nft.wasm": "tx_mint_nft.f2ed21521c676f04be4c6278cc60bec83265c3750437c87d9ea681b830767d71.wasm", - "tx_transfer.wasm": "tx_transfer.b6fc342f4a76918874e6d037a3864e4369dbba7cd7d558622e7a723e3d854da3.wasm", - "tx_unbond.wasm": "tx_unbond.6e7316d08bf8ab9a6fb1889f64a5a2265ee0399661dbb48e33555170545d1c7c.wasm", - "tx_update_vp.wasm": "tx_update_vp.6d291dadb43545a809ba33fe26582b7984c67c65f05e363a93dbc62e06a33484.wasm", - "tx_vote_proposal.wasm": "tx_vote_proposal.d0a87d58f64f46586ae3d83852deee269e22520f4780875c05aaf1731044c356.wasm", - "tx_withdraw.wasm": "tx_withdraw.e5dcc5ef2362018c1fa5ea02912528ee929aa7b6fefcf06f4ccf7509bfa52283.wasm", - "vp_nft.wasm": "vp_nft.9be5a821bc7b3075b917e8ead45893502d82cc7417e6af50dfd3f6baf36243e0.wasm", - "vp_testnet_faucet.wasm": "vp_testnet_faucet.8e2e45ff165d40dc8249188aca108e5cba86ac5dd1cd989b0237cadd4b66bfdf.wasm", - "vp_token.wasm": "vp_token.4a0446f20e7436de1e889c640a11644d1a1295c4d29e45b24582df2b9ed3176e.wasm", - "vp_user.wasm": "vp_user.eb1d6f1f524c28571ad0f21f75371aa635257313cea2702b9a70e5022fe6c3ef.wasm" -} + "tx_bond.wasm": "tx_bond.b8e6b8698ba36ab33797b33298490114b4046ce63bacea70a8bb2ed8fafb3aa5.wasm", + "tx_ibc.wasm": "tx_ibc.a8e7e017343354da9e4af574aed95d028b1cb878594b15097b31bdf5ce526a32.wasm", + "tx_init_account.wasm": "tx_init_account.871b80f632ef665c29aa04db36a4863e635d831bcd1f13132397bfb1130b4215.wasm", + "tx_init_proposal.wasm": "tx_init_proposal.a684218dea27cef4566fce9f7bafed4f872ee2ba0c092f0ce142eccda7680070.wasm", + "tx_init_validator.wasm": "tx_init_validator.7f2b68315dbcd93783e3a280e4af1920497883156e986fb0e41e774eedd8090f.wasm", + "tx_transfer.wasm": "tx_transfer.aea2de64a6ef2039e943b218e44c1126c79927e794c4aaa8f9a4330f288afa27.wasm", + "tx_unbond.wasm": "tx_unbond.7dc35dce8b5168a47d0ceed7077f7a772f0ade719d6b3c427d6b82ff807dd0c1.wasm", + "tx_update_vp.wasm": "tx_update_vp.825cb98ac070d7cd50d14ba0b53e08e1f658a2a2481322814037f4a9dad4a17b.wasm", + "tx_vote_proposal.wasm": "tx_vote_proposal.29e1ab4023e75b7ac3f6aac48585b2871e38cc2239b054ddeac6029458b870d0.wasm", + "tx_withdraw.wasm": "tx_withdraw.d53c26dca99ecd5626cb74f5036ecccfb95cf48fa6dbfaab2b185c7c3f99d48d.wasm", + "vp_testnet_faucet.wasm": "vp_testnet_faucet.537d787e157ea1abe9df6e7e59f62d3a3faf97e2f6205ff0c4736cf974920922.wasm", + "vp_token.wasm": "vp_token.2b756bdcd5045e43267ca25c3795251425f2a867a39fc0893121efae8420ad07.wasm", + "vp_user.wasm": "vp_user.c0bf61881558afba85b1b1a46eb965417cba0f918eae5c836e57734cbf3efc3f.wasm" +} \ No newline at end of file From 258b9107feb765df89688cea08130e9ebb4349e2 Mon Sep 17 00:00:00 2001 From: brentstone Date: Thu, 3 Nov 2022 23:26:28 -0400 Subject: [PATCH 22/38] wasm tx test for changing validator commission rate --- proof_of_stake/src/lib.rs | 52 +++-- shared/src/ledger/pos/mod.rs | 10 + shared/src/types/transaction/pos.rs | 21 ++ tx_prelude/src/proof_of_stake.rs | 16 ++ wasm/checksums.json | 31 +-- wasm/wasm_source/Cargo.toml | 1 + wasm/wasm_source/Makefile | 1 + wasm/wasm_source/src/lib.rs | 2 + .../src/tx_change_validator_commission.rs | 181 ++++++++++++++++++ 9 files changed, 286 insertions(+), 29 deletions(-) create mode 100644 wasm/wasm_source/src/tx_change_validator_commission.rs diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs index 9f9b86e474..951f43640b 100644 --- a/proof_of_stake/src/lib.rs +++ b/proof_of_stake/src/lib.rs @@ -183,6 +183,10 @@ pub trait PosActions: PosReadOnly { /// Error in `PosActions::withdraw_tokens` type WithdrawError: From + From>; + /// Error in `PosActions::change_commission_rate` + type CommissionRateChangeError: From + + From>; + /// Write PoS parameters. fn write_pos_params( &mut self, @@ -534,48 +538,54 @@ pub trait PosActions: PosReadOnly { /// Change the commission rate of a validator fn change_validator_commission_rate( &mut self, - params: &PosParams, validator: &Self::Address, new_rate: Decimal, current_epoch: impl Into, - ) -> Result<(), CommissionRateChangeError> { + ) -> Result<(), Self::CommissionRateChangeError> { if new_rate < Decimal::ZERO { return Err(CommissionRateChangeError::NegativeRate( new_rate, validator.clone(), - )); + ) + .into()); } let current_epoch = current_epoch.into(); let max_change = self .read_validator_max_commission_rate_change(validator) .map_err(|_| { - CommissionRateChangeError::NoMaxSetInStorage(validator) - }) - .unwrap() - .unwrap(); + CommissionRateChangeError::NoMaxSetInStorage(validator.clone()) + })? + .ok_or_else(|| { + CommissionRateChangeError::CannotRead(validator.clone()) + })?; let mut commission_rates = match self.read_validator_commission_rate(validator) { Ok(Some(rates)) => rates, _ => { return Err(CommissionRateChangeError::CannotRead( validator.clone(), - )); + ) + .into()); } }; + let params = self.read_pos_params()?; + let rate_at_pipeline = *commission_rates - .get_at_offset(current_epoch, DynEpochOffset::PipelineLen, params) + .get_at_offset(current_epoch, DynEpochOffset::PipelineLen, ¶ms) .expect("Could not find a rate in given epoch"); if new_rate == rate_at_pipeline { return Err(CommissionRateChangeError::ChangeIsZero( validator.clone(), - )); + ) + .into()); } + let rate_before_pipeline = *commission_rates .get_at_offset( current_epoch - 1, DynEpochOffset::PipelineLen, - params, + ¶ms, ) .expect("Could not find a rate in given epoch"); let change_from_prev = new_rate - rate_before_pipeline; @@ -583,7 +593,8 @@ pub trait PosActions: PosReadOnly { return Err(CommissionRateChangeError::RateChangeTooLarge( change_from_prev, validator.clone(), - )); + ) + .into()); } commission_rates.update_from_offset( |val, _epoch| { @@ -591,11 +602,12 @@ pub trait PosActions: PosReadOnly { }, current_epoch, DynEpochOffset::PipelineLen, - params, + ¶ms, ); self.write_validator_commission_rate(validator, commission_rates) - .map_err(|_| CommissionRateChangeError::CannotWrite(validator)) - .unwrap(); + .map_err(|_| { + CommissionRateChangeError::CannotWrite(validator.clone()) + })?; Ok(()) } @@ -1128,7 +1140,15 @@ where #[derive(Error, Debug)] pub enum CommissionRateChangeError
where - Address: Display + Debug + Clone + PartialOrd + Ord + Hash, + Address: Display + + Debug + + Clone + + PartialOrd + + Ord + + Hash + + BorshDeserialize + + BorshSerialize + + BorshSchema, { #[error("Unexpected negative commission rate {0} for validator {1}")] NegativeRate(Decimal, Address), diff --git a/shared/src/ledger/pos/mod.rs b/shared/src/ledger/pos/mod.rs index 62c754a7da..64e2babfd3 100644 --- a/shared/src/ledger/pos/mod.rs +++ b/shared/src/ledger/pos/mod.rs @@ -126,6 +126,16 @@ impl From> } } +impl From> + for storage_api::Error +{ + fn from( + err: namada_proof_of_stake::CommissionRateChangeError
, + ) -> Self { + Self::new(err) + } +} + #[macro_use] mod macros { /// Implement `PosReadOnly` for a type that implements diff --git a/shared/src/types/transaction/pos.rs b/shared/src/types/transaction/pos.rs index b6cba21df3..8119eb2310 100644 --- a/shared/src/types/transaction/pos.rs +++ b/shared/src/types/transaction/pos.rs @@ -1,6 +1,7 @@ //! Types used for PoS system transactions use borsh::{BorshDeserialize, BorshSchema, BorshSerialize}; +use rust_decimal::Decimal; use serde::{Deserialize, Serialize}; use crate::types::address::Address; @@ -53,3 +54,23 @@ pub struct Withdraw { /// from self-bonds, the validator is also the source pub source: Option
, } + +/// A change to the validator commission rate. +#[derive( + Debug, + Clone, + PartialEq, + BorshSerialize, + BorshDeserialize, + BorshSchema, + Hash, + Eq, + Serialize, + Deserialize, +)] +pub struct CommissionChange { + /// Validator address + pub validator: Address, + /// The new commission rate + pub new_rate: Decimal, +} diff --git a/tx_prelude/src/proof_of_stake.rs b/tx_prelude/src/proof_of_stake.rs index e732597461..5d95921664 100644 --- a/tx_prelude/src/proof_of_stake.rs +++ b/tx_prelude/src/proof_of_stake.rs @@ -74,6 +74,21 @@ impl Ctx { ) } + /// Change validator commission rate. + pub fn change_validator_commission_rate( + &mut self, + validator: &Address, + rate: &Decimal, + ) -> TxResult { + let current_epoch = self.get_block_epoch()?; + namada_proof_of_stake::PosActions::change_validator_commission_rate( + self, + validator, + *rate, + current_epoch, + ) + } + /// Attempt to initialize a validator account. On success, returns the /// initialized validator account's address. pub fn init_validator( @@ -118,6 +133,7 @@ namada::impl_pos_read_only! { impl namada_proof_of_stake::PosActions for Ctx { type BecomeValidatorError = crate::Error; type BondError = crate::Error; + type CommissionRateChangeError = crate::Error; type UnbondError = crate::Error; type WithdrawError = crate::Error; diff --git a/wasm/checksums.json b/wasm/checksums.json index 379e4d6c40..99ca7176c5 100644 --- a/wasm/checksums.json +++ b/wasm/checksums.json @@ -1,15 +1,20 @@ { - "tx_bond.wasm": "tx_bond.b8e6b8698ba36ab33797b33298490114b4046ce63bacea70a8bb2ed8fafb3aa5.wasm", - "tx_ibc.wasm": "tx_ibc.a8e7e017343354da9e4af574aed95d028b1cb878594b15097b31bdf5ce526a32.wasm", - "tx_init_account.wasm": "tx_init_account.871b80f632ef665c29aa04db36a4863e635d831bcd1f13132397bfb1130b4215.wasm", - "tx_init_proposal.wasm": "tx_init_proposal.a684218dea27cef4566fce9f7bafed4f872ee2ba0c092f0ce142eccda7680070.wasm", - "tx_init_validator.wasm": "tx_init_validator.7f2b68315dbcd93783e3a280e4af1920497883156e986fb0e41e774eedd8090f.wasm", - "tx_transfer.wasm": "tx_transfer.aea2de64a6ef2039e943b218e44c1126c79927e794c4aaa8f9a4330f288afa27.wasm", - "tx_unbond.wasm": "tx_unbond.7dc35dce8b5168a47d0ceed7077f7a772f0ade719d6b3c427d6b82ff807dd0c1.wasm", - "tx_update_vp.wasm": "tx_update_vp.825cb98ac070d7cd50d14ba0b53e08e1f658a2a2481322814037f4a9dad4a17b.wasm", - "tx_vote_proposal.wasm": "tx_vote_proposal.29e1ab4023e75b7ac3f6aac48585b2871e38cc2239b054ddeac6029458b870d0.wasm", - "tx_withdraw.wasm": "tx_withdraw.d53c26dca99ecd5626cb74f5036ecccfb95cf48fa6dbfaab2b185c7c3f99d48d.wasm", - "vp_testnet_faucet.wasm": "vp_testnet_faucet.537d787e157ea1abe9df6e7e59f62d3a3faf97e2f6205ff0c4736cf974920922.wasm", - "vp_token.wasm": "vp_token.2b756bdcd5045e43267ca25c3795251425f2a867a39fc0893121efae8420ad07.wasm", - "vp_user.wasm": "vp_user.c0bf61881558afba85b1b1a46eb965417cba0f918eae5c836e57734cbf3efc3f.wasm" + "tx_bond.wasm": "tx_bond.c74583608a2474b70c60f4378d11a9bee9b624aad64418221452e0f728b616b7.wasm", + "tx_change_validator_commission.wasm": "tx_change_validator_commission.e418ee939e2ca0248c0546c446e2513504cbcf04f054d709d4a0f159cfac9211.wasm", + "tx_from_intent.wasm": "tx_from_intent.e00a9b09cbeba1a5c61d9f93a9b5c42e5eaf0246db1fa2e78d5de7c9ede87955.wasm", + "tx_ibc.wasm": "tx_ibc.0951cba0a9487915b9a6194c6bd7c5fea0d3d056fd9ff0b47f3035c0157eb4f4.wasm", + "tx_init_account.wasm": "tx_init_account.63984b48d4a46b82f6de27d6879a3fb68ceca5cc03c8b27e8dc67f7fbc17d88c.wasm", + "tx_init_nft.wasm": "tx_init_nft.68cb966f08114092918f64e47b25574ee2a062386e38a0d3faeda29b9ff289f8.wasm", + "tx_init_proposal.wasm": "tx_init_proposal.53997c9a43e741838600b28a5bbb34a8e9207803952d79afd0c9777bfd223cd0.wasm", + "tx_init_validator.wasm": "tx_init_validator.ac82ac4c501b774b62cf77311ff0374f37f9874c58b53bffbe1c608b5ef258a5.wasm", + "tx_mint_nft.wasm": "tx_mint_nft.ef93c00a3fd0597e3551e81b4abc7fcd7e7a5f890705839d578189d11f5091ff.wasm", + "tx_transfer.wasm": "tx_transfer.abda15ddfce5e8b6307d6a2974fdf949b67631ca0b7a30f1ad233b6a4f4d5da1.wasm", + "tx_unbond.wasm": "tx_unbond.4e4c2efa864786756739efce1163532509ba10aad5dcb983eeffac5ec84395e0.wasm", + "tx_update_vp.wasm": "tx_update_vp.8e13d0ded993cdb837d2feab46758e8c7a92353511e9e558750ab76113d82a43.wasm", + "tx_vote_proposal.wasm": "tx_vote_proposal.f14bbea254f91d230f8966fae1446d4dd1a677fb491225aaa7153557498de644.wasm", + "tx_withdraw.wasm": "tx_withdraw.4ff2483bfcd2710bda5507e51407a46a6a4cc4ed1418a379e92aa29a03c4dd27.wasm", + "vp_nft.wasm": "vp_nft.ca95f995ebe0854f3c8501abcd9d65babc0040bf55554612a8509e7642b8296b.wasm", + "vp_testnet_faucet.wasm": "vp_testnet_faucet.49b35412b92610033b091bab844cc0760530c7379d3f0aaf58c538ac3d4452d8.wasm", + "vp_token.wasm": "vp_token.927d28326b54b3ff59b5447939f7fa42e1b5a4bc2c1a6f8c51ff603402d71a28.wasm", + "vp_user.wasm": "vp_user.b520d389415f8051b8ac5be38093ab503cf762dc219f4efec39c10da5362882e.wasm" } \ No newline at end of file diff --git a/wasm/wasm_source/Cargo.toml b/wasm/wasm_source/Cargo.toml index 5abce7f1e1..c66eea0480 100644 --- a/wasm/wasm_source/Cargo.toml +++ b/wasm/wasm_source/Cargo.toml @@ -23,6 +23,7 @@ tx_unbond = ["namada_tx_prelude"] tx_update_vp = ["namada_tx_prelude"] tx_vote_proposal = ["namada_tx_prelude"] tx_withdraw = ["namada_tx_prelude"] +tx_change_validator_commission = ["namada_tx_prelude"] vp_testnet_faucet = ["namada_vp_prelude", "once_cell"] vp_token = ["namada_vp_prelude"] vp_user = ["namada_vp_prelude", "once_cell", "rust_decimal"] diff --git a/wasm/wasm_source/Makefile b/wasm/wasm_source/Makefile index ce4655c39a..e525d77d55 100644 --- a/wasm/wasm_source/Makefile +++ b/wasm/wasm_source/Makefile @@ -15,6 +15,7 @@ wasms += tx_transfer wasms += tx_unbond wasms += tx_update_vp wasms += tx_withdraw +wasms += tx_change_validator_commission wasms += vp_testnet_faucet wasms += vp_token wasms += vp_user diff --git a/wasm/wasm_source/src/lib.rs b/wasm/wasm_source/src/lib.rs index 9075c60153..de33356cdd 100644 --- a/wasm/wasm_source/src/lib.rs +++ b/wasm/wasm_source/src/lib.rs @@ -1,5 +1,7 @@ #[cfg(feature = "tx_bond")] pub mod tx_bond; +#[cfg(feature = "tx_change_validator_commission")] +pub mod tx_change_validator_commission; #[cfg(feature = "tx_ibc")] pub mod tx_ibc; #[cfg(feature = "tx_init_account")] diff --git a/wasm/wasm_source/src/tx_change_validator_commission.rs b/wasm/wasm_source/src/tx_change_validator_commission.rs new file mode 100644 index 0000000000..65a78d1af5 --- /dev/null +++ b/wasm/wasm_source/src/tx_change_validator_commission.rs @@ -0,0 +1,181 @@ +//! A tx for a validator to change their commission rate for PoS rewards. + +use namada_tx_prelude::transaction::pos::CommissionChange; +use namada_tx_prelude::*; + +#[transaction] +fn apply_tx(ctx: &mut Ctx, tx_data: Vec) -> TxResult { + let signed = SignedTxData::try_from_slice(&tx_data[..]) + .wrap_err("failed to decode SignedTxData")?; + let data = signed.data.ok_or_err_msg("Missing data")?; + let CommissionChange { + validator, + new_rate, + } = transaction::pos::CommissionChange::try_from_slice(&data[..]) + .wrap_err("failed to decode Decimal value")?; + ctx.change_validator_commission_rate(&validator, &new_rate) +} + +#[cfg(test)] +mod tests { + use namada::ledger::pos::PosParams; + use namada::proto::Tx; + use namada::types::storage::Epoch; + use namada_tests::log::test; + use namada_tests::native_vp::pos::init_pos; + use namada_tests::native_vp::TestNativeVpEnv; + use namada_tests::tx::*; + use namada_tx_prelude::address::testing::{ + arb_established_address, + }; + use namada_tx_prelude::key::testing::arb_common_keypair; + use namada_tx_prelude::key::RefTo; + use namada_tx_prelude::proof_of_stake::parameters::testing::arb_pos_params; + use namada_tx_prelude::token; + use namada_vp_prelude::proof_of_stake::{ + CommissionRates, GenesisValidator, PosVP, + }; + use proptest::prelude::*; + use rust_decimal::Decimal; + + use super::*; + + proptest! { + /// In this test we setup the ledger and PoS system with an arbitrary + /// initial state with 1 genesis validator and arbitrary PoS parameters. We then + /// generate an arbitrary bond that we'd like to apply. + /// + /// After we apply the bond, we check that all the storage values + /// in PoS system have been updated as expected and then we also check + /// that this transaction is accepted by the PoS validity predicate. + #[test] + fn test_tx_change_validator_commissions( + initial_rate in arb_rate(), + max_change in arb_rate(), + commission_change in arb_commission_change(), + // A key to sign the transaction + key in arb_common_keypair(), + pos_params in arb_pos_params()) { + test_tx_change_validator_commission_aux(commission_change, initial_rate, max_change, key, pos_params).unwrap() + } + } + + fn test_tx_change_validator_commission_aux( + commission_change: transaction::pos::CommissionChange, + initial_rate: Decimal, + max_change: Decimal, + key: key::common::SecretKey, + pos_params: PosParams, + ) -> TxResult { + let consensus_key = key::testing::keypair_1().ref_to(); + let genesis_validators = [GenesisValidator { + address: commission_change.validator.clone(), + tokens: token::Amount::from(1_000_000), + consensus_key, + commission_rate: initial_rate, + max_commission_rate_change: max_change, + }]; + + println!("\nInitial rate = {}\nMax change = {}\nNew rate = {}",initial_rate,max_change,commission_change.new_rate.clone()); + + init_pos(&genesis_validators[..], &pos_params, Epoch(0)); + + let tx_code = vec![]; + let tx_data = commission_change.try_to_vec().unwrap(); + let tx = Tx::new(tx_code, Some(tx_data)); + let signed_tx = tx.sign(&key); + let tx_data = signed_tx.data.unwrap(); + + println!("\ndbg0\n"); + // Read the data before the tx is executed + let commission_rates_pre: CommissionRates = ctx() + .read_validator_commission_rate(&commission_change.validator)? + .expect("PoS validator must have commission rates"); + let commission_rate = *commission_rates_pre + .get(0) + .expect("PoS validator must have commission rate at genesis"); + assert_eq!(commission_rate, initial_rate); + println!("\ndbg1\n"); + + apply_tx(ctx(), tx_data)?; + println!("\ndbg2\n"); + + // Read the data after the tx is executed + + // The following storage keys should be updated: + + // - `#{PoS}/validator/#{validator}/commission_rate` + println!("dbg2.1"); + + let commission_rates_post: CommissionRates = ctx() + .read_validator_commission_rate(&commission_change.validator)?.unwrap(); + + dbg!(&commission_rates_pre); + dbg!(&commission_rates_post); + + // Before pipeline, the commission rates should not change + for epoch in 0..pos_params.pipeline_len { + assert_eq!( + commission_rates_pre.get(epoch), + commission_rates_post.get(epoch), + "The commission rates before the pipeline offset must not change \ + - checking in epoch: {epoch}" + ); + assert_eq!( + Some(&initial_rate), + commission_rates_post.get(epoch), + "The commission rates before the pipeline offset must not change \ + - checking in epoch: {epoch}" + ); + } + println!("\ndbg3\n"); + + // After pipeline, the commission rates should have changed + for epoch in pos_params.pipeline_len..=pos_params.unbonding_len { + assert_ne!( + commission_rates_pre.get(epoch), + commission_rates_post.get(epoch), + "The commission rate after the pipeline offset must have changed \ + - checking in epoch: {epoch}" + ); + assert_eq!( + Some(&commission_change.new_rate), + commission_rates_post.get(epoch), + "The commission rate after the pipeline offset must be the new_rate \ + - checking in epoch: {epoch}" + ); + } + + println!("\ndbg4\n"); + + // Use the tx_env to run PoS VP + let tx_env = tx_host_env::take(); + let vp_env = TestNativeVpEnv::from_tx_env(tx_env, address::POS); + let result = vp_env.validate_tx(PosVP::new); + let result = + result.expect("Validation of valid changes must not fail!"); + assert!( + result, + "PoS Validity predicate must accept this transaction" + ); + println!("\ndbg5\n"); + + Ok(()) + } + + fn arb_rate() -> impl Strategy { + (0..=100_000u64).prop_map(|num| { + Decimal::from(num) / Decimal::new(100_000, 0) + }) + } + + fn arb_commission_change() + -> impl Strategy { + (arb_established_address(), arb_rate()).prop_map( + |(validator, new_rate)| transaction::pos::CommissionChange { + validator: Address::Established(validator), + new_rate, + }, + ) + } +} From 6610a65009413238b6569d562acce24fa8308b56 Mon Sep 17 00:00:00 2001 From: brentstone Date: Thu, 3 Nov 2022 23:28:21 -0400 Subject: [PATCH 23/38] fix error convention --- proof_of_stake/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs index 951f43640b..f88da14983 100644 --- a/proof_of_stake/src/lib.rs +++ b/proof_of_stake/src/lib.rs @@ -423,7 +423,7 @@ pub trait PosActions: PosReadOnly { }; let mut bond = match self.read_bond(&bond_id)? { Some(val) => val, - None => Err(UnbondError::NoBondFound)?, + None => return Err(UnbondError::NoBondFound.into()), }; let unbond = self.read_unbond(&bond_id)?; let mut validator_total_deltas = self From 563d218333f4887d1303d10c313bf38927c16d02 Mon Sep 17 00:00:00 2001 From: brentstone Date: Fri, 4 Nov 2022 15:04:21 -0400 Subject: [PATCH 24/38] commission change wasm tx test: fix and update validation --- proof_of_stake/src/validation.rs | 83 ++++++++++++++++++- shared/src/ledger/pos/vp.rs | 16 +++- .../src/tx_change_validator_commission.rs | 69 +++++++++------ 3 files changed, 139 insertions(+), 29 deletions(-) diff --git a/proof_of_stake/src/validation.rs b/proof_of_stake/src/validation.rs index 13356cbb55..922e6b3f2e 100644 --- a/proof_of_stake/src/validation.rs +++ b/proof_of_stake/src/validation.rs @@ -16,7 +16,7 @@ use crate::btree_set::BTreeSetShims; use crate::epoched::DynEpochOffset; use crate::parameters::PosParams; use crate::types::{ - BondId, Bonds, Epoch, PublicKeyTmRawHash, Slash, Slashes, + BondId, Bonds, CommissionRates, Epoch, PublicKeyTmRawHash, Slash, Slashes, TotalVotingPowers, Unbonds, ValidatorConsensusKeys, ValidatorSets, ValidatorState, ValidatorStates, ValidatorTotalDeltas, ValidatorVotingPowers, VotingPower, VotingPowerDelta, WeightedValidator, @@ -50,6 +50,12 @@ where MissingNewValidatorConsensusKey(u64), #[error("Invalid validator consensus key update in epoch {0}")] InvalidValidatorConsensusKeyUpdate(u64), + #[error("Unexpectedly missing commission rate value for validator {0}")] + ValidatorCommissionRateIsRequired(Address), + #[error("Missing new validator commission rate in epoch {0}")] + MissingNewValidatorCommissionRate(u64), + #[error("Invalid validator commission rate update in epoch {0}")] + InvalidValidatorCommissionRateUpdate(u64), #[error("Unexpectedly missing total deltas value for validator {0}")] MissingValidatorTotalDeltas(Address), #[error("The sum of total deltas for validator {0} are negative")] @@ -279,6 +285,8 @@ where TotalDeltas(Data>), /// Voting power update VotingPowerUpdate(Data), + /// Commission rate update + CommissionRateUpdate(Data), } /// Data update with prior and posterior state. @@ -305,6 +313,7 @@ pub struct NewValidator { has_voting_power: bool, has_address_raw_hash: Option, voting_power: VotingPower, + has_commission_rate: bool, } /// Validation constants @@ -797,9 +806,14 @@ where has_voting_power, has_address_raw_hash, voting_power, + has_commission_rate, } = &new_validator; // The new validator must have set all the required fields - if !(*has_state && *has_total_deltas && *has_voting_power) { + if !(*has_state + && *has_total_deltas + && *has_voting_power + && *has_commission_rate) + { errors.push(Error::InvalidNewValidator( address.clone(), new_validator.clone(), @@ -1132,6 +1146,15 @@ where address, data, ), + CommissionRateUpdate(data) => { + Self::validator_commission_rate( + constants, + errors, + new_validators, + address, + data, + ) + } }, Balance(data) => Self::balance(errors, balance_delta, data), Bond { id, data, slashes } => { @@ -1469,6 +1492,62 @@ where } } + fn validator_commission_rate( + constants: &Constants, + errors: &mut Vec>, + new_validators: &mut HashMap>, + address: Address, + data: Data, + ) { + match (data.pre, data.post) { + // Should this case ever happen since commission rate should be init + // at genesis? (same w consensus key tho) + (None, Some(post)) => { + if post.last_update() != constants.current_epoch { + errors.push(Error::InvalidLastUpdate) + } + // The value must be known at pipeline epoch + match post.get(constants.pipeline_epoch) { + Some(_) => { + let validator = + new_validators.entry(address).or_default(); + validator.has_commission_rate = true; + } + _ => errors.push(Error::MissingNewValidatorCommissionRate( + constants.pipeline_epoch.into(), + )), + } + } + (Some(pre), Some(post)) => { + if post.last_update() != constants.current_epoch { + errors.push(Error::InvalidLastUpdate) + } + // Before pipeline epoch, the commission rate must not change + for epoch in Epoch::iter_range( + constants.current_epoch, + constants.pipeline_offset, + ) { + match (pre.get(epoch), post.get(epoch)) { + (Some(rate_pre), Some(rate_post)) + if rate_pre == rate_post => + { + continue; + } + _ => errors.push( + Error::InvalidValidatorCommissionRateUpdate( + epoch.into(), + ), + ), + } + } + } + (Some(_), None) => { + errors.push(Error::ValidatorCommissionRateIsRequired(address)) + } + (None, None) => {} + } + } + #[allow(clippy::too_many_arguments)] fn validator_voting_power( params: &PosParams, diff --git a/shared/src/ledger/pos/vp.rs b/shared/src/ledger/pos/vp.rs index 185a1bbc4d..bf6b67e6d2 100644 --- a/shared/src/ledger/pos/vp.rs +++ b/shared/src/ledger/pos/vp.rs @@ -33,8 +33,8 @@ use crate::ledger::native_vp::{ self, Ctx, CtxPostStorageRead, CtxPreStorageRead, NativeVp, }; use crate::ledger::pos::{ - is_validator_address_raw_hash_key, is_validator_consensus_key_key, - is_validator_state_key, + is_validator_address_raw_hash_key, is_validator_commission_rate_key, + is_validator_consensus_key_key, is_validator_state_key, }; use crate::ledger::storage::{self as ledger_storage, StorageHasher}; use crate::ledger::storage_api::{self, StorageRead}; @@ -262,6 +262,18 @@ where TotalVotingPowers::try_from_slice(&bytes[..]).ok() }); changes.push(TotalVotingPower(Data { pre, post })); + } else if let Some(address) = is_validator_commission_rate_key(key) + { + let pre = self.ctx.pre().read_bytes(key)?.and_then(|bytes| { + CommissionRates::try_from_slice(&bytes[..]).ok() + }); + let post = self.ctx.post().read_bytes(key)?.and_then(|bytes| { + CommissionRates::try_from_slice(&bytes[..]).ok() + }); + changes.push(Validator { + address: address.clone(), + update: CommissionRateUpdate(Data { pre, post }), + }); } else if key.segments.get(0) == Some(&addr.to_db_key()) { // Unknown changes to this address space are disallowed tracing::info!("PoS unrecognized key change {} rejected", key); diff --git a/wasm/wasm_source/src/tx_change_validator_commission.rs b/wasm/wasm_source/src/tx_change_validator_commission.rs index 65a78d1af5..301902f181 100644 --- a/wasm/wasm_source/src/tx_change_validator_commission.rs +++ b/wasm/wasm_source/src/tx_change_validator_commission.rs @@ -25,9 +25,7 @@ mod tests { use namada_tests::native_vp::pos::init_pos; use namada_tests::native_vp::TestNativeVpEnv; use namada_tests::tx::*; - use namada_tx_prelude::address::testing::{ - arb_established_address, - }; + use namada_tx_prelude::address::testing::arb_established_address; use namada_tx_prelude::key::testing::arb_common_keypair; use namada_tx_prelude::key::RefTo; use namada_tx_prelude::proof_of_stake::parameters::testing::arb_pos_params; @@ -36,6 +34,7 @@ mod tests { CommissionRates, GenesisValidator, PosVP, }; use proptest::prelude::*; + use rust_decimal::prelude::ToPrimitive; use rust_decimal::Decimal; use super::*; @@ -50,13 +49,11 @@ mod tests { /// that this transaction is accepted by the PoS validity predicate. #[test] fn test_tx_change_validator_commissions( - initial_rate in arb_rate(), - max_change in arb_rate(), - commission_change in arb_commission_change(), + commission_state_change in arb_commission_info(), // A key to sign the transaction key in arb_common_keypair(), pos_params in arb_pos_params()) { - test_tx_change_validator_commission_aux(commission_change, initial_rate, max_change, key, pos_params).unwrap() + test_tx_change_validator_commission_aux(commission_state_change.2, commission_state_change.0, commission_state_change.1, key, pos_params).unwrap() } } @@ -76,8 +73,6 @@ mod tests { max_commission_rate_change: max_change, }]; - println!("\nInitial rate = {}\nMax change = {}\nNew rate = {}",initial_rate,max_change,commission_change.new_rate.clone()); - init_pos(&genesis_validators[..], &pos_params, Epoch(0)); let tx_code = vec![]; @@ -108,7 +103,8 @@ mod tests { println!("dbg2.1"); let commission_rates_post: CommissionRates = ctx() - .read_validator_commission_rate(&commission_change.validator)?.unwrap(); + .read_validator_commission_rate(&commission_change.validator)? + .unwrap(); dbg!(&commission_rates_pre); dbg!(&commission_rates_post); @@ -118,14 +114,14 @@ mod tests { assert_eq!( commission_rates_pre.get(epoch), commission_rates_post.get(epoch), - "The commission rates before the pipeline offset must not change \ - - checking in epoch: {epoch}" + "The commission rates before the pipeline offset must not \ + change - checking in epoch: {epoch}" ); assert_eq!( Some(&initial_rate), commission_rates_post.get(epoch), - "The commission rates before the pipeline offset must not change \ - - checking in epoch: {epoch}" + "The commission rates before the pipeline offset must not \ + change - checking in epoch: {epoch}" ); } println!("\ndbg3\n"); @@ -135,14 +131,14 @@ mod tests { assert_ne!( commission_rates_pre.get(epoch), commission_rates_post.get(epoch), - "The commission rate after the pipeline offset must have changed \ - - checking in epoch: {epoch}" + "The commission rate after the pipeline offset must have \ + changed - checking in epoch: {epoch}" ); assert_eq!( Some(&commission_change.new_rate), commission_rates_post.get(epoch), - "The commission rate after the pipeline offset must be the new_rate \ - - checking in epoch: {epoch}" + "The commission rate after the pipeline offset must be the \ + new_rate - checking in epoch: {epoch}" ); } @@ -163,19 +159,42 @@ mod tests { Ok(()) } - fn arb_rate() -> impl Strategy { - (0..=100_000u64).prop_map(|num| { - Decimal::from(num) / Decimal::new(100_000, 0) - }) + fn arb_rate(min: Decimal, max: Decimal) -> impl Strategy { + let int_min: u64 = (min * Decimal::from(100_000_u64)) + .to_u64() + .unwrap_or_default(); + let int_max: u64 = (max * Decimal::from(100_000_u64)).to_u64().unwrap(); + (int_min..int_max) + .prop_map(|num| Decimal::from(num) / Decimal::from(100_000_u64)) } - fn arb_commission_change() - -> impl Strategy { - (arb_established_address(), arb_rate()).prop_map( + fn arb_commission_change( + rate_pre: Decimal, + max_change: Decimal, + ) -> impl Strategy { + let min = rate_pre - max_change; + let max = rate_pre + max_change; + (arb_established_address(), arb_rate(min, max)).prop_map( |(validator, new_rate)| transaction::pos::CommissionChange { validator: Address::Established(validator), new_rate, }, ) } + + fn arb_commission_info() + -> impl Strategy + { + let min = Decimal::ZERO; + let max = Decimal::ONE; + (arb_rate(min, max), arb_rate(min, max)).prop_flat_map( + |(rate, change)| { + ( + Just(rate), + Just(change), + arb_commission_change(rate, change), + ) + }, + ) + } } From 7d652e3fa2d43dd4abd268014d05d95ad3b333a5 Mon Sep 17 00:00:00 2001 From: brentstone Date: Fri, 4 Nov 2022 15:05:06 -0400 Subject: [PATCH 25/38] bug fix: consensus key validation error --- proof_of_stake/src/validation.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/proof_of_stake/src/validation.rs b/proof_of_stake/src/validation.rs index 922e6b3f2e..0e3109b715 100644 --- a/proof_of_stake/src/validation.rs +++ b/proof_of_stake/src/validation.rs @@ -46,6 +46,8 @@ where InvalidNewValidatorState(u64), #[error("Invalid validator state update in epoch {0}")] InvalidValidatorStateUpdate(u64), + #[error("Unexpectedly missing consensus key value for validator {0}")] + ValidatorConsensusKeyIsRequired(Address), #[error("Missing new validator consensus key in epoch {0}")] MissingNewValidatorConsensusKey(u64), #[error("Invalid validator consensus key update in epoch {0}")] @@ -1319,7 +1321,7 @@ where } } (Some(_), None) => { - errors.push(Error::ValidatorStateIsRequired(address)) + errors.push(Error::ValidatorConsensusKeyIsRequired(address)) } (None, None) => {} } From 9b4f2be63544ead596ab24aa17bcff54137c8ebd Mon Sep 17 00:00:00 2001 From: brentstone Date: Fri, 4 Nov 2022 15:05:52 -0400 Subject: [PATCH 26/38] fix get of epoched commission rate before pipeline --- proof_of_stake/src/epoched.rs | 7 ++++++- proof_of_stake/src/lib.rs | 5 ++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/proof_of_stake/src/epoched.rs b/proof_of_stake/src/epoched.rs index f13bec3ee0..cc2f24f1a8 100644 --- a/proof_of_stake/src/epoched.rs +++ b/proof_of_stake/src/epoched.rs @@ -128,6 +128,8 @@ pub enum DynEpochOffset { PipelineLen, /// Offset at unbonding length. UnbondingLen, + /// Offset at pipeline length - 1. + PipelineLenMinusOne, } impl DynEpochOffset { /// Find the value of a given offset from PoS parameters. @@ -135,6 +137,7 @@ impl DynEpochOffset { match self { DynEpochOffset::PipelineLen => params.pipeline_len, DynEpochOffset::UnbondingLen => params.unbonding_len, + DynEpochOffset::PipelineLenMinusOne => params.pipeline_len - 1, } } } @@ -1223,7 +1226,9 @@ mod tests { Some(DynEpochOffset::PipelineLen) => { Just(DynEpochOffset::PipelineLen).boxed() } - Some(DynEpochOffset::UnbondingLen) | None => prop_oneof![ + Some(DynEpochOffset::UnbondingLen) + | Some(DynEpochOffset::PipelineLenMinusOne) + | None => prop_oneof![ Just(DynEpochOffset::PipelineLen), Just(DynEpochOffset::UnbondingLen), ] diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs index f88da14983..4032b5c6d2 100644 --- a/proof_of_stake/src/lib.rs +++ b/proof_of_stake/src/lib.rs @@ -580,11 +580,10 @@ pub trait PosActions: PosReadOnly { .into()); } - let rate_before_pipeline = *commission_rates .get_at_offset( - current_epoch - 1, - DynEpochOffset::PipelineLen, + current_epoch, + DynEpochOffset::PipelineLenMinusOne, ¶ms, ) .expect("Could not find a rate in given epoch"); From 6778fd04fba9631af5eb0aa97c33951185c1d099 Mon Sep 17 00:00:00 2001 From: brentstone Date: Fri, 4 Nov 2022 18:13:09 -0400 Subject: [PATCH 27/38] add max change info to query of validator commission rate --- apps/src/lib/client/rpc.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/apps/src/lib/client/rpc.rs b/apps/src/lib/client/rpc.rs index 37b8249998..1fa8b954a5 100644 --- a/apps/src/lib/client/rpc.rs +++ b/apps/src/lib/client/rpc.rs @@ -33,6 +33,7 @@ use namada::types::key::*; use namada::types::storage::{Epoch, Key, KeySeg, PrefixValue}; use namada::types::token::{balance_key, Amount}; use namada::types::{address, storage, token}; +use rust_decimal::Decimal; use crate::cli::{self, args, Context}; use crate::client::tendermint_rpc_types::TxResponse; @@ -991,19 +992,30 @@ pub async fn query_commission_rate( let validator = ctx.get(&validator); let validator_commission_key = pos::validator_commission_rate_key(&validator); + let validator_max_commission_change_key = + pos::validator_max_commission_rate_change_key(&validator); let commission_rates = query_storage_value::( &client, &validator_commission_key, ) .await; + let max_rate_change = query_storage_value::( + &client, + &validator_max_commission_change_key, + ) + .await; + let max_rate_change = + max_rate_change.expect("No max rate change found"); let commission_rates = commission_rates.expect("No commission rate found "); match commission_rates.get(epoch) { Some(rate) => { println!( - "Validator {} commission rate: {}", + "Validator {} commission rate: {}, max change per \ + epoch: {}", validator.encode(), - *rate + *rate, + max_rate_change, ) } None => { From b9003544204c1aef553c821512e3ed107c367cf4 Mon Sep 17 00:00:00 2001 From: brentstone Date: Mon, 7 Nov 2022 16:22:18 -0500 Subject: [PATCH 28/38] fix pos state machine test --- Cargo.lock | 1 + proof_of_stake/src/parameters.rs | 8 ++++++ tests/Cargo.toml | 1 + tests/src/native_vp/pos.rs | 35 +++++++++++++++++++++++++-- wasm/Cargo.lock | 1 + wasm_for_tests/wasm_source/Cargo.lock | 1 + 6 files changed, 45 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 98b3c64417..50436d2264 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3117,6 +3117,7 @@ dependencies = [ "proptest", "prost", "rand 0.8.5", + "rust_decimal", "serde_json", "sha2 0.9.9", "tempfile", diff --git a/proof_of_stake/src/parameters.rs b/proof_of_stake/src/parameters.rs index 1c265bf1a8..ae3a881d42 100644 --- a/proof_of_stake/src/parameters.rs +++ b/proof_of_stake/src/parameters.rs @@ -153,6 +153,7 @@ mod tests { #[cfg(any(test, feature = "testing"))] pub mod testing { use proptest::prelude::*; + use rust_decimal::Decimal; use super::*; @@ -177,4 +178,11 @@ pub mod testing { } } } + + /// Get an arbitrary rate - a Decimal value between 0 and 1 inclusive, with + /// some fixed precision + pub fn arb_rate() -> impl Strategy { + (0..100_001_u64) + .prop_map(|num| Decimal::from(num) / Decimal::from(100_000_u64)) + } } diff --git a/tests/Cargo.toml b/tests/Cargo.toml index 74056ba00b..491819fdf2 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -25,6 +25,7 @@ tempfile = "3.2.0" tracing = "0.1.30" tracing-subscriber = {version = "0.3.7", default-features = false, features = ["env-filter", "fmt"]} derivative = "2.2.0" +rust_decimal = "1.26.1" [dev-dependencies] namada_apps = {path = "../apps", default-features = false, features = ["testing"]} diff --git a/tests/src/native_vp/pos.rs b/tests/src/native_vp/pos.rs index 2a748f0259..385bc7e96e 100644 --- a/tests/src/native_vp/pos.rs +++ b/tests/src/native_vp/pos.rs @@ -400,6 +400,7 @@ mod tests { ValidPosAction::InitValidator { address, consensus_key, + commission_rate: _, } => { !state.is_validator(address) && !state.is_used_key(consensus_key) @@ -578,6 +579,7 @@ pub mod testing { use namada_tx_prelude::proof_of_stake::epoched::{ DynEpochOffset, Epoched, EpochedDelta, }; + use namada_tx_prelude::proof_of_stake::parameters::testing::arb_rate; use namada_tx_prelude::proof_of_stake::types::{ Bond, Unbond, ValidatorState, VotingPower, VotingPowerDelta, WeightedValidator, @@ -587,6 +589,7 @@ pub mod testing { }; use namada_tx_prelude::{Address, StorageRead, StorageWrite}; use proptest::prelude::*; + use rust_decimal::Decimal; use crate::tx::{self, tx_host_env}; @@ -611,6 +614,7 @@ pub mod testing { InitValidator { address: Address, consensus_key: PublicKey, + commission_rate: Decimal, }, Bond { amount: token::Amount, @@ -700,6 +704,10 @@ pub mod testing { #[derivative(Debug = "ignore")] consensus_key: PublicKey, }, + ValidatorCommissionRate { + address: Address, + rate: Decimal, + }, } pub fn arb_valid_pos_action( @@ -717,11 +725,13 @@ pub mod testing { let init_validator = ( address::testing::arb_established_address(), key::testing::arb_common_keypair(), + arb_rate(), ) - .prop_map(|(addr, consensus_key)| { + .prop_map(|(addr, consensus_key, commission_rate)| { ValidPosAction::InitValidator { address: Address::Established(addr), consensus_key: consensus_key.ref_to(), + commission_rate, } }); @@ -869,6 +879,7 @@ pub mod testing { ValidPosAction::InitValidator { address, consensus_key, + commission_rate, } => { let offset = DynEpochOffset::PipelineLen; vec![ @@ -902,10 +913,14 @@ pub mod testing { offset, }, PosStorageChange::ValidatorVotingPower { - validator: address, + validator: address.clone(), vp_delta: 0, offset: Either::Left(offset), }, + PosStorageChange::ValidatorCommissionRate { + address, + rate: commission_rate, + }, ] } ValidPosAction::Bond { @@ -1487,6 +1502,22 @@ pub mod testing { unbonds.delete_current(current_epoch, params); tx::ctx().write_unbond(&bond_id, unbonds).unwrap(); } + // TODO: figure this out + PosStorageChange::ValidatorCommissionRate { address, rate } => { + let rates = tx::ctx() + .read_validator_commission_rate(&address) + .unwrap() + .map(|mut rates| { + rates.set(rate, current_epoch, params); + rates + }) + .unwrap_or_else(|| { + Epoched::init_at_genesis(rate, current_epoch) + }); + tx::ctx() + .write_validator_commission_rate(&address, rates) + .unwrap(); + } } } diff --git a/wasm/Cargo.lock b/wasm/Cargo.lock index 3345a04b2c..c2d7446397 100644 --- a/wasm/Cargo.lock +++ b/wasm/Cargo.lock @@ -1434,6 +1434,7 @@ dependencies = [ "namada_tx_prelude", "namada_vp_prelude", "prost", + "rust_decimal", "serde_json", "sha2 0.9.9", "tempfile", diff --git a/wasm_for_tests/wasm_source/Cargo.lock b/wasm_for_tests/wasm_source/Cargo.lock index 27dc824168..d9edbae570 100644 --- a/wasm_for_tests/wasm_source/Cargo.lock +++ b/wasm_for_tests/wasm_source/Cargo.lock @@ -1434,6 +1434,7 @@ dependencies = [ "namada_tx_prelude", "namada_vp_prelude", "prost", + "rust_decimal", "serde_json", "sha2 0.9.9", "tempfile", From 7b6de40da92cebd270056b7200a709b4bc09bc8f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 7 Nov 2022 21:59:32 +0000 Subject: [PATCH 29/38] [ci] wasm checksums update --- wasm/checksums.json | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/wasm/checksums.json b/wasm/checksums.json index 99ca7176c5..7e1d1f8195 100644 --- a/wasm/checksums.json +++ b/wasm/checksums.json @@ -1,20 +1,16 @@ { - "tx_bond.wasm": "tx_bond.c74583608a2474b70c60f4378d11a9bee9b624aad64418221452e0f728b616b7.wasm", - "tx_change_validator_commission.wasm": "tx_change_validator_commission.e418ee939e2ca0248c0546c446e2513504cbcf04f054d709d4a0f159cfac9211.wasm", - "tx_from_intent.wasm": "tx_from_intent.e00a9b09cbeba1a5c61d9f93a9b5c42e5eaf0246db1fa2e78d5de7c9ede87955.wasm", - "tx_ibc.wasm": "tx_ibc.0951cba0a9487915b9a6194c6bd7c5fea0d3d056fd9ff0b47f3035c0157eb4f4.wasm", - "tx_init_account.wasm": "tx_init_account.63984b48d4a46b82f6de27d6879a3fb68ceca5cc03c8b27e8dc67f7fbc17d88c.wasm", - "tx_init_nft.wasm": "tx_init_nft.68cb966f08114092918f64e47b25574ee2a062386e38a0d3faeda29b9ff289f8.wasm", - "tx_init_proposal.wasm": "tx_init_proposal.53997c9a43e741838600b28a5bbb34a8e9207803952d79afd0c9777bfd223cd0.wasm", - "tx_init_validator.wasm": "tx_init_validator.ac82ac4c501b774b62cf77311ff0374f37f9874c58b53bffbe1c608b5ef258a5.wasm", - "tx_mint_nft.wasm": "tx_mint_nft.ef93c00a3fd0597e3551e81b4abc7fcd7e7a5f890705839d578189d11f5091ff.wasm", - "tx_transfer.wasm": "tx_transfer.abda15ddfce5e8b6307d6a2974fdf949b67631ca0b7a30f1ad233b6a4f4d5da1.wasm", - "tx_unbond.wasm": "tx_unbond.4e4c2efa864786756739efce1163532509ba10aad5dcb983eeffac5ec84395e0.wasm", - "tx_update_vp.wasm": "tx_update_vp.8e13d0ded993cdb837d2feab46758e8c7a92353511e9e558750ab76113d82a43.wasm", - "tx_vote_proposal.wasm": "tx_vote_proposal.f14bbea254f91d230f8966fae1446d4dd1a677fb491225aaa7153557498de644.wasm", - "tx_withdraw.wasm": "tx_withdraw.4ff2483bfcd2710bda5507e51407a46a6a4cc4ed1418a379e92aa29a03c4dd27.wasm", - "vp_nft.wasm": "vp_nft.ca95f995ebe0854f3c8501abcd9d65babc0040bf55554612a8509e7642b8296b.wasm", - "vp_testnet_faucet.wasm": "vp_testnet_faucet.49b35412b92610033b091bab844cc0760530c7379d3f0aaf58c538ac3d4452d8.wasm", - "vp_token.wasm": "vp_token.927d28326b54b3ff59b5447939f7fa42e1b5a4bc2c1a6f8c51ff603402d71a28.wasm", - "vp_user.wasm": "vp_user.b520d389415f8051b8ac5be38093ab503cf762dc219f4efec39c10da5362882e.wasm" + "tx_bond.wasm": "tx_bond.4cca44c22f0b367a3780ce64143438fbf506382aed39a29315253e8c0b968d0d.wasm", + "tx_change_validator_commission.wasm": "tx_change_validator_commission.8c624dd710dff9c1fdd0e4576cd8193b92920ef8e51bf19835a0800311f7c90a.wasm", + "tx_ibc.wasm": "tx_ibc.92339ccc594462fc8fc29d27fcdf5c49da3c3f42c49e276c0089258b74b50b56.wasm", + "tx_init_account.wasm": "tx_init_account.3ef2dda05d678a39d03307a5f93ecef6521403c468c4fc96f4150fc169a5865f.wasm", + "tx_init_proposal.wasm": "tx_init_proposal.bad7704b1e96410b63142edf367e6aad1b79eca0a3a28f579ab64712a47b2c74.wasm", + "tx_init_validator.wasm": "tx_init_validator.5acc5e8e9a8657d1b1a62dac8cc8756946ab6b78a4ec0e6d7acb27531e54f4c5.wasm", + "tx_transfer.wasm": "tx_transfer.24996732a3f9f356bf698128c41ac15a570e661a7fa741048d391412f925d9f3.wasm", + "tx_unbond.wasm": "tx_unbond.25d400d6246d68c4696e2b5dd84d9eb32cc05adba046ef44864937b5baaefdb1.wasm", + "tx_update_vp.wasm": "tx_update_vp.ab2f6485f1536d0bceab1f12e2aef4712ebc0e1495c92198fb57ec35642c710b.wasm", + "tx_vote_proposal.wasm": "tx_vote_proposal.bd76fab06d5afafbd101a133ac8ade475fe25484f044445c3e39981eb5a2b84f.wasm", + "tx_withdraw.wasm": "tx_withdraw.01b4185d41565fc59f8976999faa738ad4e5388e30ee279078c4ff059b084e05.wasm", + "vp_testnet_faucet.wasm": "vp_testnet_faucet.3938c15abd8b2cfad97c87d48d1a84817a7aaf597d4c868356a4dbb7e2517a4b.wasm", + "vp_token.wasm": "vp_token.800e7f696b7f41c970e2464316ed09be8d3d63bd58c32e8e3434b203b56d9015.wasm", + "vp_user.wasm": "vp_user.b23ce44da223c501d4a5bb891cba4a033bba3cf6d877a1268ced3707e0e71bd0.wasm" } \ No newline at end of file From 302a08cc3a1078bfbd0df19f2873ad59c524b57a Mon Sep 17 00:00:00 2001 From: brentstone Date: Tue, 8 Nov 2022 12:06:29 -0500 Subject: [PATCH 30/38] changes in response to review comments --- apps/src/lib/cli.rs | 6 +- apps/src/lib/client/rpc.rs | 87 +++++++++---------- proof_of_stake/src/parameters.rs | 2 +- proof_of_stake/src/validation.rs | 28 +++++- shared/src/ledger/pos/vp.rs | 10 ++- shared/src/ledger/queries/router.rs | 1 - tests/src/native_vp/pos.rs | 1 - .../src/tx_change_validator_commission.rs | 10 +-- 8 files changed, 81 insertions(+), 64 deletions(-) diff --git a/apps/src/lib/cli.rs b/apps/src/lib/cli.rs index 5e85efdd5d..499e21d432 100644 --- a/apps/src/lib/cli.rs +++ b/apps/src/lib/cli.rs @@ -2101,7 +2101,7 @@ pub mod args { /// Common query args pub query: Query, /// Address of a validator - pub validator: Option, + pub validator: WalletAddress, /// Epoch in which to find commission rate pub epoch: Option, } @@ -2109,7 +2109,7 @@ pub mod args { impl Args for QueryCommissionRate { fn parse(matches: &ArgMatches) -> Self { let query = Query::parse(matches); - let validator = VALIDATOR_OPT.parse(matches); + let validator = VALIDATOR.parse(matches); let epoch = EPOCH.parse(matches); Self { query, @@ -2120,7 +2120,7 @@ pub mod args { fn def(app: App) -> App { app.add_args::() - .arg(VALIDATOR_OPT.def().about( + .arg(VALIDATOR.def().about( "The validator's address whose commission rate to query.", )) .arg(EPOCH.def().about( diff --git a/apps/src/lib/client/rpc.rs b/apps/src/lib/client/rpc.rs index 1fa8b954a5..6245e07fc1 100644 --- a/apps/src/lib/client/rpc.rs +++ b/apps/src/lib/client/rpc.rs @@ -976,7 +976,7 @@ pub async fn query_voting_power(ctx: Context, args: args::QueryVotingPower) { println!("Total voting power: {}", total_voting_power); } -/// Query PoS commssion rate +/// Query PoS validator's commission rate pub async fn query_commission_rate( ctx: Context, args: args::QueryCommissionRate, @@ -985,51 +985,50 @@ pub async fn query_commission_rate( Some(epoch) => epoch, None => query_epoch(args.query.clone()).await, }; - let client = HttpClient::new(args.query.ledger_address).unwrap(); - - match args.validator { - Some(validator) => { - let validator = ctx.get(&validator); - let validator_commission_key = - pos::validator_commission_rate_key(&validator); - let validator_max_commission_change_key = - pos::validator_max_commission_rate_change_key(&validator); - let commission_rates = query_storage_value::( - &client, - &validator_commission_key, - ) - .await; - let max_rate_change = query_storage_value::( - &client, - &validator_max_commission_change_key, - ) - .await; - let max_rate_change = - max_rate_change.expect("No max rate change found"); - let commission_rates = - commission_rates.expect("No commission rate found "); - match commission_rates.get(epoch) { - Some(rate) => { - println!( - "Validator {} commission rate: {}, max change per \ - epoch: {}", - validator.encode(), - *rate, - max_rate_change, - ) - } - None => { - println!( - "No commission rate found for {} in epoch {}", - validator.encode(), - epoch - ) - } + let client = HttpClient::new(args.query.ledger_address.clone()).unwrap(); + let validator = ctx.get(&args.validator); + let is_validator = + is_validator(&validator, args.query.ledger_address).await; + + if is_validator { + let validator_commission_key = + pos::validator_commission_rate_key(&validator); + let validator_max_commission_change_key = + pos::validator_max_commission_rate_change_key(&validator); + let commission_rates = query_storage_value::( + &client, + &validator_commission_key, + ) + .await; + let max_rate_change = query_storage_value::( + &client, + &validator_max_commission_change_key, + ) + .await; + let max_rate_change = + max_rate_change.expect("No max rate change found"); + let commission_rates = + commission_rates.expect("No commission rate found "); + match commission_rates.get(epoch) { + Some(rate) => { + println!( + "Validator {} commission rate: {}, max change per epoch: \ + {}", + validator.encode(), + *rate, + max_rate_change, + ) + } + None => { + println!( + "No commission rate found for {} in epoch {}", + validator.encode(), + epoch + ) } } - None => { - println!("No validator found from the args") - } + } else { + println!("Cannot find validator with address {}", validator); } } diff --git a/proof_of_stake/src/parameters.rs b/proof_of_stake/src/parameters.rs index ae3a881d42..c5f32267cc 100644 --- a/proof_of_stake/src/parameters.rs +++ b/proof_of_stake/src/parameters.rs @@ -182,7 +182,7 @@ pub mod testing { /// Get an arbitrary rate - a Decimal value between 0 and 1 inclusive, with /// some fixed precision pub fn arb_rate() -> impl Strategy { - (0..100_001_u64) + (0..=100_000_u64) .prop_map(|num| Decimal::from(num) / Decimal::from(100_000_u64)) } } diff --git a/proof_of_stake/src/validation.rs b/proof_of_stake/src/validation.rs index 0e3109b715..61374fca67 100644 --- a/proof_of_stake/src/validation.rs +++ b/proof_of_stake/src/validation.rs @@ -10,6 +10,7 @@ use std::ops::{Add, AddAssign, Neg, Sub, SubAssign}; use borsh::{BorshDeserialize, BorshSchema, BorshSerialize}; use derivative::Derivative; +use rust_decimal::Decimal; use thiserror::Error; use crate::btree_set::BTreeSetShims; @@ -288,7 +289,7 @@ where /// Voting power update VotingPowerUpdate(Data), /// Commission rate update - CommissionRateUpdate(Data), + CommissionRate(Data, Decimal), } /// Data update with prior and posterior state. @@ -1148,13 +1149,14 @@ where address, data, ), - CommissionRateUpdate(data) => { + CommissionRate(data, max_change) => { Self::validator_commission_rate( constants, errors, new_validators, address, data, + max_change, ) } }, @@ -1500,10 +1502,9 @@ where new_validators: &mut HashMap>, address: Address, data: Data, + max_change: Decimal, ) { match (data.pre, data.post) { - // Should this case ever happen since commission rate should be init - // at genesis? (same w consensus key tho) (None, Some(post)) => { if post.last_update() != constants.current_epoch { errors.push(Error::InvalidLastUpdate) @@ -1542,6 +1543,25 @@ where ), } } + // At the pipeline epoch, the rate must change by no larger than + // `max_change` relative to the previous epoch + match ( + pre.get(constants.pipeline_epoch - 1), + post.get(constants.pipeline_epoch), + ) { + (Some(prev_rate), Some(new_rate)) => { + if (new_rate - prev_rate).abs() > max_change { + errors.push( + Error::InvalidValidatorCommissionRateUpdate( + constants.pipeline_epoch.into(), + ), + ) + } + } + _ => errors.push(Error::ValidatorCommissionRateIsRequired( + address, + )), + } } (Some(_), None) => { errors.push(Error::ValidatorCommissionRateIsRequired(address)) diff --git a/shared/src/ledger/pos/vp.rs b/shared/src/ledger/pos/vp.rs index bf6b67e6d2..dcebda6a3c 100644 --- a/shared/src/ledger/pos/vp.rs +++ b/shared/src/ledger/pos/vp.rs @@ -264,6 +264,14 @@ where changes.push(TotalVotingPower(Data { pre, post })); } else if let Some(address) = is_validator_commission_rate_key(key) { + let max_change = self + .ctx + .pre() + .read_bytes(&validator_max_commission_rate_change_key( + address, + ))? + .and_then(|bytes| Decimal::try_from_slice(&bytes[..]).ok()) + .unwrap(); let pre = self.ctx.pre().read_bytes(key)?.and_then(|bytes| { CommissionRates::try_from_slice(&bytes[..]).ok() }); @@ -272,7 +280,7 @@ where }); changes.push(Validator { address: address.clone(), - update: CommissionRateUpdate(Data { pre, post }), + update: CommissionRate(Data { pre, post }, max_change), }); } else if key.segments.get(0) == Some(&addr.to_db_key()) { // Unknown changes to this address space are disallowed diff --git a/shared/src/ledger/queries/router.rs b/shared/src/ledger/queries/router.rs index e4823e5ad7..67e76c55bf 100644 --- a/shared/src/ledger/queries/router.rs +++ b/shared/src/ledger/queries/router.rs @@ -410,7 +410,6 @@ macro_rules! pattern_and_handler_to_method { ::Error > where CLIENT: $crate::ledger::queries::Client + std::marker::Sync { - println!("IMMA VEC!!!!!!"); let path = self.storage_value_path( $( $param ),* ); let $crate::ledger::queries::ResponseQuery { diff --git a/tests/src/native_vp/pos.rs b/tests/src/native_vp/pos.rs index 385bc7e96e..753d716736 100644 --- a/tests/src/native_vp/pos.rs +++ b/tests/src/native_vp/pos.rs @@ -1502,7 +1502,6 @@ pub mod testing { unbonds.delete_current(current_epoch, params); tx::ctx().write_unbond(&bond_id, unbonds).unwrap(); } - // TODO: figure this out PosStorageChange::ValidatorCommissionRate { address, rate } => { let rates = tx::ctx() .read_validator_commission_rate(&address) diff --git a/wasm/wasm_source/src/tx_change_validator_commission.rs b/wasm/wasm_source/src/tx_change_validator_commission.rs index 301902f181..0f72355af5 100644 --- a/wasm/wasm_source/src/tx_change_validator_commission.rs +++ b/wasm/wasm_source/src/tx_change_validator_commission.rs @@ -81,7 +81,6 @@ mod tests { let signed_tx = tx.sign(&key); let tx_data = signed_tx.data.unwrap(); - println!("\ndbg0\n"); // Read the data before the tx is executed let commission_rates_pre: CommissionRates = ctx() .read_validator_commission_rate(&commission_change.validator)? @@ -90,17 +89,14 @@ mod tests { .get(0) .expect("PoS validator must have commission rate at genesis"); assert_eq!(commission_rate, initial_rate); - println!("\ndbg1\n"); apply_tx(ctx(), tx_data)?; - println!("\ndbg2\n"); // Read the data after the tx is executed // The following storage keys should be updated: // - `#{PoS}/validator/#{validator}/commission_rate` - println!("dbg2.1"); let commission_rates_post: CommissionRates = ctx() .read_validator_commission_rate(&commission_change.validator)? @@ -124,7 +120,6 @@ mod tests { change - checking in epoch: {epoch}" ); } - println!("\ndbg3\n"); // After pipeline, the commission rates should have changed for epoch in pos_params.pipeline_len..=pos_params.unbonding_len { @@ -142,8 +137,6 @@ mod tests { ); } - println!("\ndbg4\n"); - // Use the tx_env to run PoS VP let tx_env = tx_host_env::take(); let vp_env = TestNativeVpEnv::from_tx_env(tx_env, address::POS); @@ -154,7 +147,6 @@ mod tests { result, "PoS Validity predicate must accept this transaction" ); - println!("\ndbg5\n"); Ok(()) } @@ -164,7 +156,7 @@ mod tests { .to_u64() .unwrap_or_default(); let int_max: u64 = (max * Decimal::from(100_000_u64)).to_u64().unwrap(); - (int_min..int_max) + (int_min..=int_max) .prop_map(|num| Decimal::from(num) / Decimal::from(100_000_u64)) } From dcfd6afb2e259e14304b3a6a81dfee81d3842b3d Mon Sep 17 00:00:00 2001 From: brentstone Date: Tue, 8 Nov 2022 21:42:14 -0500 Subject: [PATCH 31/38] validator VP that checks source and signature for a commission rate change tx --- wasm/wasm_source/Cargo.toml | 1 + wasm/wasm_source/src/lib.rs | 3 + wasm/wasm_source/src/vp_validator.rs | 759 +++++++++++++++++++++++++++ 3 files changed, 763 insertions(+) create mode 100644 wasm/wasm_source/src/vp_validator.rs diff --git a/wasm/wasm_source/Cargo.toml b/wasm/wasm_source/Cargo.toml index c66eea0480..a9682c5d9f 100644 --- a/wasm/wasm_source/Cargo.toml +++ b/wasm/wasm_source/Cargo.toml @@ -27,6 +27,7 @@ tx_change_validator_commission = ["namada_tx_prelude"] vp_testnet_faucet = ["namada_vp_prelude", "once_cell"] vp_token = ["namada_vp_prelude"] vp_user = ["namada_vp_prelude", "once_cell", "rust_decimal"] +vp_validator = ["namada_vp_prelude", "once_cell", "rust_decimal"] [dependencies] namada_tx_prelude = {path = "../../tx_prelude", optional = true} diff --git a/wasm/wasm_source/src/lib.rs b/wasm/wasm_source/src/lib.rs index de33356cdd..1afd097a45 100644 --- a/wasm/wasm_source/src/lib.rs +++ b/wasm/wasm_source/src/lib.rs @@ -26,3 +26,6 @@ pub mod vp_testnet_faucet; pub mod vp_token; #[cfg(feature = "vp_user")] pub mod vp_user; + +#[cfg(feature = "vp_validator")] +pub mod vp_validator; diff --git a/wasm/wasm_source/src/vp_validator.rs b/wasm/wasm_source/src/vp_validator.rs new file mode 100644 index 0000000000..798cb12241 --- /dev/null +++ b/wasm/wasm_source/src/vp_validator.rs @@ -0,0 +1,759 @@ +//! A basic validator VP. +//! +//! Like the user VP, this VP currently provides a signature verification +//! against a public key for sending tokens (receiving tokens is permissive). +//! +//! It allows to bond, unbond and withdraw tokens to and from PoS system with a +//! valid signature. +//! +//! Currently, the only difference with respect to the user VP is for a tx to +//! change a validator's commission rate: we require a valid signature only from +//! the validator whose commission rate is being changed. +//! +//! Any other storage key changes are allowed only with a valid signature. + +use namada_vp_prelude::storage::KeySeg; +use namada_vp_prelude::*; +use once_cell::unsync::Lazy; + +enum KeyType<'a> { + Token(&'a Address), + PoS, + Vp(&'a Address), + GovernanceVote(&'a Address), + Unknown, +} + +impl<'a> From<&'a storage::Key> for KeyType<'a> { + fn from(key: &'a storage::Key) -> KeyType<'a> { + if let Some(address) = token::is_any_token_balance_key(key) { + Self::Token(address) + } else if let Some((_, address)) = + token::is_any_multitoken_balance_key(key) + { + Self::Token(address) + } else if proof_of_stake::is_pos_key(key) { + Self::PoS + } else if gov_storage::is_vote_key(key) { + let voter_address = gov_storage::get_voter_address(key); + if let Some(address) = voter_address { + Self::GovernanceVote(address) + } else { + Self::Unknown + } + } else if let Some(address) = key.is_validity_predicate() { + Self::Vp(address) + } else { + Self::Unknown + } + } +} + +#[validity_predicate] +fn validate_tx( + ctx: &Ctx, + tx_data: Vec, + addr: Address, + keys_changed: BTreeSet, + verifiers: BTreeSet
, +) -> VpResult { + debug_log!( + "vp_user called with user addr: {}, key_changed: {:?}, verifiers: {:?}", + addr, + keys_changed, + verifiers + ); + + let signed_tx_data = + Lazy::new(|| SignedTxData::try_from_slice(&tx_data[..])); + + let valid_sig = Lazy::new(|| match &*signed_tx_data { + Ok(signed_tx_data) => { + let pk = key::get(ctx, &addr); + match pk { + Ok(Some(pk)) => { + matches!( + ctx.verify_tx_signature(&pk, &signed_tx_data.sig), + Ok(true) + ) + } + _ => false, + } + } + _ => false, + }); + + if !is_valid_tx(ctx, &tx_data)? { + return reject(); + } + + for key in keys_changed.iter() { + let key_type: KeyType = key.into(); + let is_valid = match key_type { + KeyType::Token(owner) => { + if owner == &addr { + let pre: token::Amount = + ctx.read_pre(key)?.unwrap_or_default(); + let post: token::Amount = + ctx.read_post(key)?.unwrap_or_default(); + let change = post.change() - pre.change(); + // debit has to signed, credit doesn't + let valid = change >= 0 || *valid_sig; + debug_log!( + "token key: {}, change: {}, valid_sig: {}, valid \ + modification: {}", + key, + change, + *valid_sig, + valid + ); + valid + } else { + debug_log!( + "This address ({}) is not of owner ({}) of token key: \ + {}", + addr, + owner, + key + ); + // If this is not the owner, allow any change + true + } + } + KeyType::PoS => { + // Allow the account to be used in PoS + let bond_id = proof_of_stake::is_bond_key(key) + .or_else(|| proof_of_stake::is_unbond_key(key)); + let valid_bond_or_unbond_change = match bond_id { + Some(bond_id) => { + // Bonds and unbonds changes for this address + // must be signed + bond_id.source != addr || *valid_sig + } + None => { + // Any other PoS changes are allowed without signature + true + } + }; + let comm = + proof_of_stake::is_validator_commission_rate_key(key); + let valid_commission_rate_change = match comm { + Some(source) => *source == addr && *valid_sig, + None => true, + }; + let valid = + valid_bond_or_unbond_change && valid_commission_rate_change; + debug_log!( + "PoS key {} {}", + key, + if valid { "accepted" } else { "rejected" } + ); + valid + } + KeyType::GovernanceVote(voter) => { + if voter == &addr { + *valid_sig + } else { + true + } + } + KeyType::Vp(owner) => { + let has_post: bool = ctx.has_key_post(key)?; + if owner == &addr { + if has_post { + let vp: Vec = ctx.read_bytes_post(key)?.unwrap(); + *valid_sig && is_vp_whitelisted(ctx, &vp)? + } else { + false + } + } else { + let vp: Vec = ctx.read_bytes_post(key)?.unwrap(); + is_vp_whitelisted(ctx, &vp)? + } + } + KeyType::Unknown => { + if key.segments.get(0) == Some(&addr.to_db_key()) { + // Unknown changes to this address space require a valid + // signature + *valid_sig + } else { + // Unknown changes anywhere else are permitted + true + } + } + }; + if !is_valid { + debug_log!("key {} modification failed vp", key); + return reject(); + } + } + + accept() +} + +#[cfg(test)] +mod tests { + use address::testing::arb_non_internal_address; + // Use this as `#[test]` annotation to enable logging + use namada_tests::log::test; + use namada_tests::tx::{self, tx_host_env, TestTxEnv}; + use namada_tests::vp::vp_host_env::storage::Key; + use namada_tests::vp::*; + use namada_tx_prelude::{StorageWrite, TxEnv}; + use namada_vp_prelude::key::RefTo; + use proptest::prelude::*; + use storage::testing::arb_account_storage_key_no_vp; + + use super::*; + + const VP_ALWAYS_TRUE_WASM: &str = + "../../wasm_for_tests/vp_always_true.wasm"; + + /// Test that no-op transaction (i.e. no storage modifications) accepted. + #[test] + fn test_no_op_transaction() { + let tx_data: Vec = vec![]; + let addr: Address = address::testing::established_address_1(); + let keys_changed: BTreeSet = BTreeSet::default(); + let verifiers: BTreeSet
= BTreeSet::default(); + + // The VP env must be initialized before calling `validate_tx` + vp_host_env::init(); + + assert!( + validate_tx(&CTX, tx_data, addr, keys_changed, verifiers).unwrap() + ); + } + + /// Test that a credit transfer is accepted. + #[test] + fn test_credit_transfer_accepted() { + // Initialize a tx environment + let mut tx_env = TestTxEnv::default(); + + let vp_owner = address::testing::established_address_1(); + let source = address::testing::established_address_2(); + let token = address::nam(); + let amount = token::Amount::from(10_098_123); + + // Spawn the accounts to be able to modify their storage + tx_env.spawn_accounts([&vp_owner, &source, &token]); + + // Credit the tokens to the source before running the transaction to be + // able to transfer from it + tx_env.credit_tokens(&source, &token, amount); + + // Initialize VP environment from a transaction + vp_host_env::init_from_tx(vp_owner.clone(), tx_env, |address| { + // Apply transfer in a transaction + tx_host_env::token::transfer( + tx::ctx(), + &source, + address, + &token, + None, + amount, + ) + .unwrap(); + }); + + let vp_env = vp_host_env::take(); + let tx_data: Vec = vec![]; + let keys_changed: BTreeSet = + vp_env.all_touched_storage_keys(); + let verifiers: BTreeSet
= BTreeSet::default(); + vp_host_env::set(vp_env); + assert!( + validate_tx(&CTX, tx_data, vp_owner, keys_changed, verifiers) + .unwrap() + ); + } + + /// Test that a debit transfer without a valid signature is rejected. + #[test] + fn test_unsigned_debit_transfer_rejected() { + // Initialize a tx environment + let mut tx_env = TestTxEnv::default(); + + let vp_owner = address::testing::established_address_1(); + let target = address::testing::established_address_2(); + let token = address::nam(); + let amount = token::Amount::from(10_098_123); + + // Spawn the accounts to be able to modify their storage + tx_env.spawn_accounts([&vp_owner, &target, &token]); + + // Credit the tokens to the VP owner before running the transaction to + // be able to transfer from it + tx_env.credit_tokens(&vp_owner, &token, amount); + + // Initialize VP environment from a transaction + vp_host_env::init_from_tx(vp_owner.clone(), tx_env, |address| { + // Apply transfer in a transaction + tx_host_env::token::transfer( + tx::ctx(), + address, + &target, + &token, + None, + amount, + ) + .unwrap(); + }); + + let vp_env = vp_host_env::take(); + let tx_data: Vec = vec![]; + let keys_changed: BTreeSet = + vp_env.all_touched_storage_keys(); + let verifiers: BTreeSet
= BTreeSet::default(); + vp_host_env::set(vp_env); + assert!( + !validate_tx(&CTX, tx_data, vp_owner, keys_changed, verifiers) + .unwrap() + ); + } + + /// Test that a debit transfer with a valid signature is accepted. + #[test] + fn test_signed_debit_transfer_accepted() { + // Initialize a tx environment + let mut tx_env = TestTxEnv::default(); + + let vp_owner = address::testing::established_address_1(); + let keypair = key::testing::keypair_1(); + let public_key = keypair.ref_to(); + let target = address::testing::established_address_2(); + let token = address::nam(); + let amount = token::Amount::from(10_098_123); + + // Spawn the accounts to be able to modify their storage + tx_env.spawn_accounts([&vp_owner, &target, &token]); + + // Credit the tokens to the VP owner before running the transaction to + // be able to transfer from it + tx_env.credit_tokens(&vp_owner, &token, amount); + + tx_env.write_public_key(&vp_owner, &public_key); + + // Initialize VP environment from a transaction + vp_host_env::init_from_tx(vp_owner.clone(), tx_env, |address| { + // Apply transfer in a transaction + tx_host_env::token::transfer( + tx::ctx(), + address, + &target, + &token, + None, + amount, + ) + .unwrap(); + }); + + let mut vp_env = vp_host_env::take(); + let tx = vp_env.tx.clone(); + let signed_tx = tx.sign(&keypair); + let tx_data: Vec = signed_tx.data.as_ref().cloned().unwrap(); + vp_env.tx = signed_tx; + let keys_changed: BTreeSet = + vp_env.all_touched_storage_keys(); + let verifiers: BTreeSet
= BTreeSet::default(); + vp_host_env::set(vp_env); + assert!( + validate_tx(&CTX, tx_data, vp_owner, keys_changed, verifiers) + .unwrap() + ); + } + + /// Test that a transfer on with accounts other than self is accepted. + #[test] + fn test_transfer_between_other_parties_accepted() { + // Initialize a tx environment + let mut tx_env = TestTxEnv::default(); + + let vp_owner = address::testing::established_address_1(); + let source = address::testing::established_address_2(); + let target = address::testing::established_address_3(); + let token = address::nam(); + let amount = token::Amount::from(10_098_123); + + // Spawn the accounts to be able to modify their storage + tx_env.spawn_accounts([&vp_owner, &source, &target, &token]); + + // Credit the tokens to the VP owner before running the transaction to + // be able to transfer from it + tx_env.credit_tokens(&source, &token, amount); + + // Initialize VP environment from a transaction + vp_host_env::init_from_tx(vp_owner.clone(), tx_env, |address| { + tx::ctx().insert_verifier(address).unwrap(); + // Apply transfer in a transaction + tx_host_env::token::transfer( + tx::ctx(), + &source, + &target, + &token, + None, + amount, + ) + .unwrap(); + }); + + let vp_env = vp_host_env::take(); + let tx_data: Vec = vec![]; + let keys_changed: BTreeSet = + vp_env.all_touched_storage_keys(); + let verifiers: BTreeSet
= BTreeSet::default(); + vp_host_env::set(vp_env); + assert!( + validate_tx(&CTX, tx_data, vp_owner, keys_changed, verifiers) + .unwrap() + ); + } + + prop_compose! { + /// Generates an account address and a storage key inside its storage. + fn arb_account_storage_subspace_key() + // Generate an address + (address in arb_non_internal_address()) + // Generate a storage key other than its VP key (VP cannot be + // modified directly via `write`, it has to be modified via + // `tx::update_validity_predicate`. + (storage_key in arb_account_storage_key_no_vp(address.clone()), + // Use the generated address too + address in Just(address)) + -> (Address, Key) { + (address, storage_key) + } + } + + proptest! { + /// Test that an unsigned tx that performs arbitrary storage writes or + /// deletes to the account is rejected. + #[test] + fn test_unsigned_arb_storage_write_rejected( + (vp_owner, storage_key) in arb_account_storage_subspace_key(), + // Generate bytes to write. If `None`, delete from the key instead + storage_value in any::>>(), + ) { + // Initialize a tx environment + let mut tx_env = TestTxEnv::default(); + + // Spawn all the accounts in the storage key to be able to modify + // their storage + let storage_key_addresses = storage_key.find_addresses(); + tx_env.spawn_accounts(storage_key_addresses); + + // Initialize VP environment from a transaction + vp_host_env::init_from_tx(vp_owner.clone(), tx_env, |_address| { + // Write or delete some data in the transaction + if let Some(value) = &storage_value { + tx::ctx().write(&storage_key, value).unwrap(); + } else { + tx::ctx().delete(&storage_key).unwrap(); + } + }); + + let vp_env = vp_host_env::take(); + let tx_data: Vec = vec![]; + let keys_changed: BTreeSet = + vp_env.all_touched_storage_keys(); + let verifiers: BTreeSet
= BTreeSet::default(); + vp_host_env::set(vp_env); + assert!(!validate_tx(&CTX, tx_data, vp_owner, keys_changed, verifiers).unwrap()); + } + } + + proptest! { + /// Test that a signed tx that performs arbitrary storage writes or + /// deletes to the account is accepted. + #[test] + fn test_signed_arb_storage_write( + (vp_owner, storage_key) in arb_account_storage_subspace_key(), + // Generate bytes to write. If `None`, delete from the key instead + storage_value in any::>>(), + ) { + // Initialize a tx environment + let mut tx_env = TestTxEnv::default(); + + let keypair = key::testing::keypair_1(); + let public_key = keypair.ref_to(); + + // Spawn all the accounts in the storage key to be able to modify + // their storage + let storage_key_addresses = storage_key.find_addresses(); + tx_env.spawn_accounts(storage_key_addresses); + + tx_env.write_public_key(&vp_owner, &public_key); + + // Initialize VP environment from a transaction + vp_host_env::init_from_tx(vp_owner.clone(), tx_env, |_address| { + // Write or delete some data in the transaction + if let Some(value) = &storage_value { + tx::ctx().write(&storage_key, value).unwrap(); + } else { + tx::ctx().delete(&storage_key).unwrap(); + } + }); + + let mut vp_env = vp_host_env::take(); + let tx = vp_env.tx.clone(); + let signed_tx = tx.sign(&keypair); + let tx_data: Vec = signed_tx.data.as_ref().cloned().unwrap(); + vp_env.tx = signed_tx; + let keys_changed: BTreeSet = + vp_env.all_touched_storage_keys(); + let verifiers: BTreeSet
= BTreeSet::default(); + vp_host_env::set(vp_env); + assert!(validate_tx(&CTX, tx_data, vp_owner, keys_changed, verifiers).unwrap()); + } + } + + /// Test that a validity predicate update without a valid signature is + /// rejected. + #[test] + fn test_unsigned_vp_update_rejected() { + // Initialize a tx environment + let mut tx_env = TestTxEnv::default(); + + let vp_owner = address::testing::established_address_1(); + let vp_code = + std::fs::read(VP_ALWAYS_TRUE_WASM).expect("cannot load wasm"); + + // Spawn the accounts to be able to modify their storage + tx_env.spawn_accounts([&vp_owner]); + + // Initialize VP environment from a transaction + vp_host_env::init_from_tx(vp_owner.clone(), tx_env, |address| { + // Update VP in a transaction + tx::ctx() + .update_validity_predicate(address, &vp_code) + .unwrap(); + }); + + let vp_env = vp_host_env::take(); + let tx_data: Vec = vec![]; + let keys_changed: BTreeSet = + vp_env.all_touched_storage_keys(); + let verifiers: BTreeSet
= BTreeSet::default(); + vp_host_env::set(vp_env); + assert!( + !validate_tx(&CTX, tx_data, vp_owner, keys_changed, verifiers) + .unwrap() + ); + } + + /// Test that a validity predicate update with a valid signature is + /// accepted. + #[test] + fn test_signed_vp_update_accepted() { + // Initialize a tx environment + let mut tx_env = TestTxEnv::default(); + tx_env.init_parameters(None, None, None); + + let vp_owner = address::testing::established_address_1(); + let keypair = key::testing::keypair_1(); + let public_key = keypair.ref_to(); + let vp_code = + std::fs::read(VP_ALWAYS_TRUE_WASM).expect("cannot load wasm"); + + // Spawn the accounts to be able to modify their storage + tx_env.spawn_accounts([&vp_owner]); + + tx_env.write_public_key(&vp_owner, &public_key); + + // Initialize VP environment from a transaction + vp_host_env::init_from_tx(vp_owner.clone(), tx_env, |address| { + // Update VP in a transaction + tx::ctx() + .update_validity_predicate(address, &vp_code) + .unwrap(); + }); + + let mut vp_env = vp_host_env::take(); + let tx = vp_env.tx.clone(); + let signed_tx = tx.sign(&keypair); + let tx_data: Vec = signed_tx.data.as_ref().cloned().unwrap(); + vp_env.tx = signed_tx; + let keys_changed: BTreeSet = + vp_env.all_touched_storage_keys(); + let verifiers: BTreeSet
= BTreeSet::default(); + vp_host_env::set(vp_env); + assert!( + validate_tx(&CTX, tx_data, vp_owner, keys_changed, verifiers) + .unwrap() + ); + } + + /// Test that a validity predicate update is rejected if not whitelisted + #[test] + fn test_signed_vp_update_not_whitelisted_rejected() { + // Initialize a tx environment + let mut tx_env = TestTxEnv::default(); + tx_env.init_parameters(None, Some(vec!["some_hash".to_string()]), None); + + let vp_owner = address::testing::established_address_1(); + let keypair = key::testing::keypair_1(); + let public_key = keypair.ref_to(); + let vp_code = + std::fs::read(VP_ALWAYS_TRUE_WASM).expect("cannot load wasm"); + + // Spawn the accounts to be able to modify their storage + tx_env.spawn_accounts([&vp_owner]); + + tx_env.write_public_key(&vp_owner, &public_key); + + // Initialize VP environment from a transaction + vp_host_env::init_from_tx(vp_owner.clone(), tx_env, |address| { + // Update VP in a transaction + tx::ctx() + .update_validity_predicate(address, &vp_code) + .unwrap(); + }); + + let mut vp_env = vp_host_env::take(); + let tx = vp_env.tx.clone(); + let signed_tx = tx.sign(&keypair); + let tx_data: Vec = signed_tx.data.as_ref().cloned().unwrap(); + vp_env.tx = signed_tx; + let keys_changed: BTreeSet = + vp_env.all_touched_storage_keys(); + let verifiers: BTreeSet
= BTreeSet::default(); + vp_host_env::set(vp_env); + assert!( + !validate_tx(&CTX, tx_data, vp_owner, keys_changed, verifiers) + .unwrap() + ); + } + + /// Test that a validity predicate update is accepted if whitelisted + #[test] + fn test_signed_vp_update_whitelisted_accepted() { + // Initialize a tx environment + let mut tx_env = TestTxEnv::default(); + + let vp_owner = address::testing::established_address_1(); + let keypair = key::testing::keypair_1(); + let public_key = keypair.ref_to(); + let vp_code = + std::fs::read(VP_ALWAYS_TRUE_WASM).expect("cannot load wasm"); + + let vp_hash = sha256(&vp_code); + tx_env.init_parameters(None, Some(vec![vp_hash.to_string()]), None); + + // Spawn the accounts to be able to modify their storage + tx_env.spawn_accounts([&vp_owner]); + + tx_env.write_public_key(&vp_owner, &public_key); + + // Initialize VP environment from a transaction + vp_host_env::init_from_tx(vp_owner.clone(), tx_env, |address| { + // Update VP in a transaction + tx::ctx() + .update_validity_predicate(address, &vp_code) + .unwrap(); + }); + + let mut vp_env = vp_host_env::take(); + let tx = vp_env.tx.clone(); + let signed_tx = tx.sign(&keypair); + let tx_data: Vec = signed_tx.data.as_ref().cloned().unwrap(); + vp_env.tx = signed_tx; + let keys_changed: BTreeSet = + vp_env.all_touched_storage_keys(); + let verifiers: BTreeSet
= BTreeSet::default(); + vp_host_env::set(vp_env); + assert!( + validate_tx(&CTX, tx_data, vp_owner, keys_changed, verifiers) + .unwrap() + ); + } + + /// Test that a tx is rejected if not whitelisted + #[test] + fn test_tx_not_whitelisted_rejected() { + // Initialize a tx environment + let mut tx_env = TestTxEnv::default(); + + let vp_owner = address::testing::established_address_1(); + let keypair = key::testing::keypair_1(); + let public_key = keypair.ref_to(); + let vp_code = + std::fs::read(VP_ALWAYS_TRUE_WASM).expect("cannot load wasm"); + + let vp_hash = sha256(&vp_code); + tx_env.init_parameters( + None, + Some(vec![vp_hash.to_string()]), + Some(vec!["some_hash".to_string()]), + ); + + // Spawn the accounts to be able to modify their storage + tx_env.spawn_accounts([&vp_owner]); + + tx_env.write_public_key(&vp_owner, &public_key); + + // Initialize VP environment from a transaction + vp_host_env::init_from_tx(vp_owner.clone(), tx_env, |address| { + // Update VP in a transaction + tx::ctx() + .update_validity_predicate(address, &vp_code) + .unwrap(); + }); + + let mut vp_env = vp_host_env::take(); + let tx = vp_env.tx.clone(); + let signed_tx = tx.sign(&keypair); + let tx_data: Vec = signed_tx.data.as_ref().cloned().unwrap(); + vp_env.tx = signed_tx; + let keys_changed: BTreeSet = + vp_env.all_touched_storage_keys(); + let verifiers: BTreeSet
= BTreeSet::default(); + vp_host_env::set(vp_env); + assert!( + !validate_tx(&CTX, tx_data, vp_owner, keys_changed, verifiers) + .unwrap() + ); + } + + #[test] + fn test_tx_whitelisted_accepted() { + // Initialize a tx environment + let mut tx_env = TestTxEnv::default(); + + let vp_owner = address::testing::established_address_1(); + let keypair = key::testing::keypair_1(); + let public_key = keypair.ref_to(); + let vp_code = + std::fs::read(VP_ALWAYS_TRUE_WASM).expect("cannot load wasm"); + + // hardcoded hash of VP_ALWAYS_TRUE_WASM + tx_env.init_parameters(None, None, Some(vec!["E3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855".to_string()])); + + // Spawn the accounts to be able to modify their storage + tx_env.spawn_accounts([&vp_owner]); + + tx_env.write_public_key(&vp_owner, &public_key); + + // Initialize VP environment from a transaction + vp_host_env::init_from_tx(vp_owner.clone(), tx_env, |address| { + // Update VP in a transaction + tx::ctx() + .update_validity_predicate(address, &vp_code) + .unwrap(); + }); + + let mut vp_env = vp_host_env::take(); + let tx = vp_env.tx.clone(); + let signed_tx = tx.sign(&keypair); + let tx_data: Vec = signed_tx.data.as_ref().cloned().unwrap(); + vp_env.tx = signed_tx; + let keys_changed: BTreeSet = + vp_env.all_touched_storage_keys(); + let verifiers: BTreeSet
= BTreeSet::default(); + vp_host_env::set(vp_env); + assert!( + validate_tx(&CTX, tx_data, vp_owner, keys_changed, verifiers) + .unwrap() + ); + } +} From 9068938684212ec42f9e26341547c517a3e5c352 Mon Sep 17 00:00:00 2001 From: brentstone Date: Tue, 8 Nov 2022 23:55:04 -0500 Subject: [PATCH 32/38] add max commission rate info to validation and pos state machine test --- proof_of_stake/src/validation.rs | 65 +++++++++++++++++++++++++++++--- shared/src/ledger/pos/vp.rs | 21 +++++++++-- tests/src/native_vp/pos.rs | 50 ++++++++++++++++++++---- 3 files changed, 120 insertions(+), 16 deletions(-) diff --git a/proof_of_stake/src/validation.rs b/proof_of_stake/src/validation.rs index 61374fca67..43858fed86 100644 --- a/proof_of_stake/src/validation.rs +++ b/proof_of_stake/src/validation.rs @@ -179,6 +179,16 @@ where NewValidatorMissingInValidatorSet(Address), #[error("Validator set has not been updated for new validators.")] MissingValidatorSetUpdate, + #[error( + "Changing the maximum commission rate change per epoch for validator \ + {0} is forbidden." + )] + ValidatorMaxCommissionRateChangeForbidden(Address), + #[error( + "Invalid value of maximum commission rate change per epoch for \ + validator {0}, got {1}." + )] + InvalidMaxCommissionRateChange(Address, Decimal), } /// An update of PoS data. @@ -289,7 +299,9 @@ where /// Voting power update VotingPowerUpdate(Data), /// Commission rate update - CommissionRate(Data, Decimal), + CommissionRate(Data, Option), + /// Maximum commission rate change update + MaxCommissionRateChange(Data), } /// Data update with prior and posterior state. @@ -317,6 +329,7 @@ pub struct NewValidator { has_address_raw_hash: Option, voting_power: VotingPower, has_commission_rate: bool, + has_max_commission_rate_change: bool, } /// Validation constants @@ -810,12 +823,14 @@ where has_address_raw_hash, voting_power, has_commission_rate, + has_max_commission_rate_change, } = &new_validator; // The new validator must have set all the required fields if !(*has_state && *has_total_deltas && *has_voting_power - && *has_commission_rate) + && *has_commission_rate + && *has_max_commission_rate_change) { errors.push(Error::InvalidNewValidator( address.clone(), @@ -1159,6 +1174,14 @@ where max_change, ) } + MaxCommissionRateChange(data) => { + Self::validator_max_commission_rate_change( + errors, + new_validators, + address, + data, + ) + } }, Balance(data) => Self::balance(errors, balance_delta, data), Bond { id, data, slashes } => { @@ -1502,14 +1525,14 @@ where new_validators: &mut HashMap>, address: Address, data: Data, - max_change: Decimal, + max_change: Option, ) { match (data.pre, data.post) { (None, Some(post)) => { if post.last_update() != constants.current_epoch { errors.push(Error::InvalidLastUpdate) } - // The value must be known at pipeline epoch + // The value must be known at the pipeline epoch match post.get(constants.pipeline_epoch) { Some(_) => { let validator = @@ -1525,7 +1548,11 @@ where if post.last_update() != constants.current_epoch { errors.push(Error::InvalidLastUpdate) } - // Before pipeline epoch, the commission rate must not change + if max_change.is_none() { + errors.push(Error::InvalidLastUpdate) + } + // Before the pipeline epoch, the commission rate must not + // change for epoch in Epoch::iter_range( constants.current_epoch, constants.pipeline_offset, @@ -1550,7 +1577,9 @@ where post.get(constants.pipeline_epoch), ) { (Some(prev_rate), Some(new_rate)) => { - if (new_rate - prev_rate).abs() > max_change { + if (new_rate - prev_rate).abs() + > max_change.unwrap_or_default() + { errors.push( Error::InvalidValidatorCommissionRateUpdate( constants.pipeline_epoch.into(), @@ -1570,6 +1599,30 @@ where } } + fn validator_max_commission_rate_change( + errors: &mut Vec>, + new_validators: &mut HashMap>, + address: Address, + data: Data, + ) { + match (data.pre, data.post) { + (None, Some(post)) => { + if post < Decimal::ZERO || post > Decimal::ONE { + errors.push(Error::InvalidMaxCommissionRateChange( + address.clone(), + post, + )) + } + + let validator = new_validators.entry(address).or_default(); + validator.has_max_commission_rate_change = true; + } + _ => errors.push(Error::ValidatorMaxCommissionRateChangeForbidden( + address, + )), + } + } + #[allow(clippy::too_many_arguments)] fn validator_voting_power( params: &PosParams, diff --git a/shared/src/ledger/pos/vp.rs b/shared/src/ledger/pos/vp.rs index dcebda6a3c..8dd7a64179 100644 --- a/shared/src/ledger/pos/vp.rs +++ b/shared/src/ledger/pos/vp.rs @@ -34,7 +34,8 @@ use crate::ledger::native_vp::{ }; use crate::ledger::pos::{ is_validator_address_raw_hash_key, is_validator_commission_rate_key, - is_validator_consensus_key_key, is_validator_state_key, + is_validator_consensus_key_key, + is_validator_max_commission_rate_change_key, is_validator_state_key, }; use crate::ledger::storage::{self as ledger_storage, StorageHasher}; use crate::ledger::storage_api::{self, StorageRead}; @@ -270,8 +271,7 @@ where .read_bytes(&validator_max_commission_rate_change_key( address, ))? - .and_then(|bytes| Decimal::try_from_slice(&bytes[..]).ok()) - .unwrap(); + .and_then(|bytes| Decimal::try_from_slice(&bytes[..]).ok()); let pre = self.ctx.pre().read_bytes(key)?.and_then(|bytes| { CommissionRates::try_from_slice(&bytes[..]).ok() }); @@ -282,6 +282,21 @@ where address: address.clone(), update: CommissionRate(Data { pre, post }, max_change), }); + } else if let Some(address) = + is_validator_max_commission_rate_change_key(key) + { + let pre = + self.ctx.pre().read_bytes(key)?.and_then(|bytes| { + Decimal::try_from_slice(&bytes[..]).ok() + }); + let post = + self.ctx.post().read_bytes(key)?.and_then(|bytes| { + Decimal::try_from_slice(&bytes[..]).ok() + }); + changes.push(Validator { + address: address.clone(), + update: MaxCommissionRateChange(Data { pre, post }), + }); } else if key.segments.get(0) == Some(&addr.to_db_key()) { // Unknown changes to this address space are disallowed tracing::info!("PoS unrecognized key change {} rejected", key); diff --git a/tests/src/native_vp/pos.rs b/tests/src/native_vp/pos.rs index 753d716736..9085f0c110 100644 --- a/tests/src/native_vp/pos.rs +++ b/tests/src/native_vp/pos.rs @@ -401,7 +401,10 @@ mod tests { address, consensus_key, commission_rate: _, + max_commission_rate_change: _, } => { + // TODO: should there be preconditions for commission + // rates here? !state.is_validator(address) && !state.is_used_key(consensus_key) } @@ -615,6 +618,7 @@ pub mod testing { address: Address, consensus_key: PublicKey, commission_rate: Decimal, + max_commission_rate_change: Decimal, }, Bond { amount: token::Amount, @@ -708,6 +712,10 @@ pub mod testing { address: Address, rate: Decimal, }, + ValidatorMaxCommissionRateChange { + address: Address, + change: Decimal, + }, } pub fn arb_valid_pos_action( @@ -726,14 +734,23 @@ pub mod testing { address::testing::arb_established_address(), key::testing::arb_common_keypair(), arb_rate(), + arb_rate(), ) - .prop_map(|(addr, consensus_key, commission_rate)| { - ValidPosAction::InitValidator { - address: Address::Established(addr), - consensus_key: consensus_key.ref_to(), + .prop_map( + |( + addr, + consensus_key, commission_rate, - } - }); + max_commission_rate_change, + )| { + ValidPosAction::InitValidator { + address: Address::Established(addr), + consensus_key: consensus_key.ref_to(), + commission_rate, + max_commission_rate_change, + } + }, + ); if validators.is_empty() { // When there is no validator, we can only initialize new ones @@ -880,6 +897,7 @@ pub mod testing { address, consensus_key, commission_rate, + max_commission_rate_change, } => { let offset = DynEpochOffset::PipelineLen; vec![ @@ -918,9 +936,13 @@ pub mod testing { offset: Either::Left(offset), }, PosStorageChange::ValidatorCommissionRate { - address, + address: address.clone(), rate: commission_rate, }, + PosStorageChange::ValidatorMaxCommissionRateChange { + address, + change: max_commission_rate_change, + }, ] } ValidPosAction::Bond { @@ -1517,6 +1539,20 @@ pub mod testing { .write_validator_commission_rate(&address, rates) .unwrap(); } + PosStorageChange::ValidatorMaxCommissionRateChange { + address, + change, + } => { + let max_change = tx::ctx() + .read_validator_max_commission_rate_change(&address) + .unwrap() + .unwrap_or(change); + tx::ctx() + .write_validator_max_commission_rate_change( + &address, max_change, + ) + .unwrap(); + } } } From 9b175c4c2af46da85188387a887c22295e0fd7f1 Mon Sep 17 00:00:00 2001 From: brentstone Date: Tue, 8 Nov 2022 23:55:43 -0500 Subject: [PATCH 33/38] fix: critical flaw in pos VP that was prematurely returning true --- shared/src/ledger/pos/vp.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/shared/src/ledger/pos/vp.rs b/shared/src/ledger/pos/vp.rs index 8dd7a64179..41b08ebd9b 100644 --- a/shared/src/ledger/pos/vp.rs +++ b/shared/src/ledger/pos/vp.rs @@ -303,7 +303,6 @@ where return Ok(false); } else { // Unknown changes anywhere else are permitted - return Ok(true); } } From fea59bd83799dd1a000c6fb795b5d78ee36a3714 Mon Sep 17 00:00:00 2001 From: brentstone Date: Tue, 8 Nov 2022 22:40:16 -0500 Subject: [PATCH 34/38] add vp validator to wasms --- genesis/dev.toml | 7 ++++++- genesis/e2e-tests-single-node.toml | 7 ++++++- wasm/wasm_source/Makefile | 1 + 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/genesis/dev.toml b/genesis/dev.toml index 2baa503dfe..15f1283ef6 100644 --- a/genesis/dev.toml +++ b/genesis/dev.toml @@ -14,7 +14,7 @@ tokens = 200000 # Amount of the validator's genesis token balance which is not staked. non_staked_balance = 100000 # VP for the validator account -validator_vp = "vp_user" +validator_vp = "vp_validator" # Commission rate for rewards commission_rate = 0.05 # Maximum change per epoch in the commission rate @@ -127,6 +127,11 @@ filename = "vp_user.wasm" # SHA-256 hash of the wasm file sha256 = "dc7b97f0448f2369bd2401c3c1d8898f53cac8c464a8c1b1f7f81415a658625d" +# Default validator VP +[wasm.vp_validator] +# filename (relative to wasm path used by the node) +filename = "vp_validator.wasm" + # Token VP [wasm.vp_token] filename = "vp_token.wasm" diff --git a/genesis/e2e-tests-single-node.toml b/genesis/e2e-tests-single-node.toml index 88634561c3..b2cd0d6892 100644 --- a/genesis/e2e-tests-single-node.toml +++ b/genesis/e2e-tests-single-node.toml @@ -10,7 +10,7 @@ tokens = 200000 # Amount of the validator's genesis token balance which is not staked. non_staked_balance = 1000000000000 # VP for the validator account -validator_vp = "vp_user" +validator_vp = "vp_validator" # Commission rate for rewards commission_rate = 0.05 # Maximum change per epoch in the commission rate @@ -122,6 +122,11 @@ filename = "vp_user.wasm" # SHA-256 hash of the wasm file sha256 = "dc7b97f0448f2369bd2401c3c1d8898f53cac8c464a8c1b1f7f81415a658625d" +# Default validator VP +[wasm.vp_validator] +# filename (relative to wasm path used by the node) +filename = "vp_validator.wasm" + # Token VP [wasm.vp_token] filename = "vp_token.wasm" diff --git a/wasm/wasm_source/Makefile b/wasm/wasm_source/Makefile index e525d77d55..b7842bd9f0 100644 --- a/wasm/wasm_source/Makefile +++ b/wasm/wasm_source/Makefile @@ -19,6 +19,7 @@ wasms += tx_change_validator_commission wasms += vp_testnet_faucet wasms += vp_token wasms += vp_user +wasms += vp_validator # Build all wasms in release mode all: $(wasms) From 757ba1d348e701ca4d2fbec959128d059cc9f5f3 Mon Sep 17 00:00:00 2001 From: brentstone Date: Wed, 9 Nov 2022 16:17:21 -0500 Subject: [PATCH 35/38] async tx to change validator commission rate --- apps/src/lib/cli.rs | 36 ++++++++++++++++ apps/src/lib/client/tx.rs | 87 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 122 insertions(+), 1 deletion(-) diff --git a/apps/src/lib/cli.rs b/apps/src/lib/cli.rs index 499e21d432..733f1e87d9 100644 --- a/apps/src/lib/cli.rs +++ b/apps/src/lib/cli.rs @@ -2095,6 +2095,42 @@ pub mod args { } } + #[derive(Clone, Debug)] + /// Commission rate change args + pub struct TxCommissionRateChange { + /// Common tx arguments + pub tx: Tx, + /// Validator address (should be self) + pub validator: WalletAddress, + /// Value to which the tx changes the commission rate + pub rate: Decimal, + } + + impl Args for TxCommissionRateChange { + fn parse(matches: &ArgMatches) -> Self { + let tx = Tx::parse(matches); + let validator = VALIDATOR.parse(matches); + let rate = COMMISSION_RATE.parse(matches); + Self { + tx, + validator, + rate, + } + } + + fn def(app: App) -> App { + app.add_args::() + .arg(VALIDATOR.def().about( + "The validator's address whose commission rate to change.", + )) + .arg( + COMMISSION_RATE + .def() + .about("The desired new commission rate."), + ) + } + } + /// Query PoS commission rate #[derive(Clone, Debug)] pub struct QueryCommissionRate { diff --git a/apps/src/lib/client/tx.rs b/apps/src/lib/client/tx.rs index 6a07204695..8526e8f5e2 100644 --- a/apps/src/lib/client/tx.rs +++ b/apps/src/lib/client/tx.rs @@ -9,7 +9,7 @@ use async_std::io::{self}; use borsh::BorshSerialize; use itertools::Either::*; use namada::ledger::governance::storage as gov_storage; -use namada::ledger::pos::{BondId, Bonds, Unbonds}; +use namada::ledger::pos::{BondId, Bonds, CommissionRates, Unbonds}; use namada::proto::Tx; use namada::types::address::{nam, Address}; use namada::types::governance::{ @@ -52,6 +52,7 @@ const VP_USER_WASM: &str = "vp_user.wasm"; const TX_BOND_WASM: &str = "tx_bond.wasm"; const TX_UNBOND_WASM: &str = "tx_unbond.wasm"; const TX_WITHDRAW_WASM: &str = "tx_withdraw.wasm"; +const TX_CHANGE_COMMISSION_WASM: &str = "tx_change_validator_commission.wasm"; const ENV_VAR_ANOMA_TENDERMINT_WEBSOCKET_TIMEOUT: &str = "ANOMA_TENDERMINT_WEBSOCKET_TIMEOUT"; @@ -975,6 +976,90 @@ pub async fn submit_withdraw(ctx: Context, args: args::Withdraw) { process_tx(ctx, &args.tx, tx, Some(default_signer)).await; } +pub async fn submit_validator_commission_change( + ctx: Context, + args: args::TxCommissionRateChange, +) { + let epoch = rpc::query_epoch(args::Query { + ledger_address: args.tx.ledger_address.clone(), + }) + .await; + + let tx_code = ctx.read_wasm(TX_CHANGE_COMMISSION_WASM); + let client = HttpClient::new(args.tx.ledger_address.clone()).unwrap(); + + // Check that the submitter of the tx is a validator + match ctx.wallet.get_validator_data() { + Some(data) => { + if args.rate < Decimal::ZERO || args.rate > Decimal::ONE { + eprintln!( + "Invalid new commission rate, received {}", + args.rate + ); + if !args.tx.force { + safe_exit(1) + } + } + + let commission_rate_key = + ledger::pos::validator_commission_rate_key(&data.address); + let max_commission_rate_change_key = + ledger::pos::validator_max_commission_rate_change_key( + &data.address, + ); + let commission_rates = rpc::query_storage_value::( + &client, + &commission_rate_key, + ) + .await; + let max_change = rpc::query_storage_value::( + &client, + &max_commission_rate_change_key, + ) + .await; + + match (commission_rates, max_change) { + (Some(rates), Some(max_change)) => { + // Assuming that pipeline length = 2 + let rate_next_epoch = rates.get(epoch + 1).unwrap(); + if (args.rate - rate_next_epoch).abs() > max_change { + eprintln!( + "New rate is too large of a change with respect \ + to the predecessor epoch in which the rate will \ + take effect." + ); + if !args.tx.force { + safe_exit(1) + } + } + } + _ => { + eprintln!("Error retrieving from storage"); + if !args.tx.force { + safe_exit(1) + } + } + } + } + None => { + eprintln!("Cannot change the commission rate of the validator"); + if !args.tx.force { + safe_exit(1) + } + } + } + + let data = pos::CommissionChange { + validator: ctx.get(&args.validator), + new_rate: args.rate, + }; + let data = data.try_to_vec().expect("Encoding tx data shouldn't fail"); + + let tx = Tx::new(tx_code, Some(data)); + let default_signer = &args.validator; + process_tx(ctx, &args.tx, tx, Some(default_signer)).await; +} + /// Submit transaction and wait for result. Returns a list of addresses /// initialized in the transaction if any. In dry run, this is always empty. async fn process_tx( From 42b0bfaedc5b7baf2a192cf1e7e884778c87438a Mon Sep 17 00:00:00 2001 From: brentstone Date: Thu, 10 Nov 2022 13:15:48 -0500 Subject: [PATCH 36/38] addressing 2nd round of review comments --- apps/src/lib/client/tx.rs | 90 +++++++++++++--------------- tests/src/native_vp/pos.rs | 2 - wasm/wasm_source/src/vp_validator.rs | 3 +- 3 files changed, 43 insertions(+), 52 deletions(-) diff --git a/apps/src/lib/client/tx.rs b/apps/src/lib/client/tx.rs index 8526e8f5e2..56c53ecd5c 100644 --- a/apps/src/lib/client/tx.rs +++ b/apps/src/lib/client/tx.rs @@ -988,65 +988,57 @@ pub async fn submit_validator_commission_change( let tx_code = ctx.read_wasm(TX_CHANGE_COMMISSION_WASM); let client = HttpClient::new(args.tx.ledger_address.clone()).unwrap(); - // Check that the submitter of the tx is a validator - match ctx.wallet.get_validator_data() { - Some(data) => { - if args.rate < Decimal::ZERO || args.rate > Decimal::ONE { - eprintln!( - "Invalid new commission rate, received {}", - args.rate - ); - if !args.tx.force { - safe_exit(1) - } + let validator = ctx.get(&args.validator); + if rpc::is_validator(&validator, args.tx.ledger_address.clone()).await { + if args.rate < Decimal::ZERO || args.rate > Decimal::ONE { + eprintln!("Invalid new commission rate, received {}", args.rate); + if !args.tx.force { + safe_exit(1) } + } - let commission_rate_key = - ledger::pos::validator_commission_rate_key(&data.address); - let max_commission_rate_change_key = - ledger::pos::validator_max_commission_rate_change_key( - &data.address, - ); - let commission_rates = rpc::query_storage_value::( - &client, - &commission_rate_key, - ) - .await; - let max_change = rpc::query_storage_value::( - &client, - &max_commission_rate_change_key, - ) - .await; + let commission_rate_key = + ledger::pos::validator_commission_rate_key(&validator); + let max_commission_rate_change_key = + ledger::pos::validator_max_commission_rate_change_key(&validator); + let commission_rates = rpc::query_storage_value::( + &client, + &commission_rate_key, + ) + .await; + let max_change = rpc::query_storage_value::( + &client, + &max_commission_rate_change_key, + ) + .await; - match (commission_rates, max_change) { - (Some(rates), Some(max_change)) => { - // Assuming that pipeline length = 2 - let rate_next_epoch = rates.get(epoch + 1).unwrap(); - if (args.rate - rate_next_epoch).abs() > max_change { - eprintln!( - "New rate is too large of a change with respect \ - to the predecessor epoch in which the rate will \ - take effect." - ); - if !args.tx.force { - safe_exit(1) - } - } - } - _ => { - eprintln!("Error retrieving from storage"); + match (commission_rates, max_change) { + (Some(rates), Some(max_change)) => { + // Assuming that pipeline length = 2 + let rate_next_epoch = rates.get(epoch + 1).unwrap(); + if (args.rate - rate_next_epoch).abs() > max_change { + eprintln!( + "New rate is too large of a change with respect to \ + the predecessor epoch in which the rate will take \ + effect." + ); if !args.tx.force { safe_exit(1) } } } - } - None => { - eprintln!("Cannot change the commission rate of the validator"); - if !args.tx.force { - safe_exit(1) + _ => { + eprintln!("Error retrieving from storage"); + if !args.tx.force { + safe_exit(1) + } } } + } else { + eprintln!("The given address {validator} is not a validator."); + if !args.tx.force { + safe_exit(1) + } } let data = pos::CommissionChange { diff --git a/tests/src/native_vp/pos.rs b/tests/src/native_vp/pos.rs index 9085f0c110..ef7fcf937c 100644 --- a/tests/src/native_vp/pos.rs +++ b/tests/src/native_vp/pos.rs @@ -403,8 +403,6 @@ mod tests { commission_rate: _, max_commission_rate_change: _, } => { - // TODO: should there be preconditions for commission - // rates here? !state.is_validator(address) && !state.is_used_key(consensus_key) } diff --git a/wasm/wasm_source/src/vp_validator.rs b/wasm/wasm_source/src/vp_validator.rs index 798cb12241..bdf41d792f 100644 --- a/wasm/wasm_source/src/vp_validator.rs +++ b/wasm/wasm_source/src/vp_validator.rs @@ -137,8 +137,9 @@ fn validate_tx( }; let comm = proof_of_stake::is_validator_commission_rate_key(key); + // Validator's commission rate change must be signed let valid_commission_rate_change = match comm { - Some(source) => *source == addr && *valid_sig, + Some(source) => *source != addr || *valid_sig, None => true, }; let valid = From 476877e96b3cdcadec603dd02aeccca0f854e66a Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 10 Nov 2022 18:43:55 +0000 Subject: [PATCH 37/38] [ci] wasm checksums update --- wasm/checksums.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/wasm/checksums.json b/wasm/checksums.json index 7e1d1f8195..43bea93213 100644 --- a/wasm/checksums.json +++ b/wasm/checksums.json @@ -12,5 +12,6 @@ "tx_withdraw.wasm": "tx_withdraw.01b4185d41565fc59f8976999faa738ad4e5388e30ee279078c4ff059b084e05.wasm", "vp_testnet_faucet.wasm": "vp_testnet_faucet.3938c15abd8b2cfad97c87d48d1a84817a7aaf597d4c868356a4dbb7e2517a4b.wasm", "vp_token.wasm": "vp_token.800e7f696b7f41c970e2464316ed09be8d3d63bd58c32e8e3434b203b56d9015.wasm", - "vp_user.wasm": "vp_user.b23ce44da223c501d4a5bb891cba4a033bba3cf6d877a1268ced3707e0e71bd0.wasm" + "vp_user.wasm": "vp_user.b23ce44da223c501d4a5bb891cba4a033bba3cf6d877a1268ced3707e0e71bd0.wasm", + "vp_validator.wasm": "vp_validator.9410b90c08504de60f124b4eb72ff486046c1966f5798826bca5c5fbd2398633.wasm" } \ No newline at end of file From 6b92f2cee3d29de57670f95518312b7b8a50842c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Thu, 10 Nov 2022 19:49:06 +0100 Subject: [PATCH 38/38] changelog: add #695 --- .../unreleased/features/695-validator-commission-rates.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 .changelog/unreleased/features/695-validator-commission-rates.md diff --git a/.changelog/unreleased/features/695-validator-commission-rates.md b/.changelog/unreleased/features/695-validator-commission-rates.md new file mode 100644 index 0000000000..086227b595 --- /dev/null +++ b/.changelog/unreleased/features/695-validator-commission-rates.md @@ -0,0 +1,4 @@ +- Allow to set validator's commission rates and a limit on change of commission + rate per epoch. Commission rate can be changed via a transaction authorized + by the validator, but the limit is immutable value, set when the validator's + account is initialized. ([#695](https://github.com/anoma/namada/pull/695)) \ No newline at end of file