Skip to content

Commit

Permalink
update feedback review
Browse files Browse the repository at this point in the history
  • Loading branch information
cryptoAtwill committed Dec 16, 2024
1 parent 3146752 commit 63c0cb5
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 185 deletions.
55 changes: 17 additions & 38 deletions fendermint/app/src/app.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -397,40 +397,6 @@ where
_ => Err(anyhow!("invalid app state json")),
}
}

fn create_check_state(&self) -> anyhow::Result<FvmExecState<ReadOnlyBlockstore<SS>>> {
let db = self.state_store_clone();
let state = self.committed_state()?;

// This would create a partial state, but some client scenarios need the full one.
// FvmCheckState::new(db, state.state_root(), state.chain_id())
// .context("error creating check state")?

let mut state = FvmExecState::new(
ReadOnlyBlockstore::new(db),
self.multi_engine.as_ref(),
state.block_height.try_into()?,
state.state_params,
)
.context("error creating check state")?;

// load txn priority calculator
let end = state.block_height() as u64;
let start = end
.saturating_sub(state.txn_priority_calculator().len() as u64)
.max(1);
for h in start..=end {
let Ok((state_params, _)) = self.state_params_at_height(FvmQueryHeight::Height(h))
else {
continue;
};
state
.txn_priority_calculator_mut()
.base_fee_updated(state_params.base_fee);
}

Ok(state)
}
}

// NOTE: The `Application` interface doesn't allow failures at the moment. The protobuf
Expand Down Expand Up @@ -595,7 +561,22 @@ where

let state = match guard.take() {
Some(state) => state,
None => self.create_check_state()?,
None => {
let db = self.state_store_clone();
let state = self.committed_state()?;

// This would create a partial state, but some client scenarios need the full one.
// FvmCheckState::new(db, state.state_root(), state.chain_id())
// .context("error creating check state")?

FvmExecState::new(
ReadOnlyBlockstore::new(db),
self.multi_engine.as_ref(),
state.block_height.try_into()?,
state.state_params,
)
.context("error creating check state")?
}
};

let (state, result) = self
Expand All @@ -619,9 +600,7 @@ where
mpool_received_trace.message = Some(Message::from(&ret.message));

let priority = state.txn_priority_calculator().priority(&ret.message);
let mut t = to_check_tx(ret);
t.priority = priority;
t
to_check_tx(ret, priority)
}
},
};
Expand Down
3 changes: 2 additions & 1 deletion fendermint/app/src/tmconv.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ pub fn to_deliver_tx(
}
}

