Skip to content

fatxpool: add fallback for ready at light#8533

Merged
iulianbarbu merged 33 commits intoparitytech:masterfrom
iulianbarbu:ib-add-fallback-for-ready-at-light
May 22, 2025
Merged

fatxpool: add fallback for ready at light#8533
iulianbarbu merged 33 commits intoparitytech:masterfrom
iulianbarbu:ib-add-fallback-for-ready-at-light

Conversation

@iulianbarbu
Copy link
Contributor

@iulianbarbu iulianbarbu commented May 14, 2025 •
edited
Loading

Description

Proposing a new block on top of an existing parent block considers the best ready transactions provided by the txpool to be included in the new block. Whenever the given parent hash to build on top of is part of a fork up to the finalized block which has no best block notified to the pool, it might be the case that the proposer will rely on ready_at_light (due to various reason not in our control), and when that's the case, the ready transactions set will be empty.

This PR adds a fallback for the ready_at_light where we consider the ready txs of the most recent view processed by the txpool, even if those txs might be invalid

Closes #8213
Closes #6056

Integration

N/A

Review Notes

In terms of testing, I updated an existing test which already exercises ready_at_light in the scope of the newly added fallback.

@iulianbarbu
add ready_at_light fallback
aa5bea3 
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu
update test
b54e528 
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu
improve comment
9e4b754 
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu
fix comment
be1d112 
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu iulianbarbu self-assigned this May 14, 2025
@iulianbarbu iulianbarbu added the I5-enhancement An additional feature request. label May 14, 2025
@iulianbarbu
Copy link
Contributor Author

iulianbarbu commented May 14, 2025

/cmd prdoc

github-actions bot and others added 4 commits May 14, 2025 13:03
@github-actions
Update from github-actions[bot] running command 'prdoc'
a42f414
@iulianbarbu
fix prdoc
489e1c9 
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu
Merge branch 'master' into ib-add-fallback-for-ready-at-light
475ae31
@iulianbarbu
try another prdoc fix
67bc04f 
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@michalkucharczyk
Copy link
Contributor

michalkucharczyk commented May 15, 2025

@iulianbarbu if you are reading through this part of code, maybe you would like to address this one:
#6056

Should also be easy one ;), and could be done in this PR.

@iulianbarbu
Copy link
Contributor Author

iulianbarbu commented May 15, 2025

Will address #6056 too, and will create a new test too.

@michalkucharczyk
Copy link
Contributor

michalkucharczyk commented May 15, 2025

Also, new fallback might be worth mentioning in the doc here:

https://github.com/paritytech/polkadot-sdk/blob/67bc04fa348e7f3ddcafdb33411de32cdd1727e8/substrate/client/transaction-pool/src/fork_aware_txpool/mod.rs#L215C1-L219C31

@michalkucharczyk
Copy link
Contributor

michalkucharczyk commented May 15, 2025

Some nits left, overall looks good 👍

iulianbarbu and others added 6 commits May 15, 2025 19:07
@iulianbarbu
revert old test and create a new one
83f970b 
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu
add missing assert
6c0b382 
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu @michalkucharczyk
Update substrate/client/transaction-pool/src/fork_aware_txpool/fork_a…
9a29656 
…ware_txpool.rs

Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>
@iulianbarbu
Merge branch 'master' into ib-add-fallback-for-ready-at-light
87418e1
@iulianbarbu
refactor the ready_at_light best view search
6727942 
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu
adjust docs
b858a23 
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu
Copy link
Contributor Author

iulianbarbu commented May 15, 2025

/cmd fmt

iulianbarbu
iulianbarbu commented May 16, 2025
View reviewed changes
iulianbarbu and others added 3 commits May 18, 2025 15:40
@iulianbarbu
fix comment
ebcf7aa 
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu
Merge branch 'master' into ib-add-fallback-for-ready-at-light
fbd6669
@iulianbarbu
fix build error
64d0e7a 
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
michalkucharczyk
michalkucharczyk reviewed May 19, 2025
View reviewed changes
Copy link
Contributor

@michalkucharczyk michalkucharczyk left a comment

Choose a reason for hiding this comment

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

The logic looks good. I proposed some renaming and signature adjustments.

iulianbarbu and others added 4 commits May 19, 2025 12:41
@iulianbarbu @michalkucharczyk
Update substrate/client/transaction-pool/tests/fatp.rs
ccc8cf2 
Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>
@iulianbarbu @michalkucharczyk
Update substrate/client/transaction-pool/src/fork_aware_txpool/mod.rs
1fe3507 
Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>
@iulianbarbu
more polishing after review
9d47e15 
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu
more polishing
5d7c1fb 
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu
Copy link
Contributor Author

