From bcd736417aebe11b97fb1760ec2eb6a234fad831 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 | 4 + offchain/Cargo.lock | 3 + offchain/advance-runner/Cargo.toml | 3 + offchain/advance-runner/src/config.rs | 11 ++- offchain/advance-runner/src/dapp_contract.rs | 46 ++++++++++ offchain/advance-runner/src/lib.rs | 1 + offchain/advance-runner/src/runner.rs | 18 ++-- .../advance-runner/src/snapshot/config.rs | 46 +++++++++- .../advance-runner/src/snapshot/disabled.rs | 20 ++--- .../advance-runner/src/snapshot/fs_manager.rs | 85 ++++++++++++------- offchain/advance-runner/src/snapshot/mod.rs | 9 +- offchain/advance-runner/tests/fixtures/mod.rs | 5 +- offchain/contracts/build.rs | 2 +- offchain/contracts/src/lib.rs | 1 + 14 files changed, 186 insertions(+), 68 deletions(-) create mode 100644 offchain/advance-runner/src/dapp_contract.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 67c0ea712..716a4f381 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Added snapshot validation. The node will now check if the snapshot's template hash matches the one stored in the blockchain + ## [0.9.1] 2023-06-14 ### Changed diff --git a/offchain/Cargo.lock b/offchain/Cargo.lock index 71877c7d7..2929608f0 100644 --- a/offchain/Cargo.lock +++ b/offchain/Cargo.lock @@ -247,6 +247,7 @@ dependencies = [ "async-trait", "backoff", "clap", + "contracts", "env_logger", "grpc-interfaces", "hex", @@ -255,6 +256,7 @@ dependencies = [ "rollups-events", "sha3", "snafu", + "state-fold-types", "tempfile", "test-fixtures", "test-log", @@ -263,6 +265,7 @@ dependencies = [ "tonic 0.9.2", "tracing", "tracing-subscriber", + "url", "uuid 1.4.0", ] diff --git a/offchain/advance-runner/Cargo.toml b/offchain/advance-runner/Cargo.toml index 8857ceb30..d4a66dc4a 100644 --- a/offchain/advance-runner/Cargo.toml +++ b/offchain/advance-runner/Cargo.toml @@ -16,14 +16,17 @@ rollups-events = { path = "../rollups-events" } async-trait.workspace = true backoff = { workspace = true, features = ["tokio"] } clap = { workspace = true, features = ["derive", "env"] } +contracts = { path = "../contracts" } hex.workspace = true sha3 = { workspace = true, features = ["std"] } snafu.workspace = true +state-fold-types = { workspace = true, features = ["ethers"] } tokio = { workspace = true, features = ["macros", "time", "rt-multi-thread"] } tonic.workspace = true tracing.workspace = true tracing-subscriber = { workspace = true, features = ["env-filter"] } uuid = { workspace = true, features = ["v4"] } +url.workspace = true [dev-dependencies] test-fixtures = { path = "../test-fixtures" } diff --git a/offchain/advance-runner/src/config.rs b/offchain/advance-runner/src/config.rs index 398078cd0..69bd226a5 100644 --- a/offchain/advance-runner/src/config.rs +++ b/offchain/advance-runner/src/config.rs @@ -27,12 +27,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..8b7b5c5a6 --- /dev/null +++ b/offchain/advance-runner/src/dapp_contract.rs @@ -0,0 +1,46 @@ +// (c) Cartesi and individual authors (see AUTHORS) +// SPDX-License-Identifier: Apache-2.0 (see LICENSE) + +use contracts::cartesi_dapp::CartesiDApp; +use rollups_events::{Address, Hash}; +use snafu::{ResultExt, Snafu}; +use state_fold_types::ethers::{ + prelude::ContractError, + providers::{Http, HttpRateLimitRetryPolicy, Provider, RetryClient}, +}; +use std::sync::Arc; +use url::Url; + +const MAX_RETRIES: u32 = 10; +const INITIAL_BACKOFF: u64 = 1000; + +#[derive(Debug, Snafu)] +pub enum DappContractError { + #[snafu(display("failed to obtain hash from dapp contract"))] + ContractError { + 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(ContractSnafu)?; + + Ok(Hash::new(template_hash)) +} diff --git a/offchain/advance-runner/src/lib.rs b/offchain/advance-runner/src/lib.rs index b3761f29d..7685b0c77 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..53467b6a1 100644 --- a/offchain/advance-runner/src/runner.rs +++ b/offchain/advance-runner/src/runner.rs @@ -1,12 +1,11 @@ // (c) Cartesi and individual authors (see AUTHORS) // SPDX-License-Identifier: Apache-2.0 (see LICENSE) -use rollups_events::{Event, InputMetadata, RollupsData, RollupsInput}; -use snafu::{ResultExt, Snafu}; - use crate::broker::{BrokerFacade, BrokerFacadeError}; use crate::server_manager::{ServerManagerError, ServerManagerFacade}; use crate::snapshot::SnapshotManager; +use rollups_events::{Event, InputMetadata, RollupsData, RollupsInput}; +use snafu::{ResultExt, Snafu}; #[derive(Debug, Snafu)] pub enum RunnerError { @@ -53,8 +52,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 +120,11 @@ impl Runner { .context(GetLatestSnapshotSnafu)?; tracing::info!(?snapshot, "got latest snapshot"); - let offchain_hash = self - .snapshot_manager - .get_template_hash(&snapshot) + self.snapshot_manager + .validate(&snapshot) .await - .context(GetSnapshotHashSnafu)?; - tracing::trace!(?offchain_hash, "got snapshot hash"); + .context(ValidateSnapshotSnafu)?; + tracing::trace!("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..3f134bf80 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 snafu::{ensure, Snafu}; +use rollups_events::Address; +use snafu::{ensure, ResultExt, 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,27 @@ 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; + let provider_http_endpoint = if validation_enabled { + if let Some(endpoint) = &cli_config.provider_http_endpoint { + let url = + Url::parse(&endpoint).context(EndpointParseSnafu)?; + Some(url) + } else { + return Err( + SnapshotConfigError::NoProviderEndpointError {}, + ); + } + } else { + None + }; + Ok(SnapshotConfig::FileSystem(FSManagerConfig { snapshot_dir, snapshot_latest, + validation_enabled, + provider_http_endpoint, + dapp_address, })) } else { Ok(SnapshotConfig::Disabled) @@ -39,18 +63,25 @@ 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 {}, + + #[snafu(display("provider_http_endpoint isn't a valid URL"))] + EndpointParseError { source: url::ParseError }, } #[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 +92,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)] + 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..5343a0f8e 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,28 @@ 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.clone(), + 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)) + match offchain_hash == onchain_hash { + true => Ok(()), + false => Err(FSSnapshotError::HashMismatchError), + } + } else { + tracing::trace!( + "snapshot validation is disabled, accepting snapshot" + ); + Ok(()) + } } } @@ -272,7 +282,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 +292,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 +333,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 +617,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 +632,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..5911d7e46 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; @@ -33,9 +32,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 the Snapshot template hash with the one 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 f36d6f18e..192a48fc0 100644 --- a/offchain/advance-runner/tests/fixtures/mod.rs +++ b/offchain/advance-runner/tests/fixtures/mod.rs @@ -66,7 +66,7 @@ impl AdvanceRunnerFixture { let dapp_metadata = DAppMetadata { chain_id, - dapp_address, + dapp_address: dapp_address.clone(), }; let broker_config = BrokerConfig { @@ -83,6 +83,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 993e3cc7c..6c510ffe3 100644 --- a/offchain/contracts/build.rs +++ b/offchain/contracts/build.rs @@ -18,7 +18,7 @@ fn main() -> Result<(), Box> { ("InputBox", "inputs/InputBox", "input_box.rs"), ("Authority", "consensus/authority/Authority", "authority.rs"), ("History", "history/History", "history.rs"), - // ("CartesiDApp", "dapp/CartesiDApp", "cartesi_dapp.rs"), + ("CartesiDApp", "dapp/CartesiDApp", "cartesi_dapp.rs"), // ( // "CartesiDAppFactory", // "dapp/CartesiDAppFactory", 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);