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

Remove usage of "unwrap" in the stake component #3011

Merged
merged 1 commit into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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