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

shielded-pool: delegate supply tracking to components #4020

Merged
merged 14 commits into from
Mar 15, 2024
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
6 changes: 3 additions & 3 deletions crates/core/app/src/action_handler/actions/submit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use penumbra_keys::keys::{FullViewingKey, NullifierKey};
use penumbra_proto::{DomainType, StateWriteProto as _};
use penumbra_sct::component::clock::EpochRead;
use penumbra_sct::component::tree::SctRead;
use penumbra_shielded_pool::component::SupplyWrite;
use penumbra_shielded_pool::component::AssetRegistry;
use penumbra_transaction::{AuthorizationData, Transaction, TransactionPlan, WitnessData};

use crate::action_handler::AppActionHandler;
Expand Down Expand Up @@ -276,12 +276,12 @@ impl AppActionHandler for ProposalSubmit {
// Register the denom for the voting proposal NFT
state
.register_denom(&ProposalNft::deposit(proposal_id).denom())
.await?;
.await;

// Register the denom for the vote receipt tokens
state
.register_denom(&VotingReceiptToken::new(proposal_id).denom())
.await?;
.await;

// Set the proposal state to voting (votes start immediately)
state.put_proposal_state(proposal_id, ProposalState::Voting);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use anyhow::Result;
use async_trait::async_trait;
use cnidarium::StateWrite;
use penumbra_proto::StateWriteProto as _;
use penumbra_shielded_pool::component::SupplyWrite;
use penumbra_shielded_pool::component::AssetRegistry;

use crate::action_handler::ActionHandler;
use crate::component::{StateReadExt as _, StateWriteExt as _};
Expand Down Expand Up @@ -60,7 +60,7 @@ impl ActionHandler for ProposalDepositClaim {
}
.denom(),
)
.await?;
.await;

// Set the proposal state to claimed
state.put_proposal_state(*proposal, ProposalState::Claimed { outcome });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use anyhow::Result;
use async_trait::async_trait;
use cnidarium::StateWrite;
use penumbra_proto::StateWriteProto as _;
use penumbra_shielded_pool::component::SupplyWrite;
use penumbra_shielded_pool::component::AssetRegistry;

