Skip to content

Commit

Permalink
Remove usage of "unwrap" in the stake component
Browse files Browse the repository at this point in the history
  • Loading branch information
zbuc committed Sep 13, 2023
1 parent cc784a2 commit 01f2262
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -93,16 +93,21 @@ impl ActionHandler for validator::Definition {
async fn execute<S: StateWrite>(&self, mut state: S) -> Result<()> {
let v = self;

let cur_epoch = state.get_current_epoch().await.unwrap();
let cur_epoch = state
.get_current_epoch()
.await
.context("should be able to get current epoch during validator definition execution")?;

if state
.validator(&v.validator.identity_key)
.await
.unwrap()
.context("should be able to fetch validator during validator definition execution")?
.is_some()
{
// This is an existing validator definition.
state.update_validator(v.validator.clone()).await.unwrap();
state.update_validator(v.validator.clone()).await.context(
"should be able to update validator during validator definition execution",
)?;
} else {
// This is a new validator definition.
// Set the default rates and state.
Expand All @@ -127,7 +132,7 @@ impl ActionHandler for validator::Definition {
state
.add_validator(v.validator.clone(), cur_rate_data, next_rate_data)
.await
.unwrap();
.context("should be able to add validator during validator definition execution")?;
}

Ok(())
Expand Down
97 changes: 71 additions & 26 deletions crates/core/component/stake/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::{
};

use ::metrics::{decrement_gauge, gauge, increment_gauge};
use anyhow::{anyhow, Result};
use anyhow::{anyhow, Context, Result};
use async_trait::async_trait;
use futures::{FutureExt, StreamExt, TryFutureExt, TryStreamExt};
use penumbra_asset::{Value, STAKING_TOKEN_ASSET_ID};
Expand Down Expand Up @@ -57,7 +57,7 @@ fn validator_address(ck: &PublicKey) -> [u8; 20] {
let ck_bytes = ck.to_bytes();
let addr: [u8; 20] = Sha256::digest(&ck_bytes).as_slice()[0..20]
.try_into()
.unwrap();
.expect("Sha256 digest should be 20-bytes long");

addr
}
Expand Down Expand Up @@ -298,7 +298,13 @@ pub(crate) trait StakingImpl: StateWriteExt {
let end_height = self.get_block_height().await?;

for height in epoch_to_end.start_height..=end_height {
let changes = self.delegation_changes(height.try_into().unwrap()).await?;
let changes = self
.delegation_changes(
height
.try_into()
.context("should be able to convert u64 into block height")?,
)
.await?;
for d in changes.delegations {
delegations_by_validator
.entry(d.validator_identity.clone())
Expand Down Expand Up @@ -495,8 +501,14 @@ pub(crate) trait StakingImpl: StateWriteExt {
let mut zero_power = Vec::new();

for v in self.validator_identity_list().await? {
let state = self.validator_state(&v).await?.unwrap();
let power = self.validator_power(&v).await?.unwrap();
let state = self
.validator_state(&v)
.await?
.context("should be able to fetch validator state")?;
let power = self
.validator_power(&v)
.await?
.context("should be able to fetch validator power")?;
if matches!(state, validator::State::Active | validator::State::Inactive) {
if power == 0 {
zero_power.push((v, power));
Expand Down Expand Up @@ -536,7 +548,10 @@ pub(crate) trait StakingImpl: StateWriteExt {
let current_epoch = self.get_current_epoch().await?;

for v in self.validator_identity_list().await? {
let state = self.validator_bonding_state(&v).await?.unwrap();
let state = self
.validator_bonding_state(&v)
.await?
.context("should be able to fetch validator bonding state")?;
if let validator::BondingState::Unbonding { unbonding_epoch } = state {
if unbonding_epoch <= current_epoch.index {
self.set_validator_bonding_state(&v, validator::BondingState::Unbonded)
Expand Down Expand Up @@ -617,11 +632,15 @@ pub(crate) trait StakingImpl: StateWriteExt {
// Save the validator updates to send to Tendermint.
let tendermint_validator_updates = voting_power_by_consensus_key
.iter()
.map(|(ck, power)| Update {
pub_key: *ck,
power: (*power).try_into().unwrap(),
.map(|(ck, power)| {
Ok(Update {
pub_key: *ck,
power: (*power)
.try_into()
.context("should be able to convert u64 into validator Power")?,
})
})
.collect();
.collect::<Result<Vec<_>>>()?;
self.put_tendermint_validator_updates(tendermint_validator_updates);

// Record the new consensus keys we will have told tendermint about.
Expand Down Expand Up @@ -694,9 +713,8 @@ pub(crate) trait StakingImpl: StateWriteExt {
while let Some(data) = js.join_next().await.transpose()? {
if let Some((identity_key, consensus_key, mut uptime)) = data? {
// for some reason last_commit_info has truncated sha256 hashes
let addr: [u8; 20] = Sha256::digest(&consensus_key.to_bytes()).as_slice()[0..20]
.try_into()
.unwrap();
let addr: [u8; 20] =
Sha256::digest(&consensus_key.to_bytes()).as_slice()[0..20].try_into()?;

let voted = did_address_vote
.get(&addr)
Expand All @@ -715,7 +733,7 @@ pub(crate) trait StakingImpl: StateWriteExt {
);
gauge!(metrics::MISSED_BLOCKS, uptime.num_missed_blocks() as f64, "identity_key" => identity_key.to_string());

uptime.mark_height_as_signed(height, voted).unwrap();
uptime.mark_height_as_signed(height, voted)?;
if uptime.num_missed_blocks() as u64 >= params.missed_blocks_maximum {
self.set_validator_state(&identity_key, validator::State::Jailed)
.await?;
Expand Down Expand Up @@ -875,8 +893,14 @@ impl Component for Staking {

#[instrument(name = "staking", skip(state, app_state))]
async fn init_chain<S: StateWrite>(mut state: S, app_state: &genesis::AppState) {
let starting_height = state.get_block_height().await.unwrap();
let starting_epoch = state.epoch_by_height(starting_height).await.unwrap();
let starting_height = state
.get_block_height()
.await
.expect("should be able to get initial block height");
let starting_epoch = state
.epoch_by_height(starting_height)
.await
.expect("should be able to get initial epoch");
let epoch_index = starting_epoch.index;

// Delegations require knowing the rates for the next epoch, so
Expand Down Expand Up @@ -909,18 +933,24 @@ impl Component for Staking {
// and there is a separate key containing the list of all validator keys.
for validator in &app_state.validators {
// Parse the proto into a domain type.
let validator = Validator::try_from(validator.clone()).unwrap();
let validator = Validator::try_from(validator.clone())
.expect("should be able to parse genesis validator");

state
.add_genesis_validator(&genesis_allocations, &genesis_base_rate, validator)
.await
.unwrap();
.expect("should be able to add genesis validator to state");
}

// Finally, record that there were no delegations in this block, so the data
// isn't missing when we process the first epoch transition.
state
.set_delegation_changes(starting_height.try_into().unwrap(), Default::default())
.set_delegation_changes(
starting_height
.try_into()
.expect("should be able to convert u64 into block height"),
Default::default(),
)
.await;

// Build the initial validator set update.
Expand All @@ -929,7 +959,10 @@ impl Component for Staking {
state_key::current_consensus_keys().to_owned(),
CurrentConsensusKeys::default(),
);
state.build_tendermint_validator_updates().await.unwrap();
state
.build_tendermint_validator_updates()
.await
.expect("should be able to build initial tendermint validator updates");
}

#[instrument(name = "staking", skip(state, begin_block))]
Expand All @@ -941,13 +974,16 @@ impl Component for Staking {
// For each validator identified as byzantine by tendermint, update its
// state to be slashed
for evidence in begin_block.byzantine_validators.iter() {
state.process_evidence(evidence).await.unwrap();
state
.process_evidence(evidence)
.await
.expect("should be able to process tendermint ABCI evidence");
}

state
.track_uptime(&begin_block.last_commit_info)
.await
.unwrap();
.expect("should be able to track uptime");
}

#[instrument(name = "staking", skip(state, end_block))]
Expand All @@ -959,20 +995,29 @@ impl Component for Staking {
// Write the delegation changes for this block.
state
.set_delegation_changes(
end_block.height.try_into().unwrap(),
end_block
.height
.try_into()
.expect("should be able to convert i64 into block height"),
state.stub_delegation_changes().clone(),
)
.await;
}

#[instrument(name = "staking", skip(state))]
async fn end_epoch<S: StateWrite + 'static>(state: &mut Arc<S>) -> anyhow::Result<()> {
let state = Arc::get_mut(state).expect("state should be unique");
let cur_epoch = state.get_current_epoch().await.unwrap();
let state = Arc::get_mut(state).context("state should be unique")?;
let cur_epoch = state
.get_current_epoch()
.await
.context("should be able to get current epoch during end_epoch")?;
state.end_epoch(cur_epoch).await?;
// Since we only update the validator set at epoch boundaries,
// we only need to build the validator set updates here in end_epoch.
state.build_tendermint_validator_updates().await.unwrap();
state
.build_tendermint_validator_updates()
.await
.context("should be able to build tendermint validator updates")?;
Ok(())
}
}
Expand Down
1 change: 1 addition & 0 deletions crates/core/component/stake/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![cfg_attr(docsrs, feature(doc_cfg))]
#![deny(clippy::unwrap_used)]
#![allow(clippy::clone_on_copy)]

mod changes;
Expand Down
10 changes: 7 additions & 3 deletions crates/core/component/stake/src/penalty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,13 @@ impl Penalty {

/// Apply this `Penalty` to an `Amount` of unbonding tokens.
pub fn apply_to(&self, amount: Amount) -> Amount {
let penalized_amount =
(u128::try_from(amount).unwrap()) * (1_0000_0000 - self.0 as u128) / 1_0000_0000;
Amount::try_from(u64::try_from(penalized_amount).unwrap()).unwrap()
let penalized_amount = (u128::try_from(amount).expect("amount should be a valid u128"))
* (1_0000_0000 - self.0 as u128)
/ 1_0000_0000;
Amount::try_from(
u64::try_from(penalized_amount).expect("penalized amount should fit in u64"),
)
.expect("all u64 values should be valid Amounts")
}

/// Helper method to compute the effect of an UndelegateClaim on the
Expand Down
6 changes: 3 additions & 3 deletions crates/core/component/stake/src/rate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ impl RateData {
// upconvert to u128 intermediates and panic if the result is too large (unlikely)
((unbonded_amount as u128 * 1_0000_0000) / self.validator_exchange_rate as u128)
.try_into()
.unwrap()
.expect("delegation amount should fit in u128")
}

pub fn slash(&self, penalty: Penalty) -> Self {
Expand All @@ -107,7 +107,7 @@ impl RateData {
u64::try_from(
(self.validator_exchange_rate as u128 * penalty.0 as u128) / 1_0000_0000,
)
.unwrap(),
.expect("penalty should fit in u64"),
);

slashed
Expand Down Expand Up @@ -140,7 +140,7 @@ impl RateData {
((total_delegation_tokens * self.validator_exchange_rate as u128)
/ base_rate_data.base_exchange_rate as u128)
.try_into()
.unwrap()
.expect("voting power should fit in u64")
}

/// Uses this `RateData` to build a `Delegate` transaction action that
Expand Down
17 changes: 14 additions & 3 deletions crates/core/component/stake/src/undelegate_claim/proof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,20 @@ impl UndelegateClaimProof {
Proof::deserialize_compressed_unchecked(&self.0[..]).map_err(|e| anyhow::anyhow!(e))?;

let mut public_inputs = Vec::new();
public_inputs.extend(balance_commitment.0.to_field_elements().unwrap());
public_inputs.extend(unbonding_id.0.to_field_elements().unwrap());
public_inputs.extend(penalty.to_field_elements().unwrap());
public_inputs.extend(
balance_commitment
.0
.to_field_elements()
.ok_or(anyhow::anyhow!(
"could not convert balance commitment to field elements"
))?,
);
public_inputs.extend(unbonding_id.0.to_field_elements().ok_or(anyhow::anyhow!(
"could not convert unbonding id to field elements"
))?);
public_inputs.extend(penalty.to_field_elements().ok_or(anyhow::anyhow!(
"could not convert penalty to field elements"
))?);

tracing::trace!(?public_inputs);
let start = std::time::Instant::now();
Expand Down

0 comments on commit 01f2262

Please sign in to comment.