From 3440ead761530bf917ee8f8963f628563d7d0e6a Mon Sep 17 00:00:00 2001 From: cryptoAtwill <108330426+cryptoAtwill@users.noreply.github.com> Date: Wed, 8 May 2024 17:51:07 +0800 Subject: [PATCH] Better logging of checkpoint and its associated structs (#743) Co-authored-by: Akosh Farkash --- Cargo.lock | 2 + ipc/api/Cargo.toml | 2 + ipc/api/src/address.rs | 22 +++++ ipc/api/src/checkpoint.rs | 70 +++++++++++++++- ipc/api/src/cross.rs | 7 +- ipc/api/src/lib.rs | 83 +++++++++++++++++++ ipc/api/src/subnet_id.rs | 4 + .../commands/checkpoint/bottomup_bundles.rs | 12 +-- .../commands/checkpoint/list_checkpoints.rs | 35 -------- ipc/cli/src/commands/checkpoint/mod.rs | 6 -- ipc/provider/src/checkpoint.rs | 11 ++- ipc/provider/src/lib.rs | 7 +- ipc/provider/src/manager/evm/manager.rs | 11 ++- ipc/provider/src/manager/subnet.rs | 5 +- ipc/wallet/src/evm/mod.rs | 7 +- 15 files changed, 224 insertions(+), 60 deletions(-) delete mode 100644 ipc/cli/src/commands/checkpoint/list_checkpoints.rs diff --git a/Cargo.lock b/Cargo.lock index 0ea864e19..2fdee3f5e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4993,8 +4993,10 @@ dependencies = [ "serde", "serde_json", "serde_tuple", + "serde_with 2.3.3", "strum 0.26.1", "thiserror", + "tracing", ] [[package]] diff --git a/ipc/api/Cargo.toml b/ipc/api/Cargo.toml index 011c8e99d..9cbcdceb1 100644 --- a/ipc/api/Cargo.toml +++ b/ipc/api/Cargo.toml @@ -26,6 +26,8 @@ serde_tuple = { workspace = true } strum = { workspace = true } thiserror = { workspace = true } ethers = { workspace = true } +tracing = { workspace = true } +serde_with = { workspace = true, features = ["hex"] } ipc_actors_abis = { workspace = true } ipc-types = { workspace = true } diff --git a/ipc/api/src/address.rs b/ipc/api/src/address.rs index cfdac0b30..7985d13bc 100644 --- a/ipc/api/src/address.rs +++ b/ipc/api/src/address.rs @@ -2,7 +2,9 @@ // SPDX-License-Identifier: MIT use crate::error::Error; use crate::subnet_id::SubnetID; +use crate::{deserialize_human_readable_str, HumanReadable}; use fvm_shared::address::{Address, Protocol}; +use serde::ser::Error as SerializeError; use serde_tuple::{Deserialize_tuple, Serialize_tuple}; use std::{fmt, str::FromStr}; @@ -98,6 +100,26 @@ impl FromStr for IPCAddress { } } +impl serde_with::SerializeAs for HumanReadable { + fn serialize_as(address: &IPCAddress, serializer: S) -> Result + where + S: serde::Serializer, + { + if serializer.is_human_readable() { + address + .to_string() + .map_err(|e| { + S::Error::custom(format!("cannot convert ipc address to string: {e}")) + })? + .serialize(serializer) + } else { + address.serialize(serializer) + } + } +} + +deserialize_human_readable_str!(IPCAddress); + #[cfg(test)] mod tests { use crate::address::IPCAddress; diff --git a/ipc/api/src/checkpoint.rs b/ipc/api/src/checkpoint.rs index 0f2dd6683..88f41f891 100644 --- a/ipc/api/src/checkpoint.rs +++ b/ipc/api/src/checkpoint.rs @@ -4,6 +4,7 @@ use crate::cross::IpcEnvelope; use crate::subnet_id::SubnetID; +use crate::HumanReadable; use cid::multihash::Code; use cid::multihash::MultihashDigest; use cid::Cid; @@ -13,7 +14,9 @@ use fvm_shared::address::Address; use fvm_shared::clock::ChainEpoch; use fvm_shared::econ::TokenAmount; use lazy_static::lazy_static; -use serde::{Deserialize, Serialize}; +use serde::ser::SerializeSeq; +use serde::{Deserialize, Serialize, Serializer}; +use serde_with::serde_as; use std::fmt::{Display, Formatter}; lazy_static! { @@ -49,19 +52,23 @@ impl Display for QuorumReachedEvent { } /// The collection of items for the bottom up checkpoint submission +#[serde_as] #[derive(PartialEq, Eq, Clone, Debug, Serialize, Deserialize)] pub struct BottomUpCheckpointBundle { pub checkpoint: BottomUpCheckpoint, /// The list of signatures that have signed the checkpoint hash + #[serde_as(as = "Vec")] pub signatures: Vec, /// The list of addresses that have signed the checkpoint hash pub signatories: Vec
, } /// The collection of items for the bottom up checkpoint submission +#[serde_as] #[derive(PartialEq, Eq, Clone, Debug, Serialize, Deserialize)] pub struct BottomUpMsgBatch { /// Child subnet ID, for replay protection from other subnets where the exact same validators operate + #[serde_as(as = "HumanReadable")] pub subnet_id: SubnetID, /// The height of the child subnet at which the batch was cut pub block_height: ChainEpoch, @@ -69,6 +76,7 @@ pub struct BottomUpMsgBatch { pub msgs: Vec, } +#[serde_as] #[derive(PartialEq, Eq, Clone, Debug, Serialize, Deserialize)] pub struct BottomUpCheckpoint { /// Child subnet ID, for replay protection from other subnets where the exact same validators operate. @@ -78,6 +86,7 @@ pub struct BottomUpCheckpoint { /// Has to follow the previous checkpoint by checkpoint period. pub block_height: ChainEpoch, /// The hash of the block. + #[serde_as(as = "HumanReadable")] pub block_hash: Vec, /// The number of the membership (validator set) which is going to sign the next checkpoint. /// This one expected to be signed by the validators from the membership reported in the previous checkpoint. @@ -86,3 +95,62 @@ pub struct BottomUpCheckpoint { /// The list of messages for execution pub msgs: Vec, } + +pub fn serialize_vec_bytes_to_vec_hex, S>( + data: &[T], + s: S, +) -> Result +where + S: Serializer, +{ + let mut seq = s.serialize_seq(Some(data.len()))?; + for element in data { + seq.serialize_element(&hex::encode(element))?; + } + seq.end() +} + +#[cfg(test)] +mod tests { + use crate::address::IPCAddress; + use crate::checkpoint::Signature; + use crate::subnet_id::SubnetID; + use crate::HumanReadable; + use fvm_shared::address::Address; + use serde::{Deserialize, Serialize}; + use serde_with::serde_as; + use std::str::FromStr; + + #[test] + fn test_serialization_vec_vec_u8() { + #[serde_as] + #[derive(Debug, Serialize, Deserialize, PartialEq)] + struct T { + #[serde_as(as = "Vec")] + d: Vec, + #[serde_as(as = "HumanReadable")] + subnet_id: SubnetID, + #[serde_as(as = "HumanReadable")] + ipc_address: IPCAddress, + } + + let subnet_id = + SubnetID::from_str("/r31415926/f2xwzbdu7z5sam6hc57xxwkctciuaz7oe5omipwbq").unwrap(); + let ipc_address = IPCAddress::new(&subnet_id, &Address::new_id(101)).unwrap(); + + let t = T { + d: vec![vec![1; 30], vec![2; 30]], + subnet_id, + ipc_address, + }; + let s = serde_json::to_string(&t).unwrap(); + assert_eq!( + s, + r#"{"d":["010101010101010101010101010101010101010101010101010101010101","020202020202020202020202020202020202020202020202020202020202"],"subnet_id":"/r31415926/f2xwzbdu7z5sam6hc57xxwkctciuaz7oe5omipwbq","ipc_address":"/r31415926/f2xwzbdu7z5sam6hc57xxwkctciuaz7oe5omipwbq:f0101"}"# + ); + + let r: T = serde_json::from_str(&s).unwrap(); + + assert_eq!(r, t); + } +} diff --git a/ipc/api/src/cross.rs b/ipc/api/src/cross.rs index ee6eaccf5..6c3e70b39 100644 --- a/ipc/api/src/cross.rs +++ b/ipc/api/src/cross.rs @@ -4,12 +4,15 @@ use crate::address::IPCAddress; use crate::subnet_id::SubnetID; +use crate::HumanReadable; use anyhow::anyhow; use fvm_shared::address::Address; use fvm_shared::econ::TokenAmount; use serde::{Deserialize, Serialize}; use serde_tuple::{Deserialize_tuple, Serialize_tuple}; +use serde_with::serde_as; +#[serde_as] #[derive(PartialEq, Eq, Clone, Debug, Serialize, Deserialize)] pub struct IpcEnvelope { /// Type of message being propagated. @@ -18,12 +21,14 @@ pub struct IpcEnvelope { /// It makes sense to extract from the encoded message /// all shared fields required by all message, so they /// can be inspected without having to decode the message. + #[serde_as(as = "HumanReadable")] pub to: IPCAddress, /// Value included in the envelope pub value: TokenAmount, /// address sending the message pub from: IPCAddress, /// abi.encoded message + #[serde_as(as = "HumanReadable")] pub message: Vec, /// outgoing nonce for the envelope. /// This nonce is set by the gateway when committing the message for propagation @@ -104,7 +109,7 @@ impl IpcEnvelope { } /// Type of cross-net messages currently supported -#[derive(PartialEq, Eq, Clone, Debug, Serialize, Deserialize)] +#[derive(PartialEq, Eq, Clone, Debug, Serialize, Deserialize, strum::Display)] #[repr(u8)] pub enum IpcMsgKind { /// for cross-net messages that move native token, i.e. fund/release. diff --git a/ipc/api/src/lib.rs b/ipc/api/src/lib.rs index bfad747e2..1b61454de 100644 --- a/ipc/api/src/lib.rs +++ b/ipc/api/src/lib.rs @@ -1,7 +1,10 @@ // Copyright 2022-2024 Protocol Labs // SPDX-License-Identifier: MIT +use ethers::utils::hex; use fvm_shared::{address::Address, econ::TokenAmount}; use ipc_types::EthAddress; +use serde::de::Error as SerdeError; +use serde::{Deserialize, Serialize, Serializer}; use std::str::FromStr; pub mod address; @@ -31,3 +34,83 @@ pub fn ethers_address_to_fil_address(addr: ðers::types::Address) -> anyhow::R let eth_addr = EthAddress::from_str(&raw_addr)?; Ok(Address::from(eth_addr)) } + +/// Marker type for serialising data to/from string +pub struct HumanReadable; + +#[macro_export] +macro_rules! serialise_human_readable_str { + ($typ:ty) => { + impl serde_with::SerializeAs<$typ> for $crate::HumanReadable { + fn serialize_as(value: &$typ, serializer: S) -> Result + where + S: serde::Serializer, + { + if serializer.is_human_readable() { + value.to_string().serialize(serializer) + } else { + value.serialize(serializer) + } + } + } + }; +} + +#[macro_export] +macro_rules! deserialize_human_readable_str { + ($typ:ty) => { + use serde::de::Error as DeserializeError; + use serde::{Deserialize, Serialize}; + + impl<'de> serde_with::DeserializeAs<'de, $typ> for $crate::HumanReadable { + fn deserialize_as(deserializer: D) -> Result<$typ, D::Error> + where + D: serde::de::Deserializer<'de>, + { + if deserializer.is_human_readable() { + let s = String::deserialize(deserializer)?; + <$typ>::from_str(&s).map_err(|_| { + D::Error::custom(format!( + "cannot parse from str {}", + core::any::type_name::<$typ>() + )) + }) + } else { + <$typ>::deserialize(deserializer) + } + } + } + }; +} + +#[macro_export] +macro_rules! as_human_readable_str { + ($typ:ty) => { + $crate::serialise_human_readable_str!($typ); + $crate::deserialize_human_readable_str!($typ); + }; +} + +impl serde_with::SerializeAs> for HumanReadable { + fn serialize_as(source: &Vec, serializer: S) -> Result + where + S: Serializer, + { + hex::encode(source).serialize(serializer) + } +} + +impl<'de> serde_with::DeserializeAs<'de, Vec> for HumanReadable { + fn deserialize_as(deserializer: D) -> Result, D::Error> + where + D: serde::de::Deserializer<'de>, + { + if deserializer.is_human_readable() { + let s = String::deserialize(deserializer)?; + Ok(hex::decode(s) + .map_err(|e| D::Error::custom(format!("cannot parse from str {e}")))?) + } else { + Vec::::deserialize(deserializer) + } + } +} diff --git a/ipc/api/src/subnet_id.rs b/ipc/api/src/subnet_id.rs index 18f94d64a..0aa758649 100644 --- a/ipc/api/src/subnet_id.rs +++ b/ipc/api/src/subnet_id.rs @@ -9,6 +9,8 @@ use std::fmt::Write; use std::hash::{Hash, Hasher}; use std::str::FromStr; +use crate::as_human_readable_str; + use crate::error::Error; /// MaxChainID is the maximum chain ID value @@ -26,6 +28,8 @@ pub struct SubnetID { children: Vec
, } +as_human_readable_str!(SubnetID); + lazy_static! { pub static ref UNDEF: SubnetID = SubnetID { root: 0, diff --git a/ipc/cli/src/commands/checkpoint/bottomup_bundles.rs b/ipc/cli/src/commands/checkpoint/bottomup_bundles.rs index 66fdc4dc6..b9212fc55 100644 --- a/ipc/cli/src/commands/checkpoint/bottomup_bundles.rs +++ b/ipc/cli/src/commands/checkpoint/bottomup_bundles.rs @@ -27,12 +27,12 @@ impl CommandLineHandler for GetBottomUpBundles { let subnet = SubnetID::from_str(&arguments.subnet)?; for h in arguments.from_epoch..=arguments.to_epoch { - let bundle = provider.get_bottom_up_bundle(&subnet, h).await?; - println!( - "checkpoint: {:?}, signatures: {:?}, signatories: {:?}", - bundle.checkpoint, bundle.signatures, bundle.signatories, - ); - println!("{bundle:?}"); + let Some(bundle) = provider.get_bottom_up_bundle(&subnet, h).await? else { + continue; + }; + + println!("bottom up checkpoint bundle at height: {}", h); + println!("{}", serde_json::to_string(&bundle)?); } Ok(()) diff --git a/ipc/cli/src/commands/checkpoint/list_checkpoints.rs b/ipc/cli/src/commands/checkpoint/list_checkpoints.rs deleted file mode 100644 index 4b352a548..000000000 --- a/ipc/cli/src/commands/checkpoint/list_checkpoints.rs +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright 2022-2024 Protocol Labs -// SPDX-License-Identifier: MIT -//! List checkpoints cli command - -use std::fmt::Debug; - -use async_trait::async_trait; -use clap::Args; -use fvm_shared::clock::ChainEpoch; - -use crate::{CommandLineHandler, GlobalArguments}; - -/// The command to list checkpoints committed in a subnet actor. -pub(crate) struct ListBottomUpCheckpoints; - -#[async_trait] -impl CommandLineHandler for ListBottomUpCheckpoints { - type Arguments = ListBottomUpCheckpointsArgs; - - async fn handle(_global: &GlobalArguments, arguments: &Self::Arguments) -> anyhow::Result<()> { - log::debug!("list checkpoints with args: {:?}", arguments); - todo!() - } -} - -#[derive(Debug, Args)] -#[command(about = "List bottom-up checkpoints")] -pub(crate) struct ListBottomUpCheckpointsArgs { - #[arg(long, help = "The subnet id of the checkpointing subnet")] - pub subnet: String, - #[arg(long, help = "Include checkpoints from this epoch")] - pub from_epoch: ChainEpoch, - #[arg(long, help = "Include checkpoints up to this epoch")] - pub to_epoch: ChainEpoch, -} diff --git a/ipc/cli/src/commands/checkpoint/mod.rs b/ipc/cli/src/commands/checkpoint/mod.rs index 4ef7c7acd..c408f6802 100644 --- a/ipc/cli/src/commands/checkpoint/mod.rs +++ b/ipc/cli/src/commands/checkpoint/mod.rs @@ -4,9 +4,6 @@ use crate::commands::checkpoint::bottomup_bundles::{GetBottomUpBundles, GetBotto use crate::commands::checkpoint::bottomup_height::{ LastBottomUpCheckpointHeight, LastBottomUpCheckpointHeightArgs, }; -use crate::commands::checkpoint::list_checkpoints::{ - ListBottomUpCheckpoints, ListBottomUpCheckpointsArgs, -}; use crate::commands::checkpoint::list_validator_changes::{ ListValidatorChanges, ListValidatorChangesArgs, }; @@ -19,7 +16,6 @@ use clap::{Args, Subcommand}; mod bottomup_bundles; mod bottomup_height; -mod list_checkpoints; mod list_validator_changes; mod quorum_reached; mod relayer; @@ -35,7 +31,6 @@ pub(crate) struct CheckpointCommandsArgs { impl CheckpointCommandsArgs { pub async fn handle(&self, global: &GlobalArguments) -> anyhow::Result<()> { match &self.command { - Commands::ListBottomup(args) => ListBottomUpCheckpoints::handle(global, args).await, Commands::Relayer(args) => BottomUpRelayer::handle(global, args).await, Commands::ListValidatorChanges(args) => { ListValidatorChanges::handle(global, args).await @@ -53,7 +48,6 @@ impl CheckpointCommandsArgs { #[derive(Debug, Subcommand)] pub(crate) enum Commands { - ListBottomup(ListBottomUpCheckpointsArgs), Relayer(BottomUpRelayerArgs), ListValidatorChanges(ListValidatorChangesArgs), ListBottomupBundle(GetBottomUpBundlesArgs), diff --git a/ipc/provider/src/checkpoint.rs b/ipc/provider/src/checkpoint.rs index fd9281126..0cb77efe1 100644 --- a/ipc/provider/src/checkpoint.rs +++ b/ipc/provider/src/checkpoint.rs @@ -177,8 +177,15 @@ impl BottomUpCheckpointMan let bundle = self .child_handler .checkpoint_bundle_at(event.height) - .await?; - tracing::debug!("bottom up bundle: {bundle:?}"); + .await? + .ok_or_else(|| { + anyhow!( + "expected checkpoint at height {} but none found", + event.height + ) + })?; + + log::debug!("bottom up bundle: {bundle:?}"); // We support parallel checkpoint submission using FIFO order with a limited parallelism (controlled by // the size of submission_semaphore). diff --git a/ipc/provider/src/lib.rs b/ipc/provider/src/lib.rs index 43787becc..4cf58c1ee 100644 --- a/ipc/provider/src/lib.rs +++ b/ipc/provider/src/lib.rs @@ -660,8 +660,11 @@ impl IpcProvider { &self, subnet: &SubnetID, height: ChainEpoch, - ) -> anyhow::Result { - let conn = self.get_connection(subnet)?; + ) -> anyhow::Result> { + let conn = match self.connection(subnet) { + None => return Err(anyhow!("target subnet not found")), + Some(conn) => conn, + }; conn.manager().checkpoint_bundle_at(height).await } diff --git a/ipc/provider/src/manager/evm/manager.rs b/ipc/provider/src/manager/evm/manager.rs index ef785ca13..4c67ea3bd 100644 --- a/ipc/provider/src/manager/evm/manager.rs +++ b/ipc/provider/src/manager/evm/manager.rs @@ -1167,7 +1167,7 @@ impl BottomUpCheckpointRelayer for EthSubnetManager { async fn checkpoint_bundle_at( &self, height: ChainEpoch, - ) -> anyhow::Result { + ) -> anyhow::Result> { let contract = gateway_getter_facet::GatewayGetterFacet::new( self.ipc_contract_info.gateway_addr, Arc::new(self.ipc_contract_info.provider.clone()), @@ -1177,6 +1177,11 @@ impl BottomUpCheckpointRelayer for EthSubnetManager { .get_checkpoint_signature_bundle(U256::from(height)) .call() .await?; + + if checkpoint.block_height.as_u64() == 0 { + return Ok(None); + } + let checkpoint = BottomUpCheckpoint::try_from(checkpoint)?; let signatories = signatories .into_iter() @@ -1187,11 +1192,11 @@ impl BottomUpCheckpointRelayer for EthSubnetManager { .map(|s| s.to_vec()) .collect::>(); - Ok(BottomUpCheckpointBundle { + Ok(Some(BottomUpCheckpointBundle { checkpoint, signatures, signatories, - }) + })) } async fn quorum_reached_events(&self, height: ChainEpoch) -> Result> { diff --git a/ipc/provider/src/manager/subnet.rs b/ipc/provider/src/manager/subnet.rs index e9bea5784..becf56bc7 100644 --- a/ipc/provider/src/manager/subnet.rs +++ b/ipc/provider/src/manager/subnet.rs @@ -270,7 +270,10 @@ pub trait BottomUpCheckpointRelayer: Send + Sync { /// Get the checkpoint period, i.e the number of blocks to submit bottom up checkpoints. async fn checkpoint_period(&self, subnet_id: &SubnetID) -> Result; /// Get the checkpoint bundle at a specific height. If it does not exist, it will through error. - async fn checkpoint_bundle_at(&self, height: ChainEpoch) -> Result; + async fn checkpoint_bundle_at( + &self, + height: ChainEpoch, + ) -> Result>; /// Queries the signature quorum reached events at target height. async fn quorum_reached_events(&self, height: ChainEpoch) -> Result>; /// Get the current epoch in the current subnet diff --git a/ipc/wallet/src/evm/mod.rs b/ipc/wallet/src/evm/mod.rs index 2a1ce183e..05dd985fb 100644 --- a/ipc/wallet/src/evm/mod.rs +++ b/ipc/wallet/src/evm/mod.rs @@ -11,10 +11,10 @@ use std::fmt::{Display, Formatter}; use std::hash::Hash; use zeroize::Zeroize; -pub use crate::evm::persistent::{PersistentKeyInfo, PersistentKeyStore}; - #[cfg(feature = "with-ethers")] -pub use std::str::FromStr; +use std::str::FromStr; + +pub use crate::evm::persistent::{PersistentKeyInfo, PersistentKeyStore}; pub const DEFAULT_KEYSTORE_NAME: &str = "evm_keystore.json"; @@ -101,6 +101,7 @@ impl From for EthKeyAddress { } } +#[cfg(feature = "with-ethers")] impl TryFrom for fvm_shared::address::Address { type Error = hex::FromHexError;