Add function to update txpool on preconfirmation #2861

AurelienFT · 2025-03-12T08:57:29Z

Linked Issues/PRs

Resolve #2860

Description

When a pre-confirmation status has arrived to the pool the pool can take action to updates some states

Checklist

Breaking changes are clearly marked as such in the PR description and changelog
New behavior is reflected in tests
The specification matches the implemented behavior (link update PR if changes are needed)

Before requesting review

I have reviewed the code myself
I have created follow-up issues caused by this PR and linked them here

…nfirmations_txpool

crates/services/txpool_v2/src/pool_worker.rs

rymnc · 2025-03-12T20:05:41Z

crates/services/txpool_v2/src/pool_worker.rs

+                            self.expired_transactions_and_dependents(tx_and_dependents_ids);
+                        }
+                        PoolTxUpdateRequest::PreconfirmedTransactions { txs } => {
+                            self.preconfirmed_transactions(txs);


can we pass an iterator instead?

Suggested change

self.preconfirmed_transactions(txs);

self.preconfirmed_transactions(txs.iter());

rymnc · 2025-03-12T20:07:13Z

crates/services/txpool_v2/src/pool_worker.rs

        }
    }

+    fn preconfirmed_transactions(&mut self, txs: Vec<(TxId, PreconfirmationStatus)>) {


Suggested change

fn preconfirmed_transactions(&mut self, txs: Vec<(TxId, PreconfirmationStatus)>) {

fn preconfirmed_transactions(&mut self, txs: impl Iterator<Item = &(TxId, PreconfirmationStatus)>) {

perhaps?

rymnc · 2025-03-12T20:08:12Z

crates/services/txpool_v2/src/pool_worker.rs

    }

-    fn remove_and_coin_dependents(&mut self, tx_ids: (Vec<TxId>, Error)) {
+    fn expired_transactions_and_dependents(&mut self, tx_ids: (Vec<TxId>, Error)) {


i think we can take an iterator here too, no?

I don't like the type transformation it requires to write to make this works when calling this function because of the tuple, I would pass this one.

rymnc · 2025-03-12T20:08:41Z

crates/services/txpool_v2/src/shared_state.rs

 #[derive(Clone)]
 pub struct SharedState {
-    pub(crate) request_remove_sender: mpsc::Sender<PoolRemoveRequest>,
+    pub(crate) request_remove_sender: mpsc::Sender<PoolTxUpdateRequest>,


Suggested change

pub(crate) request_remove_sender: mpsc::Sender<PoolTxUpdateRequest>,

pub(crate) request_update_sender: mpsc::Sender<PoolTxUpdateRequest>,

rymnc · 2025-03-12T20:08:55Z

crates/services/txpool_v2/src/shared_state.rs

+
+    pub fn notify_preconfirmed_txs(&self, txs: Vec<(TxId, PreconfirmationStatus)>) {
+        if let Err(e) = self
+            .request_remove_sender


Suggested change

.request_remove_sender

.request_update_sender

rymnc · 2025-03-12T20:09:07Z

crates/services/txpool_v2/src/shared_state.rs

            .collect();

        if let Err(e) = self
            .request_remove_sender


Suggested change

.request_remove_sender

.request_update_sender

rymnc · 2025-03-12T20:09:29Z

crates/services/txpool_v2/src/pool_worker.rs

    thread_management_sender: Sender<ThreadManagementRequest>,
    pub(super) request_insert_sender: Sender<PoolInsertRequest>,
-    pub(super) request_remove_sender: Sender<PoolRemoveRequest>,
+    pub(super) request_remove_sender: Sender<PoolTxUpdateRequest>,


Suggested change

pub(super) request_remove_sender: Sender<PoolTxUpdateRequest>,

pub(super) request_update_sender: Sender<PoolTxUpdateRequest>,

rymnc · 2025-03-12T20:15:15Z

crates/services/txpool_v2/src/pending_pool.rs

+    pub fn new_known_tx<'a>(
        &mut self,
-        new_known_tx: ArcPoolTx,
+        new_known_tx_outputs: impl Iterator<Item = &'a Output>,


perhaps this Output/iterator should be boxed?
difficult to say without knowing average size of outputs that we have, but in the case the user has 255 outputs this would be overusing the cache and move other imp code out of the cpu cache

since we have a limit on the total number of outputs, perhaps we can use smallvec?

(i only write this because of the comment that says "we expect it to be called a lot")

xgreenx · 2025-03-13T04:08:55Z

crates/services/txpool_v2/src/pool_worker.rs


-pub(super) enum PoolRemoveRequest {
+pub(super) enum PoolTxUpdateRequest {
+    PreconfirmedTransactions {


I think instead of doing that, the transaction pool should subscribe for all pre-confirmation that comes from the TxStatusUpdater.

In that case you also don't need SkippedTransactions, because you will receive PreConfirmationSqueezedOut.

I made this in a new PR (easier): #2882 . However it's still the service that receive the data and pass it to the worker if necessary to minimize the usage of the worker thread.

xgreenx · 2025-03-13T04:12:06Z

crates/services/txpool_v2/src/extracted_outputs.rs

+                | Output::Change { .. }
+                | Output::Variable { .. } => {


Resolved outputs only contain Change and Variable outputs=D

BTW, we need to make a note to add an integration test to check that we can submit transaction that uses Change or Variable output form its parent after receiving a preconfiramtion

AurelienFT · 2025-03-18T14:32:48Z

Closing in favor of #2882 to address @xgreenx comment. Thanks for the review @rymnc I adressed your comments in the new PR.

AurelienFT and others added 11 commits March 11, 2025 21:09

Add expiration check on pending pool and fix error on saved_outputs

b67b2e1

Changelog

7b8a21c

Fix test fakyness

5fc6db4

Fix test

683d89e

Merge branch 'master' into add_timer_based_expiration_pending_pool

b779d2d

Refactor extracted outputs cache

b071c93

Update extract outputs to work with TxId useful for skipped transactions

1c7e283

update changelog

9949937

Fix clippy

29ae565

fmt

2cf1efb

Add function to update txpool

43a8a13

AurelienFT requested review from Dentosal, MitchTurner and xgreenx as code owners March 12, 2025 08:57

Changelog

8b0a319

AurelienFT requested a review from a team as a code owner March 12, 2025 08:59

AurelienFT and others added 3 commits March 12, 2025 10:15

Use outputs to update pool components

e61aa8f

Remove expiration out of timer

e90de63

Merge branch 'add_timer_based_expiration_pending_pool' into use_preco…

0a3759e

…nfirmations_txpool

AurelienFT self-assigned this Mar 12, 2025

rymnc reviewed Mar 12, 2025

View reviewed changes

crates/services/txpool_v2/src/pool_worker.rs Show resolved Hide resolved

rymnc reviewed Mar 12, 2025

View reviewed changes

xgreenx reviewed Mar 13, 2025

View reviewed changes

Base automatically changed from add_timer_based_expiration_pending_pool to master March 13, 2025 08:31

AurelienFT requested a review from netrome as a code owner March 13, 2025 08:31

Merge branch 'master' into use_preconfirmations_txpool

c258ff7

AurelienFT closed this Mar 18, 2025

	self.preconfirmed_transactions(txs);
	self.preconfirmed_transactions(txs.iter());

	fn preconfirmed_transactions(&mut self, txs: Vec<(TxId, PreconfirmationStatus)>) {
	fn preconfirmed_transactions(&mut self, txs: impl Iterator<Item = &(TxId, PreconfirmationStatus)>) {

	pub(crate) request_remove_sender: mpsc::Sender<PoolTxUpdateRequest>,
	pub(crate) request_update_sender: mpsc::Sender<PoolTxUpdateRequest>,

	pub(super) request_remove_sender: Sender<PoolTxUpdateRequest>,
	pub(super) request_update_sender: Sender<PoolTxUpdateRequest>,

Add function to update txpool on preconfirmation #2861

Add function to update txpool on preconfirmation #2861

Uh oh!

Conversation

AurelienFT commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Linked Issues/PRs

Description

Checklist

Before requesting review

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rymnc Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AurelienFT commented Mar 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AurelienFT commented Mar 12, 2025 •

edited

Loading

rymnc Mar 12, 2025 •

edited

Loading