Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adapted the block producer to react on the outdated transactions from the TxPool #2002

Merged
merged 6 commits into from
Jul 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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/).
- [#1948](https://github.com/FuelLabs/fuel-core/pull/1948): Add new `AlgorithmV1` and `AlgorithmUpdaterV1` for the gas price. Include tools for analysis

### Changed
- [#2002](https://github.com/FuelLabs/fuel-core/pull/2002): Adapted the block producer to react to checked transactions that were using another version of consensus parameters during validation in the TxPool. After an upgrade of the consensus parameters of the network, TxPool could store invalid `Checked` transactions. This change fixes that by tracking the version that was used to validate the transactions.
- [#1999](https://github.com/FuelLabs/fuel-core/pull/1999): Minimize the number of panics in the codebase.
- [#1990](https://github.com/FuelLabs/fuel-core/pull/1990): Use latest view for mutate GraphQL queries after modification of the node.
- [#1992](https://github.com/FuelLabs/fuel-core/pull/1992): Parse multiple relayer contracts, `RELAYER-V2-LISTENING-CONTRACTS` env variable using a `,` delimiter.
Expand Down
Binary file not shown.
94 changes: 93 additions & 1 deletion crates/fuel-core/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ mod tests {
use fuel_core::database::Database;
use fuel_core_executor::{
executor::OnceTransactionsSource,
ports::RelayerPort,
ports::{
MaybeCheckedTransaction,
RelayerPort,
},
refs::ContractRef,
};
use fuel_core_storage::{
Expand All @@ -32,6 +35,7 @@ mod tests {
PartialFuelBlock,
},
header::{
ApplicationHeader,
ConsensusHeader,
PartialBlockHeader,
},
Expand All @@ -52,6 +56,7 @@ mod tests {
fuel_crypto::SecretKey,
fuel_merkle::sparse,
fuel_tx::{
consensus_parameters::gas::GasCostsValuesV1,
field::{
InputContract,
Inputs,
Expand All @@ -76,8 +81,10 @@ mod tests {
Cacheable,
ConsensusParameters,
Create,
DependentCost,
FeeParameters,
Finalizable,
GasCostsValues,
Output,
Receipt,
Script,
Expand All @@ -104,6 +111,7 @@ mod tests {
checked_transaction::{
CheckError,
EstimatePredicates,
IntoChecked,
},
interpreter::{
ExecutableTransaction,
Expand Down Expand Up @@ -2730,6 +2738,90 @@ mod tests {
assert!(result.is_ok(), "{result:?}")
}

#[test]
fn verifying_during_production_consensus_parameters_version_works() {
let mut rng = StdRng::seed_from_u64(2322u64);
let predicate: Vec<u8> = vec![op::ret(RegId::ONE)].into_iter().collect();
let owner = Input::predicate_owner(&predicate);
let amount = 1000;
let cheap_consensus_parameters = ConsensusParameters::default();

let mut tx = TransactionBuilder::script(vec![], vec![])
.max_fee_limit(amount)
.add_input(Input::coin_predicate(
rng.gen(),
owner,
amount,
AssetId::BASE,
rng.gen(),
0,
predicate,
vec![],
))
.finalize();
tx.estimate_predicates(
&cheap_consensus_parameters.clone().into(),
MemoryInstance::new(),
)
.unwrap();

// Given
let gas_costs: GasCostsValues = GasCostsValuesV1 {
vm_initialization: DependentCost::HeavyOperation {
base: u32::MAX as u64,
gas_per_unit: 0,
},
..GasCostsValuesV1::free()
}
.into();
let expensive_consensus_parameters_version = 0;
let mut expensive_consensus_parameters = ConsensusParameters::default();
expensive_consensus_parameters.set_gas_costs(gas_costs.into());
let config = Config {
consensus_parameters: expensive_consensus_parameters.clone(),
..Default::default()
};
let producer = create_executor(Database::default(), config.clone());

let cheap_consensus_parameters_version = 1;
let cheaply_checked_tx = MaybeCheckedTransaction::CheckedTransaction(
tx.into_checked_basic(0u32.into(), &cheap_consensus_parameters)
.unwrap()
.into(),
cheap_consensus_parameters_version,
);

// When
let ExecutionResult {
skipped_transactions,
..
} = producer
.produce_without_commit_with_source(Components {
header_to_produce: PartialBlockHeader {
application: ApplicationHeader {
consensus_parameters_version:
expensive_consensus_parameters_version,
..Default::default()
},
..Default::default()
},
transactions_source: OnceTransactionsSource::new_maybe_checked(vec![
cheaply_checked_tx,
]),
coinbase_recipient: Default::default(),
gas_price: 1,
})
.unwrap()
.into_result();

// Then
assert_eq!(skipped_transactions.len(), 1);
assert!(matches!(
skipped_transactions[0].1,
ExecutorError::InvalidTransaction(_)
));
}

#[cfg(feature = "relayer")]
mod relayer {
use super::*;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,16 @@ impl SharedState {
}

pub fn latest_consensus_parameters(&self) -> Arc<ConsensusParameters> {
self.latest_consensus_parameters_with_version().1
}

pub fn latest_consensus_parameters_with_version(
&self,
) -> (ConsensusParametersVersion, Arc<ConsensusParameters>) {
let version = *self.latest_consensus_parameters_version.lock();
self.get_consensus_parameters(&version)
.expect("The latest consensus parameters always are available unless this function was called before regenesis.")
let params = self.get_consensus_parameters(&version)
.expect("The latest consensus parameters always are available unless this function was called before regenesis.");
(version, params)
}
}

Expand Down
7 changes: 6 additions & 1 deletion crates/fuel-core/src/service/adapters/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@ impl fuel_core_executor::ports::TransactionsSource for TransactionsSource {
self.txpool
.select_transactions(gas_limit)
.into_iter()
.map(|tx| MaybeCheckedTransaction::CheckedTransaction(tx.as_ref().into()))
.map(|tx| {
MaybeCheckedTransaction::CheckedTransaction(
tx.as_ref().into(),
tx.used_consensus_parameters_version(),
)
})
.collect()
}
}
Expand Down
7 changes: 5 additions & 2 deletions crates/fuel-core/src/service/adapters/txpool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use fuel_core_txpool::ports::{
MemoryPool,
};
use fuel_core_types::{
blockchain::header::ConsensusParametersVersion,
entities::{
coins::coin::CompressedCoin,
relayer::message::Message,
Expand Down Expand Up @@ -149,8 +150,10 @@ impl GasPriceProvider for StaticGasPrice {
}

impl ConsensusParametersProviderTrait for ConsensusParametersProvider {
fn latest_consensus_parameters(&self) -> Arc<ConsensusParameters> {
self.shared_state.latest_consensus_parameters()
fn latest_consensus_parameters(
&self,
) -> (ConsensusParametersVersion, Arc<ConsensusParameters>) {
self.shared_state.latest_consensus_parameters_with_version()
}
}

Expand Down
27 changes: 23 additions & 4 deletions crates/services/executor/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,12 @@ impl OnceTransactionsSource {
),
}
}

pub fn new_maybe_checked(transactions: Vec<MaybeCheckedTransaction>) -> Self {
Self {
transactions: ParkingMutex::new(transactions),
}
}
}

impl TransactionsSource for OnceTransactionsSource {
Expand Down Expand Up @@ -697,10 +703,13 @@ where
{
let block_header = partial_block.header;
let block_height = block_header.height();
let consensus_parameters_version = block_header.consensus_parameters_version;
let relayed_tx_iter = forced_transactions.into_iter();
for transaction in relayed_tx_iter {
let maybe_checked_transaction =
MaybeCheckedTransaction::CheckedTransaction(transaction);
for checked in relayed_tx_iter {
let maybe_checked_transaction = MaybeCheckedTransaction::CheckedTransaction(
checked,
consensus_parameters_version,
);
let tx_id = maybe_checked_transaction.id(&self.consensus_params.chain_id());
match self.execute_transaction_and_commit(
partial_block,
Expand Down Expand Up @@ -1038,11 +1047,21 @@ where
header: &PartialBlockHeader,
) -> ExecutorResult<CheckedTransaction> {
let block_height = *header.height();
let actual_version = header.consensus_parameters_version;
let checked_tx = match tx {
MaybeCheckedTransaction::Transaction(tx) => tx
.into_checked_basic(block_height, &self.consensus_params)?
.into(),
MaybeCheckedTransaction::CheckedTransaction(checked_tx) => checked_tx,
MaybeCheckedTransaction::CheckedTransaction(checked_tx, checked_version) => {
if actual_version == checked_version {
checked_tx
} else {
let checked_tx: Checked<Transaction> = checked_tx.into();
let (tx, _) = checked_tx.into();
tx.into_checked_basic(block_height, &self.consensus_params)?
.into()
}
}
};
Ok(checked_tx)
}
Expand Down
42 changes: 25 additions & 17 deletions crates/services/executor/src/ports.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use fuel_core_types::{
blockchain::primitives::DaBlockHeight,
blockchain::{
header::ConsensusParametersVersion,
primitives::DaBlockHeight,
},
fuel_tx,
fuel_tx::{
TxId,
Expand All @@ -12,28 +15,33 @@ use fuel_core_types::{

/// The wrapper around either `Transaction` or `CheckedTransaction`.
pub enum MaybeCheckedTransaction {
CheckedTransaction(CheckedTransaction),
CheckedTransaction(CheckedTransaction, ConsensusParametersVersion),
Transaction(fuel_tx::Transaction),
}

impl MaybeCheckedTransaction {
pub fn id(&self, chain_id: &ChainId) -> TxId {
match self {
MaybeCheckedTransaction::CheckedTransaction(CheckedTransaction::Script(
tx,
)) => tx.id(),
MaybeCheckedTransaction::CheckedTransaction(CheckedTransaction::Create(
tx,
)) => tx.id(),
MaybeCheckedTransaction::CheckedTransaction(CheckedTransaction::Mint(tx)) => {
tx.id()
}
MaybeCheckedTransaction::CheckedTransaction(CheckedTransaction::Upgrade(
tx,
)) => tx.id(),
MaybeCheckedTransaction::CheckedTransaction(CheckedTransaction::Upload(
tx,
)) => tx.id(),
MaybeCheckedTransaction::CheckedTransaction(
CheckedTransaction::Script(tx),
_,
) => tx.id(),
MaybeCheckedTransaction::CheckedTransaction(
CheckedTransaction::Create(tx),
_,
) => tx.id(),
MaybeCheckedTransaction::CheckedTransaction(
CheckedTransaction::Mint(tx),
_,
) => tx.id(),
MaybeCheckedTransaction::CheckedTransaction(
CheckedTransaction::Upgrade(tx),
_,
) => tx.id(),
MaybeCheckedTransaction::CheckedTransaction(
CheckedTransaction::Upload(tx),
_,
) => tx.id(),
MaybeCheckedTransaction::Transaction(tx) => tx.id(chain_id),
}
}
Expand Down
5 changes: 4 additions & 1 deletion crates/services/txpool/src/ports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::types::GasPrice;
use fuel_core_services::stream::BoxStream;
use fuel_core_storage::Result as StorageResult;
use fuel_core_types::{
blockchain::header::ConsensusParametersVersion,
entities::{
coins::coin::CompressedCoin,
relayer::message::Message,
Expand Down Expand Up @@ -81,7 +82,9 @@ pub trait MemoryPool {
#[cfg_attr(feature = "test-helpers", mockall::automock)]
pub trait ConsensusParametersProvider {
/// Get latest consensus parameters.
fn latest_consensus_parameters(&self) -> Arc<ConsensusParameters>;
fn latest_consensus_parameters(
&self,
) -> (ConsensusParametersVersion, Arc<ConsensusParameters>);
}

#[async_trait::async_trait]
Expand Down
11 changes: 8 additions & 3 deletions crates/services/txpool/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ where
new_transaction = self.gossiped_tx_stream.next() => {
if let Some(GossipData { data: Some(tx), message_id, peer_id }) = new_transaction {
let current_height = *self.tx_pool_shared_state.current_height.lock();
let params = self
let (version, params) = self
.tx_pool_shared_state
.consensus_parameters_provider
.latest_consensus_parameters();
Expand All @@ -265,6 +265,7 @@ where
.in_scope(|| {
self.tx_pool_shared_state.txpool.lock().insert(
&self.tx_pool_shared_state.tx_status_sender,
version,
txs
)
});
Expand Down Expand Up @@ -400,7 +401,7 @@ where
) -> Vec<Result<InsertionResult, Error>> {
// verify txs
let current_height = *self.current_height.lock();
let params = self
let (version, params) = self
.consensus_parameters_provider
.latest_consensus_parameters();

Expand Down Expand Up @@ -428,7 +429,11 @@ where
.collect();

// insert txs
let insertion = { self.txpool.lock().insert(&self.tx_status_sender, valid_txs) };
let insertion = {
self.txpool
.lock()
.insert(&self.tx_status_sender, version, valid_txs)
};

for (ret, tx) in insertion.iter().zip(txs.into_iter()) {
match ret {
Expand Down
2 changes: 1 addition & 1 deletion crates/services/txpool/src/service/test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ impl TestContextBuilder {
MockConsensusParametersProvider::default();
consensus_parameters_provider
.expect_latest_consensus_parameters()
.returning(|| Arc::new(Default::default()));
.returning(|| (0, Arc::new(Default::default())));

let service = new_service(
config,
Expand Down
Loading
Loading