use crate::{
action_handler::ActionHandler,
Expand Down Expand Up @@ -45,7 +45,7 @@ impl ActionHandler for ProposalWithdraw {
// Register the denom for the withdrawn proposal NFT
state
.register_denom(&ProposalNft::unbonding_deposit(*proposal).denom())
.await?;
.await;

state.record_proto(event::proposal_withdraw(self));

Expand Down
4 changes: 2 additions & 2 deletions crates/core/component/governance/src/component/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use penumbra_sct::{
component::{clock::EpochRead, tree::SctRead},
Nullifier,
};
use penumbra_shielded_pool::component::SupplyRead;
use penumbra_shielded_pool::component::AssetRegistryRead;
use penumbra_stake::{
component::{validator_handler::ValidatorDataRead, ConsensusIndexRead},
DelegationToken, GovernanceKey, IdentityKey,
Expand Down Expand Up @@ -272,7 +272,7 @@ pub trait StateReadExt: StateRead + penumbra_stake::StateReadExt {
/// Look up the validator for a given asset ID, if it is a delegation token.
async fn validator_by_delegation_asset(&self, asset_id: asset::Id) -> Result<IdentityKey> {
// Attempt to find the denom for the asset ID of the specified value
let Some(denom) = self.denom_by_asset(&asset_id).await? else {
let Some(denom) = self.denom_by_asset(&asset_id).await else {
anyhow::bail!("asset ID {} does not correspond to a known denom", asset_id);
};

Expand Down
4 changes: 2 additions & 2 deletions crates/core/component/shielded-pool/src/component.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
//! The Penumbra shielded pool [`Component`] and [`ActionHandler`] implementations.

mod action_handler;
mod assets;
mod metrics;
mod note_manager;
mod shielded_pool;
mod supply;
mod transfer;

pub use self::metrics::register_metrics;
pub use assets::{AssetRegistry, AssetRegistryRead};
pub use note_manager::NoteManager;
pub use shielded_pool::{ShieldedPool, StateReadExt, StateWriteExt};
pub use supply::{SupplyRead, SupplyWrite};
pub use transfer::Ics20Transfer;

pub mod rpc;
36 changes: 36 additions & 0 deletions crates/core/component/shielded-pool/src/component/assets.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
use async_trait::async_trait;
use cnidarium::{StateRead, StateWrite};
use penumbra_asset::asset::{self, Metadata};
use penumbra_proto::{StateReadProto, StateWriteProto};

use tracing::instrument;

use crate::state_key;

#[async_trait]
pub trait AssetRegistryRead: StateRead {
async fn denom_by_asset(&self, asset_id: &asset::Id) -> Option<Metadata> {
self.get(&state_key::denom_by_asset(asset_id))
.await
.expect("no deserialization error")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: Will this potentially fail for benign resigns we nevertheless want to throw upstream, e.g. wrong storage path? IDK what the convention here is though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, the failure mode for this is if we have a cache miss that hits storage and fails. The most likely reason this happens is some disk failure that cause storage degradation. Either way, it's irrecoverable for callers and the node should halt.

}
}

impl<T: StateRead + ?Sized> AssetRegistryRead for T {}

#[async_trait]
pub trait AssetRegistry: StateWrite {
/// Register a new asset present in the shielded pool.
/// If the asset is already registered, this is a no-op.
#[instrument(skip(self))]
async fn register_denom(&mut self, denom: &Metadata) {
let asset_id = denom.id();
tracing::debug!(?asset_id, "registering asset metadata in shielded pool");

if self.denom_by_asset(&asset_id).await.is_none() {
self.put(state_key::denom_by_asset(&asset_id), denom.clone());
}
}
}

impl<T: StateWrite + ?Sized> AssetRegistry for T {}
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ use tracing::instrument;
use crate::state_key;
use crate::{Note, NotePayload, Rseed};

use super::SupplyWrite;

/// Manages the addition of new notes to the chain state.
#[async_trait]
pub trait NoteManager: StateWrite {
Expand All @@ -39,7 +37,6 @@ pub trait NoteManager: StateWrite {
// Hashing the current SCT root would be sufficient, since it will
// change every time we insert a new note. But computing the SCT root
// is very slow, so instead we hash the current position.

let position: u64 = self
.get_sct()
.await
Expand All @@ -56,9 +53,6 @@ pub trait NoteManager: StateWrite {
.try_into()?;

let note = Note::from_parts(*address, value, Rseed(rseed_bytes))?;
// Now record the note and update the total supply:
self.increase_token_supply(&value.asset_id, value.amount)
.await?;
self.add_note_payload(note.payload(), source).await;

Ok(())
Expand Down
7 changes: 2 additions & 5 deletions crates/core/component/shielded-pool/src/component/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use penumbra_proto::core::component::shielded_pool::v1::{
use tonic::Status;
use tracing::instrument;

use super::SupplyRead;
use super::AssetRegistryRead;

// TODO: Hide this and only expose a Router?
pub struct Server {
Expand Down Expand Up @@ -36,10 +36,7 @@ impl QueryService for Server {
.try_into()
.map_err(|e| Status::invalid_argument(format!("could not parse asset_id: {e}")))?;

let denom = state
.denom_by_asset(&id)
.await
.map_err(|e| Status::internal(e.to_string()))?;
let denom = state.denom_by_asset(&id).await;

let rsp = match denom {
Some(denom) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use penumbra_sct::CommitmentSource;
use tendermint::v0_37::abci;
use tracing::instrument;

use super::{NoteManager, SupplyWrite};
use super::{AssetRegistry, NoteManager};

pub struct ShieldedPool {}

Expand Down Expand Up @@ -41,10 +41,7 @@ impl Component for ShieldedPool {
"Genesis allocations contain empty note",
);

state
.register_denom(&allocation.denom())
.await
.expect("able to register denom for genesis allocation");
state.register_denom(&allocation.denom()).await;
state
.mint_note(
allocation.value(),
Expand Down
97 changes: 0 additions & 97 deletions crates/core/component/shielded-pool/src/component/supply.rs

This file was deleted.

12 changes: 2 additions & 10 deletions crates/core/component/shielded-pool/src/component/transfer.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::str::FromStr;

use crate::{
component::{NoteManager, SupplyWrite},
component::{AssetRegistry, NoteManager},
Ics20Withdrawal,
};
use anyhow::{Context, Result};
Expand Down Expand Up @@ -137,11 +137,6 @@ pub trait Ics20TransferWriteExt: StateWrite {
state_key::ics20_value_balance(&withdrawal.source_channel, &withdrawal.denom.id()),
new_value_balance,
);

// update supply tracking of burned note
self.decrease_token_supply(&withdrawal.denom.id(), withdrawal.amount)
.await
.expect("couldn't update token supply in ics20 withdrawal!");
}

self.send_packet_execute(checked_packet).await;
Expand Down Expand Up @@ -364,10 +359,7 @@ async fn recv_transfer_packet_inner<S: StateWrite>(
.as_str()
.try_into()
.context("unable to parse denom in ics20 transfer as DenomMetadata")?;
state
.register_denom(&denom)
.await
.context("unable to register denom in ics20 transfer")?;
state.register_denom(&denom).await;

let value = Value {
amount: receiver_amount,
Expand Down
8 changes: 0 additions & 8 deletions crates/core/component/shielded-pool/src/state_key.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,6 @@
use penumbra_asset::asset;
use std::string::String;

pub fn token_supply(asset_id: &asset::Id) -> String {
format!("shielded_pool/assets/{asset_id}/token_supply")
}

pub fn known_assets() -> &'static str {
"shielded_pool/known_assets"
}

pub fn denom_by_asset(asset_id: &asset::Id) -> String {
format!("shielded_pool/assets/{asset_id}/denom")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use anyhow::{ensure, Result};
use async_trait::async_trait;
use cnidarium::StateWrite;
use penumbra_sct::component::clock::EpochRead;
use penumbra_shielded_pool::component::SupplyWrite;
use penumbra_shielded_pool::component::AssetRegistry;

use crate::{
component::action_handler::ActionHandler,
Expand Down Expand Up @@ -66,9 +66,7 @@ impl ActionHandler for Undelegate {
/* ----- execution ------ */

// Register the undelegation's denom, so clients can look it up later.
state
.register_denom(&self.unbonding_token().denom())
.await?;
state.register_denom(&self.unbonding_token().denom()).await;

tracing::debug!(?self, "queuing undelegation for next epoch");
state.push_undelegation(self.clone());
Expand Down
Loading
Loading