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

Increase usage of new Fuel_txpool #238

Merged
merged 25 commits into from
Apr 4, 2022
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
93a7b1e
2 tests left
ControlCplusControlV Mar 27, 2022
0079f98
Fix all tests
ControlCplusControlV Mar 27, 2022
f891083
Fix find_tx to check mempool
ControlCplusControlV Mar 27, 2022
7cb5845
clippy and fmt
ControlCplusControlV Mar 27, 2022
48b4834
much neater
ControlCplusControlV Mar 28, 2022
27d8cde
much neater v2
ControlCplusControlV Mar 28, 2022
9d07c91
fix
ControlCplusControlV Mar 28, 2022
b6b01ab
Merge remote-tracking branch 'origin/master' into controlc/insert_rpc
ControlCplusControlV Mar 29, 2022
8de152f
Started runtime checking and moving to a new feature
ControlCplusControlV Mar 29, 2022
0f5c1e5
Merge remote-tracking branch 'origin/master' into controlc/insert_rpc
ControlCplusControlV Mar 31, 2022
6600da9
Correct error handling?
ControlCplusControlV Mar 31, 2022
7226644
forgot fmt
ControlCplusControlV Mar 31, 2022
a0c7b65
fix clippy
ControlCplusControlV Mar 31, 2022
a4f1aac
Update fuel-core/src/tx_pool.rs
ControlCplusControlV Apr 1, 2022
f76d637
Update fuel-core/src/tx_pool.rs
ControlCplusControlV Apr 1, 2022
82e2681
Update fuel-core/src/tx_pool.rs
ControlCplusControlV Apr 1, 2022
90644c8
Added Test
ControlCplusControlV Apr 2, 2022
d62c791
fmt
ControlCplusControlV Apr 2, 2022
8a5c9f7
Merge branch 'master' into controlc/insert_rpc
ControlCplusControlV Apr 2, 2022
ea8be2e
fix test
ControlCplusControlV Apr 2, 2022
715b4b8
checking status correctly
ControlCplusControlV Apr 2, 2022
88950a7
Transaction Status Success?
ControlCplusControlV Apr 2, 2022
082876b
forgot fmt again
ControlCplusControlV Apr 2, 2022
51750b7
Made tests more extensive
ControlCplusControlV Apr 2, 2022
2298094
clippy
ControlCplusControlV Apr 2, 2022
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
13 changes: 11 additions & 2 deletions fuel-core/src/schema/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,17 @@ impl TxQuery {
) -> async_graphql::Result<Option<Transaction>> {
let db = ctx.data_unchecked::<Database>();
let key = id.0;
Ok(Storage::<fuel_types::Bytes32, FuelTx>::get(db, &key)?
.map(|tx| Transaction(tx.into_owned())))

let tx_pool = ctx.data::<Arc<TxPool>>().unwrap();

let found_tx = tx_pool.pool().find(&[key]).await;

if let Some(Some(transaction)) = found_tx.get(0) {
Ok(Some(Transaction((transaction.deref()).clone())))
} else {
Ok(Storage::<fuel_types::Bytes32, FuelTx>::get(db, &key)?
.map(|tx| Transaction(tx.into_owned())))
}
}

async fn transactions(
Expand Down
42 changes: 39 additions & 3 deletions fuel-core/src/tx_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ pub enum Error {
Database(Box<dyn StdError>),
#[error("unexpected block execution error {0:?}")]
Execution(crate::executor::Error),
#[error("Tx is invalid, insertion failed {0:?}")]
Other(#[from] anyhow::Error),
}

impl From<KvStoreError> for Error {
Expand Down Expand Up @@ -83,9 +85,43 @@ impl TxPool {

pub async fn submit_tx(&self, tx: Transaction) -> Result<Bytes32, Error> {
let db = self.db.clone();
let tx_id = tx.id();

let mut tx_to_exec = tx.clone();

let includable_txs: Vec<Transaction>;

if self.executor.config.utxo_validation {
if tx_to_exec.metadata().is_none() {
tx_to_exec.precompute_metadata();
}

self.fuel_txpool
.insert(vec![Arc::new(tx_to_exec.clone())])
.await
.into_iter()
.collect::<Result<Vec<_>, _>>()?;

let includable_arc_txs = self.fuel_txpool.includable().await;

includable_txs = includable_arc_txs
.iter()
.map(|arc| Transaction::clone(&*arc))
.collect();

for included_tx in includable_arc_txs {
self.fuel_txpool.remove(&[included_tx.id()]).await;
}
} else {
includable_txs = vec![tx];
}

let tx_id = tx_to_exec.id();

// set status to submitted
db.update_tx_status(&tx_id, TransactionStatus::Submitted { time: Utc::now() })?;
db.update_tx_status(
&tx_id.clone(),
TransactionStatus::Submitted { time: Utc::now() },
)?;
Copy link
Contributor

@rakita rakita Apr 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Voxelot this seems like it needs a rework, updating status before it is executed is not a good flow, everything before block/tx inclusion shouldn't leave any trace in database, only after block is included we could have status of it.
Expected status of transaction is:

  • Pending (inside txpool)
  • Included (included in chain)

Is sdk using this information? Maybe we need to change how we get status, and do it in the same as it is done in TxQuery::transaction

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this needs rework and this was called out on the original task:

Update transaction status API to fallback to the txpool if the status doesn't exist in the database, using the Submitted status

I'm assuming there will be follow-up PR's to finish the rest of the planned work? @ControlCplusControlV

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Voxelot yes I am planning on using a fallback based on tx status rather than existence in a separate PR, as I figured there may be some overlap with #183 in part atleast, by tackling

if the transaction was very invalid (i.e. not includable in a block) this seems like it might need to be separate type of error from a tx that was partially processed within a block (i.e. utxos were spent and outputs created, but the script panicked).

As I could generate the first status based on if it errors when inserting into the tx pool (which would require utxo verification to be on to determine if a tx is invalid) and then the second would be a result of a failure but the tx was includable.

Unless you would rather that separate PR just cover lookup in mempool by status?


// setup and execute block
let current_height = db.get_block_height()?.unwrap_or_default();
Expand All @@ -103,7 +139,7 @@ impl TxPool {
// TODO: compute the current merkle root of all blocks
prev_root: Default::default(),
},
transactions: vec![tx],
transactions: includable_txs,
};
// immediately execute block
self.executor
Expand Down
25 changes: 25 additions & 0 deletions fuel-tests/tests/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,31 @@ async fn submit() {
assert_eq!(tx.id(), ret_tx.id());
}

#[tokio::test]
async fn submit_utxo_verified_tx() {
let config = Config {
utxo_validation: true,
..Config::local_node()
};

let srv = FuelService::new_node(config).await.unwrap();
let client = FuelClient::from(srv.bound_address);

let tx = Transaction::default();

let id = client.submit(&tx).await.unwrap();
// verify that the tx returned from the api matches the submitted tx
let ret_tx = client
.transaction(&id.0.to_string())
.await
.unwrap()
.unwrap()
.transaction;

let transaction_result = client.transaction_status(&ret_tx.id().to_string()).await;
assert!(transaction_result.is_ok());
Voxelot marked this conversation as resolved.
Show resolved Hide resolved
}

#[tokio::test]
async fn receipts() {
let transaction = fuel_tx::Transaction::default();
Expand Down