From ae5178366b264a4e8b24be500d5cccbdbc35943e Mon Sep 17 00:00:00 2001 From: Henry de Valence Date: Sat, 10 Aug 2024 21:37:08 -0400 Subject: [PATCH 1/4] pcli: add un-advertised support for importing Prax registry data This makes a minimal set of changes to allow `pcli` to make use of the Prax registry. In order to not pre-empt more comprehensive planning about how such an integration should work, this PR just changes `pcli` so that if the registry is placed in a `registry.json` file in the `pcli` home directory, it will be imported into the view database on startup. --- crates/bin/pcli/src/opt.rs | 9 +++++ ...p_can_sweep_a_collection_of_small_notes.rs | 1 + .../view_server_can_be_served_on_localhost.rs | 1 + crates/view/src/service.rs | 5 +++ crates/view/src/storage.rs | 33 +++++++++++++++++++ 5 files changed, 49 insertions(+) diff --git a/crates/bin/pcli/src/opt.rs b/crates/bin/pcli/src/opt.rs index d55e3ca2d6..ca4f644f01 100644 --- a/crates/bin/pcli/src/opt.rs +++ b/crates/bin/pcli/src/opt.rs @@ -155,8 +155,17 @@ impl Opt { let path = self.home.join(crate::VIEW_FILE_NAME); tracing::info!(%path, "using local view service"); + let registry_path = self.home.join("registry.json"); + // Check if the path exists or set it to nojne + let registry_path = if registry_path.exists() { + Some(registry_path) + } else { + None + }; + let svc = ViewServer::load_or_initialize( Some(path), + registry_path, &config.full_viewing_key, config.grpc_url.clone(), ) diff --git a/crates/core/app/tests/app_can_sweep_a_collection_of_small_notes.rs b/crates/core/app/tests/app_can_sweep_a_collection_of_small_notes.rs index e9909e610c..c182a4cca5 100644 --- a/crates/core/app/tests/app_can_sweep_a_collection_of_small_notes.rs +++ b/crates/core/app/tests/app_can_sweep_a_collection_of_small_notes.rs @@ -115,6 +115,7 @@ async fn app_can_sweep_a_collection_of_small_notes() -> anyhow::Result<()> { // Spawn the client-side view server... let view_server = { penumbra_view::ViewServer::load_or_initialize( + None::<&camino::Utf8Path>, None::<&camino::Utf8Path>, &*test_keys::FULL_VIEWING_KEY, grpc_url, diff --git a/crates/core/app/tests/view_server_can_be_served_on_localhost.rs b/crates/core/app/tests/view_server_can_be_served_on_localhost.rs index de8922a6ac..1ba83aabc7 100644 --- a/crates/core/app/tests/view_server_can_be_served_on_localhost.rs +++ b/crates/core/app/tests/view_server_can_be_served_on_localhost.rs @@ -91,6 +91,7 @@ async fn view_server_can_be_served_on_localhost() -> anyhow::Result<()> { // Spawn the client-side view server... let view_server = { penumbra_view::ViewServer::load_or_initialize( + None::<&camino::Utf8Path>, None::<&camino::Utf8Path>, &*test_keys::FULL_VIEWING_KEY, grpc_url, diff --git a/crates/view/src/service.rs b/crates/view/src/service.rs index 7d85e9fdbe..5c43e86c1d 100644 --- a/crates/view/src/service.rs +++ b/crates/view/src/service.rs @@ -100,6 +100,7 @@ impl ViewServer { )] pub async fn load_or_initialize( storage_path: Option>, + registry_path: Option>, fvk: &FullViewingKey, node: Url, ) -> anyhow::Result { @@ -108,6 +109,10 @@ impl ViewServer { .await? .tap(|_| tracing::debug!("storage is ready")); + if let Some(registry_path) = registry_path { + storage.load_asset_metadata(registry_path).await?; + } + Self::new(storage, node) .tap(|_| tracing::trace!("constructing view server")) .await diff --git a/crates/view/src/storage.rs b/crates/view/src/storage.rs index 701c32dd92..c59836409f 100644 --- a/crates/view/src/storage.rs +++ b/crates/view/src/storage.rs @@ -255,6 +255,36 @@ impl Storage { .await? } + /// Loads asset metadata from a JSON file and use to update the database. + pub async fn load_asset_metadata( + &self, + registry_path: impl AsRef, + ) -> anyhow::Result<()> { + tracing::debug!(registry_path = ?registry_path.as_ref(), "loading asset metadata"); + let registry_path = registry_path.as_ref(); + // Parse into a serde_json::Value first so we can get the bits we care about + let mut registry_json: serde_json::Value = serde_json::from_str( + std::fs::read_to_string(registry_path) + .context("failed to read file")? + .as_str(), + ) + .context("failed to parse JSON")?; + + let registry: BTreeMap = serde_json::value::from_value( + registry_json + .get_mut("assetById") + .ok_or_else(|| anyhow::anyhow!("missing assetById"))? + .take(), + ) + .context("could not parse asset registry")?; + + for metadata in registry.into_values() { + self.record_asset(metadata).await?; + } + + Ok(()) + } + /// Query for account balance by address pub async fn balances( &self, @@ -1030,7 +1060,10 @@ impl Storage { }).await? } + #[tracing::instrument(skip(self))] pub async fn record_asset(&self, asset: Metadata) -> anyhow::Result<()> { + tracing::debug!(?asset); + let asset_id = asset.id().to_bytes().to_vec(); let denom = asset.base_denom().denom; From 671b5765f4d64d22ea08b4b19bc2bc22770ab36a Mon Sep 17 00:00:00 2001 From: Henry de Valence Date: Sun, 11 Aug 2024 12:45:36 -0400 Subject: [PATCH 2/4] asset: don't rely on metadata ordering assumptions --- crates/core/asset/src/asset/denom_metadata.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/core/asset/src/asset/denom_metadata.rs b/crates/core/asset/src/asset/denom_metadata.rs index b2c957ad85..ae998be584 100644 --- a/crates/core/asset/src/asset/denom_metadata.rs +++ b/crates/core/asset/src/asset/denom_metadata.rs @@ -317,16 +317,19 @@ impl Metadata { if amount == 0u64.into() { return self.default_unit(); } + let mut selected_index = 0; + let mut selected_exponent = 0; for (unit_index, unit) in self.inner.units.iter().enumerate() { let unit_amount = Amount::from(10u128.pow(unit.exponent as u32)); - if amount >= unit_amount { - return Unit { - unit_index, - inner: self.inner.clone(), - }; + if unit_amount <= amount && unit.exponent >= selected_exponent { + selected_index = unit_index; + selected_exponent = unit.exponent; } } - self.base_unit() + return Unit { + unit_index: selected_index, + inner: self.inner.clone(), + }; } pub fn starts_with(&self, prefix: &str) -> bool { From c39ddc39bf179a0bfca71916770f82a098d666ef Mon Sep 17 00:00:00 2001 From: Henry de Valence Date: Sun, 11 Aug 2024 13:01:07 -0400 Subject: [PATCH 3/4] view: correctly store denom metadata --- crates/view/src/storage.rs | 33 +++++++++++------------------- crates/view/src/storage/schema.sql | 3 ++- 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/crates/view/src/storage.rs b/crates/view/src/storage.rs index c59836409f..8da8a3d74c 100644 --- a/crates/view/src/storage.rs +++ b/crates/view/src/storage.rs @@ -823,14 +823,10 @@ impl Storage { spawn_blocking(move || { pool.get()? - .prepare_cached("SELECT * FROM assets")? + .prepare_cached("SELECT metadata FROM assets")? .query_and_then([], |row| { - let _asset_id: Vec = row.get("asset_id")?; - let denom: String = row.get("denom")?; - - let denom_metadata = asset::REGISTRY - .parse_denom(&denom) - .ok_or_else(|| anyhow::anyhow!("invalid denomination {}", denom))?; + let metadata_json = row.get::<_, String>("metadata")?; + let denom_metadata = serde_json::from_str(&metadata_json)?; anyhow::Ok(denom_metadata) })? @@ -846,13 +842,10 @@ impl Storage { spawn_blocking(move || { pool.get()? - .prepare_cached("SELECT * FROM assets WHERE asset_id = ?1")? + .prepare_cached("SELECT metadata FROM assets WHERE asset_id = ?1")? .query_and_then([id], |row| { - let _asset_id: Vec = row.get("asset_id")?; - let denom: String = row.get("denom")?; - let denom_metadata = asset::REGISTRY - .parse_denom(&denom) - .ok_or_else(|| anyhow::anyhow!("invalid denomination {}", denom))?; + let metadata_json = row.get::<_, String>("metadata")?; + let denom_metadata = serde_json::from_str(&metadata_json)?; anyhow::Ok(denom_metadata) })? .next() @@ -870,13 +863,10 @@ impl Storage { spawn_blocking(move || { pool.get()? - .prepare_cached("SELECT * FROM assets WHERE denom LIKE ?1 ESCAPE '\\'")? + .prepare_cached("SELECT metadata FROM assets WHERE denom LIKE ?1 ESCAPE '\\'")? .query_and_then([pattern], |row| { - let _asset_id: Vec = row.get("asset_id")?; - let denom: String = row.get("denom")?; - let denom_metadata = asset::REGISTRY - .parse_denom(&denom) - .ok_or_else(|| anyhow::anyhow!("invalid denomination {}", denom))?; + let metadata_json = row.get::<_, String>("metadata")?; + let denom_metadata = serde_json::from_str(&metadata_json)?; anyhow::Ok(denom_metadata) })? .collect() @@ -1066,14 +1056,15 @@ impl Storage { let asset_id = asset.id().to_bytes().to_vec(); let denom = asset.base_denom().denom; + let metadata_json = serde_json::to_string(&asset)?; let pool = self.pool.clone(); spawn_blocking(move || { pool.get()? .execute( - "INSERT OR IGNORE INTO assets (asset_id, denom) VALUES (?1, ?2)", - (asset_id, denom), + "INSERT OR REPLACE INTO assets (asset_id, denom, metadata) VALUES (?1, ?2, ?3)", + (asset_id, denom, metadata_json), ) .map_err(anyhow::Error::from) }) diff --git a/crates/view/src/storage/schema.sql b/crates/view/src/storage/schema.sql index fc967e2e4e..f50e6d12f8 100644 --- a/crates/view/src/storage/schema.sql +++ b/crates/view/src/storage/schema.sql @@ -15,7 +15,8 @@ CREATE TABLE sync_height (height BIGINT NOT NULL); -- used for storing a cache of known assets CREATE TABLE assets ( asset_id BLOB PRIMARY KEY NOT NULL, - denom TEXT NOT NULL + denom TEXT NOT NULL, + metadata TEXT NOT NULL ); -- the shape information about the sct From bfd72276ac0dff70108e6c68b68ac1a1a364c670 Mon Sep 17 00:00:00 2001 From: Henry de Valence Date: Sun, 11 Aug 2024 13:02:14 -0400 Subject: [PATCH 4/4] view: improve span handling --- crates/core/asset/src/asset/denom_metadata.rs | 1 + crates/view/src/service.rs | 16 ++++++---------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/crates/core/asset/src/asset/denom_metadata.rs b/crates/core/asset/src/asset/denom_metadata.rs index ae998be584..8b2b489f59 100644 --- a/crates/core/asset/src/asset/denom_metadata.rs +++ b/crates/core/asset/src/asset/denom_metadata.rs @@ -30,6 +30,7 @@ pub struct Metadata { } // These are constructed by the asset registry. +#[derive(Debug)] pub(super) struct Inner { // The Penumbra asset ID id: Id, diff --git a/crates/view/src/service.rs b/crates/view/src/service.rs index 5c43e86c1d..85ceeea841 100644 --- a/crates/view/src/service.rs +++ b/crates/view/src/service.rs @@ -16,7 +16,7 @@ use tap::{Tap, TapFallible}; use tokio::sync::{watch, RwLock}; use tokio_stream::wrappers::WatchStream; use tonic::{async_trait, transport::Channel, Request, Response, Status}; -use tracing::instrument; +use tracing::{instrument, Instrument}; use url::Url; use penumbra_asset::{asset, asset::Metadata, Value}; @@ -91,13 +91,6 @@ pub struct ViewServer { impl ViewServer { /// Convenience method that calls [`Storage::load_or_initialize`] and then [`Self::new`]. - #[instrument( - skip_all, - fields( - path = ?storage_path.as_ref().map(|p| p.as_ref().as_str()), - url = %node, - ) - )] pub async fn load_or_initialize( storage_path: Option>, registry_path: Option>, @@ -126,22 +119,25 @@ impl ViewServer { /// To create multiple [`ViewService`]s, clone the [`ViewService`] returned /// by this method, rather than calling it multiple times. That way, each clone /// will be backed by the same scanning task, rather than each spawning its own. - #[instrument(skip_all)] pub async fn new(storage: Storage, node: Url) -> anyhow::Result { + let span = tracing::error_span!(parent: None, "view"); let channel = Channel::from_shared(node.to_string()) .with_context(|| "could not parse node URI")? .connect() + .instrument(span.clone()) .await .with_context(|| "could not connect to grpc server") .tap_err(|error| tracing::error!(?error, "could not connect to grpc server"))?; let (worker, state_commitment_tree, error_slot, sync_height_rx) = Worker::new(storage.clone(), channel) + .instrument(span.clone()) .tap(|_| tracing::trace!("constructing view server worker")) .await? .tap(|_| tracing::debug!("constructed view server worker")); - tokio::spawn(worker.run()).tap(|_| tracing::debug!("spawned view server worker")); + tokio::spawn(worker.run().instrument(span)) + .tap(|_| tracing::debug!("spawned view server worker")); Ok(Self { storage,