pub fn to_check_tx(ret: FvmCheckRet) -> response::CheckTx {
pub fn to_check_tx(ret: FvmCheckRet, priority: i64) -> response::CheckTx {
// Putting the message `log` because only `log` appears in the `tx_sync` JSON-RPC response.
let message = ret
.info
Expand All @@ -144,6 +144,7 @@ pub fn to_check_tx(ret: FvmCheckRet) -> response::CheckTx {
data,
gas_wanted: ret.gas_limit.try_into().unwrap_or(i64::MAX),
sender: ret.sender.to_string(),
priority,
..Default::default()
}
}
Expand Down
4 changes: 4 additions & 0 deletions fendermint/vm/interpreter/src/fvm/gas.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ pub struct BlockGasTracker {
}

impl BlockGasTracker {
pub fn base_fee(&self) -> &TokenAmount {
&self.base_fee
}

pub fn create<E: Executor>(executor: &mut E) -> anyhow::Result<BlockGasTracker> {
let mut ret = Self {
base_fee: Zero::zero(),
Expand Down
9 changes: 2 additions & 7 deletions fendermint/vm/interpreter/src/fvm/state/exec.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ pub type ActorAddressMap = HashMap<ActorID, Address>;
/// The result of the message application bundled with any delegated addresses of event emitters.
pub type ExecResult = anyhow::Result<(ApplyRet, ActorAddressMap)>;

const DEFAULT_BASE_FEE_HISTORY: usize = 5;

/// Parts of the state which evolve during the lifetime of the chain.
#[serde_as]
#[derive(Serialize, Deserialize, Debug, Clone, Eq, PartialEq)]
Expand Down Expand Up @@ -155,6 +153,7 @@ where
let mut executor = DefaultExecutor::new(engine, machine)?;

let block_gas_tracker = BlockGasTracker::create(&mut executor)?;
let base_fee = block_gas_tracker.base_fee().clone();

Ok(Self {
executor,
Expand All @@ -168,7 +167,7 @@ where
power_scale: params.power_scale,
},
params_dirty: false,
txn_priority: TxnPriorityCalculator::new(DEFAULT_BASE_FEE_HISTORY),
txn_priority: TxnPriorityCalculator::new(base_fee),
})
}

Expand Down Expand Up @@ -277,10 +276,6 @@ where
&self.txn_priority
}

pub fn txn_priority_calculator_mut(&mut self) -> &mut TxnPriorityCalculator {
&mut self.txn_priority
}

pub fn app_version(&self) -> u64 {
self.params.app_version
}
Expand Down
152 changes: 13 additions & 139 deletions fendermint/vm/interpreter/src/fvm/state/priority.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,63 +3,30 @@

use crate::fvm::FvmMessage;
use fvm_shared::econ::TokenAmount;
use num_traits::{ToPrimitive, Zero};
use num_traits::ToPrimitive;

/// The transaction priority calculator. The priority calculated is used to determine the ordering
/// in the mempool.
#[derive(Clone, Debug)]
pub struct TxnPriorityCalculator {
/// Ring buffer of base fee history
base_fee_history: Vec<Option<TokenAmount>>,
/// Next slot in the ring buffer
next_slot: usize,
base_fee: TokenAmount,
}

impl TxnPriorityCalculator {
pub fn new(size: usize) -> Self {
let mut v = Vec::with_capacity(size);
for _ in 0..size {
v.push(None);
}
Self {
base_fee_history: v,
next_slot: 0,
}
}

pub fn len(&self) -> usize {
self.base_fee_history.len()
}

pub fn base_fee_updated(&mut self, base_fee: TokenAmount) {
self.base_fee_history[self.next_slot] = Some(base_fee);
self.next_slot = (self.next_slot + 1) % self.base_fee_history.len();
pub fn new(base_fee: TokenAmount) -> Self {
Self { base_fee }
}

pub fn priority(&self, msg: &FvmMessage) -> i64 {
let base_fee = self.lowest_base_fee();

if msg.gas_fee_cap < base_fee {
if msg.gas_fee_cap < self.base_fee {
return i64::MIN;
}

let effective_premium = msg.gas_premium.clone().min(&msg.gas_fee_cap - base_fee);
let effective_premium = msg
.gas_premium
.clone()
.min(&msg.gas_fee_cap - &self.base_fee);
effective_premium.atto().to_i64().unwrap_or(i64::MAX)
}

fn lowest_base_fee(&self) -> TokenAmount {
let mut out: Option<TokenAmount> = None;
for v in &self.base_fee_history {
let Some(v) = v.as_ref() else { continue };

match out {
Some(min) => out = Some(min.min(v.clone())),
None => out = Some(v.clone()),
}
}

out.unwrap_or(TokenAmount::zero())
}
}

#[cfg(test)]
Expand All @@ -85,113 +52,20 @@ mod tests {
}
}

#[test]
fn base_fee_update_works() {
let mut cal = TxnPriorityCalculator::new(3);

assert_eq!(cal.lowest_base_fee(), TokenAmount::from_atto(0));

// [10, None, None]
cal.base_fee_updated(TokenAmount::from_atto(10));
assert_eq!(cal.lowest_base_fee(), TokenAmount::from_atto(10));
assert_eq!(
cal.base_fee_history,
vec![Some(TokenAmount::from_atto(10)), None, None]
);

// [10, 20, None]
cal.base_fee_updated(TokenAmount::from_atto(20));
assert_eq!(cal.lowest_base_fee(), TokenAmount::from_atto(10));
assert_eq!(
cal.base_fee_history,
vec![
Some(TokenAmount::from_atto(10)),
Some(TokenAmount::from_atto(20)),
None
]
);

// [10, 20, 5]
cal.base_fee_updated(TokenAmount::from_atto(5));
assert_eq!(cal.lowest_base_fee(), TokenAmount::from_atto(5));
assert_eq!(
cal.base_fee_history,
vec![
Some(TokenAmount::from_atto(10)),
Some(TokenAmount::from_atto(20)),
Some(TokenAmount::from_atto(5)),
]
);

// [6, 20, 5]
cal.base_fee_updated(TokenAmount::from_atto(6));
assert_eq!(cal.lowest_base_fee(), TokenAmount::from_atto(5));
assert_eq!(
cal.base_fee_history,
vec![
Some(TokenAmount::from_atto(6)),
Some(TokenAmount::from_atto(20)),
Some(TokenAmount::from_atto(5)),
]
);

// [6, 100, 5]
cal.base_fee_updated(TokenAmount::from_atto(100));
assert_eq!(cal.lowest_base_fee(), TokenAmount::from_atto(5));
assert_eq!(
cal.base_fee_history,
vec![
Some(TokenAmount::from_atto(6)),
Some(TokenAmount::from_atto(100)),
Some(TokenAmount::from_atto(5)),
]
);

// [6, 100, 10]
cal.base_fee_updated(TokenAmount::from_atto(10));
assert_eq!(cal.lowest_base_fee(), TokenAmount::from_atto(6));
assert_eq!(
cal.base_fee_history,
vec![
Some(TokenAmount::from_atto(6)),
Some(TokenAmount::from_atto(100)),
Some(TokenAmount::from_atto(10)),
]
);

// [10, 100, 10]
cal.base_fee_updated(TokenAmount::from_atto(10));
assert_eq!(cal.lowest_base_fee(), TokenAmount::from_atto(10));
assert_eq!(
cal.base_fee_history,
vec![
Some(TokenAmount::from_atto(10)),
Some(TokenAmount::from_atto(100)),
Some(TokenAmount::from_atto(10)),
]
);
}

#[test]
fn priority_calculation() {
let mut cal = TxnPriorityCalculator::new(3);

cal.base_fee_updated(TokenAmount::from_atto(10));
cal.base_fee_updated(TokenAmount::from_atto(20));
cal.base_fee_updated(TokenAmount::from_atto(30));

// lowest base fee is 10
let cal = TxnPriorityCalculator::new(TokenAmount::from_atto(30));

let msg = create_msg(TokenAmount::from_atto(1), TokenAmount::from_atto(20));
assert_eq!(cal.priority(&msg), i64::MIN);

let msg = create_msg(TokenAmount::from_atto(10), TokenAmount::from_atto(20));
assert_eq!(cal.priority(&msg), 0);
assert_eq!(cal.priority(&msg), i64::MIN);

let msg = create_msg(TokenAmount::from_atto(15), TokenAmount::from_atto(20));
let msg = create_msg(TokenAmount::from_atto(35), TokenAmount::from_atto(20));
assert_eq!(cal.priority(&msg), 5);

let msg = create_msg(TokenAmount::from_atto(40), TokenAmount::from_atto(20));
let msg = create_msg(TokenAmount::from_atto(50), TokenAmount::from_atto(20));
assert_eq!(cal.priority(&msg), 20);

let msg = create_msg(
Expand Down

0 comments on commit 63c0cb5

Please sign in to comment.