Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Optimize find delegation validators #3043

Merged
merged 13 commits into from
Apr 27, 2024
Prev Previous commit
Next Next commit
improve nomenclature and comments
  • Loading branch information
brentstone committed Apr 15, 2024
commit 14181abbe3b351ac4d0c7377eb3edec4c3932aed
2 changes: 1 addition & 1 deletion crates/apps/src/lib/node/ledger/shell/finalize_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions crates/benches/native_vps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions crates/benches/txs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
7 changes: 4 additions & 3 deletions crates/benches/vps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions crates/governance/src/storage/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
4 changes: 2 additions & 2 deletions crates/governance/src/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,11 @@ pub fn vote_proposal<S>(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())?;
}
Expand Down
6 changes: 3 additions & 3 deletions crates/governance/src/storage/proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Address>,
pub delegation_validators: Vec<Address>,
}

impl TryFrom<DefaultProposal> for InitProposalData {
Expand Down Expand Up @@ -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,
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/light_sdk/src/transaction/governance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ impl VoteProposal {
id,
vote,
voter,
delegations,
delegation_validators: delegations,
};

Self(transaction::build_tx(
Expand Down
47 changes: 18 additions & 29 deletions crates/namada/src/ledger/governance/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -302,19 +291,19 @@ 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, &current_epoch)
find_delegations(&self.ctx.pre(), voter, &current_epoch)
{
if delegations.is_empty() {
return Err(native_vp::Error::new_alloc(format!(
"No delegations found for {voter_address}"
))
.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)
})
Expand Down Expand Up @@ -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(
Expand All @@ -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 {
Expand Down Expand Up @@ -1096,20 +1085,20 @@ where
pub fn is_validator(
&self,
verifiers: &BTreeSet<Address>,
address: &Address,
delegation_address: &Address,
voter: &Address,
validator: &Address,
) -> Result<bool>
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.
Expand Down
7 changes: 4 additions & 3 deletions crates/proof_of_stake/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<S>(
storage: &S,
address: &Address,
Expand Down Expand Up @@ -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(());
}
Expand Down Expand Up @@ -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 \
Expand Down
2 changes: 1 addition & 1 deletion crates/sdk/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions crates/sdk/src/signing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand All @@ -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));
}
Expand Down
2 changes: 1 addition & 1 deletion crates/sdk/src/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2223,7 +2223,7 @@ pub async fn build_vote_proposal(
id: *proposal_id,
vote: proposal_vote,
voter: voter_address.clone(),
delegations,
delegation_validators,
};

build(
Expand Down