iulianbarbu commented May 19, 2025

/cmd fmt

michalkucharczyk
michalkucharczyk reviewed May 19, 2025
View reviewed changes
@iulianbarbu
tests polishing
897ddfb 
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu
return most recent view ready txs if finalized block number can't be …
15afbb3 
…resolved

Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu
one more round of polish
f0bc263 
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu
address nit
2570fa3 
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
bkchr
bkchr approved these changes May 21, 2025
View reviewed changes
@iulianbarbu
address basti comments
a9a337d 
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu iulianbarbu added this pull request to the merge queue May 22, 2025
Merged via the queue into paritytech:master with commit 26afcd6 May 22, 2025
248 of 249 checks passed
@iulianbarbu iulianbarbu deleted the ib-add-fallback-for-ready-at-light branch May 22, 2025 13:34
ordian added a commit that referenced this pull request May 27, 2025
@ordian
Merge branch 'master' into ao-fix-issue-64
67a1bfe 
* master: (99 commits)
  Snowbridge: Remove asset location check for compatibility (#8473)
  add poke_deposit extrinsic to pallet-bounties (#8382)
  litep2p/peerset: Reject non-reserved peers in the reserved-only mode (#8650)
  Charge deposit based on key length (#8648)
  [pallet-revive] make subscription task panic on error (#8587)
  tx/metrics: Add metrics for the RPC v2 `transactionWatch_v1_submitAndWatch` (#8345)
  Bridges: Fix - Improve try-state for pallet-xcm-bridge-hub (#8615)
  Introduce CreateBare, deprecated CreateInherent (#7597)
  Use hashbrown hashmap/hashset in validation context (#8606)
  ci: rm gitlab config (#8622)
  🔪 flaky and Zombienet tests (#8600)
  cumulus: adjust unincluded segment size metric buckets (#8617)
  Benchmark storage access on block validation (#8069)
  Revert 7934 es/remove tj changes (#8611)
  collator-protocol: add more collation observability (#8230)
  `fatxpool`: add fallback for ready at light (#8533)
  txpool: fix tx removal from unlocks set (#8500)
  XCMP weight metering: account for the MQ page position (#8344)
  fix epmb solution duplicate issue + add remote mining apparatus to epm (#8585)
  Fix generated address returned by Substrate RPC runtime call (#8504)
  ...
pgherveou pushed a commit that referenced this pull request Jun 11, 2025
@iulianbarbu @github-actions
@michalkucharczyk @pgherveou
fatxpool: add fallback for ready at light (#8533)
f70e58a 
# Description

Proposing a new block on top of an existing parent block considers the
best ready transactions provided by the txpool to be included in the new
block. Whenever the given parent hash to build on top of is part of a
fork up to the finalized block which has no best block notified to the
pool, it might be the case that the proposer will rely on
`ready_at_light` (due to various reason not in our control), and when
that's the case, the ready transactions set will be empty.

This PR adds a fallback for the `ready_at_light` where we consider the
ready txs of the most recent view processed by the txpool, even if those
txs might be invalid

Closes #8213 
Closes #6056

## Integration

N/A

## Review Notes

In terms of testing, I updated an existing test which already exercises
`ready_at_light` in the scope of the newly added fallback.

---------

Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>
alvicsam pushed a commit that referenced this pull request Oct 17, 2025
@iulianbarbu @github-actions
@michalkucharczyk @alvicsam
fatxpool: add fallback for ready at light (#8533)
a7417e4 
# Description

Proposing a new block on top of an existing parent block considers the
best ready transactions provided by the txpool to be included in the new
block. Whenever the given parent hash to build on top of is part of a
fork up to the finalized block which has no best block notified to the
pool, it might be the case that the proposer will rely on
`ready_at_light` (due to various reason not in our control), and when
that's the case, the ready transactions set will be empty.

This PR adds a fallback for the `ready_at_light` where we consider the
ready txs of the most recent view processed by the txpool, even if those
txs might be invalid

Closes #8213 
Closes #6056

## Integration

N/A

## Review Notes

In terms of testing, I updated an existing test which already exercises
`ready_at_light` in the scope of the newly added fallback.

---------

Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michalkucharczyk michalkucharczyk michalkucharczyk approved these changes

@bkchr bkchr bkchr approved these changes

@skunert skunert Awaiting requested review from skunert

@lrubasze lrubasze Awaiting requested review from lrubasze

Assignees

@iulianbarbu iulianbarbu

Labels

I5-enhancement An additional feature request.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fatxpool: add one more fallback for ready_at_light fatxpool: ready_at_light optimization

3 participants

@iulianbarbu @michalkucharczyk @bkchr