Skip to content

Commit

Permalink
exclude non-includable messages at proposal time.
Browse files Browse the repository at this point in the history
  • Loading branch information
raulk committed Jan 3, 2025
1 parent 2c40148 commit 02e23db
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 63 deletions.
50 changes: 32 additions & 18 deletions fendermint/vm/interpreter/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
use crate::fvm::state::ipc::GatewayCaller;
use crate::fvm::store::ReadOnlyBlockstore;
use crate::fvm::{topdown, EndBlockOutput, FvmApplyRet};
use crate::selector::{GasLimitSelector, MessageSelector};
use crate::{
fvm::state::FvmExecState,
fvm::FvmMessage,
Expand All @@ -17,6 +16,7 @@ use fendermint_tracing::emit;
use fendermint_vm_actor_interface::ipc;
use fendermint_vm_event::ParentFinalityMissingQuorum;
use fendermint_vm_message::ipc::ParentFinality;
use fendermint_vm_message::signed::SignedMessage;
use fendermint_vm_message::{
chain::ChainMessage,
ipc::{BottomUpCheckpoint, CertifiedMessage, IpcMessage, SignedRelayedMessage},
Expand Down Expand Up @@ -117,7 +117,7 @@ where
(chain_env, state): Self::State,
mut msgs: Vec<Self::Message>,
) -> anyhow::Result<Vec<Self::Message>> {
msgs = messages_selection(msgs, &state)?;
msgs = select_messages(msgs, &state)?;

// Collect resolved CIDs ready to be proposed from the pool.
let ckpts = atomically(|| chain_env.checkpoint_pool.collect_resolved()).await;
Expand Down Expand Up @@ -232,7 +232,7 @@ 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 {
if !signed.includable(&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);
Expand Down Expand Up @@ -582,30 +582,44 @@ fn relayed_bottom_up_ckpt_to_fvm(
Ok(msg)
}

/// Selects messages to be executed. Currently, this is a static function whose main purpose is to
/// coordinate various selectors. However, it does not have formal semantics for doing so, e.g.
/// do we daisy-chain selectors, do we parallelize, how do we treat rejections and acceptances?
/// It hasn't been well thought out yet. When we refactor the whole *Interpreter stack, we will
/// revisit this and make the selection function properly pluggable.
fn messages_selection<DB: Blockstore + Clone + 'static>(
/// Selects messages to be included in a proposal.
/// Currently, this is a static function whose main purpose is to compose various hard-coded selectors.
/// When we refactor the whole *Interpreter stack, we will revisit this and make the selection
/// function properly pluggable.
fn select_messages<DB: Blockstore + Clone + 'static>(
msgs: Vec<ChainMessage>,
state: &FvmExecState<DB>,
) -> anyhow::Result<Vec<ChainMessage>> {
let mut user_msgs = msgs
// Ensure we only have signed messages.
let signed = msgs
.into_iter()
.map(|msg| match msg {
ChainMessage::Signed(inner) => Ok(inner),
ChainMessage::Ipc(_) => Err(anyhow!("should not have ipc messages in user proposals")),
})
.collect::<anyhow::Result<Vec<_>>>()?;

// Currently only one selector, we can potentially extend to more selectors
// This selector enforces that the total cumulative gas limit of all messages is less than the
// currently active block gas limit.
let selectors = vec![GasLimitSelector {}];
for s in selectors {
user_msgs = s.select_messages(state, user_msgs)
}
let total_gas_limit = state.block_gas_tracker().available();
let gas_market = state.block_gas_tracker().current_gas_market();
let mut total_gas_limit_consumed = 0;

// Selector functions to compose.
let is_includable = |msg: &SignedMessage| msg.includable(&gas_market.base_fee);
let cumulative_gas_limit = |msg: &SignedMessage| {
let gas_limit = msg.message.gas_limit;
let accepted = total_gas_limit_consumed + gas_limit <= total_gas_limit;
if accepted {
total_gas_limit_consumed += gas_limit;
}
accepted
};

Ok(user_msgs.into_iter().map(ChainMessage::Signed).collect())
// Perform the selection.
let selected = signed
.into_iter()
.filter(is_includable)
.take_while(cumulative_gas_limit)
.map(ChainMessage::Signed)
.collect::<Vec<_>>();
Ok(selected)
}
1 change: 0 additions & 1 deletion fendermint/vm/interpreter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ pub mod signed;

#[cfg(feature = "arb")]
mod arb;
mod selector;

/// Prepare and process transaction proposals.
#[async_trait]
Expand Down
44 changes: 0 additions & 44 deletions fendermint/vm/interpreter/src/selector.rs

This file was deleted.

6 changes: 6 additions & 0 deletions fendermint/vm/message/src/signed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use fvm_shared::address::{Address, Payload};
use fvm_shared::chainid::ChainID;
use fvm_shared::crypto::signature::ops::recover_secp_public_key;
use fvm_shared::crypto::signature::{Signature, SignatureType, SECP_SIG_LEN};
use fvm_shared::econ::TokenAmount;
use fvm_shared::message::Message;
use serde::{Deserialize, Serialize};

Expand Down Expand Up @@ -300,6 +301,11 @@ impl SignedMessage {
Self::verify_signature(self.origin_kind, &self.message, &self.signature, chain_id)
}

/// Determines whether the message is includable in a block based on gas fees.
pub fn includable(&self, current_base_fee: &TokenAmount) -> bool {
&self.message.gas_fee_cap >= current_base_fee
}

/// Returns reference to the unsigned message.
pub fn message(&self) -> &Message {
&self.message
Expand Down

0 comments on commit 02e23db

Please sign in to comment.