From 8a18d9095d10ca35128fe7982e1d7084375264f5 Mon Sep 17 00:00:00 2001 From: brentstone Date: Tue, 5 Mar 2024 11:30:47 +0100 Subject: [PATCH 01/13] track all validators to which an account has bonded tokens --- crates/proof_of_stake/src/lib.rs | 34 ++++++++++++++----- crates/proof_of_stake/src/queries.rs | 42 ++++++++++++++---------- crates/proof_of_stake/src/storage.rs | 8 ++++- crates/proof_of_stake/src/storage_key.rs | 15 +++++++++ crates/proof_of_stake/src/types/mod.rs | 7 ++++ 5 files changed, 78 insertions(+), 28 deletions(-) diff --git a/crates/proof_of_stake/src/lib.rs b/crates/proof_of_stake/src/lib.rs index aa28e63e35..a3a56c2547 100644 --- a/crates/proof_of_stake/src/lib.rs +++ b/crates/proof_of_stake/src/lib.rs @@ -52,15 +52,15 @@ use crate::slashing::{ }; use crate::storage::{ below_capacity_validator_set_handle, bond_handle, - consensus_validator_set_handle, delegator_redelegated_bonds_handle, - delegator_redelegated_unbonds_handle, get_last_reward_claim_epoch, - liveness_missed_votes_handle, liveness_sum_missed_votes_handle, - read_consensus_validator_set_addresses, read_non_pos_owned_params, - read_pos_params, read_validator_last_slash_epoch, - read_validator_max_commission_rate_change, read_validator_stake, - total_bonded_handle, total_consensus_stake_handle, total_unbonded_handle, - try_insert_consensus_key, unbond_handle, update_total_deltas, - update_validator_deltas, validator_addresses_handle, + consensus_validator_set_handle, delegation_targets_handle, + delegator_redelegated_bonds_handle, delegator_redelegated_unbonds_handle, + get_last_reward_claim_epoch, liveness_missed_votes_handle, + liveness_sum_missed_votes_handle, read_consensus_validator_set_addresses, + read_non_pos_owned_params, read_pos_params, + read_validator_last_slash_epoch, read_validator_max_commission_rate_change, + read_validator_stake, total_bonded_handle, total_consensus_stake_handle, + total_unbonded_handle, try_insert_consensus_key, unbond_handle, + update_total_deltas, update_validator_deltas, validator_addresses_handle, validator_commission_rate_handle, validator_consensus_key_handle, validator_deltas_handle, validator_eth_cold_key_handle, validator_eth_hot_key_handle, validator_incoming_redelegations_handle, @@ -272,6 +272,11 @@ where tracing::debug!("\nBonds after incrementing: {bonds:#?}"); } + // Add the validator to the delegation targets + let target_validators = + delegation_targets_handle(source).at(&(current_epoch + offset)); + target_validators.insert(storage, validator.clone())?; + // Update the validator set // Allow bonding even if the validator is jailed. However, if jailed, there // must be no changes to the validator set. Check at the pipeline epoch. @@ -516,6 +521,17 @@ where bonds_handle.set(storage, new_bond_amount, bond_epoch, 0)?; } + // If the bond is now completely empty, remove the validator from the + // delegation targets + let bonds_total = bonds_handle + .get_sum(storage, pipeline_epoch, ¶ms)? + .unwrap_or_default(); + if bonds_total.is_zero() { + delegation_targets_handle(source) + .at(&pipeline_epoch) + .remove(storage, validator)?; + } + // `updatedUnbonded` // Update the unbonds in storage using the eager map computed above if !is_redelegation { diff --git a/crates/proof_of_stake/src/queries.rs b/crates/proof_of_stake/src/queries.rs index fb3d2e0c7e..78789d15a5 100644 --- a/crates/proof_of_stake/src/queries.rs +++ b/crates/proof_of_stake/src/queries.rs @@ -13,7 +13,7 @@ use namada_storage::collections::lazy_map::{NestedSubKey, SubKey}; use namada_storage::StorageRead; use crate::slashing::{find_validator_slashes, get_slashed_amount}; -use crate::storage::{bond_handle, read_pos_params, unbond_handle}; +use crate::storage::{bond_handle, delegation_targets_handle, read_pos_params, unbond_handle}; use crate::types::{ BondDetails, BondId, BondsAndUnbondsDetail, BondsAndUnbondsDetails, Slash, UnbondDetails, @@ -59,26 +59,32 @@ pub fn find_delegations( where S: StorageRead, { - let bonds_prefix = storage_key::bonds_for_source_prefix(owner); + // let bonds_prefix = storage_key::bonds_for_source_prefix(owner); let params = read_pos_params(storage)?; let mut delegations: HashMap = HashMap::new(); - for iter_result in - namada_storage::iter_prefix_bytes(storage, &bonds_prefix)? - { - let (key, _bond_bytes) = iter_result?; - let validator_address = storage_key::get_validator_address_from_bond( - &key, - ) - .ok_or_else(|| { - namada_storage::Error::new_const( - "Delegation key should contain validator address.", - ) - })?; - let deltas_sum = bond_handle(owner, &validator_address) - .get_sum(storage, *epoch, ¶ms)? - .unwrap_or_default(); - delegations.insert(validator_address, deltas_sum); + // for iter_result in + // namada_storage::iter_prefix_bytes(storage, &bonds_prefix)? + // { + // let (key, _bond_bytes) = iter_result?; + // let validator_address = storage_key::get_validator_address_from_bond( + // &key, + // ) + // .ok_or_else(|| { + // namada_storage::Error::new_const( + // "Delegation key should contain validator address.", + // ) + // })?; + // let deltas_sum = bond_handle(owner, &validator_address) + // .get_sum(storage, *epoch, ¶ms)? + // .unwrap_or_default(); + // delegations.insert(validator_address, deltas_sum); + // } + let delegation_targets = delegation_targets_handle(owner).at(epoch); + for validator in delegation_targets.iter(storage)? { + let validator = validator?; + let stake = bond_handle(owner, &validator).get_sum(storage, *epoch, ¶ms)?.unwrap_or_default(); + delegations.insert(validator, stake); } Ok(delegations) } diff --git a/crates/proof_of_stake/src/storage.rs b/crates/proof_of_stake/src/storage.rs index 6bd8448d5f..d431d9a7e4 100644 --- a/crates/proof_of_stake/src/storage.rs +++ b/crates/proof_of_stake/src/storage.rs @@ -19,7 +19,7 @@ use num_traits::CheckedAdd; use crate::storage_key::consensus_keys_key; use crate::types::{ BelowCapacityValidatorSets, BondId, Bonds, CommissionRates, - ConsensusValidatorSets, DelegatorRedelegatedBonded, + ConsensusValidatorSets, DelegationTargets, DelegatorRedelegatedBonded, DelegatorRedelegatedUnbonded, EpochedSlashes, IncomingRedelegations, LivenessMissedVotes, LivenessSumMissedVotes, OutgoingRedelegations, ReverseOrdTokenAmount, RewardsAccumulator, RewardsProducts, Slashes, @@ -251,6 +251,12 @@ pub fn total_active_deltas_handle() -> TotalDeltas { TotalDeltas::open(key) } +/// Get the storage handle to the delegation targets map +pub fn delegation_targets_handle(delegator: &Address) -> DelegationTargets { + let key = storage_key::delegation_targets_key(delegator); + DelegationTargets::open(key) +} + // ---- Storage read + write ---- /// Read PoS parameters diff --git a/crates/proof_of_stake/src/storage_key.rs b/crates/proof_of_stake/src/storage_key.rs index cab2c2b8ac..f1b2f7c8e5 100644 --- a/crates/proof_of_stake/src/storage_key.rs +++ b/crates/proof_of_stake/src/storage_key.rs @@ -61,6 +61,7 @@ const LIVENESS_MISSED_VOTES_SUM: &str = "sum_missed_votes"; const LAST_STAKED_RATIO_KEY: &str = "last_staked_ratio"; const LAST_POS_INFLATION_AMOUNT_KEY: &str = "last_inflation_amount"; const TOTAL_ACTIVE_DELTAS_KEY: &str = "total_active_deltas"; +const DELEGATION_TARGETS_PREFIX: &str = "delegation_targets"; /// Is the given key a PoS storage key? pub fn is_pos_key(key: &Key) -> bool { @@ -1083,3 +1084,17 @@ pub fn is_total_active_deltas_key(key: &Key) -> bool { false } } + +/// Storage prefix for the delegation targets. +pub fn delegation_targets_prefix() -> Key { + Key::from(ADDRESS.to_db_key()) + .push(&DELEGATION_TARGETS_PREFIX.to_owned()) + .expect("Cannot obtain a storage key") +} + +/// Storage key for the delegation targets of a delegator. +pub fn delegation_targets_key(delegator: &Address) -> Key { + delegation_targets_prefix() + .push(&delegator.to_db_key()) + .expect("Cannot obtain a storage key") +} \ No newline at end of file diff --git a/crates/proof_of_stake/src/types/mod.rs b/crates/proof_of_stake/src/types/mod.rs index 8c6259e414..adb0b8b7a1 100644 --- a/crates/proof_of_stake/src/types/mod.rs +++ b/crates/proof_of_stake/src/types/mod.rs @@ -263,6 +263,13 @@ pub type LivenessMissedVotes = NestedMap>; /// elements in the corresponding inner LazySet of [`LivenessMissedVotes`]. pub type LivenessSumMissedVotes = LazyMap; +/// The set of all target validators for a given delegator. +pub type DelegationTargets = crate::epoched::NestedEpoched< + LazySet
, + crate::epoched::OffsetPipelineLen, + crate::epoched::OffsetMaxProposalPeriodPlus, +>; + #[derive( Debug, Clone, From 17b6916d06886062cfb68f4ecd4d5246741d748f Mon Sep 17 00:00:00 2001 From: brentstone Date: Tue, 5 Mar 2024 13:19:42 +0100 Subject: [PATCH 02/13] rename and fix rpc fns to get delegator's target validators --- crates/apps/src/lib/client/rpc.rs | 21 ++++++---- .../light_sdk/src/reading/asynchronous/pos.rs | 9 +++-- crates/light_sdk/src/reading/blocking/pos.rs | 11 +++-- crates/proof_of_stake/src/queries.rs | 40 ++++++++++--------- crates/sdk/src/queries/vp/pos.rs | 8 ++-- crates/sdk/src/rpc.rs | 17 +++++--- crates/sdk/src/tx.rs | 2 +- 7 files changed, 66 insertions(+), 42 deletions(-) diff --git a/crates/apps/src/lib/client/rpc.rs b/crates/apps/src/lib/client/rpc.rs index 307c12b516..2731116c56 100644 --- a/crates/apps/src/lib/client/rpc.rs +++ b/crates/apps/src/lib/client/rpc.rs @@ -2389,13 +2389,19 @@ pub async fn query_delegations( let delegations: HashSet
= unwrap_client_response::( RPC.vp() .pos() - .delegation_validators(context.client(), &owner) + .delegation_validators(context.client(), &owner, &None) .await, ); if delegations.is_empty() { - display_line!(context.io(), "No delegations found"); + display_line!( + context.io(), + "No delegations found active in the current epoch" + ); } else { - display_line!(context.io(), "Found delegations to:"); + display_line!( + context.io(), + "Found delegations in the current epoch to:" + ); for delegation in delegations { display_line!(context.io(), " {delegation}"); } @@ -2800,25 +2806,26 @@ async fn get_validator_stake( ) } -pub async fn get_delegators_delegation< +pub async fn get_delegation_validators< C: namada::ledger::queries::Client + Sync, >( client: &C, address: &Address, ) -> HashSet
{ - namada_sdk::rpc::get_delegators_delegation(client, address) + let epoch = namada_sdk::rpc::query_epoch(client).await.unwrap(); + namada_sdk::rpc::get_delegation_validators(client, address, epoch) .await .unwrap() } -pub async fn get_delegators_delegation_at< +pub async fn get_delegations_of_delegator_at< C: namada::ledger::queries::Client + Sync, >( client: &C, address: &Address, epoch: Epoch, ) -> HashMap { - namada_sdk::rpc::get_delegators_delegation_at(client, address, epoch) + namada_sdk::rpc::get_delegations_of_delegator_at(client, address, epoch) .await .unwrap() } diff --git a/crates/light_sdk/src/reading/asynchronous/pos.rs b/crates/light_sdk/src/reading/asynchronous/pos.rs index c2e67c3f0a..f2ca4e8710 100644 --- a/crates/light_sdk/src/reading/asynchronous/pos.rs +++ b/crates/light_sdk/src/reading/asynchronous/pos.rs @@ -153,7 +153,7 @@ pub async fn get_validator_state( } /// Get the delegator's delegation -pub async fn get_delegators_delegation( +pub async fn get_delegation_validators( tendermint_addr: &str, address: &Address, ) -> Result, Error> { @@ -162,11 +162,12 @@ pub async fn get_delegators_delegation( .map_err(|e| Error::Other(e.to_string()))?, ) .map_err(|e| Error::Other(e.to_string()))?; - rpc::get_delegators_delegation(&client, address).await + let epoch = rpc::query_epoch(&client).await?; + rpc::get_delegation_validators(&client, address, epoch).await } /// Get the delegator's delegation at some epoh -pub async fn get_delegators_delegation_at( +pub async fn get_delegations_of_delegator_at( tendermint_addr: &str, address: &Address, epoch: Epoch, @@ -176,7 +177,7 @@ pub async fn get_delegators_delegation_at( .map_err(|e| Error::Other(e.to_string()))?, ) .map_err(|e| Error::Other(e.to_string()))?; - rpc::get_delegators_delegation_at(&client, address, epoch).await + rpc::get_delegations_of_delegator_at(&client, address, epoch).await } /// Query and return validator's commission rate and max commission rate diff --git a/crates/light_sdk/src/reading/blocking/pos.rs b/crates/light_sdk/src/reading/blocking/pos.rs index 231adf5349..363035ccf0 100644 --- a/crates/light_sdk/src/reading/blocking/pos.rs +++ b/crates/light_sdk/src/reading/blocking/pos.rs @@ -165,7 +165,7 @@ pub fn get_validator_state( } /// Get the delegator's delegation -pub fn get_delegators_delegation( +pub fn get_delegation_validators( tendermint_addr: &str, address: &Address, ) -> Result, Error> { @@ -175,11 +175,12 @@ pub fn get_delegators_delegation( ) .map_err(|e| Error::Other(e.to_string()))?; let rt = Runtime::new().unwrap(); - rt.block_on(rpc::get_delegators_delegation(&client, address)) + let epoch = rpc::query_epoch(&client).await?; + rt.block_on(rpc::get_delegation_validators(&client, address, epoch)) } /// Get the delegator's delegation at some epoh -pub fn get_delegators_delegation_at( +pub fn get_delegations_of_delegator_at( tendermint_addr: &str, address: &Address, epoch: Epoch, @@ -190,7 +191,9 @@ pub fn get_delegators_delegation_at( ) .map_err(|e| Error::Other(e.to_string()))?; let rt = Runtime::new().unwrap(); - rt.block_on(rpc::get_delegators_delegation_at(&client, address, epoch)) + rt.block_on(rpc::get_delegations_of_delegator_at( + &client, address, epoch, + )) } /// Query and return validator's commission rate and max commission rate diff --git a/crates/proof_of_stake/src/queries.rs b/crates/proof_of_stake/src/queries.rs index 78789d15a5..7f0c924155 100644 --- a/crates/proof_of_stake/src/queries.rs +++ b/crates/proof_of_stake/src/queries.rs @@ -25,28 +25,32 @@ use crate::{storage_key, PosParams}; pub fn find_delegation_validators( storage: &S, owner: &Address, + epoch: &Epoch, ) -> namada_storage::Result> where S: StorageRead, { - let bonds_prefix = storage_key::bonds_for_source_prefix(owner); - let mut delegations: HashSet
= HashSet::new(); - - for iter_result in - namada_storage::iter_prefix_bytes(storage, &bonds_prefix)? - { - let (key, _bond_bytes) = iter_result?; - let validator_address = storage_key::get_validator_address_from_bond( - &key, - ) - .ok_or_else(|| { - namada_storage::Error::new_const( - "Delegation key should contain validator address.", - ) - })?; - delegations.insert(validator_address); - } - Ok(delegations) + // let bonds_prefix = storage_key::bonds_for_source_prefix(owner); + // let mut delegations: HashSet
= HashSet::new(); + + // for iter_result in + // namada_storage::iter_prefix_bytes(storage, &bonds_prefix)? + // { + // let (key, _bond_bytes) = iter_result?; + // let validator_address = storage_key::get_validator_address_from_bond( + // &key, + // ) + // .ok_or_else(|| { + // namada_storage::Error::new_const( + // "Delegation key should contain validator address.", + // ) + // })?; + // delegations.insert(validator_address); + // } + let delegation_targets = delegation_targets_handle(owner).at(epoch); + delegation_targets + .iter(storage)? + .collect::, _>>() } /// Find all validators to which a given bond `owner` (or source) has a diff --git a/crates/sdk/src/queries/vp/pos.rs b/crates/sdk/src/queries/vp/pos.rs index f7f79b40af..aa623a981e 100644 --- a/crates/sdk/src/queries/vp/pos.rs +++ b/crates/sdk/src/queries/vp/pos.rs @@ -84,7 +84,7 @@ router! {POS, ( "total_stake" / [epoch: opt Epoch] ) -> token::Amount = total_stake, - ( "delegations" / [owner: Address] ) + ( "delegations" / [owner: Address] / [epoch: opt Epoch] ) -> HashSet
= delegation_validators, ( "delegations_at" / [owner: Address] / [epoch: opt Epoch] ) @@ -571,12 +571,14 @@ where fn delegation_validators( ctx: RequestCtx<'_, D, H, V, T>, owner: Address, + epoch: Option, ) -> namada_storage::Result> where D: 'static + DB + for<'iter> DBIter<'iter> + Sync, H: 'static + StorageHasher + Sync, { - find_delegation_validators(ctx.state, &owner) + let epoch = epoch.unwrap_or(ctx.state.in_mem().last_epoch); + find_delegation_validators(ctx.state, &owner, &epoch) } /// Find all the validator addresses to whom the given `owner` address has @@ -590,7 +592,7 @@ where D: 'static + DB + for<'iter> DBIter<'iter> + Sync, H: 'static + StorageHasher + Sync, { - let epoch = epoch.unwrap_or(ctx.state.in_mem().last_epoch); + let epoch: Epoch = epoch.unwrap_or(ctx.state.in_mem().last_epoch); find_delegations(ctx.state, &owner, &epoch) } diff --git a/crates/sdk/src/rpc.rs b/crates/sdk/src/rpc.rs index f5ff2658a5..16238c8846 100644 --- a/crates/sdk/src/rpc.rs +++ b/crates/sdk/src/rpc.rs @@ -792,18 +792,25 @@ pub async fn get_validator_state( ) } -/// Get the delegator's delegation -pub async fn get_delegators_delegation( +/// Get the validators to which a delegator is bonded at a certain epoch +pub async fn get_delegation_validators( client: &C, address: &Address, + epoch: Epoch, ) -> Result, error::Error> { convert_response::( - RPC.vp().pos().delegation_validators(client, address).await, + RPC.vp() + .pos() + .delegation_validators(client, address, &Some(epoch)) + .await, ) } -/// Get the delegator's delegation at some epoch -pub async fn get_delegators_delegation_at( +/// Get the delegations of a delegator at some epoch, including the validator +/// and bond amount +pub async fn get_delegations_of_delegator_at< + C: crate::queries::Client + Sync, +>( client: &C, address: &Address, epoch: Epoch, diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 2faef0c99a..5edbda6d3e 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -2177,7 +2177,7 @@ pub async fn build_vote_proposal( } else { // Get active valid validators with whom the voter has delegations // (bonds) - let delegation_vals = rpc::get_delegators_delegation_at( + let delegation_vals = rpc::get_delegations_of_delegator_at( context.client(), voter_address, proposal.voting_start_epoch, From 14181abbe3b351ac4d0c7377eb3edec4c3932aed Mon Sep 17 00:00:00 2001 From: brentstone Date: Wed, 6 Mar 2024 12:07:38 +0100 Subject: [PATCH 03/13] improve nomenclature and comments --- .../lib/node/ledger/shell/finalize_block.rs | 2 +- crates/benches/native_vps.rs | 6 ++- crates/benches/txs.rs | 4 +- crates/benches/vps.rs | 7 +-- crates/governance/src/storage/keys.rs | 4 +- crates/governance/src/storage/mod.rs | 4 +- crates/governance/src/storage/proposal.rs | 6 +-- .../light_sdk/src/transaction/governance.rs | 2 +- crates/namada/src/ledger/governance/mod.rs | 47 +++++++------------ crates/proof_of_stake/src/lib.rs | 7 +-- crates/sdk/src/error.rs | 2 +- crates/sdk/src/signing.rs | 4 +- crates/sdk/src/tx.rs | 2 +- 13 files changed, 45 insertions(+), 52 deletions(-) diff --git a/crates/apps/src/lib/node/ledger/shell/finalize_block.rs b/crates/apps/src/lib/node/ledger/shell/finalize_block.rs index f2a61a809b..ae257cab45 100644 --- a/crates/apps/src/lib/node/ledger/shell/finalize_block.rs +++ b/crates/apps/src/lib/node/ledger/shell/finalize_block.rs @@ -1286,7 +1286,7 @@ mod test_finalize_block { id: proposal_id, vote, voter: validator, - delegations: vec![], + delegation_validators: vec![], }; // Vote to accept the proposal (there's only one validator, so its // vote decides) diff --git a/crates/benches/native_vps.rs b/crates/benches/native_vps.rs index 6092900eab..dfa2863c93 100644 --- a/crates/benches/native_vps.rs +++ b/crates/benches/native_vps.rs @@ -89,7 +89,9 @@ fn governance(c: &mut Criterion) { id: 0, vote: ProposalVote::Yay, voter: defaults::albert_address(), - delegations: vec![defaults::validator_address()], + delegation_validators: vec![ + defaults::validator_address(), + ], }, None, None, @@ -105,7 +107,7 @@ fn governance(c: &mut Criterion) { id: 0, vote: ProposalVote::Nay, voter: defaults::validator_address(), - delegations: vec![], + delegation_validators: vec![], }, None, None, diff --git a/crates/benches/txs.rs b/crates/benches/txs.rs index ad68d3e1ab..3eaa0339cc 100644 --- a/crates/benches/txs.rs +++ b/crates/benches/txs.rs @@ -547,7 +547,7 @@ fn vote_proposal(c: &mut Criterion) { id: 0, vote: ProposalVote::Yay, voter: defaults::albert_address(), - delegations: vec![defaults::validator_address()], + delegation_validators: vec![defaults::validator_address()], }, None, None, @@ -560,7 +560,7 @@ fn vote_proposal(c: &mut Criterion) { id: 0, vote: ProposalVote::Nay, voter: defaults::validator_address(), - delegations: vec![], + delegation_validators: vec![], }, None, None, diff --git a/crates/benches/vps.rs b/crates/benches/vps.rs index 1faf6ba5c9..8d12d0393b 100644 --- a/crates/benches/vps.rs +++ b/crates/benches/vps.rs @@ -94,8 +94,9 @@ fn vp_implicit(c: &mut Criterion) { id: 0, vote: ProposalVote::Yay, voter: Address::from(&implicit_account.to_public()), - delegations: vec![], /* NOTE: no need to bond tokens because the - * implicit vp doesn't check that */ + delegation_validators: vec![], /* NOTE: no need to bond tokens + * because the + * implicit vp doesn't check that */ }, None, None, @@ -251,7 +252,7 @@ fn vp_user(c: &mut Criterion) { id: 0, vote: ProposalVote::Yay, voter: defaults::validator_address(), - delegations: vec![], + delegation_validators: vec![], }, None, None, diff --git a/crates/governance/src/storage/keys.rs b/crates/governance/src/storage/keys.rs index 72cc36917a..995e1fffaa 100644 --- a/crates/governance/src/storage/keys.rs +++ b/crates/governance/src/storage/keys.rs @@ -442,10 +442,10 @@ pub fn get_proposal_vote_prefix_key(id: u64) -> Key { pub fn get_vote_proposal_key( id: u64, voter_address: Address, - delegation_address: Address, + validator_address: Address, ) -> Key { get_proposal_vote_prefix_key(id) - .push(&delegation_address) + .push(&validator_address) .expect("Cannot obtain a storage key") .push(&voter_address) .expect("Cannot obtain a storage key") diff --git a/crates/governance/src/storage/mod.rs b/crates/governance/src/storage/mod.rs index f37be28bf3..8dcaf83cbe 100644 --- a/crates/governance/src/storage/mod.rs +++ b/crates/governance/src/storage/mod.rs @@ -103,11 +103,11 @@ pub fn vote_proposal(storage: &mut S, data: VoteProposalData) -> Result<()> where S: StorageRead + StorageWrite, { - for delegation in data.delegations { + for validator in data.delegation_validators { let vote_key = governance_keys::get_vote_proposal_key( data.id, data.voter.clone(), - delegation, + validator, ); storage.write(&vote_key, data.vote.clone())?; } diff --git a/crates/governance/src/storage/proposal.rs b/crates/governance/src/storage/proposal.rs index 2ced316360..8ef8443bf2 100644 --- a/crates/governance/src/storage/proposal.rs +++ b/crates/governance/src/storage/proposal.rs @@ -83,7 +83,7 @@ pub struct VoteProposalData { /// The proposal voter address pub voter: Address, /// Validators to who the voter has delegations to - pub delegations: Vec
, + pub delegation_validators: Vec
, } impl TryFrom for InitProposalData { @@ -770,13 +770,13 @@ pub mod testing { id: u64, vote in arb_proposal_vote(), voter in arb_non_internal_address(), - delegations in collection::vec(arb_non_internal_address(), 0..10), + delegation_validators in collection::vec(arb_non_internal_address(), 0..10), ) -> VoteProposalData { VoteProposalData { id, vote, voter, - delegations, + delegation_validators, } } } diff --git a/crates/light_sdk/src/transaction/governance.rs b/crates/light_sdk/src/transaction/governance.rs index e23e1e0d0c..ae97e0d4c9 100644 --- a/crates/light_sdk/src/transaction/governance.rs +++ b/crates/light_sdk/src/transaction/governance.rs @@ -113,7 +113,7 @@ impl VoteProposal { id, vote, voter, - delegations, + delegation_validators: delegations, }; Self(transaction::build_tx( diff --git a/crates/namada/src/ledger/governance/mod.rs b/crates/namada/src/ledger/governance/mod.rs index e17d0196c2..9fc01a63da 100644 --- a/crates/namada/src/ledger/governance/mod.rs +++ b/crates/namada/src/ledger/governance/mod.rs @@ -257,21 +257,10 @@ where let pre_voting_end_epoch: Epoch = self.force_read(&voting_end_epoch_key, ReadType::Pre)?; - let voter = gov_storage::get_voter_address(key); - let delegation_address = gov_storage::get_vote_delegation_address(key); - - let (voter_address, delegation_address) = - match (voter, delegation_address) { - (Some(voter_address), Some(delegator_address)) => { - (voter_address, delegator_address) - } - _ => { - return Err(native_vp::Error::new_alloc(format!( - "Vote key is not valid: {key}" - )) - .into()); - } - }; + let voter = gov_storage::get_voter_address(key) + .ok_or(Error::InvalidVoteKey(key.to_string()))?; + let validator = gov_storage::get_vote_delegation_address(key) + .ok_or(Error::InvalidVoteKey(key.to_string()))?; // Invalid proposal id if pre_counter <= proposal_id { @@ -286,8 +275,8 @@ where let vote_key = gov_storage::get_vote_proposal_key( proposal_id, - voter_address.clone(), - delegation_address.clone(), + voter.clone(), + validator.clone(), ); if self @@ -302,7 +291,7 @@ where // TODO: We should refactor this by modifying the vote proposal tx let all_delegations_are_valid = if let Ok(delegations) = - find_delegations(&self.ctx.pre(), voter_address, ¤t_epoch) + find_delegations(&self.ctx.pre(), voter, ¤t_epoch) { if delegations.is_empty() { return Err(native_vp::Error::new_alloc(format!( @@ -310,11 +299,11 @@ where )) .into()); } else { - delegations.iter().all(|(address, _)| { + delegations.iter().all(|(val_address, _)| { let vote_key = gov_storage::get_vote_proposal_key( proposal_id, - voter_address.clone(), - address.clone(), + voter.clone(), + val_address.clone(), ); self.ctx.post().has_key(&vote_key).unwrap_or(false) }) @@ -352,7 +341,7 @@ where // first check if validator, then check if delegator let is_validator = - self.is_validator(verifiers, voter_address, delegation_address)?; + self.is_validator(verifiers, voter, validator)?; if is_validator { return is_valid_validator_voting_period( @@ -374,8 +363,8 @@ where let is_delegator = self.is_delegator( pre_voting_start_epoch, verifiers, - voter_address, - delegation_address, + voter, + validator, )?; if !is_delegator { @@ -1096,20 +1085,20 @@ where pub fn is_validator( &self, verifiers: &BTreeSet
, - address: &Address, - delegation_address: &Address, + voter: &Address, + validator: &Address, ) -> Result where S: StateRead, CA: 'static + WasmCacheAccess, { - if !address.eq(delegation_address) { + if !voter.eq(validator) { return Ok(false); } - let is_validator = is_validator(&self.ctx.pre(), address)?; + let is_validator = is_validator(&self.ctx.pre(), voter)?; - Ok(is_validator && verifiers.contains(address)) + Ok(is_validator && verifiers.contains(voter)) } /// Private method to read from storage data that are 100% in storage. diff --git a/crates/proof_of_stake/src/lib.rs b/crates/proof_of_stake/src/lib.rs index a3a56c2547..79540ea942 100644 --- a/crates/proof_of_stake/src/lib.rs +++ b/crates/proof_of_stake/src/lib.rs @@ -168,7 +168,7 @@ where } /// Check if the provided address is a delegator address, optionally at a -/// particular epoch +/// particular epoch. Returns `false` if the address is a validator. pub fn is_delegator( storage: &S, address: &Address, @@ -226,6 +226,7 @@ where "Bonding token amount {} at epoch {current_epoch}", amount.to_string_native() ); + // No-op if the bond amount is 0 if amount.is_zero() { return Ok(()); } @@ -1254,8 +1255,8 @@ where )); } - // If the address is not yet a validator, it cannot have self-bonds, but it - // may have delegations. + // The address may not have any bonds if it is going to be initialized as a + // validator if has_bonds(storage, address)? { return Err(namada_storage::Error::new_const( "The given address has delegations and therefore cannot become a \ diff --git a/crates/sdk/src/error.rs b/crates/sdk/src/error.rs index c6efa2b5cb..c2f87b7651 100644 --- a/crates/sdk/src/error.rs +++ b/crates/sdk/src/error.rs @@ -171,7 +171,7 @@ pub enum TxSubmitError { /// Bond amount is zero #[error("The requested bond amount is 0.")] BondIsZero, - /// Unond amount is zero + /// Unbond amount is zero #[error("The requested unbond amount is 0.")] UnbondIsZero, /// No unbonded bonds ready to withdraw in the current epoch diff --git a/crates/sdk/src/signing.rs b/crates/sdk/src/signing.rs index cd01dca1c5..e30065caa5 100644 --- a/crates/sdk/src/signing.rs +++ b/crates/sdk/src/signing.rs @@ -1284,7 +1284,7 @@ pub async fn to_ledger_vector( format!("Vote : {}", LedgerProposalVote(&vote_proposal.vote)), format!("Voter : {}", vote_proposal.voter), ]); - for delegation in &vote_proposal.delegations { + for delegation in &vote_proposal.delegation_validators { tv.output.push(format!("Delegation : {}", delegation)); } @@ -1293,7 +1293,7 @@ pub async fn to_ledger_vector( format!("Vote : {}", LedgerProposalVote(&vote_proposal.vote)), format!("Voter : {}", vote_proposal.voter), ]); - for delegation in vote_proposal.delegations { + for delegation in vote_proposal.delegation_validators { tv.output_expert .push(format!("Delegation : {}", delegation)); } diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 5edbda6d3e..b24e8cbb76 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -2223,7 +2223,7 @@ pub async fn build_vote_proposal( id: *proposal_id, vote: proposal_vote, voter: voter_address.clone(), - delegations, + delegation_validators, }; build( From 47ec5701f648ad2e74acb3f6a5d92352b5dbd200 Mon Sep 17 00:00:00 2001 From: brentstone Date: Wed, 6 Mar 2024 13:29:39 +0100 Subject: [PATCH 04/13] don't merklize delegation targets --- crates/apps/src/lib/node/ledger/shell/mod.rs | 3 ++- crates/proof_of_stake/src/storage_key.rs | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/crates/apps/src/lib/node/ledger/shell/mod.rs b/crates/apps/src/lib/node/ledger/shell/mod.rs index 7c880e7693..f52437a9ab 100644 --- a/crates/apps/src/lib/node/ledger/shell/mod.rs +++ b/crates/apps/src/lib/node/ledger/shell/mod.rs @@ -361,7 +361,8 @@ pub fn is_merklized_storage_key(key: &namada_sdk::storage::Key) -> bool { && *key != token::storage_key::masp_convert_anchor_key() && *key != token::storage_key::masp_token_map_key() && *key != token::storage_key::masp_assets_hash_key() - || namada::ibc::storage::is_ibc_counter_key(key)) + || namada::ibc::storage::is_ibc_counter_key(key) + || namada::proof_of_stake::storage_key::is_delegation_targets_key(key)) } /// Channels for communicating with an Ethereum oracle. diff --git a/crates/proof_of_stake/src/storage_key.rs b/crates/proof_of_stake/src/storage_key.rs index f1b2f7c8e5..3f9285166c 100644 --- a/crates/proof_of_stake/src/storage_key.rs +++ b/crates/proof_of_stake/src/storage_key.rs @@ -1097,4 +1097,20 @@ pub fn delegation_targets_key(delegator: &Address) -> Key { delegation_targets_prefix() .push(&delegator.to_db_key()) .expect("Cannot obtain a storage key") -} \ No newline at end of file +} + +/// Is storage key for the delegation targets of a delegator? +pub fn is_delegation_targets_key(key: &Key) -> bool { + if key.segments.len() >= 3 { + match &key.segments[..3] { + [ + DbKeySeg::AddressSeg(addr), + DbKeySeg::StringSeg(prefix), + DbKeySeg::AddressSeg(_delegator), + ] => addr == &ADDRESS && prefix == DELEGATION_TARGETS_PREFIX, + _ => false, + } + } else { + false + } +} From ce1056e13d45fa4370ef03e8a95f22310c177ece Mon Sep 17 00:00:00 2001 From: brentstone Date: Tue, 19 Mar 2024 17:10:26 -0400 Subject: [PATCH 05/13] new approach --- crates/namada/src/ledger/governance/mod.rs | 32 +++-- crates/proof_of_stake/src/epoched.rs | 3 +- crates/proof_of_stake/src/lib.rs | 105 ++++++++++++-- crates/proof_of_stake/src/queries.rs | 147 +++++++++++++------- crates/proof_of_stake/src/tests/helpers.rs | 39 +++++- crates/proof_of_stake/src/tests/test_pos.rs | 113 ++++++++++++++- crates/proof_of_stake/src/types/mod.rs | 45 +++++- crates/sdk/src/tx.rs | 2 +- 8 files changed, 402 insertions(+), 84 deletions(-) diff --git a/crates/namada/src/ledger/governance/mod.rs b/crates/namada/src/ledger/governance/mod.rs index 9fc01a63da..53dfbb6171 100644 --- a/crates/namada/src/ledger/governance/mod.rs +++ b/crates/namada/src/ledger/governance/mod.rs @@ -257,10 +257,16 @@ where let pre_voting_end_epoch: Epoch = self.force_read(&voting_end_epoch_key, ReadType::Pre)?; - let voter = gov_storage::get_voter_address(key) - .ok_or(Error::InvalidVoteKey(key.to_string()))?; - let validator = gov_storage::get_vote_delegation_address(key) - .ok_or(Error::InvalidVoteKey(key.to_string()))?; + let voter = gov_storage::get_voter_address(key).ok_or( + native_vp::Error::new_alloc(format!( + "Failed to parse a voter from the vote key {key}", + )), + )?; + let validator = gov_storage::get_vote_delegation_address(key).ok_or( + native_vp::Error::new_alloc(format!( + "Failed to parse a validator from the vote key {key}", + )), + )?; // Invalid proposal id if pre_counter <= proposal_id { @@ -295,7 +301,7 @@ where { if delegations.is_empty() { return Err(native_vp::Error::new_alloc(format!( - "No delegations found for {voter_address}" + "No delegations found for {voter}" )) .into()); } else { @@ -310,13 +316,13 @@ where } } else { return Err(native_vp::Error::new_alloc(format!( - "Failed to query delegations for {voter_address}" + "Failed to query delegations for {voter}" )) .into()); }; if !all_delegations_are_valid { return Err(native_vp::Error::new_alloc(format!( - "Not all delegations of {voter_address} were deemed valid" + "Not all delegations of {voter} were deemed valid" )) .into()); } @@ -340,8 +346,7 @@ where } // first check if validator, then check if delegator - let is_validator = - self.is_validator(verifiers, voter, validator)?; + let is_validator = self.is_validator(verifiers, voter, validator)?; if is_validator { return is_valid_validator_voting_period( @@ -351,9 +356,9 @@ where ) .ok_or_else(|| { native_vp::Error::new_alloc(format!( - "Validator {voter_address} voted outside of the voting \ - period. Current epoch: {current_epoch}, pre voting start \ - epoch: {pre_voting_start_epoch}, pre voting end epoch: \ + "Validator {voter} voted outside of the voting period. \ + Current epoch: {current_epoch}, pre voting start epoch: \ + {pre_voting_start_epoch}, pre voting end epoch: \ {pre_voting_end_epoch}." )) .into() @@ -369,8 +374,7 @@ where if !is_delegator { return Err(native_vp::Error::new_alloc(format!( - "Address {voter_address} is neither a validator nor a \ - delegator." + "Address {voter} is neither a validator nor a delegator." )) .into()); } diff --git a/crates/proof_of_stake/src/epoched.rs b/crates/proof_of_stake/src/epoched.rs index 033f6d6f13..b8d6bf4793 100644 --- a/crates/proof_of_stake/src/epoched.rs +++ b/crates/proof_of_stake/src/epoched.rs @@ -372,7 +372,8 @@ where .unwrap() } - fn get_oldest_epoch( + /// Get the oldest epoch at which data is stored + pub fn get_oldest_epoch( &self, storage: &S, ) -> namada_storage::Result> diff --git a/crates/proof_of_stake/src/lib.rs b/crates/proof_of_stake/src/lib.rs index 79540ea942..165d074185 100644 --- a/crates/proof_of_stake/src/lib.rs +++ b/crates/proof_of_stake/src/lib.rs @@ -39,7 +39,7 @@ use namada_storage::collections::lazy_map::{self, Collectable, LazyMap}; use namada_storage::{StorageRead, StorageWrite}; pub use namada_trans_token as token; pub use parameters::{OwnedPosParams, PosParams}; -use types::into_tm_voting_power; +use types::{into_tm_voting_power, DelegationEpochs}; use crate::queries::{find_bonds, has_bonds}; use crate::rewards::{ @@ -256,15 +256,14 @@ where return Err(BondError::NotAValidator(validator.clone()).into()); } - let bond_handle = bond_handle(source, validator); - let total_bonded_handle = total_bonded_handle(validator); - if tracing::level_enabled!(tracing::Level::DEBUG) { let bonds = find_bonds(storage, source, validator)?; tracing::debug!("\nBonds before incrementing: {bonds:#?}"); } // Initialize or update the bond at the pipeline offset + let bond_handle = bond_handle(source, validator); + let total_bonded_handle = total_bonded_handle(validator); bond_handle.add(storage, amount, current_epoch, offset)?; total_bonded_handle.add(storage, amount, current_epoch, offset)?; @@ -274,9 +273,13 @@ where } // Add the validator to the delegation targets - let target_validators = - delegation_targets_handle(source).at(&(current_epoch + offset)); - target_validators.insert(storage, validator.clone())?; + add_delegation_target( + storage, + source, + validator, + current_epoch + offset, + current_epoch, + )?; // Update the validator set // Allow bonding even if the validator is jailed. However, if jailed, there @@ -528,9 +531,13 @@ where .get_sum(storage, pipeline_epoch, ¶ms)? .unwrap_or_default(); if bonds_total.is_zero() { - delegation_targets_handle(source) - .at(&pipeline_epoch) - .remove(storage, validator)?; + remove_delegation_target( + storage, + source, + validator, + pipeline_epoch, + current_epoch, + )?; } // `updatedUnbonded` @@ -2151,6 +2158,15 @@ where pipeline_epoch, )?; + // Add the dest validator to the delegation targets + add_delegation_target( + storage, + delegator, + dest_validator, + pipeline_epoch, + current_epoch, + )?; + // Update validator set for dest validator let is_jailed_or_inactive_at_pipeline = matches!( validator_state_handle(dest_validator).get( @@ -2930,3 +2946,72 @@ where Ok(()) } + +fn add_delegation_target( + storage: &mut S, + delegator: &Address, + validator: &Address, + epoch: Epoch, + _current_epoch: Epoch, +) -> namada_storage::Result<()> +where + S: StorageRead + StorageWrite, +{ + let bond_holders = delegation_targets_handle(delegator); + if let Some(delegations) = bond_holders.get(storage, validator)?.as_mut() { + let (start, end) = delegations.last_range; + if let Some(end) = end { + // Add the `last_range` pair to the `prev_ranges` and make a new + // `last_range` + delegations.prev_ranges.push((start, end)); + delegations.last_range = (epoch, None); + bond_holders.insert( + storage, + validator.clone(), + delegations.clone(), + )?; + } else { + // do nothing since the last bond is still active + } + } else { + // Make a new delegation to this source-validator pair + let first_delegation = DelegationEpochs { + prev_ranges: vec![], + last_range: (epoch, None), + }; + bond_holders.insert(storage, validator.clone(), first_delegation)?; + } + + // Todo: possibly update the data by pruning old pairs in + // `delegations.prev_ranges`?? + + Ok(()) +} + +fn remove_delegation_target( + storage: &mut S, + delegator: &Address, + validator: &Address, + epoch: Epoch, + _current_epoch: Epoch, +) -> namada_storage::Result<()> +where + S: StorageRead + StorageWrite, +{ + let validators = delegation_targets_handle(delegator); + if let Some(delegation) = validators.get(storage, validator)?.as_mut() { + debug_assert!( + delegation.last_range.1.is_none(), + "End epoch should be None since we are removing the delegation + right now!!" + ); + delegation.last_range.1 = Some(epoch); + validators.insert(storage, validator.clone(), delegation.clone())?; + } else { + panic!("Delegation should exist since we are removing it right now!!!"); + } + + // TODO: possibly update the data by pruning old pairs or validators?? + + Ok(()) +} diff --git a/crates/proof_of_stake/src/queries.rs b/crates/proof_of_stake/src/queries.rs index 7f0c924155..12adc9d01a 100644 --- a/crates/proof_of_stake/src/queries.rs +++ b/crates/proof_of_stake/src/queries.rs @@ -13,12 +13,14 @@ use namada_storage::collections::lazy_map::{NestedSubKey, SubKey}; use namada_storage::StorageRead; use crate::slashing::{find_validator_slashes, get_slashed_amount}; -use crate::storage::{bond_handle, delegation_targets_handle, read_pos_params, unbond_handle}; +use crate::storage::{ + bond_handle, delegation_targets_handle, read_pos_params, unbond_handle, +}; use crate::types::{ - BondDetails, BondId, BondsAndUnbondsDetail, BondsAndUnbondsDetails, Slash, - UnbondDetails, + BondDetails, BondId, BondsAndUnbondsDetail, BondsAndUnbondsDetails, + DelegationEpochs, Slash, UnbondDetails, }; -use crate::{storage_key, PosParams}; +use crate::{bond_amount, storage_key, PosParams}; /// Find all validators to which a given bond `owner` (or source) has a /// delegation @@ -30,27 +32,47 @@ pub fn find_delegation_validators( where S: StorageRead, { - // let bonds_prefix = storage_key::bonds_for_source_prefix(owner); - // let mut delegations: HashSet
= HashSet::new(); - - // for iter_result in - // namada_storage::iter_prefix_bytes(storage, &bonds_prefix)? - // { - // let (key, _bond_bytes) = iter_result?; - // let validator_address = storage_key::get_validator_address_from_bond( - // &key, - // ) - // .ok_or_else(|| { - // namada_storage::Error::new_const( - // "Delegation key should contain validator address.", - // ) - // })?; - // delegations.insert(validator_address); - // } - let delegation_targets = delegation_targets_handle(owner).at(epoch); - delegation_targets - .iter(storage)? - .collect::, _>>() + let validators = delegation_targets_handle(owner); + if validators.is_empty(storage)? { + return Ok(HashSet::new()); + } + + let mut delegation_targets = HashSet::
::new(); + + for validator in validators.iter(storage)? { + let ( + val, + DelegationEpochs { + prev_ranges, + last_range: (last_start, last_end), + }, + ) = validator?; + + // Now determine if the validator held a bond from delegator at epoch + if *epoch >= last_start { + // the `last_range` will tell us if there was a bond + if let Some(end) = last_end { + if *epoch < end { + delegation_targets.insert(val); + } + } else { + // this is bond is currently held + delegation_targets.insert(val); + } + } else { + // need to search through the `prev_ranges` now + for (start, end) in prev_ranges.iter().rev() { + if *epoch >= *start { + if *epoch < *end { + delegation_targets.insert(val); + } + break; + } + } + } + } + + Ok(delegation_targets) } /// Find all validators to which a given bond `owner` (or source) has a @@ -63,32 +85,55 @@ pub fn find_delegations( where S: StorageRead, { - // let bonds_prefix = storage_key::bonds_for_source_prefix(owner); - let params = read_pos_params(storage)?; - let mut delegations: HashMap = HashMap::new(); - - // for iter_result in - // namada_storage::iter_prefix_bytes(storage, &bonds_prefix)? - // { - // let (key, _bond_bytes) = iter_result?; - // let validator_address = storage_key::get_validator_address_from_bond( - // &key, - // ) - // .ok_or_else(|| { - // namada_storage::Error::new_const( - // "Delegation key should contain validator address.", - // ) - // })?; - // let deltas_sum = bond_handle(owner, &validator_address) - // .get_sum(storage, *epoch, ¶ms)? - // .unwrap_or_default(); - // delegations.insert(validator_address, deltas_sum); - // } - let delegation_targets = delegation_targets_handle(owner).at(epoch); - for validator in delegation_targets.iter(storage)? { - let validator = validator?; - let stake = bond_handle(owner, &validator).get_sum(storage, *epoch, ¶ms)?.unwrap_or_default(); - delegations.insert(validator, stake); + let validators = delegation_targets_handle(owner); + if validators.is_empty(storage)? { + return Ok(HashMap::new()); + } + + let mut delegations = HashMap::::new(); + + for validator in validators.iter(storage)? { + let ( + val, + DelegationEpochs { + prev_ranges, + last_range: (last_start, last_end), + }, + ) = validator?; + + // TODO: Should use raw bonds or slashed stake?? + let bond_amount = bond_amount( + storage, + &BondId { + source: owner.clone(), + validator: val.clone(), + }, + *epoch, + )?; + + // Now determine if the validator held a bond from delegator at epoch + if *epoch >= last_start { + // the `last_range` will tell us if there was a bond + if let Some(end) = last_end { + if *epoch < end { + // this bond was previously held + delegations.insert(val, bond_amount); + } + } else { + // this bond is currently held + delegations.insert(val, bond_amount); + } + } else { + // need to search through the `prev_ranges` now + for (start, end) in prev_ranges.iter().rev() { + if *epoch >= *start { + if *epoch < *end { + delegations.insert(val, bond_amount); + } + break; + } + } + } } Ok(delegations) } diff --git a/crates/proof_of_stake/src/tests/helpers.rs b/crates/proof_of_stake/src/tests/helpers.rs index f199c13e85..a1b81c595a 100644 --- a/crates/proof_of_stake/src/tests/helpers.rs +++ b/crates/proof_of_stake/src/tests/helpers.rs @@ -3,7 +3,9 @@ use std::ops::Range; use namada_core::address::testing::address_from_simple_seed; use namada_core::dec::Dec; -use namada_core::key::testing::common_sk_from_simple_seed; +use namada_core::key::testing::{ + common_sk_from_simple_seed, keypair_1, keypair_3, +}; use namada_core::key::{self, RefTo}; use namada_core::storage::Epoch; use namada_core::token; @@ -179,3 +181,38 @@ pub fn arb_redelegation_amounts( ) }) } + +pub fn get_genesis_validators( + num: u64, + init_stakes: Vec, +) -> Vec { + if init_stakes.len() != num as usize { + panic!("init_stakes.len() != num"); + } + let protocol_key = keypair_1().to_public(); + let eth_cold_key = keypair_3().to_public(); + let eth_hot_key = keypair_3().to_public(); + let commission_rate = Dec::new(5, 2).expect("Test failed"); + let max_commission_rate_change = Dec::new(1, 2).expect("Test failed"); + + let mut gen_vals = Vec::::new(); + for (seed, stake) in init_stakes.iter().enumerate() { + let address = address_from_simple_seed(seed as u64); + let consensus_sk = common_sk_from_simple_seed(seed as u64); + let consensus_key = consensus_sk.to_public(); + + gen_vals.push(GenesisValidator { + address, + tokens: *stake, + consensus_key, + protocol_key: protocol_key.clone(), + eth_hot_key: eth_hot_key.clone(), + eth_cold_key: eth_cold_key.clone(), + commission_rate, + max_commission_rate_change, + metadata: Default::default(), + }); + } + + gen_vals +} diff --git a/crates/proof_of_stake/src/tests/test_pos.rs b/crates/proof_of_stake/src/tests/test_pos.rs index 0f24d0c71f..0c4d256fc9 100644 --- a/crates/proof_of_stake/src/tests/test_pos.rs +++ b/crates/proof_of_stake/src/tests/test_pos.rs @@ -22,7 +22,7 @@ use token::get_effective_total_native_supply; use crate::parameters::testing::arb_pos_params; use crate::parameters::OwnedPosParams; -use crate::queries::bonds_and_unbonds; +use crate::queries::{bonds_and_unbonds, find_delegation_validators}; use crate::rewards::{ log_block_rewards_aux, update_rewards_products_and_mint_inflation, PosRewardsCalculator, @@ -38,6 +38,7 @@ use crate::storage::{ use crate::test_utils::test_init_genesis; use crate::tests::helpers::{ advance_epoch, arb_genesis_validators, arb_params_and_genesis_validators, + get_genesis_validators, }; use crate::token::{credit_tokens, read_balance}; use crate::types::{ @@ -1749,3 +1750,113 @@ fn test_jail_for_liveness_aux(validators: Vec) { &storage_clone.write_log() ); } + +#[test] +fn test_delegation_targets() { + let stakes = vec![ + token::Amount::native_whole(1), + token::Amount::native_whole(2), + ]; + let mut s = TestState::default(); + let mut current_epoch = s.in_mem().block.epoch; + let params = OwnedPosParams::default(); + + let genesis_validators = get_genesis_validators(2, stakes.clone()); + let validator1 = genesis_validators[0].address.clone(); + let validator2 = genesis_validators[1].address.clone(); + + let delegator = address::testing::gen_implicit_address(); + let staking_token = staking_token_address(&s); + credit_tokens( + &mut s, + &staking_token, + &delegator, + token::Amount::native_whole(20), + ) + .unwrap(); + + let params = test_init_genesis( + &mut s, + params, + genesis_validators.into_iter(), + current_epoch, + ) + .unwrap(); + + // Check initial delegation targets + for epoch in Epoch::iter_bounds_inclusive( + current_epoch, + current_epoch + params.pipeline_len, + ) { + println!("Epoch: {:?}", epoch); + let delegatees1 = + find_delegation_validators(&s, &validator1, &epoch).unwrap(); + let delegatees2 = + find_delegation_validators(&s, &validator2, &epoch).unwrap(); + assert_eq!(delegatees1.len(), 1); + assert_eq!(delegatees2.len(), 1); + assert!(delegatees1.contains(&validator1)); + assert!(delegatees2.contains(&validator2)); + } + + // Advance an epoch and check if the delegation targets are properly updated + // in the absence of bonds + current_epoch = advance_epoch(&mut s, ¶ms); + for epoch in Epoch::iter_bounds_inclusive( + Epoch::default(), + current_epoch + params.pipeline_len, + ) { + println!("Epoch: {:?}", epoch); + let delegatees1 = + find_delegation_validators(&s, &validator1, &epoch).unwrap(); + let delegatees2 = + find_delegation_validators(&s, &validator2, &epoch).unwrap(); + assert_eq!(delegatees1.len(), 1); + assert_eq!(delegatees2.len(), 1); + assert!(delegatees1.contains(&validator1)); + assert!(delegatees2.contains(&validator2)); + } + + // Bond from a delegator to validator1 in epoch 1 + bond_tokens( + &mut s, + Some(&delegator), + &validator1, + token::Amount::native_whole(3), + current_epoch, + None, + ) + .unwrap(); + + // Completely self-unbond from validator2 + unbond_tokens(&mut s, None, &validator2, stakes[1], current_epoch, false) + .unwrap(); + + // Check the delegation targets now + let pipeline_epoch = current_epoch + params.pipeline_len; + for epoch in + Epoch::iter_bounds_inclusive(Epoch::default(), pipeline_epoch.prev()) + { + println!("Epoch: {:?}", epoch); + let delegatees1 = + find_delegation_validators(&s, &validator1, &epoch).unwrap(); + let delegatees2 = + find_delegation_validators(&s, &validator2, &epoch).unwrap(); + assert_eq!(delegatees1.len(), 1); + assert_eq!(delegatees2.len(), 1); + assert!(delegatees1.contains(&validator1)); + assert!(delegatees2.contains(&validator2)); + } + + let delegatees1 = + find_delegation_validators(&s, &validator1, &pipeline_epoch).unwrap(); + let delegatees2 = + find_delegation_validators(&s, &validator2, &pipeline_epoch).unwrap(); + let del_delegatees = + find_delegation_validators(&s, &delegator, &pipeline_epoch).unwrap(); + assert_eq!(delegatees1.len(), 1); + assert_eq!(delegatees2.len(), 0); + assert_eq!(del_delegatees.len(), 1); + assert!(delegatees1.contains(&validator1)); + assert!(del_delegatees.contains(&validator1)); +} diff --git a/crates/proof_of_stake/src/types/mod.rs b/crates/proof_of_stake/src/types/mod.rs index adb0b8b7a1..af83afd720 100644 --- a/crates/proof_of_stake/src/types/mod.rs +++ b/crates/proof_of_stake/src/types/mod.rs @@ -263,12 +263,47 @@ pub type LivenessMissedVotes = NestedMap>; /// elements in the corresponding inner LazySet of [`LivenessMissedVotes`]. pub type LivenessSumMissedVotes = LazyMap; +/// Contains information on epoch periods (start, end) in which a delegator had +/// a bonded with a certain validator. The `end` epoch is the first epoch at +/// which the bond ceased to exist (exclusive). +#[derive(Debug, Clone, BorshSerialize, BorshDeserialize)] +pub struct DelegationEpochs { + /// Previous ranges during which a bond existed + pub prev_ranges: Vec<(Epoch, Epoch)>, + /// The last range during which a bond existed + pub last_range: (Epoch, Option), +} + /// The set of all target validators for a given delegator. -pub type DelegationTargets = crate::epoched::NestedEpoched< - LazySet
, - crate::epoched::OffsetPipelineLen, - crate::epoched::OffsetMaxProposalPeriodPlus, ->; +pub type DelegationTargets = LazyMap; + +// impl DelegationTargets { +// pub fn get_delegation_validators( +// &self, +// storage: &mut S, +// epoch: Epoch, +// ) -> namada_storage::Result>> +// where +// S: StorageRead, +// { +// let oldest_epoch = self +// .get_last_update(storage)? +// .expect("Oldest epoch should be set"); + +// if epoch < oldest_epoch { +// // Should we return an error or None? +// return Ok(None); +// } + +// let mut epoch = epoch; +// while epoch >= oldest_epoch { +// if let Some(validators) = self.get(epoch, storage)? { +// return Ok(Some(validators)); +// } +// epoch = epoch - 1; +// } +// } +// } #[derive( Debug, diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index b24e8cbb76..87234d3328 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -2223,7 +2223,7 @@ pub async fn build_vote_proposal( id: *proposal_id, vote: proposal_vote, voter: voter_address.clone(), - delegation_validators, + delegation_validators: delegations, }; build( From 341fb41b7611a4de1702c1922bff8d15cffc6b9e Mon Sep 17 00:00:00 2001 From: brentstone Date: Tue, 9 Apr 2024 19:23:19 -0600 Subject: [PATCH 06/13] fix and finish test --- crates/proof_of_stake/src/tests/test_pos.rs | 274 ++++++++++++++++++-- 1 file changed, 248 insertions(+), 26 deletions(-) diff --git a/crates/proof_of_stake/src/tests/test_pos.rs b/crates/proof_of_stake/src/tests/test_pos.rs index 0c4d256fc9..3895b08fd9 100644 --- a/crates/proof_of_stake/src/tests/test_pos.rs +++ b/crates/proof_of_stake/src/tests/test_pos.rs @@ -22,7 +22,9 @@ use token::get_effective_total_native_supply; use crate::parameters::testing::arb_pos_params; use crate::parameters::OwnedPosParams; -use crate::queries::{bonds_and_unbonds, find_delegation_validators}; +use crate::queries::{ + bonds_and_unbonds, find_delegation_validators, find_delegations, +}; use crate::rewards::{ log_block_rewards_aux, update_rewards_products_and_mint_inflation, PosRewardsCalculator, @@ -1757,8 +1759,8 @@ fn test_delegation_targets() { token::Amount::native_whole(1), token::Amount::native_whole(2), ]; - let mut s = TestState::default(); - let mut current_epoch = s.in_mem().block.epoch; + let mut storage = TestState::default(); + let mut current_epoch = storage.in_mem().block.epoch; let params = OwnedPosParams::default(); let genesis_validators = get_genesis_validators(2, stakes.clone()); @@ -1766,51 +1768,60 @@ fn test_delegation_targets() { let validator2 = genesis_validators[1].address.clone(); let delegator = address::testing::gen_implicit_address(); - let staking_token = staking_token_address(&s); + let staking_token = staking_token_address(&storage); credit_tokens( - &mut s, + &mut storage, &staking_token, &delegator, token::Amount::native_whole(20), ) .unwrap(); + credit_tokens( + &mut storage, + &staking_token, + &validator2, + token::Amount::native_whole(20), + ) + .unwrap(); let params = test_init_genesis( - &mut s, + &mut storage, params, genesis_validators.into_iter(), current_epoch, ) .unwrap(); + println!("\nValidator1: {:?}", validator1); + println!("Validator2: {:?}", validator2); + println!("Delegator: {:?}\n", delegator); + // Check initial delegation targets for epoch in Epoch::iter_bounds_inclusive( current_epoch, current_epoch + params.pipeline_len, ) { - println!("Epoch: {:?}", epoch); let delegatees1 = - find_delegation_validators(&s, &validator1, &epoch).unwrap(); + find_delegation_validators(&storage, &validator1, &epoch).unwrap(); let delegatees2 = - find_delegation_validators(&s, &validator2, &epoch).unwrap(); + find_delegation_validators(&storage, &validator2, &epoch).unwrap(); assert_eq!(delegatees1.len(), 1); assert_eq!(delegatees2.len(), 1); assert!(delegatees1.contains(&validator1)); assert!(delegatees2.contains(&validator2)); } - // Advance an epoch and check if the delegation targets are properly updated - // in the absence of bonds - current_epoch = advance_epoch(&mut s, ¶ms); + // Advance to epoch 1 and check if the delegation targets are properly + // updated in the absence of bonds + current_epoch = advance_epoch(&mut storage, ¶ms); for epoch in Epoch::iter_bounds_inclusive( Epoch::default(), current_epoch + params.pipeline_len, ) { - println!("Epoch: {:?}", epoch); let delegatees1 = - find_delegation_validators(&s, &validator1, &epoch).unwrap(); + find_delegation_validators(&storage, &validator1, &epoch).unwrap(); let delegatees2 = - find_delegation_validators(&s, &validator2, &epoch).unwrap(); + find_delegation_validators(&storage, &validator2, &epoch).unwrap(); assert_eq!(delegatees1.len(), 1); assert_eq!(delegatees2.len(), 1); assert!(delegatees1.contains(&validator1)); @@ -1819,7 +1830,7 @@ fn test_delegation_targets() { // Bond from a delegator to validator1 in epoch 1 bond_tokens( - &mut s, + &mut storage, Some(&delegator), &validator1, token::Amount::native_whole(3), @@ -1829,19 +1840,25 @@ fn test_delegation_targets() { .unwrap(); // Completely self-unbond from validator2 - unbond_tokens(&mut s, None, &validator2, stakes[1], current_epoch, false) - .unwrap(); + unbond_tokens( + &mut storage, + None, + &validator2, + stakes[1], + current_epoch, + false, + ) + .unwrap(); // Check the delegation targets now let pipeline_epoch = current_epoch + params.pipeline_len; for epoch in Epoch::iter_bounds_inclusive(Epoch::default(), pipeline_epoch.prev()) { - println!("Epoch: {:?}", epoch); let delegatees1 = - find_delegation_validators(&s, &validator1, &epoch).unwrap(); + find_delegation_validators(&storage, &validator1, &epoch).unwrap(); let delegatees2 = - find_delegation_validators(&s, &validator2, &epoch).unwrap(); + find_delegation_validators(&storage, &validator2, &epoch).unwrap(); assert_eq!(delegatees1.len(), 1); assert_eq!(delegatees2.len(), 1); assert!(delegatees1.contains(&validator1)); @@ -1849,14 +1866,219 @@ fn test_delegation_targets() { } let delegatees1 = - find_delegation_validators(&s, &validator1, &pipeline_epoch).unwrap(); + find_delegation_validators(&storage, &validator1, &pipeline_epoch) + .unwrap(); + assert_eq!(delegatees1.len(), 1); + assert!(delegatees1.contains(&validator1)); + let delegatees2 = - find_delegation_validators(&s, &validator2, &pipeline_epoch).unwrap(); + find_delegation_validators(&storage, &validator2, &pipeline_epoch) + .unwrap(); + assert!(delegatees2.is_empty()); + let del_delegatees = - find_delegation_validators(&s, &delegator, &pipeline_epoch).unwrap(); - assert_eq!(delegatees1.len(), 1); - assert_eq!(delegatees2.len(), 0); + find_delegation_validators(&storage, &delegator, &pipeline_epoch) + .unwrap(); assert_eq!(del_delegatees.len(), 1); + assert!(del_delegatees.contains(&validator1)); + + // Advance to epoch 3 + advance_epoch(&mut storage, ¶ms); + current_epoch = advance_epoch(&mut storage, ¶ms); + + // Bond from delegator to validator1 + bond_tokens( + &mut storage, + Some(&delegator), + &validator1, + token::Amount::native_whole(3), + current_epoch, + None, + ) + .unwrap(); + + // Bond from delegator to validator2 + bond_tokens( + &mut storage, + Some(&delegator), + &validator2, + token::Amount::native_whole(3), + current_epoch, + None, + ) + .unwrap(); + + // Checks + let pipeline_epoch = current_epoch + params.pipeline_len; + + // Up to epoch 2 + for epoch in + Epoch::iter_bounds_inclusive(Epoch::default(), current_epoch.prev()) + { + let delegatees1 = + find_delegation_validators(&storage, &validator1, &epoch).unwrap(); + let delegatees2 = + find_delegation_validators(&storage, &validator2, &epoch).unwrap(); + let del_delegatees = + find_delegation_validators(&storage, &delegator, &epoch).unwrap(); + assert_eq!(delegatees1.len(), 1); + assert_eq!(delegatees2.len(), 1); + assert!(delegatees1.contains(&validator1)); + assert!(delegatees2.contains(&validator2)); + assert!(del_delegatees.is_empty()); + } + + // Epochs 3-4 + for epoch in + Epoch::iter_bounds_inclusive(current_epoch, pipeline_epoch.prev()) + { + let delegatees1 = + find_delegation_validators(&storage, &validator1, &epoch).unwrap(); + let delegatees2 = + find_delegation_validators(&storage, &validator2, &epoch).unwrap(); + let del_delegatees = + find_delegation_validators(&storage, &delegator, &epoch).unwrap(); + assert_eq!(delegatees1.len(), 1); + assert!(delegatees2.is_empty()); + assert_eq!(del_delegatees.len(), 1); + assert!(delegatees1.contains(&validator1)); + assert!(del_delegatees.contains(&validator1)); + } + + // Epoch 5 (pipeline) + let delegatees1 = + find_delegation_validators(&storage, &validator1, &pipeline_epoch) + .unwrap(); + let delegatees2 = + find_delegation_validators(&storage, &validator2, &pipeline_epoch) + .unwrap(); + let del_delegatees = + find_delegation_validators(&storage, &delegator, &pipeline_epoch) + .unwrap(); + assert_eq!(delegatees1.len(), 1); + assert!(delegatees2.is_empty()); + assert_eq!(del_delegatees.len(), 2); assert!(delegatees1.contains(&validator1)); assert!(del_delegatees.contains(&validator1)); + assert!(del_delegatees.contains(&validator2)); + + // Advance to epoch 4 and self-bond from validator2 again + current_epoch = advance_epoch(&mut storage, ¶ms); + bond_tokens( + &mut storage, + None, + &validator2, + token::Amount::native_whole(1), + current_epoch, + None, + ) + .unwrap(); + + let pipeline_epoch = current_epoch + params.pipeline_len; + + // Check at pipeline epoch 6 + let delegatees1 = + find_delegation_validators(&storage, &validator1, &pipeline_epoch) + .unwrap(); + let delegatees2 = + find_delegation_validators(&storage, &validator2, &pipeline_epoch) + .unwrap(); + let del_delegatees = + find_delegation_validators(&storage, &delegator, &pipeline_epoch) + .unwrap(); + assert_eq!(delegatees1.len(), 1); + assert_eq!(delegatees2.len(), 1); + assert_eq!(del_delegatees.len(), 2); + assert!(delegatees1.contains(&validator1)); + assert!(delegatees2.contains(&validator2)); + assert!(del_delegatees.contains(&validator1)); + assert!(del_delegatees.contains(&validator2)); + + // Check everything again including the raw bond amount this time + + // Up to epoch 2 + for epoch in Epoch::iter_bounds_inclusive(Epoch::default(), Epoch(2)) { + let delegatees1 = + find_delegations(&storage, &validator1, &epoch).unwrap(); + let delegatees2 = + find_delegations(&storage, &validator2, &epoch).unwrap(); + let del_delegatees = + find_delegations(&storage, &delegator, &epoch).unwrap(); + assert_eq!(delegatees1.len(), 1); + assert_eq!(delegatees2.len(), 1); + assert!(del_delegatees.is_empty()); + assert_eq!(delegatees1.get(&validator1).unwrap(), &stakes[0]); + assert_eq!(delegatees2.get(&validator2).unwrap(), &stakes[1]); + } + + // Epochs 3-4 + for epoch in Epoch::iter_bounds_inclusive(Epoch(3), Epoch(4)) { + let delegatees1 = + find_delegations(&storage, &validator1, &epoch).unwrap(); + let delegatees2 = + find_delegations(&storage, &validator2, &epoch).unwrap(); + let del_delegatees = + find_delegations(&storage, &delegator, &epoch).unwrap(); + assert_eq!(delegatees1.len(), 1); + assert!(delegatees2.is_empty()); + assert_eq!(del_delegatees.len(), 1); + assert_eq!( + delegatees1.get(&validator1).unwrap(), + &token::Amount::native_whole(1) + ); + assert_eq!( + del_delegatees.get(&validator1).unwrap(), + &token::Amount::native_whole(3) + ); + } + + // Epoch 5 + let delegatees1 = + find_delegations(&storage, &validator1, &Epoch(5)).unwrap(); + let delegatees2 = + find_delegations(&storage, &validator2, &Epoch(5)).unwrap(); + let del_delegatees = + find_delegations(&storage, &delegator, &Epoch(5)).unwrap(); + assert_eq!(delegatees1.len(), 1); + assert!(delegatees2.is_empty()); + assert_eq!(del_delegatees.len(), 2); + assert_eq!( + delegatees1.get(&validator1).unwrap(), + &token::Amount::native_whole(1) + ); + assert_eq!( + del_delegatees.get(&validator1).unwrap(), + &token::Amount::native_whole(6) + ); + assert_eq!( + del_delegatees.get(&validator2).unwrap(), + &token::Amount::native_whole(3) + ); + + // Epoch 6 + let delegatees1 = + find_delegations(&storage, &validator1, &Epoch(6)).unwrap(); + let delegatees2 = + find_delegations(&storage, &validator2, &Epoch(6)).unwrap(); + let del_delegatees = + find_delegations(&storage, &delegator, &Epoch(6)).unwrap(); + assert_eq!(delegatees1.len(), 1); + assert_eq!(delegatees2.len(), 1); + assert_eq!(del_delegatees.len(), 2); + assert_eq!( + delegatees1.get(&validator1).unwrap(), + &token::Amount::native_whole(1) + ); + assert_eq!( + delegatees2.get(&validator2).unwrap(), + &token::Amount::native_whole(1) + ); + assert_eq!( + del_delegatees.get(&validator1).unwrap(), + &token::Amount::native_whole(6) + ); + assert_eq!( + del_delegatees.get(&validator2).unwrap(), + &token::Amount::native_whole(3) + ); } From b35f0f8b8ce1df6a47e6ac9a4a3be40f1720bb9a Mon Sep 17 00:00:00 2001 From: brentstone Date: Wed, 10 Apr 2024 09:47:44 -0600 Subject: [PATCH 07/13] fixes from review comments --- crates/proof_of_stake/src/lib.rs | 5 +++-- crates/proof_of_stake/src/types/mod.rs | 28 -------------------------- 2 files changed, 3 insertions(+), 30 deletions(-) diff --git a/crates/proof_of_stake/src/lib.rs b/crates/proof_of_stake/src/lib.rs index 165d074185..25900833d8 100644 --- a/crates/proof_of_stake/src/lib.rs +++ b/crates/proof_of_stake/src/lib.rs @@ -3000,12 +3000,13 @@ where { let validators = delegation_targets_handle(delegator); if let Some(delegation) = validators.get(storage, validator)?.as_mut() { + let (_start, end) = &mut delegation.last_range; debug_assert!( - delegation.last_range.1.is_none(), + end.is_none(), "End epoch should be None since we are removing the delegation right now!!" ); - delegation.last_range.1 = Some(epoch); + *end = Some(epoch); validators.insert(storage, validator.clone(), delegation.clone())?; } else { panic!("Delegation should exist since we are removing it right now!!!"); diff --git a/crates/proof_of_stake/src/types/mod.rs b/crates/proof_of_stake/src/types/mod.rs index af83afd720..851568bb4a 100644 --- a/crates/proof_of_stake/src/types/mod.rs +++ b/crates/proof_of_stake/src/types/mod.rs @@ -277,34 +277,6 @@ pub struct DelegationEpochs { /// The set of all target validators for a given delegator. pub type DelegationTargets = LazyMap; -// impl DelegationTargets { -// pub fn get_delegation_validators( -// &self, -// storage: &mut S, -// epoch: Epoch, -// ) -> namada_storage::Result>> -// where -// S: StorageRead, -// { -// let oldest_epoch = self -// .get_last_update(storage)? -// .expect("Oldest epoch should be set"); - -// if epoch < oldest_epoch { -// // Should we return an error or None? -// return Ok(None); -// } - -// let mut epoch = epoch; -// while epoch >= oldest_epoch { -// if let Some(validators) = self.get(epoch, storage)? { -// return Ok(Some(validators)); -// } -// epoch = epoch - 1; -// } -// } -// } - #[derive( Debug, Clone, From 8ddbcfe1364d2f9e351bb567bf9e7ffb7cddd5a5 Mon Sep 17 00:00:00 2001 From: brentstone Date: Mon, 15 Apr 2024 12:02:50 -0700 Subject: [PATCH 08/13] fix edge case (fully unbond then rebond) --- crates/proof_of_stake/src/lib.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/proof_of_stake/src/lib.rs b/crates/proof_of_stake/src/lib.rs index 25900833d8..2087bd4a91 100644 --- a/crates/proof_of_stake/src/lib.rs +++ b/crates/proof_of_stake/src/lib.rs @@ -2963,8 +2963,14 @@ where if let Some(end) = end { // Add the `last_range` pair to the `prev_ranges` and make a new // `last_range` - delegations.prev_ranges.push((start, end)); - delegations.last_range = (epoch, None); + if epoch == end { + // This case would occur if in the same epoch, the bond was + // fully unbonded, followed by the bonding of new tokens + delegations.last_range.1 = None; + } else { + delegations.prev_ranges.insert(start, end); + delegations.last_range = (epoch, None); + } bond_holders.insert( storage, validator.clone(), From 2bff6e10dd115089bd55a99efd7a34c019f96d6d Mon Sep 17 00:00:00 2001 From: brentstone Date: Mon, 15 Apr 2024 12:03:21 -0700 Subject: [PATCH 09/13] prune old `prev_ranges` data --- crates/proof_of_stake/src/lib.rs | 39 ++++++++++- crates/proof_of_stake/src/tests/test_pos.rs | 77 ++++++++++++++++++++- crates/proof_of_stake/src/types/mod.rs | 4 +- 3 files changed, 114 insertions(+), 6 deletions(-) diff --git a/crates/proof_of_stake/src/lib.rs b/crates/proof_of_stake/src/lib.rs index 2087bd4a91..c1a6d808c7 100644 --- a/crates/proof_of_stake/src/lib.rs +++ b/crates/proof_of_stake/src/lib.rs @@ -26,6 +26,7 @@ use core::fmt::Debug; use std::cmp::{self}; use std::collections::{BTreeMap, BTreeSet}; +use epoched::EpochOffset; pub use error::*; use namada_core::address::{Address, InternalAddress}; use namada_core::collections::HashSet; @@ -533,6 +534,7 @@ where if bonds_total.is_zero() { remove_delegation_target( storage, + ¶ms, source, validator, pipeline_epoch, @@ -2982,7 +2984,7 @@ where } else { // Make a new delegation to this source-validator pair let first_delegation = DelegationEpochs { - prev_ranges: vec![], + prev_ranges: BTreeMap::new(), last_range: (epoch, None), }; bond_holders.insert(storage, validator.clone(), first_delegation)?; @@ -2996,10 +2998,11 @@ where fn remove_delegation_target( storage: &mut S, + params: &PosParams, delegator: &Address, validator: &Address, epoch: Epoch, - _current_epoch: Epoch, + current_epoch: Epoch, ) -> namada_storage::Result<()> where S: StorageRead + StorageWrite, @@ -3013,12 +3016,42 @@ where right now!!" ); *end = Some(epoch); + prune_old_delegations(params, delegation, current_epoch)?; validators.insert(storage, validator.clone(), delegation.clone())?; } else { panic!("Delegation should exist since we are removing it right now!!!"); } - // TODO: possibly update the data by pruning old pairs or validators?? + Ok(()) +} + +fn prune_old_delegations( + params: &PosParams, + delegations: &mut DelegationEpochs, + current_epoch: Epoch, +) -> namada_storage::Result<()> { + let delta = + crate::epoched::OffsetMaxProposalPeriodOrSlashProcessingLenPlus::value( + params, + ); + let oldest_to_keep = current_epoch.checked_sub(delta).unwrap_or_default(); + + let to_remove = delegations + .prev_ranges + .iter() + .filter_map(|(start, end)| { + if start < &oldest_to_keep && end < &oldest_to_keep { + Some(start) + } else { + None + } + }) + .cloned() + .collect::>(); + + for start_epoch in to_remove { + delegations.prev_ranges.remove(&start_epoch); + } Ok(()) } diff --git a/crates/proof_of_stake/src/tests/test_pos.rs b/crates/proof_of_stake/src/tests/test_pos.rs index 3895b08fd9..0150cba4d5 100644 --- a/crates/proof_of_stake/src/tests/test_pos.rs +++ b/crates/proof_of_stake/src/tests/test_pos.rs @@ -20,6 +20,7 @@ use proptest::test_runner::Config; use test_log::test; use token::get_effective_total_native_supply; +use crate::epoched::EpochOffset; use crate::parameters::testing::arb_pos_params; use crate::parameters::OwnedPosParams; use crate::queries::{ @@ -31,7 +32,8 @@ use crate::rewards::{ }; use crate::slashing::{process_slashes, slash}; use crate::storage::{ - get_consensus_key_set, liveness_sum_missed_votes_handle, + delegation_targets_handle, get_consensus_key_set, + liveness_sum_missed_votes_handle, read_below_threshold_validator_set_addresses, read_consensus_validator_set_addresses_with_stake, read_total_stake, read_validator_deltas_value, rewards_accumulator_handle, @@ -2081,4 +2083,77 @@ fn test_delegation_targets() { del_delegatees.get(&validator2).unwrap(), &token::Amount::native_whole(3) ); + + // Advance enough epochs for a relevant action to prune old data + let num_to_advance = + crate::epoched::OffsetMaxProposalPeriodOrSlashProcessingLenPlus::value( + ¶ms, + ); + for _ in 0..num_to_advance { + advance_epoch(&mut storage, ¶ms); + } + current_epoch = storage.in_mem().block.epoch; + + // Redelegate fully from validator1 to validator2 + redelegate_tokens( + &mut storage, + &delegator, + &validator1, + &validator2, + current_epoch, + token::Amount::native_whole(6), + ) + .unwrap(); + + let de_d1 = delegation_targets_handle(&delegator) + .get(&storage, &validator1) + .unwrap() + .unwrap(); + let de_d2 = delegation_targets_handle(&delegator) + .get(&storage, &validator2) + .unwrap() + .unwrap(); + assert!(de_d1.prev_ranges.is_empty()); + assert_eq!( + de_d1.last_range.1, + Some(current_epoch + params.pipeline_len) + ); + assert!(de_d2.prev_ranges.is_empty()); + assert!(de_d2.last_range.1.is_none()); + + // Fully self-unbond validator2 to see if old data is pruned + unbond_tokens( + &mut storage, + None, + &validator2, + token::Amount::native_whole(1), + current_epoch, + false, + ) + .unwrap(); + + let de_2 = delegation_targets_handle(&validator2) + .get(&storage, &validator2) + .unwrap() + .unwrap(); + assert!(de_2.prev_ranges.is_empty()); + assert_eq!(de_2.last_range.1, Some(current_epoch + params.pipeline_len)); + + // Self-bond validator2 to check that no data is pushed to `prev_ranges` + bond_tokens( + &mut storage, + None, + &validator2, + token::Amount::native_whole(2), + current_epoch, + None, + ) + .unwrap(); + + let de_2 = delegation_targets_handle(&validator2) + .get(&storage, &validator2) + .unwrap() + .unwrap(); + assert!(de_2.prev_ranges.is_empty()); + assert_eq!(de_2.last_range.1, None); } diff --git a/crates/proof_of_stake/src/types/mod.rs b/crates/proof_of_stake/src/types/mod.rs index 851568bb4a..61fd02463b 100644 --- a/crates/proof_of_stake/src/types/mod.rs +++ b/crates/proof_of_stake/src/types/mod.rs @@ -268,8 +268,8 @@ pub type LivenessSumMissedVotes = LazyMap; /// which the bond ceased to exist (exclusive). #[derive(Debug, Clone, BorshSerialize, BorshDeserialize)] pub struct DelegationEpochs { - /// Previous ranges during which a bond existed - pub prev_ranges: Vec<(Epoch, Epoch)>, + /// Previous ranges during which a bond existed (Map) + pub prev_ranges: BTreeMap, /// The last range during which a bond existed pub last_range: (Epoch, Option), } From f670a03b6d6b2cb3d50240f2391adf0b3d13d487 Mon Sep 17 00:00:00 2001 From: brentstone Date: Mon, 15 Apr 2024 12:05:29 -0700 Subject: [PATCH 10/13] changelog: add #3043 --- .../unreleased/improvements/3043-delegation-validators.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/unreleased/improvements/3043-delegation-validators.md diff --git a/.changelog/unreleased/improvements/3043-delegation-validators.md b/.changelog/unreleased/improvements/3043-delegation-validators.md new file mode 100644 index 0000000000..d28e01d4cf --- /dev/null +++ b/.changelog/unreleased/improvements/3043-delegation-validators.md @@ -0,0 +1,3 @@ +- Optimize the finding of validators to which a delegator has bonds at a + given epoch. Now keeps data in storage rather than iterating over all bonds. + ([\#3043](https://github.com/anoma/namada/pull/3043)) \ No newline at end of file From aafc6db8a3f75c90ea463a66f5a63d62d33277ce Mon Sep 17 00:00:00 2001 From: brentstone Date: Tue, 16 Apr 2024 09:31:11 -0700 Subject: [PATCH 11/13] fix from comments --- crates/proof_of_stake/src/lib.rs | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/crates/proof_of_stake/src/lib.rs b/crates/proof_of_stake/src/lib.rs index c1a6d808c7..a1881ef837 100644 --- a/crates/proof_of_stake/src/lib.rs +++ b/crates/proof_of_stake/src/lib.rs @@ -3036,22 +3036,9 @@ fn prune_old_delegations( ); let oldest_to_keep = current_epoch.checked_sub(delta).unwrap_or_default(); - let to_remove = delegations + delegations .prev_ranges - .iter() - .filter_map(|(start, end)| { - if start < &oldest_to_keep && end < &oldest_to_keep { - Some(start) - } else { - None - } - }) - .cloned() - .collect::>(); - - for start_epoch in to_remove { - delegations.prev_ranges.remove(&start_epoch); - } + .retain(|_start, end| *end >= oldest_to_keep); Ok(()) } From e174eadf0f726cab39703ebd232a487349bb7262 Mon Sep 17 00:00:00 2001 From: brentstone Date: Wed, 17 Apr 2024 12:58:19 -0700 Subject: [PATCH 12/13] use unslashed bond amounts in `find_delegations` --- crates/proof_of_stake/src/lib.rs | 40 +++++++++++++++++++++++----- crates/proof_of_stake/src/queries.rs | 5 ++-- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/crates/proof_of_stake/src/lib.rs b/crates/proof_of_stake/src/lib.rs index a1881ef837..9bdfd66ebb 100644 --- a/crates/proof_of_stake/src/lib.rs +++ b/crates/proof_of_stake/src/lib.rs @@ -1587,18 +1587,15 @@ where commission_handle.set(storage, new_rate, current_epoch, params.pipeline_len) } -/// Get the total bond amount, including slashes, for a given bond ID and epoch. -/// Returns the bond amount after slashing. For future epochs the value is -/// subject to change. -pub fn bond_amount( +fn bond_amounts_for_query( storage: &S, + params: &PosParams, bond_id: &BondId, epoch: Epoch, -) -> namada_storage::Result +) -> namada_storage::Result> where S: StorageRead, { - let params = read_pos_params(storage)?; // Outer key is the start epoch used to calculate slashes. let mut amounts: BTreeMap = BTreeMap::default(); @@ -1702,6 +1699,37 @@ where } } } + Ok(amounts) +} + +/// Get the total bond amount, without applying slashes, for a given bond ID and +/// epoch. For future epochs, the value is subject to change. +pub fn raw_bond_amount( + storage: &S, + bond_id: &BondId, + epoch: Epoch, +) -> namada_storage::Result +where + S: StorageRead, +{ + let params = read_pos_params(storage)?; + let amounts = bond_amounts_for_query(storage, ¶ms, bond_id, epoch)?; + Ok(amounts.values().cloned().sum()) +} + +/// Get the total bond amount, including slashes, for a given bond ID and epoch. +/// Returns the bond amount after slashing. For future epochs, the value is +/// subject to change. +pub fn bond_amount( + storage: &S, + bond_id: &BondId, + epoch: Epoch, +) -> namada_storage::Result +where + S: StorageRead, +{ + let params = read_pos_params(storage)?; + let mut amounts = bond_amounts_for_query(storage, ¶ms, bond_id, epoch)?; if !amounts.is_empty() { let slashes = find_validator_slashes(storage, &bond_id.validator)?; diff --git a/crates/proof_of_stake/src/queries.rs b/crates/proof_of_stake/src/queries.rs index 12adc9d01a..5d643d2f48 100644 --- a/crates/proof_of_stake/src/queries.rs +++ b/crates/proof_of_stake/src/queries.rs @@ -20,7 +20,7 @@ use crate::types::{ BondDetails, BondId, BondsAndUnbondsDetail, BondsAndUnbondsDetails, DelegationEpochs, Slash, UnbondDetails, }; -use crate::{bond_amount, storage_key, PosParams}; +use crate::{raw_bond_amount, storage_key, PosParams}; /// Find all validators to which a given bond `owner` (or source) has a /// delegation @@ -101,8 +101,7 @@ where }, ) = validator?; - // TODO: Should use raw bonds or slashed stake?? - let bond_amount = bond_amount( + let bond_amount = raw_bond_amount( storage, &BondId { source: owner.clone(), From 4336b33af55a36bff898ea4aa0420864b434535b Mon Sep 17 00:00:00 2001 From: brentstone Date: Thu, 18 Apr 2024 11:00:17 -0700 Subject: [PATCH 13/13] better comment --- crates/proof_of_stake/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/proof_of_stake/src/lib.rs b/crates/proof_of_stake/src/lib.rs index 9bdfd66ebb..72ca2995ff 100644 --- a/crates/proof_of_stake/src/lib.rs +++ b/crates/proof_of_stake/src/lib.rs @@ -3018,8 +3018,9 @@ where bond_holders.insert(storage, validator.clone(), first_delegation)?; } - // Todo: possibly update the data by pruning old pairs in - // `delegations.prev_ranges`?? + // Only prune in `remove_delegation_target` to keep the operations lean. + // After all, `prev_ranges` only grows when `remove_delegation_target` is + // called. Ok(()) }