Skip to content

Commit

Permalink
feat: Use timestamp of last block when dry running transactions (#2206)
Browse files Browse the repository at this point in the history
## Description
Closes #2128 

This PR adds the option to specify a timestamp in
BlockProducer::dry_run. When no timestamp is provided, the block
producer will use the time stamp of the previous block, instead of using
`Tai64::now()` (previous behavior).

## Checklist
- [x] Breaking changes are clearly marked as such in the PR description
and changelog
- [x] New behavior is reflected in tests

### Before requesting review
- [x] I have reviewed the code myself
- [x] I have created follow-up issues caused by this PR and linked them
here

### After merging, notify other teams

- [ ] [Rust SDK](https://github.com/FuelLabs/fuels-rs/)
  • Loading branch information
netrome authored Sep 19, 2024
1 parent 525c48f commit 9d5ae1e
Show file tree
Hide file tree
Showing 8 changed files with 240 additions and 56 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Changed

#### Breaking
- [2206](https://github.com/FuelLabs/fuel-core/pull/2206): Use timestamp of last block when dry running transactions.
- [2153](https://github.com/FuelLabs/fuel-core/pull/2153): Updated default gas costs for the local testnet configuration to match `fuel-core 0.35.0`.

## [Version 0.36.0]
Expand Down
1 change: 1 addition & 0 deletions crates/fuel-core/src/graphql_api/ports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ pub trait BlockProducerPort: Send + Sync {
&self,
transactions: Vec<Transaction>,
height: Option<BlockHeight>,
time: Option<Tai64>,
utxo_validation: Option<bool>,
gas_price: Option<u64>,
) -> anyhow::Result<Vec<TransactionExecutionStatus>>;
Expand Down
3 changes: 2 additions & 1 deletion crates/fuel-core/src/schema/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,8 @@ impl TxMutation {
let tx_statuses = block_producer
.dry_run_txs(
transactions,
None,
None, // TODO(#1749): Pass parameter from API
None, // TODO(#1749): Pass parameter from API
utxo_validation,
gas_price.map(|x| x.into()),
)
Expand Down
3 changes: 2 additions & 1 deletion crates/fuel-core/src/service/adapters/graphql_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,12 @@ impl BlockProducerPort for BlockProducerAdapter {
&self,
transactions: Vec<Transaction>,
height: Option<BlockHeight>,
time: Option<Tai64>,
utxo_validation: Option<bool>,
gas_price: Option<u64>,
) -> anyhow::Result<Vec<TransactionExecutionStatus>> {
self.block_producer
.dry_run(transactions, height, utxo_validation, gas_price)
.dry_run(transactions, height, time, utxo_validation, gas_price)
.await
}
}
Expand Down
29 changes: 19 additions & 10 deletions crates/services/producer/src/block_producer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,9 @@ where

let source = tx_source(height);

let header = self.new_header(height, block_time).await?;
let header = self
.new_header_with_new_da_height(height, block_time)
.await?;

let gas_price = self.calculate_gas_price().await?;

Expand Down Expand Up @@ -268,26 +270,33 @@ where
GasPriceProvider: GasPriceProviderConstraint,
ConsensusProvider: ConsensusParametersProvider,
{
// TODO: Support custom `block_time` for `dry_run`.
/// Simulates multiple transactions without altering any state. Does not acquire the production lock.
/// since it is basically a "read only" operation and shouldn't get in the way of normal
/// production.
pub async fn dry_run(
&self,
transactions: Vec<Transaction>,
height: Option<BlockHeight>,
time: Option<Tai64>,
utxo_validation: Option<bool>,
gas_price: Option<u64>,
) -> anyhow::Result<Vec<TransactionExecutionStatus>> {
let view = self.view_provider.latest_view()?;
let height = height.unwrap_or_else(|| {
view.latest_height()
.unwrap_or_default()
let latest_height = view.latest_height().unwrap_or_default();

let simulated_height = height.unwrap_or_else(|| {
latest_height
.succ()
.expect("It is impossible to overflow the current block height")
});

let header = self._new_header(height, Tai64::now())?;
let simulated_time = time.unwrap_or_else(|| {
view.get_block(&latest_height)
.map(|block| block.header().time())
.unwrap_or(Tai64::UNIX_EPOCH)
});

let header = self.new_header(simulated_height, simulated_time)?;

let gas_price = if let Some(inner) = gas_price {
inner
Expand Down Expand Up @@ -341,12 +350,12 @@ where
ConsensusProvider: ConsensusParametersProvider,
{
/// Create the header for a new block at the provided height
async fn new_header(
async fn new_header_with_new_da_height(
&self,
height: BlockHeight,
block_time: Tai64,
) -> anyhow::Result<PartialBlockHeader> {
let mut block_header = self._new_header(height, block_time)?;
let mut block_header = self.new_header(height, block_time)?;
let previous_da_height = block_header.da_height;
let gas_limit = self
.consensus_parameters_provider
Expand All @@ -367,7 +376,7 @@ where
block_time: Tai64,
da_height: DaBlockHeight,
) -> anyhow::Result<PartialBlockHeader> {
let mut block_header = self._new_header(height, block_time)?;
let mut block_header = self.new_header(height, block_time)?;
block_header.application.da_height = da_height;
Ok(block_header)
}
Expand Down Expand Up @@ -414,7 +423,7 @@ where
}
}

fn _new_header(
fn new_header(
&self,
height: BlockHeight,
block_time: Tai64,
Expand Down
175 changes: 133 additions & 42 deletions crates/services/producer/src/block_producer/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,104 @@ mod produce_and_execute_block_txpool {
}
}

// Tests for the `dry_run` method.
mod dry_run {
use super::*;

#[tokio::test]
async fn dry_run__executes_with_given_timestamp() {
// Given
let simulated_block_time = Tai64::from_unix(1337);
let executor = MockExecutorWithCapture::default();
let ctx = TestContext::default_from_executor(executor.clone());

// When
let _ = ctx
.producer()
.dry_run(vec![], None, Some(simulated_block_time), None, None)
.await;

// Then
assert_eq!(executor.captured_block_timestamp(), simulated_block_time);
}

#[tokio::test]
async fn dry_run__executes_with_past_timestamp() {
// Given
let simulated_block_time = Tai64::UNIX_EPOCH;
let last_block_time = Tai64::from_unix(1337);

let executor = MockExecutorWithCapture::default();
let ctx = TestContextBuilder::new()
.with_prev_time(last_block_time)
.build_with_executor(executor.clone());

// When
let _ = ctx
.producer()
.dry_run(vec![], None, Some(simulated_block_time), None, None)
.await;

// Then
assert_eq!(executor.captured_block_timestamp(), simulated_block_time);
}

#[tokio::test]
async fn dry_run__uses_last_block_timestamp_when_no_time_provided() {
// Given
let last_block_time = Tai64::from_unix(1337);

let executor = MockExecutorWithCapture::default();
let ctx = TestContextBuilder::new()
.with_prev_time(last_block_time)
.build_with_executor(executor.clone());

// When
let _ = ctx.producer().dry_run(vec![], None, None, None, None).await;

// Then
assert_eq!(executor.captured_block_timestamp(), last_block_time);
}

#[tokio::test]
async fn dry_run__errors_early_if_height_is_lower_than_chain_tip() {
// Given
let last_block_height = BlockHeight::new(42);

let executor = MockExecutorWithCapture::default();
let ctx = TestContextBuilder::new()
.with_prev_height(last_block_height)
.build_with_executor(executor.clone());

// When
let _ = ctx
.producer()
.dry_run(vec![], last_block_height.pred(), None, None, None)
.await
.expect_err("expected failure");

// Then
assert!(executor.has_no_captured_block_timestamp());
}

impl MockExecutorWithCapture<Transaction> {
fn captured_block_timestamp(&self) -> Tai64 {
*self
.captured
.lock()
.unwrap()
.as_ref()
.expect("should have captured a block")
.header_to_produce
.time()
}

fn has_no_captured_block_timestamp(&self) -> bool {
self.captured.lock().unwrap().is_none()
}
}
}

use fuel_core_types::fuel_tx::field::MintGasPrice;
use proptest::{
prop_compose,
Expand Down Expand Up @@ -804,6 +902,7 @@ struct TestContextBuilder {
prev_da_height: DaBlockHeight,
block_gas_limit: Option<u64>,
prev_height: BlockHeight,
prev_time: Tai64,
}

impl TestContextBuilder {
Expand All @@ -814,6 +913,7 @@ impl TestContextBuilder {
prev_da_height: 1u64.into(),
block_gas_limit: None,
prev_height: 0u32.into(),
prev_time: Tai64::UNIX_EPOCH,
}
}

Expand Down Expand Up @@ -846,16 +946,25 @@ impl TestContextBuilder {
self
}

fn build(&self) -> TestContext<MockExecutor> {
fn with_prev_time(mut self, prev_time: Tai64) -> Self {
self.prev_time = prev_time;
self
}

fn pre_existing_blocks(&self) -> Arc<Mutex<HashMap<BlockHeight, CompressedBlock>>> {
let da_height = self.prev_da_height;
let previous_block = PartialFuelBlock {
let height = self.prev_height;
let time = self.prev_time;

let block = PartialFuelBlock {
header: PartialBlockHeader {
application: ApplicationHeader {
da_height,
..Default::default()
},
consensus: ConsensusHeader {
height: self.prev_height,
height,
time,
..Default::default()
},
},
Expand All @@ -865,67 +974,49 @@ impl TestContextBuilder {
.unwrap()
.compress(&Default::default());

let db = MockDb {
blocks: Arc::new(Mutex::new(
vec![(self.prev_height, previous_block)]
.into_iter()
.collect(),
)),
consensus_parameters_version: 0,
state_transition_bytecode_version: 0,
};
Arc::new(Mutex::new(HashMap::from_iter(Some((height, block)))))
}

fn build(self) -> TestContext<MockExecutor> {
let block_gas_limit = self.block_gas_limit.unwrap_or_default();

let mock_relayer = MockRelayer {
latest_block_height: self.latest_block_height,
latest_da_blocks_with_costs: self.blocks_with_gas_costs.clone(),
..Default::default()
};

let db = MockDb {
blocks: self.pre_existing_blocks(),
consensus_parameters_version: 0,
state_transition_bytecode_version: 0,
};

TestContext {
relayer: mock_relayer,
block_gas_limit: self.block_gas_limit.unwrap_or_default(),
block_gas_limit,
..TestContext::default_from_db(db)
}
}

fn build_with_executor<Ex>(&self, executor: Ex) -> TestContext<Ex> {
let da_height = self.prev_da_height;
let previous_block = PartialFuelBlock {
header: PartialBlockHeader {
application: ApplicationHeader {
da_height,
..Default::default()
},
consensus: ConsensusHeader {
height: self.prev_height,
..Default::default()
},
},
transactions: vec![],
}
.generate(&[], Default::default())
.unwrap()
.compress(&Default::default());

let db = MockDb {
blocks: Arc::new(Mutex::new(
vec![(self.prev_height, previous_block)]
.into_iter()
.collect(),
)),
consensus_parameters_version: 0,
state_transition_bytecode_version: 0,
};
fn build_with_executor<Ex>(self, executor: Ex) -> TestContext<Ex> {
let block_gas_limit = self.block_gas_limit.unwrap_or_default();

let mock_relayer = MockRelayer {
latest_block_height: self.latest_block_height,
latest_da_blocks_with_costs: self.blocks_with_gas_costs.clone(),
..Default::default()
};

let db = MockDb {
blocks: self.pre_existing_blocks(),
consensus_parameters_version: 0,
state_transition_bytecode_version: 0,
};

TestContext {
relayer: mock_relayer,
block_gas_limit: self.block_gas_limit.unwrap_or_default(),
block_gas_limit,
..TestContext::default_from_db_and_executor(db, executor)
}
}
Expand Down
14 changes: 14 additions & 0 deletions crates/services/producer/src/mocks.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::ports::{
BlockProducer,
BlockProducerDatabase,
DryRunner,
Relayer,
TxPool,
};
Expand Down Expand Up @@ -37,6 +38,7 @@ use fuel_core_types::{
Error as ExecutorError,
ExecutionResult,
Result as ExecutorResult,
TransactionExecutionStatus,
UncommittedResult,
},
txpool::ArcPoolTx,
Expand Down Expand Up @@ -227,6 +229,18 @@ impl BlockProducer<Vec<Transaction>> for MockExecutorWithCapture<Transaction> {
}
}

impl DryRunner for MockExecutorWithCapture<Transaction> {
fn dry_run(
&self,
block: Components<Vec<Transaction>>,
_utxo_validation: Option<bool>,
) -> ExecutorResult<Vec<TransactionExecutionStatus>> {
*self.captured.lock().unwrap() = Some(block);

Ok(Vec::new())
}
}

impl<Tx> Default for MockExecutorWithCapture<Tx> {
fn default() -> Self {
Self {
Expand Down
Loading

0 comments on commit 9d5ae1e

Please sign in to comment.