From 30998178aeba01eb2240a6e99b5edf4692b35cca Mon Sep 17 00:00:00 2001 From: Dmitrii Novikov Date: Mon, 21 Oct 2024 15:56:53 +0400 Subject: [PATCH] feat(ethexe): handle freed pages of programs memory (#4302) --- Makefile | 1 + ethexe/cli/src/config.rs | 7 +++--- ethexe/processor/src/handling/events.rs | 6 ++--- ethexe/processor/src/handling/mod.rs | 4 +-- ethexe/processor/src/lib.rs | 4 +-- ethexe/runtime/common/src/journal.rs | 28 +++++++++++++++------ ethexe/signer/src/lib.rs | 6 ++--- utils/calc-stack-height/src/main.rs | 4 +-- utils/wasm-builder/src/crate_info.rs | 7 +++--- utils/wasm-optimizer/src/cargo_toolchain.rs | 6 ++--- 10 files changed, 42 insertions(+), 31 deletions(-) diff --git a/Makefile b/Makefile index ff5ca2f4fef..4a9cbb907ed 100644 --- a/Makefile +++ b/Makefile @@ -6,6 +6,7 @@ ethexe-pre-commit: ethexe-contracts-pre-commit ethexe-pre-commit-no-contracts ethexe-pre-commit-no-contracts: @ echo " > Formatting ethexe" && cargo +nightly fmt --all -- --config imports_granularity=Crate,edition=2021 @ echo " >> Clippy checking ethexe" && cargo clippy -p "ethexe-*" --all-targets --all-features -- --no-deps -D warnings + @ echo " >>> Testing ethexe" && cargo test -p "ethexe-*" # Building ethexe contracts .PHONY: ethexe-contracts-pre-commit diff --git a/ethexe/cli/src/config.rs b/ethexe/cli/src/config.rs index cb3f8f7e34a..09267a16625 100644 --- a/ethexe/cli/src/config.rs +++ b/ethexe/cli/src/config.rs @@ -19,8 +19,7 @@ //! Application config in one place. use crate::args::Args; - -use anyhow::{Context as _, Result}; +use anyhow::{ensure, Context as _, Result}; use directories::ProjectDirs; use ethexe_network::NetworkEventLoopConfig; use ethexe_prometheus_endpoint::Registry; @@ -173,14 +172,14 @@ impl TryFrom for Config { let sequencer = ConfigPublicKey::new(&args.sequencer_key).context("invalid sequencer key")?; - anyhow::ensure!( + ensure!( args.tmp || sequencer != ConfigPublicKey::Random, "random key for sequencer is only allowed with `--tmp` flag" ); let validator = ConfigPublicKey::new(&args.validator_key).context("invalid validator key")?; - anyhow::ensure!( + ensure!( args.tmp || validator != ConfigPublicKey::Random, "random key for validator is only allowed with `--tmp` flag" ); diff --git a/ethexe/processor/src/handling/events.rs b/ethexe/processor/src/handling/events.rs index ed44a3a295d..78762ae2966 100644 --- a/ethexe/processor/src/handling/events.rs +++ b/ethexe/processor/src/handling/events.rs @@ -17,7 +17,7 @@ // along with this program. If not, see . use crate::Processor; -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, ensure, Result}; use ethexe_common::{ mirror::RequestEvent as MirrorEvent, router::{RequestEvent as RouterEvent, ValueClaim}, @@ -272,12 +272,12 @@ impl Processor { } pub fn handle_new_program(&mut self, program_id: ProgramId, code_id: CodeId) -> Result<()> { - anyhow::ensure!( + ensure!( self.db.original_code(code_id).is_some(), "code existence must be checked on router" ); - anyhow::ensure!( + ensure!( self.db.program_code_id(program_id).is_none(), "program duplicates must be checked on router" ); diff --git a/ethexe/processor/src/handling/mod.rs b/ethexe/processor/src/handling/mod.rs index ded4b37d209..46e9cd7927d 100644 --- a/ethexe/processor/src/handling/mod.rs +++ b/ethexe/processor/src/handling/mod.rs @@ -17,7 +17,7 @@ // along with this program. If not, see . use crate::Processor; -use anyhow::Result; +use anyhow::{ensure, Result}; use ethexe_db::CodesStorage; use ethexe_runtime_common::{ state::{ComplexStorage as _, Dispatch}, @@ -65,7 +65,7 @@ impl Processor { } self.db.mutate_state(state_hash, |processor, state| { - anyhow::ensure!(state.program.is_active(), "program should be active"); + ensure!(state.program.is_active(), "program should be active"); state.queue_hash = processor .modify_queue(state.queue_hash.clone(), |queue| queue.extend(dispatches))?; diff --git a/ethexe/processor/src/lib.rs b/ethexe/processor/src/lib.rs index 2ae8e701ef7..dcc94933e5a 100644 --- a/ethexe/processor/src/lib.rs +++ b/ethexe/processor/src/lib.rs @@ -18,7 +18,7 @@ //! Program's execution service for eGPU. -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, ensure, Result}; use ethexe_common::{mirror::RequestEvent as MirrorEvent, BlockRequestEvent}; use ethexe_db::{BlockMetaStorage, CodesStorage, Database}; use ethexe_runtime_common::{state::Storage, InBlockTransitions}; @@ -176,7 +176,7 @@ impl OverlaidProcessor { .read_state(state_hash) .ok_or_else(|| anyhow!("unreachable: state partially presents in storage"))?; - anyhow::ensure!( + ensure!( !state.requires_init_message(), "program isn't yet initialized" ); diff --git a/ethexe/runtime/common/src/journal.rs b/ethexe/runtime/common/src/journal.rs index 3f55b91e76c..03541a6959c 100644 --- a/ethexe/runtime/common/src/journal.rs +++ b/ethexe/runtime/common/src/journal.rs @@ -6,7 +6,7 @@ use crate::{ InBlockTransitions, }; use alloc::{collections::BTreeMap, vec, vec::Vec}; -use anyhow::Result; +use anyhow::{bail, Result}; use core::num::NonZero; use core_processor::{ common::{DispatchOutcome, JournalHandler}, @@ -83,15 +83,13 @@ impl JournalHandler for Handler<'_, S> { self.update_state(program_id, |state| { match &mut state.program { Program::Active(ActiveProgram { initialized, .. }) if *initialized => { - anyhow::bail!("an attempt to initialize already initialized program") + bail!("an attempt to initialize already initialized program") } Program::Active(ActiveProgram { ref mut initialized, .. }) => *initialized = true, - _ => anyhow::bail!( - "an attempt to dispatch init message for inactive program" - ), + _ => bail!("an attempt to dispatch init message for inactive program"), }; Ok(()) @@ -360,7 +358,7 @@ impl JournalHandler for Handler<'_, S> { ref mut pages_hash, .. }) = state.program else { - anyhow::bail!("an attempt to update pages data of inactive program"); + bail!("an attempt to update pages data of inactive program"); }; let new_pages = storage.store_pages(pages_data); @@ -383,18 +381,32 @@ impl JournalHandler for Handler<'_, S> { self.update_state_with_storage(program_id, |storage, state| { let Program::Active(ActiveProgram { ref mut allocations_hash, + ref mut pages_hash, .. }) = state.program else { - anyhow::bail!("an attempt to update allocations of inactive program"); + bail!("an attempt to update allocations of inactive program"); }; - // TODO (breathx): remove data for difference pages. + let mut removed_pages = vec![]; + *allocations_hash = storage.modify_allocations(allocations_hash.clone(), |allocations| { + removed_pages = allocations + .difference(&new_allocations) + .flat_map(|i| i.iter()) + .flat_map(|i| i.to_iter()) + .collect(); + *allocations = new_allocations; })?; + *pages_hash = storage.modify_memory_pages(pages_hash.clone(), |pages| { + for page in removed_pages { + pages.remove(&page); + } + })?; + Ok(()) }); } diff --git a/ethexe/signer/src/lib.rs b/ethexe/signer/src/lib.rs index 58afaebc779..6b8778e652d 100644 --- a/ethexe/signer/src/lib.rs +++ b/ethexe/signer/src/lib.rs @@ -25,7 +25,7 @@ pub use digest::{Digest, ToDigest}; pub use sha3; pub use signature::Signature; -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, bail, Result}; use gprimitives::{ActorId, H160}; use parity_scale_codec::{Decode, Encode}; use sha3::Digest as _; @@ -205,7 +205,7 @@ impl Signer { } } - anyhow::bail!("Address not found: {}", address); + bail!("Address not found: {}", address); } pub fn get_key_by_addr(&self, address: Address) -> Result> { @@ -279,7 +279,7 @@ impl Signer { let bytes = fs::read(key_path)?; if bytes.len() != 32 { - anyhow::bail!("Invalid key length: {:?}", bytes); + bail!("Invalid key length: {:?}", bytes); } buf.copy_from_slice(&bytes); diff --git a/utils/calc-stack-height/src/main.rs b/utils/calc-stack-height/src/main.rs index e8e00678703..b35018f6dc5 100644 --- a/utils/calc-stack-height/src/main.rs +++ b/utils/calc-stack-height/src/main.rs @@ -16,7 +16,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use anyhow::anyhow; +use anyhow::{anyhow, ensure}; use gear_core::code::{Code, TryNewCodeConfig}; use gear_wasm_instrument::{SystemBreakCode, STACK_HEIGHT_EXPORT_NAME}; use std::{env, fs}; @@ -142,7 +142,7 @@ fn main() -> anyhow::Result<()> { ); if let Some(schedule_stack_height) = schedule.limits.stack_height { - anyhow::ensure!( + ensure!( schedule_stack_height <= stack_height, "Stack height in runtime schedule must be decreased" ); diff --git a/utils/wasm-builder/src/crate_info.rs b/utils/wasm-builder/src/crate_info.rs index 87f854ffc74..59fff9b5c07 100644 --- a/utils/wasm-builder/src/crate_info.rs +++ b/utils/wasm-builder/src/crate_info.rs @@ -16,12 +16,11 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use anyhow::{Context, Result}; +use crate::{builder_error::BuilderError, multiple_crate_versions}; +use anyhow::{ensure, Context, Result}; use cargo_metadata::{Metadata, MetadataCommand, Package}; use std::{collections::BTreeMap, path::Path}; -use crate::{builder_error::BuilderError, multiple_crate_versions}; - /// Helper to get a crate info extracted from the `Cargo.toml`. #[derive(Debug, Default)] pub struct CrateInfo { @@ -38,7 +37,7 @@ pub struct CrateInfo { impl CrateInfo { /// Create a new `CrateInfo` from a path to the `Cargo.toml`. pub fn from_manifest(manifest_path: &Path) -> Result { - anyhow::ensure!( + ensure!( manifest_path.exists(), BuilderError::ManifestPathInvalid(manifest_path.to_path_buf()) ); diff --git a/utils/wasm-optimizer/src/cargo_toolchain.rs b/utils/wasm-optimizer/src/cargo_toolchain.rs index 3c9bdc47d67..aef48151f55 100644 --- a/utils/wasm-optimizer/src/cargo_toolchain.rs +++ b/utils/wasm-optimizer/src/cargo_toolchain.rs @@ -16,7 +16,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use anyhow::{anyhow, Context, Result}; +use anyhow::{anyhow, ensure, Context, Result}; use regex::Regex; use std::{borrow::Cow, process::Command, sync::LazyLock}; @@ -48,7 +48,7 @@ impl Toolchain { .output() .context("`rustup` command failed")?; - anyhow::ensure!( + ensure!( output.status.success(), "`rustup` exit code is not successful" ); @@ -90,7 +90,7 @@ impl Toolchain { /// Checks whether the toolchain is recommended. pub fn check_recommended_toolchain(&self) -> Result<()> { let toolchain = Self::PINNED_NIGHTLY_TOOLCHAIN; - anyhow::ensure!( + ensure!( self.raw_toolchain_str() == toolchain, anyhow!( "recommended toolchain `{x}` not found, install it using the command:\n\