From 529434360f696ce7c41dbc12e15c09190a1343c3 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 25 Oct 2023 13:57:21 +0100 Subject: [PATCH 1/4] FM-316: Add some context --- fendermint/vm/interpreter/src/chain.rs | 30 ++++++++++++------- .../vm/interpreter/src/fvm/state/ipc.rs | 2 ++ fendermint/vm/interpreter/src/fvm/topdown.rs | 10 +++++-- 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/fendermint/vm/interpreter/src/chain.rs b/fendermint/vm/interpreter/src/chain.rs index 4875683b..a62c510d 100644 --- a/fendermint/vm/interpreter/src/chain.rs +++ b/fendermint/vm/interpreter/src/chain.rs @@ -8,7 +8,7 @@ use crate::{ signed::{SignedMessageApplyRes, SignedMessageCheckRes, SyntheticMessage, VerifiableMessage}, CheckInterpreter, ExecInterpreter, GenesisInterpreter, ProposalInterpreter, QueryInterpreter, }; -use anyhow::{anyhow, Context}; +use anyhow::{bail, Context}; use async_stm::atomically; use async_trait::async_trait; use fendermint_vm_actor_interface::ipc; @@ -209,7 +209,8 @@ where let (state, ret) = self .inner .deliver(state, VerifiableMessage::Synthetic(smsg)) - .await?; + .await + .context("failed to deliver bottom up checkpoint")?; // If successful, add the CID to the background resolution pool. let is_success = match ret { @@ -236,9 +237,7 @@ where } IpcMessage::TopDownExec(p) => { if !provider.is_enabled() { - return Err(anyhow!( - "cannot execute IPC top-down message: parent provider disabled" - )); + bail!("cannot execute IPC top-down message: parent provider disabled"); } // commit parent finality first @@ -249,26 +248,34 @@ where finality.clone(), &provider, ) - .await?; + .await + .context("failed to commit finality")?; // error happens if we cannot get the validator set from ipc agent after retries let validator_changes = provider .validator_changes_from(prev_height + 1, finality.height) - .await?; + .await + .context("failed to fetch validator changes")?; + self.gateway_caller - .store_validator_changes(&mut state, validator_changes)?; + .store_validator_changes(&mut state, validator_changes) + .context("failed to store validator changes")?; // error happens if we cannot get the cross messages from ipc agent after retries let msgs = provider .top_down_msgs_from(prev_height + 1, p.height as u64, &finality.block_hash) - .await?; + .await + .context("failed to fetch top down messages")?; + let ret = topdown::execute_topdown_msgs(&self.gateway_caller, &mut state, msgs) - .await?; + .await + .context("failed to execute top down messages")?; atomically(|| { provider.set_new_finality(finality.clone(), prev_finality.clone()) }) .await; + tracing::debug!("new finality updated: {:?}", finality); Ok(((pool, provider, state), ChainMessageApplyRet::Ipc(ret))) @@ -328,7 +335,8 @@ where let (state, ret) = self .inner .check(state, VerifiableMessage::Synthetic(msg), is_recheck) - .await?; + .await + .context("failed to check bottom up resolve")?; Ok((state, Ok(ret))) } diff --git a/fendermint/vm/interpreter/src/fvm/state/ipc.rs b/fendermint/vm/interpreter/src/fvm/state/ipc.rs index 87cbec3f..6f013380 100644 --- a/fendermint/vm/interpreter/src/fvm/state/ipc.rs +++ b/fendermint/vm/interpreter/src/fvm/state/ipc.rs @@ -203,9 +203,11 @@ impl GatewayCaller { finality: IPCParentFinality, ) -> anyhow::Result> { let evm_finality = router::ParentFinality::try_from(finality)?; + let (has_committed, prev_finality) = self .router .call(state, |c| c.commit_parent_finality(evm_finality))?; + Ok(if !has_committed { None } else { diff --git a/fendermint/vm/interpreter/src/fvm/topdown.rs b/fendermint/vm/interpreter/src/fvm/topdown.rs index 81601d84..363a6e2f 100644 --- a/fendermint/vm/interpreter/src/fvm/topdown.rs +++ b/fendermint/vm/interpreter/src/fvm/topdown.rs @@ -6,6 +6,7 @@ use crate::chain::TopDownFinalityProvider; use crate::fvm::state::ipc::GatewayCaller; use crate::fvm::state::FvmExecState; use crate::fvm::FvmApplyRet; +use anyhow::Context; use fendermint_vm_topdown::{BlockHeight, IPCParentFinality, ParentViewProvider}; use fvm_ipld_blockstore::Blockstore; use fvm_shared::econ::TokenAmount; @@ -28,14 +29,16 @@ where } else { (provider.genesis_epoch()?, None) }; + tracing::debug!( "commit finality parsed: prev_height {prev_height}, prev_finality: {prev_finality:?}" ); + Ok((prev_height, prev_finality)) } /// Execute the top down messages implicitly. Before the execution, mint to the gateway of the funds -/// transferred in the messages. +/// transferred in the messages, and increase the circulating supply with the incoming value. pub async fn execute_topdown_msgs( gateway_caller: &GatewayCaller, state: &mut FvmExecState, @@ -45,7 +48,10 @@ where DB: Blockstore + Sync + Send + 'static, { let total_value: TokenAmount = messages.iter().map(|a| a.msg.value.clone()).sum(); - gateway_caller.mint_to_gateway(state, total_value)?; + + gateway_caller + .mint_to_gateway(state, total_value) + .context("failed to mint to gateway")?; gateway_caller.apply_cross_messages(state, messages) } From 88db4e0f32db3937d31ee73e0e85cfe3b4a208c9 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 25 Oct 2023 16:10:59 +0100 Subject: [PATCH 2/4] FM-316: Add FvmUpdatableParams to help not forget to save --- fendermint/app/src/app.rs | 16 +++++++- .../vm/interpreter/src/fvm/state/exec.rs | 37 +++++++++++++++---- .../vm/interpreter/src/fvm/state/genesis.rs | 5 ++- .../vm/interpreter/src/fvm/state/mod.rs | 2 +- 4 files changed, 49 insertions(+), 11 deletions(-) diff --git a/fendermint/app/src/app.rs b/fendermint/app/src/app.rs index 0d6927c7..7cb1f86b 100644 --- a/fendermint/app/src/app.rs +++ b/fendermint/app/src/app.rs @@ -22,6 +22,7 @@ use fendermint_vm_interpreter::chain::{ }; use fendermint_vm_interpreter::fvm::state::{ empty_state_tree, CheckStateRef, FvmExecState, FvmGenesisState, FvmQueryState, FvmStateParams, + FvmUpdatableParams, }; use fendermint_vm_interpreter::fvm::store::ReadOnlyBlockstore; use fendermint_vm_interpreter::fvm::{FvmApplyRet, FvmGenesisOutput}; @@ -725,9 +726,20 @@ where let mut state = self.committed_state()?; state.block_height = exec_state.block_height().try_into()?; state.state_params.timestamp = exec_state.timestamp(); - state.state_params.state_root = exec_state.commit().context("failed to commit FVM")?; - let state_root = state.state_root(); + let ( + state_root, + FvmUpdatableParams { + power_scale, + circ_supply, + }, + _, + ) = exec_state.commit().context("failed to commit FVM")?; + + state.state_params.state_root = state_root; + state.state_params.power_scale = power_scale; + state.state_params.circ_supply = circ_supply; + let app_hash = state.app_hash(); tracing::debug!( diff --git a/fendermint/vm/interpreter/src/fvm/state/exec.rs b/fendermint/vm/interpreter/src/fvm/state/exec.rs index ff8f6f39..c59dff6a 100644 --- a/fendermint/vm/interpreter/src/fvm/state/exec.rs +++ b/fendermint/vm/interpreter/src/fvm/state/exec.rs @@ -48,6 +48,22 @@ pub struct FvmStateParams { pub power_scale: PowerScale, } +/// Parts of the state which can be updated by message execution, apart from the actor state. +/// +/// This is just a technical thing to help us not forget about saving something. +/// +/// TODO: `base_fee` should surely be here. +#[derive(Debug)] +pub struct FvmUpdatableParams { + /// The circulating supply changes if IPC is elabled and + /// funds/releases are carried out with the parent. + pub circ_supply: TokenAmount, + /// Conversion between collateral and voting power. + /// Doesn't change at the moment but in theory it could, + /// and it doesn't have a place within the FVM. + pub power_scale: PowerScale, +} + pub type MachineBlockstore = as Machine>::Blockstore; /// A state we create for the execution of all the messages in a block. @@ -64,8 +80,10 @@ where /// execution interpreter without having to add yet another piece to track at the app level. block_hash: Option, - /// Conversion between collateral and voting power. - power_scale: PowerScale, + params: FvmUpdatableParams, + + /// Indicate whether the parameters have been updated. + params_dirty: bool, } impl FvmExecState @@ -90,7 +108,7 @@ where // * base_fee; by default it's zero let mut mc = nc.for_epoch(block_height, params.timestamp.0, params.state_root); mc.set_base_fee(params.base_fee); - mc.set_circulating_supply(params.circ_supply); + mc.set_circulating_supply(params.circ_supply.clone()); // Creating a new machine every time is prohibitively slow. // let ec = EngineConfig::from(&nc); @@ -103,7 +121,11 @@ where Ok(Self { executor, block_hash: None, - power_scale: params.power_scale, + params: FvmUpdatableParams { + circ_supply: params.circ_supply, + power_scale: params.power_scale, + }, + params_dirty: false, }) } @@ -142,8 +164,9 @@ where /// semantics we can hope to provide if the middlewares call each other: did it go /// all the way down, or did it stop somewhere? Easier to have one commit of the state /// as a whole. - pub fn commit(mut self) -> anyhow::Result { - self.executor.flush() + pub fn commit(mut self) -> anyhow::Result<(Cid, FvmUpdatableParams, bool)> { + let cid = self.executor.flush()?; + Ok((cid, self.params, self.params_dirty)) } /// The height of the currently executing block. @@ -163,7 +186,7 @@ where /// Conversion between collateral and voting power. pub fn power_scale(&self) -> PowerScale { - self.power_scale + self.params.power_scale } /// Get a mutable reference to the underlying [StateTree]. diff --git a/fendermint/vm/interpreter/src/fvm/state/genesis.rs b/fendermint/vm/interpreter/src/fvm/state/genesis.rs index ee64d90e..046dab5a 100644 --- a/fendermint/vm/interpreter/src/fvm/state/genesis.rs +++ b/fendermint/vm/interpreter/src/fvm/state/genesis.rs @@ -151,7 +151,10 @@ where pub fn commit(self) -> anyhow::Result { match self.stage { Stage::Tree(mut state_tree) => Ok(state_tree.flush()?), - Stage::Exec(exec_state) => exec_state.commit(), + Stage::Exec(exec_state) => match exec_state.commit()? { + (_, _, true) => bail!("FVM parameters are not expected to be updated in genesis"), + (cid, _, _) => Ok(cid), + }, } } diff --git a/fendermint/vm/interpreter/src/fvm/state/mod.rs b/fendermint/vm/interpreter/src/fvm/state/mod.rs index 5937a365..ec5a5a5c 100644 --- a/fendermint/vm/interpreter/src/fvm/state/mod.rs +++ b/fendermint/vm/interpreter/src/fvm/state/mod.rs @@ -12,7 +12,7 @@ mod snapshot; use std::sync::Arc; pub use check::FvmCheckState; -pub use exec::{FvmExecState, FvmStateParams}; +pub use exec::{FvmExecState, FvmStateParams, FvmUpdatableParams}; pub use genesis::{empty_state_tree, FvmGenesisState}; pub use query::FvmQueryState; pub use snapshot::{Snapshot, V1Snapshot}; From 3af5b83decc42a8cd17a7cebe1a159bf3c7a925e Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 25 Oct 2023 16:25:48 +0100 Subject: [PATCH 3/4] FM-316: Update circulating supply with minted tokens --- .../vm/interpreter/src/fvm/state/exec.rs | 18 ++++++++++++++++++ fendermint/vm/interpreter/src/fvm/topdown.rs | 6 +++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/fendermint/vm/interpreter/src/fvm/state/exec.rs b/fendermint/vm/interpreter/src/fvm/state/exec.rs index c59dff6a..1d763d73 100644 --- a/fendermint/vm/interpreter/src/fvm/state/exec.rs +++ b/fendermint/vm/interpreter/src/fvm/state/exec.rs @@ -80,6 +80,7 @@ where /// execution interpreter without having to add yet another piece to track at the app level. block_hash: Option, + /// State of parameters that are outside the control of the FVM but can change and need to be persisted. params: FvmUpdatableParams, /// Indicate whether the parameters have been updated. @@ -224,6 +225,23 @@ where Ok(emitters) } + + /// Update the circulating supply, effective from the next block. + pub fn update_circ_supply(&mut self, f: F) + where + F: FnOnce(&mut TokenAmount), + { + self.update_params(|p| f(&mut p.circ_supply)) + } + + /// Update the parameters and mark them as dirty. + fn update_params(&mut self, f: F) + where + F: FnOnce(&mut FvmUpdatableParams), + { + f(&mut self.params); + self.params_dirty = true; + } } impl HasChainID for FvmExecState diff --git a/fendermint/vm/interpreter/src/fvm/topdown.rs b/fendermint/vm/interpreter/src/fvm/topdown.rs index 363a6e2f..f0da4c2e 100644 --- a/fendermint/vm/interpreter/src/fvm/topdown.rs +++ b/fendermint/vm/interpreter/src/fvm/topdown.rs @@ -50,8 +50,12 @@ where let total_value: TokenAmount = messages.iter().map(|a| a.msg.value.clone()).sum(); gateway_caller - .mint_to_gateway(state, total_value) + .mint_to_gateway(state, total_value.clone()) .context("failed to mint to gateway")?; + state.update_circ_supply(|circ_supply| { + *circ_supply += total_value; + }); + gateway_caller.apply_cross_messages(state, messages) } From 37cf4684e8a3e205973972d1c22e7748ae300c5b Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Thu, 26 Oct 2023 11:28:24 +0100 Subject: [PATCH 4/4] Update fendermint/vm/interpreter/src/fvm/state/exec.rs Co-authored-by: cryptoAtwill <108330426+cryptoAtwill@users.noreply.github.com> --- fendermint/vm/interpreter/src/fvm/state/exec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fendermint/vm/interpreter/src/fvm/state/exec.rs b/fendermint/vm/interpreter/src/fvm/state/exec.rs index 1d763d73..477e94a4 100644 --- a/fendermint/vm/interpreter/src/fvm/state/exec.rs +++ b/fendermint/vm/interpreter/src/fvm/state/exec.rs @@ -55,7 +55,7 @@ pub struct FvmStateParams { /// TODO: `base_fee` should surely be here. #[derive(Debug)] pub struct FvmUpdatableParams { - /// The circulating supply changes if IPC is elabled and + /// The circulating supply changes if IPC is enabled and /// funds/releases are carried out with the parent. pub circ_supply: TokenAmount, /// Conversion between collateral and voting power.