fix(node): harden transaction inclusion logic to account for fee market

fendermint/actors/api/src/gas_market.rs

@@ -16,6 +16,8 @@ pub struct Reading {
     pub block_gas_limit: Gas,
     /// The current base fee for the block.
     pub base_fee: TokenAmount,
+    /// The minimum allowable base fee.
+    pub min_base_fee: TokenAmount,
 }
 
 /// The current utilization for the client to report to the gas market.

fendermint/actors/gas_market/eip1559/src/lib.rs

@@ -97,6 +97,7 @@ impl fendermint_actors_api::gas_market::GasMarket for Actor {
         Ok(fendermint_actors_api::gas_market::Reading {
             block_gas_limit: st.constants.block_gas_limit,
             base_fee: st.base_fee,
+            min_base_fee: st.constants.minimal_base_fee,
         })
     }
 
@@ -111,6 +112,7 @@ impl fendermint_actors_api::gas_market::GasMarket for Actor {
             Ok(fendermint_actors_api::gas_market::Reading {
                 block_gas_limit: st.constants.block_gas_limit,
                 base_fee: st.base_fee.clone(),
+                min_base_fee: st.constants.minimal_base_fee.clone(),
             })
         })
     }
@@ -229,10 +231,10 @@ mod tests {
         let r = rt.call::<Actor>(
             Method::SetConstants as u64,
             IpldBlock::serialize_cbor(&Constants {
-                minimal_base_fee: Default::default(),
                 elasticity_multiplier: 0,
                 base_fee_max_change_denominator: 0,
                 block_gas_limit: 20,
+                ..Default::default()
             })
             .unwrap(),
         );
@@ -266,6 +268,7 @@ mod tests {
             .unwrap();
         let reading = r.deserialize::<Reading>().unwrap();
         assert_eq!(reading.base_fee, TokenAmount::from_atto(112));
+        assert_eq!(reading.min_base_fee, Constants::default().minimal_base_fee)
     }
 
     #[test]
@@ -292,6 +295,7 @@ mod tests {
             .unwrap();
         let reading = r.deserialize::<Reading>().unwrap();
         assert_eq!(reading.base_fee, TokenAmount::from_atto(100));
+        assert_eq!(reading.min_base_fee, Constants::default().minimal_base_fee)
     }
 
     #[test]
@@ -318,6 +322,7 @@ mod tests {
             .unwrap();
         let reading = r.deserialize::<Reading>().unwrap();
         assert_eq!(reading.base_fee, TokenAmount::from_atto(88));
+        assert_eq!(reading.min_base_fee, Constants::default().minimal_base_fee)
     }
 
     #[test]

fendermint/testing/materializer/tests/docker_tests/standalone.rs

@@ -43,7 +43,7 @@ where
 /// from the ethereum API instance it was sent to even before it is included in the block.
 #[serial_test::serial]
 #[tokio::test]
-async fn test_sent_tx_found_in_mempool() {
+async fn test_sent_tx_found_in_mempool_and_invalid_rejected() {
     with_testnet(
         MANIFEST,
         |manifest| {
@@ -90,6 +90,16 @@ async fn test_sent_tx_found_in_mempool() {
                     }
                 }
 
+                // This transaction is invalid because the gas fee cap is below the network's minimum base fee.
+                // It should be rejected at submission time via CheckTx (which will also reject it at block validation time).
+                let invalid = Eip1559TransactionRequest::new()
+                    .to(to)
+                    .max_fee_per_gas(1)
+                    .value(1);
+                if let Ok(_) = middleware.send_transaction(invalid, None).await {
+                    bail!("expected invalid transaction to error on submission")
+                }
+
                 Ok(())
             };
 

fendermint/vm/interpreter/src/chain.rs

@@ -32,6 +32,7 @@ use fvm_ipld_encoding::RawBytes;
 use fvm_shared::clock::ChainEpoch;
 use fvm_shared::econ::TokenAmount;
 use num_traits::Zero;
+use std::any::Any;
 use std::sync::Arc;
 
 /// A resolution pool for bottom-up and top-down checkpoints.
@@ -190,6 +191,7 @@ where
         msgs: Vec<Self::Message>,
     ) -> anyhow::Result<bool> {
         let mut block_gas_usage = 0;
+        let gas_market = state.block_gas_tracker().current_gas_market();
 
         for msg in msgs {
             match msg {
@@ -228,6 +230,14 @@ where
                     }
                 }
                 ChainMessage::Signed(signed) => {
+                    // We do not need to check against the minimium base fee because the gas market
+                    // is guaranteed to be capped at it anyway.
+                    if signed.message.gas_fee_cap < gas_market.base_fee {
+                        // We do not accept blocks containing transactions with gas parameters below the current base fee.
+                        // Producing an invalid block like this should penalize the validator going forward.
+                        return Ok(false);
+                    }
+
                     block_gas_usage += signed.message.gas_limit;
                 }
                 _ => {}
@@ -462,6 +472,7 @@ impl<I, DB> CheckInterpreter for ChainMessageInterpreter<I, DB>
 where
     DB: Blockstore + Clone + 'static + Send + Sync,
     I: CheckInterpreter<Message = VerifiableMessage, Output = SignedMessageCheckRes>,
+    I::State: Any,
 {
     type State = I::State;
     type Message = ChainMessage;
@@ -475,6 +486,22 @@ where
     ) -> anyhow::Result<(Self::State, Self::Output)> {
         match msg {
             ChainMessage::Signed(msg) => {
+                // TODO The recursive, generic Interpreter design is extremely inflexible, and ultimately useless.
+                //  We will untangle this later (see https://github.com/consensus-shipyard/ipc/issues/1241).
+                //  In the meantime we are compelled to downcast to access the actual types in use.
+                //  This check cannot be performed in the last interpreter (which has access to the FvmExecState) because
+                //  that one returns an exit code, whereas illegal messages do not lead to any execution and therefore
+                //  they do not benefit from an exit code.
+                let fvm_exec_state = (&state as &dyn Any)
+                    .downcast_ref::<FvmExecState<ReadOnlyBlockstore<DB>>>()
+                    .context("inner state not downcastable to FvmExecState")?;
+                let gas_market = fvm_exec_state.block_gas_tracker().current_gas_market();
+                if msg.message.gas_limit > gas_market.block_gas_limit
+                    || msg.message.gas_fee_cap < gas_market.min_base_fee
+                {
+                    return Ok((state, Err(IllegalMessage)));
+                }
+
                 let (state, ret) = self
                     .inner
                     .check(state, VerifiableMessage::Signed(msg), is_recheck)

fendermint/vm/interpreter/src/fvm/gas.rs

@@ -15,39 +15,31 @@ use num_traits::Zero;
 
 #[derive(Debug, Clone)]
 pub struct BlockGasTracker {
-    /// The current base fee.
-    base_fee: TokenAmount,
-    /// The current block gas limit.
-    block_gas_limit: Gas,
+    /// The currently active gas market reading.
+    current_gas_market: Reading,
     /// The cumulative gas premiums claimable by the block producer.
     cumul_gas_premium: TokenAmount,
     /// The accumulated gas usage throughout the block.
     cumul_gas_used: Gas,
 }
 
 impl BlockGasTracker {
-    pub fn base_fee(&self) -> &TokenAmount {
-        &self.base_fee
+    pub fn current_gas_market(&self) -> &Reading {
+        &self.current_gas_market
     }
 
     pub fn create<E: Executor>(executor: &mut E) -> anyhow::Result<BlockGasTracker> {
-        let mut ret = Self {
-            base_fee: Zero::zero(),
-            block_gas_limit: Zero::zero(),
+        Self::read_gas_market(executor).map(|reading| Self {
+            current_gas_market: reading,
             cumul_gas_premium: Zero::zero(),
             cumul_gas_used: Zero::zero(),
-        };
-
-        let reading = Self::read_gas_market(executor)?;
-
-        ret.base_fee = reading.base_fee;
-        ret.block_gas_limit = reading.block_gas_limit;
-
-        Ok(ret)
+        })
     }
 
     pub fn available(&self) -> Gas {
-        self.block_gas_limit.saturating_sub(self.cumul_gas_used)
+        self.current_gas_market
+            .block_gas_limit
+            .saturating_sub(self.cumul_gas_used)
     }
 
     pub fn ensure_sufficient_gas(&self, msg: &FvmMessage) -> anyhow::Result<()> {
@@ -66,7 +58,7 @@ impl BlockGasTracker {
         self.cumul_gas_used = self.cumul_gas_used.saturating_add(ret.msg_receipt.gas_used);
 
         // sanity check, should not happen; only trace if it does so we can debug later.
-        if self.cumul_gas_used >= self.block_gas_limit {
+        if self.cumul_gas_used >= self.current_gas_market.block_gas_limit {
             tracing::warn!("out of block gas; cumulative gas used exceeds block gas limit!");
         }
     }

fendermint/vm/interpreter/src/fvm/state/exec.rs

@@ -188,7 +188,7 @@ where
         let mut executor = DefaultExecutor::new(engine.clone(), machine)?;
 
         let block_gas_tracker = BlockGasTracker::create(&mut executor)?;
-        let base_fee = block_gas_tracker.base_fee().clone();
+        let base_fee = block_gas_tracker.current_gas_market().base_fee.clone();
 
         Ok(Self {
             executor,