From 91dc04b24e2ad31b710efc168981673c72e193ac Mon Sep 17 00:00:00 2001 From: Victor Yves Crispim Date: Fri, 28 Jul 2023 15:19:51 -0300 Subject: [PATCH] feat(advance-runner)!: validate snapshot hash The snapshot's template hash gets compared with its counterpart in the blockchain, causing the advance-runner to stop if it does not match. BREAKING CHANGE: adds the PROVIDER_HTTP_ENDPOINT parameter, which is required to validate snapshots. Validation is enabled by default, but can be disabled by setting SNAPSHOT_VALIDATION_ENABLED to false --- CHANGELOG.md | 1 + offchain/Cargo.lock | 3 + offchain/advance-runner/Cargo.toml | 7 +- offchain/advance-runner/src/config.rs | 11 ++- offchain/advance-runner/src/dapp_contract.rs | 44 ++++++++++ offchain/advance-runner/src/lib.rs | 1 + offchain/advance-runner/src/runner.rs | 17 ++-- .../advance-runner/src/snapshot/config.rs | 36 +++++++- .../advance-runner/src/snapshot/disabled.rs | 20 ++--- .../advance-runner/src/snapshot/fs_manager.rs | 84 ++++++++++++------- offchain/advance-runner/src/snapshot/mod.rs | 18 ++-- offchain/advance-runner/tests/fixtures/mod.rs | 5 +- offchain/contracts/build.rs | 3 +- offchain/contracts/src/lib.rs | 1 + 14 files changed, 184 insertions(+), 67 deletions(-) create mode 100644 offchain/advance-runner/src/dapp_contract.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 57ad99619..e0ada1b70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added authority claimer service to support reader mode - Added support to `POST` *inspect state* requests +- Added snapshot validation. The node will now check whether the snapshot's template hash matches the one stored in the blockchain ### Changed diff --git a/offchain/Cargo.lock b/offchain/Cargo.lock index c9fe685a7..2632d121d 100644 --- a/offchain/Cargo.lock +++ b/offchain/Cargo.lock @@ -247,7 +247,9 @@ dependencies = [ "async-trait", "backoff", "clap", + "contracts", "env_logger", + "ethers", "grpc-interfaces", "hex", "http-health-check", @@ -264,6 +266,7 @@ dependencies = [ "tonic", "tracing", "tracing-subscriber", + "url", "uuid 1.4.1", ] diff --git a/offchain/advance-runner/Cargo.toml b/offchain/advance-runner/Cargo.toml index 623b52d07..f1c7d7d28 100644 --- a/offchain/advance-runner/Cargo.toml +++ b/offchain/advance-runner/Cargo.toml @@ -9,6 +9,7 @@ name = "cartesi-rollups-advance-runner" path = "src/main.rs" [dependencies] +contracts = { path = "../contracts" } grpc-interfaces = { path = "../grpc-interfaces" } http-health-check = { path = "../http-health-check" } log = { path = "../log" } @@ -17,13 +18,15 @@ rollups-events = { path = "../rollups-events" } async-trait.workspace = true backoff = { workspace = true, features = ["tokio"] } clap = { workspace = true, features = ["derive", "env"] } +ethers.workspace = true hex.workspace = true sha3 = { workspace = true, features = ["std"] } snafu.workspace = true tokio = { workspace = true, features = ["macros", "time", "rt-multi-thread"] } tonic.workspace = true -tracing.workspace = true tracing-subscriber = { workspace = true, features = ["env-filter"] } +tracing.workspace = true +url.workspace = true uuid = { workspace = true, features = ["v4"] } [dev-dependencies] @@ -32,5 +35,5 @@ test-fixtures = { path = "../test-fixtures" } env_logger.workspace = true rand.workspace = true tempfile.workspace = true -testcontainers.workspace = true test-log = { workspace = true, features = ["trace"] } +testcontainers.workspace = true diff --git a/offchain/advance-runner/src/config.rs b/offchain/advance-runner/src/config.rs index 4c44e5bee..0aa8d5da6 100644 --- a/offchain/advance-runner/src/config.rs +++ b/offchain/advance-runner/src/config.rs @@ -29,12 +29,15 @@ impl AdvanceRunnerConfig { pub fn parse() -> Result { let cli_config = CLIConfig::parse(); let broker_config = cli_config.broker_cli_config.into(); - let dapp_metadata = cli_config.dapp_metadata_cli_config.into(); + let dapp_metadata: DAppMetadata = + cli_config.dapp_metadata_cli_config.into(); let server_manager_config = ServerManagerConfig::parse_from_cli(cli_config.sm_cli_config); - let snapshot_config = - SnapshotConfig::parse_from_cli(cli_config.snapshot_cli_config) - .context(SnapshotConfigSnafu)?; + let snapshot_config = SnapshotConfig::new( + cli_config.snapshot_cli_config, + dapp_metadata.dapp_address.clone(), + ) + .context(SnapshotConfigSnafu)?; let backoff_max_elapsed_duration = Duration::from_millis(cli_config.backoff_max_elapsed_duration); let healthcheck_port = cli_config.healthcheck_port; diff --git a/offchain/advance-runner/src/dapp_contract.rs b/offchain/advance-runner/src/dapp_contract.rs new file mode 100644 index 000000000..ad3b9c8b9 --- /dev/null +++ b/offchain/advance-runner/src/dapp_contract.rs @@ -0,0 +1,44 @@ +// (c) Cartesi and individual authors (see AUTHORS) +// SPDX-License-Identifier: Apache-2.0 (see LICENSE) + +use contracts::cartesi_dapp::CartesiDApp; +use ethers::{ + prelude::ContractError, + providers::{Http, HttpRateLimitRetryPolicy, Provider, RetryClient}, +}; +use rollups_events::{Address, Hash}; +use snafu::{ResultExt, Snafu}; +use std::sync::Arc; +use url::Url; + +const MAX_RETRIES: u32 = 10; +const INITIAL_BACKOFF: u64 = 1000; + +#[derive(Debug, Snafu)] +#[snafu(display("failed to obtain hash from dapp contract"))] +pub struct DappContractError { + source: ContractError>>, +} + +pub async fn get_template_hash( + dapp_address: &Address, + provider_http_endpoint: Url, +) -> Result { + let provider = Provider::new(RetryClient::new( + Http::new(provider_http_endpoint), + Box::new(HttpRateLimitRetryPolicy), + MAX_RETRIES, + INITIAL_BACKOFF, + )); + + let cartesi_dapp = + CartesiDApp::new(dapp_address.inner(), Arc::new(provider)); + + let template_hash = cartesi_dapp + .get_template_hash() + .call() + .await + .context(DappContractSnafu)?; + + Ok(Hash::new(template_hash)) +} diff --git a/offchain/advance-runner/src/lib.rs b/offchain/advance-runner/src/lib.rs index 7d8c616be..b1dfaed21 100644 --- a/offchain/advance-runner/src/lib.rs +++ b/offchain/advance-runner/src/lib.rs @@ -16,6 +16,7 @@ pub use error::AdvanceRunnerError; mod broker; pub mod config; +mod dapp_contract; mod error; pub mod runner; mod server_manager; diff --git a/offchain/advance-runner/src/runner.rs b/offchain/advance-runner/src/runner.rs index 21004e27c..032454ea2 100644 --- a/offchain/advance-runner/src/runner.rs +++ b/offchain/advance-runner/src/runner.rs @@ -53,8 +53,8 @@ pub enum RunnerError { ))] ParentIdMismatchError { expected: String, got: String }, - #[snafu(display("failed to get hash from snapshot "))] - GetSnapshotHashError { source: SnapError }, + #[snafu(display("failed to validate snapshot"))] + ValidateSnapshotError { source: SnapError }, } type Result = std::result::Result>; @@ -121,12 +121,13 @@ impl Runner { .context(GetLatestSnapshotSnafu)?; tracing::info!(?snapshot, "got latest snapshot"); - let offchain_hash = self - .snapshot_manager - .get_template_hash(&snapshot) - .await - .context(GetSnapshotHashSnafu)?; - tracing::trace!(?offchain_hash, "got snapshot hash"); + if snapshot.is_template() { + self.snapshot_manager + .validate(&snapshot) + .await + .context(ValidateSnapshotSnafu)?; + tracing::info!("template snapshot is valid"); + } let event_id = self .broker diff --git a/offchain/advance-runner/src/snapshot/config.rs b/offchain/advance-runner/src/snapshot/config.rs index cb0b50846..3b25d6d8d 100644 --- a/offchain/advance-runner/src/snapshot/config.rs +++ b/offchain/advance-runner/src/snapshot/config.rs @@ -2,13 +2,18 @@ // SPDX-License-Identifier: Apache-2.0 (see LICENSE) use clap::Parser; +use rollups_events::Address; use snafu::{ensure, Snafu}; use std::path::PathBuf; +use url::Url; #[derive(Debug, Clone)] pub struct FSManagerConfig { pub snapshot_dir: PathBuf, pub snapshot_latest: PathBuf, + pub validation_enabled: bool, + pub provider_http_endpoint: Option, + pub dapp_address: Address, } #[derive(Debug, Clone)] @@ -18,8 +23,9 @@ pub enum SnapshotConfig { } impl SnapshotConfig { - pub fn parse_from_cli( + pub fn new( cli_config: SnapshotCLIConfig, + dapp_address: Address, ) -> Result { if cli_config.snapshot_enabled { let snapshot_dir = PathBuf::from(cli_config.snapshot_dir); @@ -28,9 +34,22 @@ impl SnapshotConfig { let snapshot_latest = PathBuf::from(cli_config.snapshot_latest); ensure!(snapshot_latest.is_symlink(), SymlinkSnafu); + let validation_enabled = cli_config.snapshot_validation_enabled; + if validation_enabled { + ensure!( + cli_config.provider_http_endpoint.is_some(), + NoProviderEndpointSnafu, + ); + } + + let provider_http_endpoint = cli_config.provider_http_endpoint; + Ok(SnapshotConfig::FileSystem(FSManagerConfig { snapshot_dir, snapshot_latest, + validation_enabled, + provider_http_endpoint, + dapp_address, })) } else { Ok(SnapshotConfig::Disabled) @@ -39,18 +58,22 @@ impl SnapshotConfig { } #[derive(Debug, Snafu)] +#[allow(clippy::enum_variant_names)] pub enum SnapshotConfigError { #[snafu(display("Snapshot dir isn't a directory"))] DirError {}, #[snafu(display("Snapshot latest isn't a symlink"))] SymlinkError {}, + + #[snafu(display("A provider http endpoint is required"))] + NoProviderEndpointError {}, } #[derive(Parser, Debug)] #[command(name = "snapshot")] pub struct SnapshotCLIConfig { - /// If set to false, disable snapshots + /// If set to false, disables snapshots. Enabled by default #[arg(long, env, default_value_t = true)] snapshot_enabled: bool, @@ -61,4 +84,13 @@ pub struct SnapshotCLIConfig { /// Path to the symlink of the latest snapshot #[arg(long, env)] snapshot_latest: String, + + /// If set to false, disables snapshot validation. Enabled by default + #[arg(long, env, default_value_t = true)] + snapshot_validation_enabled: bool, + + /// The endpoint for a JSON-RPC provider. + /// Required if SNAPSHOT_VALIDATION_ENABLED is `true` + #[arg(long, env, value_parser = Url::parse)] + provider_http_endpoint: Option, } diff --git a/offchain/advance-runner/src/snapshot/disabled.rs b/offchain/advance-runner/src/snapshot/disabled.rs index 5b22459b8..bc82f6bc1 100644 --- a/offchain/advance-runner/src/snapshot/disabled.rs +++ b/offchain/advance-runner/src/snapshot/disabled.rs @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 (see LICENSE) use super::{Snapshot, SnapshotManager}; -use rollups_events::Hash; #[derive(Debug)] pub struct SnapshotDisabled {} @@ -17,7 +16,7 @@ impl SnapshotManager for SnapshotDisabled { /// Get the most recent snapshot #[tracing::instrument(level = "trace", skip_all)] - async fn get_latest(&self) -> Result { + async fn get_latest(&self) -> Result { tracing::trace!("snapshots disabled; returning default"); Ok(Default::default()) } @@ -28,26 +27,21 @@ impl SnapshotManager for SnapshotDisabled { &self, _: u64, _: u64, - ) -> Result { + ) -> Result { tracing::trace!("snapshots disabled; returning default"); Ok(Default::default()) } /// Set the most recent snapshot #[tracing::instrument(level = "trace", skip_all)] - async fn set_latest( - &self, - _: Snapshot, - ) -> Result<(), SnapshotDisabledError> { + async fn set_latest(&self, _: Snapshot) -> Result<(), Self::Error> { tracing::trace!("snapshots disabled; ignoring"); Ok(()) } - async fn get_template_hash( - &self, - _: &Snapshot, - ) -> Result { - tracing::trace!("snapshots disabled; returning default"); - Ok(Hash::default()) + #[tracing::instrument(level = "trace", skip_all)] + async fn validate(&self, _: &Snapshot) -> Result<(), Self::Error> { + tracing::trace!("snapshots disabled; ignoring"); + Ok(()) } } diff --git a/offchain/advance-runner/src/snapshot/fs_manager.rs b/offchain/advance-runner/src/snapshot/fs_manager.rs index 9cf01b496..d86ddd789 100644 --- a/offchain/advance-runner/src/snapshot/fs_manager.rs +++ b/offchain/advance-runner/src/snapshot/fs_manager.rs @@ -8,6 +8,8 @@ use std::fs::{self, File}; use std::io::Read; use std::path::{Path, PathBuf}; +use crate::dapp_contract::DappContractError; + use super::config::FSManagerConfig; use super::{Snapshot, SnapshotManager}; @@ -83,6 +85,12 @@ pub enum FSSnapshotError { path: PathBuf, source: std::io::Error, }, + + #[snafu(display("failed to call the dapp contract"))] + OnchainError { source: DappContractError }, + + #[snafu(display("snapshot hash does not match its on-chain counterpart"))] + HashMismatchError, } #[derive(Debug)] @@ -213,26 +221,27 @@ impl SnapshotManager for FSSnapshotManager { Ok(()) } - /// Reads the binary contents of the hash file in snapshot's directory - /// and converts them to a `Hash`. It assumes the file was created correctly - /// and makes no checks in this regard. #[tracing::instrument(level = "trace", skip_all)] - async fn get_template_hash( - &self, - snapshot: &Snapshot, - ) -> Result { - let path = snapshot.path.join(HASH_FILE); - tracing::trace!(?path, "opening hash file at"); - let file = File::open(path.clone()) - .context(OpenHashSnafu { path: path.clone() })?; + async fn validate(&self, snapshot: &Snapshot) -> Result<(), Self::Error> { + if self.config.validation_enabled { + let offchain_hash = snapshot.get_hash().await?; + + let onchain_hash = crate::dapp_contract::get_template_hash( + &self.config.dapp_address, + self.config.provider_http_endpoint.clone().unwrap(), + ) + .await + .context(OnchainSnafu)?; - let mut buffer = [0_u8; HASH_SIZE]; - let bytes = file - .take(HASH_SIZE as u64) - .read(&mut buffer) - .context(ReadHashSnafu { path: path.clone() })?; - tracing::trace!("read {bytes} bytes from file"); - Ok(Hash::new(buffer)) + if offchain_hash == onchain_hash { + Ok(()) + } else { + Err(FSSnapshotError::HashMismatchError) + } + } else { + tracing::trace!("snapshot validation disabled, accepting snapshot"); + Ok(()) + } } } @@ -272,7 +281,7 @@ fn decode_filename(path: &Path) -> Result<(u64, u64), FSSnapshotError> { impl TryFrom for Snapshot { type Error = FSSnapshotError; - fn try_from(path: PathBuf) -> Result { + fn try_from(path: PathBuf) -> Result { let (epoch, processed_input_count) = decode_filename(&path)?; Ok(Snapshot { path, @@ -282,6 +291,27 @@ impl TryFrom for Snapshot { } } +impl Snapshot { + /// Reads the binary contents of the hash file in snapshot's directory + /// and converts them to a `Hash`. It assumes the file was created correctly + /// and makes no checks in this regard. + #[tracing::instrument(level = "trace", skip_all)] + pub async fn get_hash(&self) -> Result { + let path = self.path.join(HASH_FILE); + tracing::trace!(?path, "opening hash file at"); + let file = File::open(path.clone()) + .context(OpenHashSnafu { path: path.clone() })?; + + let mut buffer = [0_u8; HASH_SIZE]; + let bytes = file + .take(HASH_SIZE as u64) + .read(&mut buffer) + .context(ReadHashSnafu { path: path.clone() })?; + tracing::trace!("read {bytes} bytes from file"); + Ok(Hash::new(buffer)) + } +} + #[cfg(test)] mod tests { use super::*; @@ -302,6 +332,9 @@ mod tests { let config = FSManagerConfig { snapshot_dir, snapshot_latest, + validation_enabled: false, + provider_http_endpoint: None, + dapp_address: Default::default(), }; let manager = FSSnapshotManager::new(config); Self { tempdir, manager } @@ -583,11 +616,7 @@ mod tests { }; assert_eq!( - state - .manager - .get_template_hash(&snap) - .await - .expect("get template hash should work"), + snap.get_hash().await.expect("get hash should work"), Hash::new(hash) ); } @@ -602,12 +631,7 @@ mod tests { path, }; - let err = state - .manager - .get_template_hash(&snap) - .await - .expect_err("get template hash should fail"); - + let err = snap.get_hash().await.expect_err("get hash should fail"); assert!(matches!(err, FSSnapshotError::OpenHashError { .. })) } } diff --git a/offchain/advance-runner/src/snapshot/mod.rs b/offchain/advance-runner/src/snapshot/mod.rs index f06402901..9b8c35acd 100644 --- a/offchain/advance-runner/src/snapshot/mod.rs +++ b/offchain/advance-runner/src/snapshot/mod.rs @@ -1,7 +1,6 @@ // (c) Cartesi and individual authors (see AUTHORS) // SPDX-License-Identifier: Apache-2.0 (see LICENSE) -use rollups_events::Hash; use std::path::PathBuf; pub mod config; @@ -16,6 +15,15 @@ pub struct Snapshot { pub processed_input_count: u64, } +impl Snapshot { + /// Verifies if this is the template snapshot. The template snapshot is a + /// Cartesi Machine snapshot taken right after its creation and before it + /// processes any inputs + pub fn is_template(&self) -> bool { + self.epoch == 0 && self.processed_input_count == 0 + } +} + #[async_trait::async_trait] pub trait SnapshotManager { type Error: snafu::Error; @@ -33,9 +41,7 @@ pub trait SnapshotManager { /// Set the most recent snapshot async fn set_latest(&self, snapshot: Snapshot) -> Result<(), Self::Error>; - /// Get the snapshot's template hash - async fn get_template_hash( - &self, - snapshot: &Snapshot, - ) -> Result; + /// Compares `Snapshot`'s hash with the template hash stored on-chain, + /// failing if they don't match + async fn validate(&self, snapshot: &Snapshot) -> Result<(), Self::Error>; } diff --git a/offchain/advance-runner/tests/fixtures/mod.rs b/offchain/advance-runner/tests/fixtures/mod.rs index 745aef3c7..10f17cfb3 100644 --- a/offchain/advance-runner/tests/fixtures/mod.rs +++ b/offchain/advance-runner/tests/fixtures/mod.rs @@ -68,7 +68,7 @@ impl AdvanceRunnerFixture { let dapp_metadata = DAppMetadata { chain_id, - dapp_address, + dapp_address: dapp_address.clone(), }; let broker_config = BrokerConfig { @@ -85,6 +85,9 @@ impl AdvanceRunnerFixture { snapshot_latest: snapshot_dir .expect("Should have a Path") .join("latest"), + validation_enabled: false, + provider_http_endpoint: None, + dapp_address, }) } else { SnapshotConfig::Disabled diff --git a/offchain/contracts/build.rs b/offchain/contracts/build.rs index 36af02e66..442e1261c 100644 --- a/offchain/contracts/build.rs +++ b/offchain/contracts/build.rs @@ -17,12 +17,13 @@ fn main() -> Result<(), Box> { let tempdir = tempfile::tempdir()?; let tarball = tempdir.path().join("rollups.tgz"); download_contracts(&tarball)?; - unzip_contracts(&tarball, &tempdir.path())?; + unzip_contracts(&tarball, tempdir.path())?; let contracts = vec![ ("inputs", "InputBox", "input_box.rs"), ("consensus/authority", "Authority", "authority.rs"), ("history", "History", "history.rs"), + ("dapp", "CartesiDApp", "cartesi_dapp.rs"), ]; for (contract_path, contract_name, bindings_file_name) in contracts { let source_path = path(tempdir.path(), contract_path, contract_name); diff --git a/offchain/contracts/src/lib.rs b/offchain/contracts/src/lib.rs index edf4e32a5..b81fbdd3a 100644 --- a/offchain/contracts/src/lib.rs +++ b/offchain/contracts/src/lib.rs @@ -18,3 +18,4 @@ macro_rules! contract { contract!(input_box); contract!(authority); contract!(history); +contract!(cartesi_dapp);