Skip to content

Commit

Permalink
Bugfix: Calling of the contract with enabled utxo_validation fails (#…
Browse files Browse the repository at this point in the history
…1717)

The fix for the [#1657](#1657)
to include the contract into `ContractsInfo` table.

Thanks @Torres-ssf for pointing and debugging it.

---------

Co-authored-by: xgreenx <[email protected]>
  • Loading branch information
Brandon Vrooman and xgreenx authored Mar 4, 2024
1 parent 634bc16 commit 3bdecfd
Show file tree
Hide file tree
Showing 11 changed files with 195 additions and 31 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

### Changed

- [#1717](https://github.com/FuelLabs/fuel-core/pull/1717): The fix for the [#1657](https://github.com/FuelLabs/fuel-core/pull/1657) to include the contract into `ContractsInfo` table.
- [#1657](https://github.com/FuelLabs/fuel-core/pull/1657): Upgrade to `fuel-vm` 0.46.0.
- [#1671](https://github.com/FuelLabs/fuel-core/pull/1671): The logic related to the `FuelBlockIdsToHeights` is moved to the off-chain worker.
- [#1663](https://github.com/FuelLabs/fuel-core/pull/1663): Reduce the punishment criteria for mempool gossipping.
Expand Down
13 changes: 10 additions & 3 deletions crates/fuel-core/src/service/adapters/consensus_module/poa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ use fuel_core_types::{
BlockImportInfo,
UncommittedResult as UncommittedImporterResult,
},
executor::UncommittedResult,
executor::{
Error as ExecutorError,
UncommittedResult,
},
txpool::ArcPoolTx,
},
tai64::Tai64,
Expand Down Expand Up @@ -82,8 +85,12 @@ impl TransactionPool for TxPoolAdapter {
self.service.total_consumable_gas()
}

fn remove_txs(&self, ids: Vec<TxId>) -> Vec<ArcPoolTx> {
self.service.remove_txs(ids)
fn remove_txs(&self, ids: Vec<(TxId, ExecutorError)>) -> Vec<ArcPoolTx> {
self.service.remove_txs(
ids.into_iter()
.map(|(tx_id, err)| (tx_id, err.to_string()))
.collect(),
)
}

fn transaction_status_events(&self) -> BoxStream<TxId> {
Expand Down
7 changes: 5 additions & 2 deletions crates/services/consensus_module/poa/src/ports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ use fuel_core_types::{
BlockImportInfo,
UncommittedResult as UncommittedImportResult,
},
executor::UncommittedResult as UncommittedExecutionResult,
executor::{
Error as ExecutorError,
UncommittedResult as UncommittedExecutionResult,
},
txpool::ArcPoolTx,
},
tai64::Tai64,
Expand All @@ -35,7 +38,7 @@ pub trait TransactionPool: Send + Sync {

fn total_consumable_gas(&self) -> u64;

fn remove_txs(&self, tx_ids: Vec<TxId>) -> Vec<ArcPoolTx>;
fn remove_txs(&self, tx_ids: Vec<(TxId, ExecutorError)>) -> Vec<ArcPoolTx>;

fn transaction_status_events(&self) -> BoxStream<TxId>;
}
Expand Down
2 changes: 1 addition & 1 deletion crates/services/consensus_module/poa/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ where
tx_id,
err
);
tx_ids_to_remove.push(tx_id);
tx_ids_to_remove.push((tx_id, err));
}
self.txpool.remove_txs(tx_ids_to_remove);

Expand Down
11 changes: 6 additions & 5 deletions crates/services/consensus_module/poa/src/service_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,15 +232,15 @@ impl MockTransactionPool {
.sum()
});
let removed = txs.clone();
txpool
.expect_remove_txs()
.returning(move |tx_ids: Vec<TxId>| {
txpool.expect_remove_txs().returning(
move |tx_ids: Vec<(TxId, ExecutorError)>| {
let mut guard = removed.lock().unwrap();
for id in tx_ids {
for (id, _) in tx_ids {
guard.retain(|tx| tx.id(&ChainId::default()) == id);
}
vec![]
});
},
);

TxPoolContext {
txpool,
Expand Down Expand Up @@ -308,6 +308,7 @@ async fn remove_skipped_transactions() {
let mut txpool = MockTransactionPool::no_tx_updates();
// Test created for only for this check.
txpool.expect_remove_txs().returning(move |skipped_ids| {
let skipped_ids: Vec<_> = skipped_ids.into_iter().map(|(id, _)| id).collect();
// Transform transactions into ids.
let skipped_transactions: Vec<_> = skipped_transactions
.iter()
Expand Down
17 changes: 16 additions & 1 deletion crates/services/executor/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ use fuel_core_types::{
MintAssetId,
MintGasPrice,
OutputContract,
Salt,
TxPointer as TxPointerField,
},
input,
Expand Down Expand Up @@ -1017,7 +1018,7 @@ where
.into();
let reverted = vm_result.should_revert();

let (state, mut tx, receipts) = vm_result.into_inner();
let (state, mut tx, receipts): (_, Tx, _) = vm_result.into_inner();
#[cfg(debug_assertions)]
{
tx.precompute(&self.config.consensus_parameters.chain_id)?;
Expand Down Expand Up @@ -1073,6 +1074,20 @@ where
})
}

// TODO: Move to an off-chain worker: https://github.com/FuelLabs/fuel-core/issues/1721
if let Some(create) = tx.as_create() {
let contract_id = create
.metadata()
.as_ref()
.expect("The metadata always should exist after VM execution stage")
.contract_id;
let salt = *create.salt();
tx_st_transaction
.as_mut()
.storage::<ContractsInfo>()
.insert(&contract_id, &(salt.into()))?;
}

let final_tx = tx.into();

// Store tx into the block db transaction
Expand Down
8 changes: 4 additions & 4 deletions crates/services/txpool/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,8 @@ impl<P2P, ViewProvider> SharedState<P2P, ViewProvider> {
self.txpool.lock().consumable_gas()
}

pub fn remove_txs(&self, ids: Vec<TxId>) -> Vec<ArcPoolTx> {
self.txpool.lock().remove(&self.tx_status_sender, &ids)
pub fn remove_txs(&self, ids: Vec<(TxId, String)>) -> Vec<ArcPoolTx> {
self.txpool.lock().remove(&self.tx_status_sender, ids)
}

pub fn find(&self, ids: Vec<TxId>) -> Vec<Option<TxInfo>> {
Expand All @@ -330,8 +330,8 @@ impl<P2P, ViewProvider> SharedState<P2P, ViewProvider> {
sorted_txs
}

pub fn remove(&self, ids: Vec<TxId>) -> Vec<ArcPoolTx> {
self.txpool.lock().remove(&self.tx_status_sender, &ids)
pub fn remove(&self, ids: Vec<(TxId, String)>) -> Vec<ArcPoolTx> {
self.txpool.lock().remove(&self.tx_status_sender, ids)
}

pub fn new_tx_notification_subscribe(&self) -> broadcast::Receiver<TxId> {
Expand Down
19 changes: 14 additions & 5 deletions crates/services/txpool/src/service/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,15 +246,23 @@ async fn simple_insert_removal_subscription() {
}

// remove them
service
.shared
.remove(vec![tx1.cached_id().unwrap(), tx2.cached_id().unwrap()]);
service.shared.remove(vec![
(
tx1.cached_id().unwrap(),
"Because of the test purposes".to_string(),
),
(
tx2.cached_id().unwrap(),
"Because of the test purposes".to_string(),
),
]);

let update = tx1_subscribe_updates.next().await.unwrap();
assert_eq!(
update,
TxStatusMessage::Status(TransactionStatus::SqueezedOut {
reason: "Transaction removed.".to_string()
reason: "Transaction squeezed out because Because of the test purposes"
.to_string()
}),
"Second message in tx1 stream should be squeezed out"
);
Expand All @@ -263,7 +271,8 @@ async fn simple_insert_removal_subscription() {
assert_eq!(
update,
TxStatusMessage::Status(TransactionStatus::SqueezedOut {
reason: "Transaction removed.".to_string()
reason: "Transaction squeezed out because Because of the test purposes"
.to_string()
}),
"Second message in tx2 stream should be squeezed out"
);
Expand Down
17 changes: 11 additions & 6 deletions crates/services/txpool/src/txpool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,15 +206,20 @@ impl<ViewProvider> TxPool<ViewProvider> {
pub fn remove(
&mut self,
tx_status_sender: &TxStatusChange,
tx_ids: &[TxId],
tx_ids: Vec<(TxId, String)>,
) -> Vec<ArcPoolTx> {
let mut removed = Vec::new();
for tx_id in tx_ids {
let rem = self.remove_by_tx_id(tx_id);
tx_status_sender.send_squeezed_out(*tx_id, Error::Removed);
for (tx_id, reason) in tx_ids.into_iter() {
let rem = self.remove_by_tx_id(&tx_id);
tx_status_sender.send_squeezed_out(tx_id, Error::SqueezedOut(reason.clone()));
for dependent_tx in rem.iter() {
if tx_id != &dependent_tx.id() {
tx_status_sender.send_squeezed_out(dependent_tx.id(), Error::Removed);
if tx_id != dependent_tx.id() {
tx_status_sender.send_squeezed_out(
dependent_tx.id(),
Error::SqueezedOut(
format!("Parent transaction with {tx_id}, was removed because of the {reason}")
)
);
}
}
removed.extend(rem.into_iter());
Expand Down
6 changes: 6 additions & 0 deletions crates/types/src/entities/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,9 @@ impl From<Salt> for ContractsInfoTypeV1 {
Self { salt }
}
}

impl From<Salt> for ContractsInfoType {
fn from(salt: Salt) -> Self {
ContractsInfoType::V1(salt.into())
}
}
125 changes: 121 additions & 4 deletions tests/tests/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,15 @@ use crate::helpers::{
TestContext,
TestSetupBuilder,
};
use fuel_core::service::{
Config,
FuelService,
use fuel_core::{
database::{
database_description::on_chain::OnChain,
Database,
},
service::{
Config,
FuelService,
},
};
use fuel_core_client::client::{
pagination::{
Expand All @@ -18,12 +24,123 @@ use fuel_core_types::{
fuel_asm::*,
fuel_tx::*,
fuel_types::canonical::Serialize,
fuel_vm::*,
fuel_vm::{
checked_transaction::IntoChecked,
*,
},
};
use rand::SeedableRng;

use fuel_core::chain_config::{
ChainConfig,
CoinConfig,
StateConfig,
};
use rstest::rstest;

const SEED: u64 = 2322;

#[tokio::test]
async fn calling_the_contract_with_enabled_utxo_validation_is_successful() {
let mut rng = rand::rngs::StdRng::seed_from_u64(0xBAADF00D);
let secret = SecretKey::random(&mut rng);
let amount = 10000;
let owner = Input::owner(&secret.public_key());
let utxo_id_1 = UtxoId::new([1; 32].into(), 0);
let utxo_id_2 = UtxoId::new([1; 32].into(), 1);

let config = Config {
debug: true,
utxo_validation: true,
chain_conf: ChainConfig {
initial_state: Some(StateConfig {
coins: Some(vec![
CoinConfig {
tx_id: Some(*utxo_id_1.tx_id()),
output_index: Some(utxo_id_1.output_index()),
owner,
amount,
asset_id: AssetId::BASE,
..Default::default()
},
CoinConfig {
tx_id: Some(*utxo_id_2.tx_id()),
output_index: Some(utxo_id_2.output_index()),
owner,
amount,
asset_id: AssetId::BASE,
..Default::default()
},
]),
..Default::default()
}),
..ChainConfig::local_testnet()
},
..Config::local_node()
};
let node = FuelService::from_database(Database::<OnChain>::in_memory(), config)
.await
.unwrap();
let client = FuelClient::from(node.bound_address);

// Given
let contract_input = {
let bytecode: Witness = vec![].into();
let salt = Salt::zeroed();
let contract = Contract::from(bytecode.as_ref());
let code_root = contract.root();
let balance_root = Contract::default_state_root();
let state_root = Contract::default_state_root();

let contract_id = contract.id(&salt, &code_root, &state_root);
let output = Output::contract_created(contract_id, state_root);
let create_tx = TransactionBuilder::create(bytecode, salt, vec![])
.add_unsigned_coin_input(
secret,
utxo_id_1,
amount,
Default::default(),
Default::default(),
)
.add_output(output)
.finalize_as_transaction()
.into_checked(Default::default(), &Default::default())
.expect("Cannot check transaction");

let contract_input = Input::contract(
UtxoId::new(create_tx.id(), 1),
balance_root,
state_root,
Default::default(),
contract_id,
);

client
.submit_and_await_commit(create_tx.transaction())
.await
.expect("cannot insert tx into transaction pool");

contract_input
};

// When
let contract_tx = TransactionBuilder::script(vec![], vec![])
.add_input(contract_input)
.add_unsigned_coin_input(
secret,
utxo_id_2,
amount,
Default::default(),
Default::default(),
)
.add_output(Output::contract(0, Default::default(), Default::default()))
.finalize_as_transaction();
let tx_status = client.submit_and_await_commit(&contract_tx).await.unwrap();

// Then
assert!(matches!(tx_status, TransactionStatus::Success { .. }));
}

#[rstest]
#[tokio::test]
async fn test_contract_balance(
Expand Down

0 comments on commit 3bdecfd

Please sign in to comment.