fix: 🚑 nonce management in the blockchain service #549
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…Labs/storage-hub into fix/nonce-manager-bcsv
ffarall
reviewed
Oct 31, 2025
ffarall
added
B5-clientnoteworthy
…Labs/storage-hub into fix/nonce-manager-bcsv
…Labs/storage-hub into fix/nonce-manager-bcsv
…Labs/storage-hub into fix/nonce-manager-bcsv
Karrq
pushed a commit
that referenced
this pull request
Nov 3, 2025
* feat: ✨ initial transaction pool and nonce manager logic * feat: ♻️ refactor watches to sync with new transaction pool * fix: 🐛 issue with status subscriptions * fix: 🩹 also process tx status updates on finality * feat: 🔊 improve log clarity by specifying which tx is being dropped * test: ✅ add Retracted and Usurped test cases for new logic * fix: 🎨 run cargo fmt * feat: ✨ add event extraction from successful transactions * style: 🎨 run cargo fmt * test: ✅ add Invalid transaction and nonce gap fulfilling test * feat: 🔊 Add error logs in case of failing to send status update of transactions * feat: ✅ add a way to select log level for actors in integration tests * feat: 🔊 Add logging of which call is related to which hash * fix: 🩹 Remove leftover `only: true` flag * perf: ⚡ Decrease retry timout in test * fix: 🩹 amend review (pool -> manager) * feat: ✨ add tip tracking in transaction manager * docs: 💡 add permalink to Polkadot SDK's `TransactionStatus` * test: ✅ rename and fix transaction manager integration test * fix: ✅ single volunteer test * test: ✅ improve BSP volunteer for multiple files test * feat: ✨ Allow arbitrary strings for log level in tests * feat: 🔊 Improve logs of transactions adding nonce * fix: 🐛 add initial check when watching for tx success * fix: 🐛 check for the `ExtrinsicSuccess` event to ensure the new ext didn't fail * test: ✅ minor test fixes * fix: 🩹 Fix typos and names of tests * fix: 🐛 Update transaction status in `run()` cycle of blockchain service * fix: 🩹 Remove processing of transaction status in block import and block finalised * fix: 🐛 skip to correct block when testing MSP charge * fix: 🩹 Remove `only: true` flag from test * fix: 🩹 Move `tx_status_receiver` out of `actor` struct * fix: 🐛 Avoid cross-talk between updates from replaced transactions * test: ✅ fix invalid transaction test by dropping retry tx --------- Co-authored-by: Facundo Farall <[email protected]>
snowmead
pushed a commit
that referenced
this pull request
Nov 3, 2025
* feat: ✨ initial transaction pool and nonce manager logic * feat: ♻️ refactor watches to sync with new transaction pool * fix: 🐛 issue with status subscriptions * fix: 🩹 also process tx status updates on finality * feat: 🔊 improve log clarity by specifying which tx is being dropped * test: ✅ add Retracted and Usurped test cases for new logic * fix: 🎨 run cargo fmt * feat: ✨ add event extraction from successful transactions * style: 🎨 run cargo fmt * test: ✅ add Invalid transaction and nonce gap fulfilling test * feat: 🔊 Add error logs in case of failing to send status update of transactions * feat: ✅ add a way to select log level for actors in integration tests * feat: 🔊 Add logging of which call is related to which hash * fix: 🩹 Remove leftover `only: true` flag * perf: ⚡ Decrease retry timout in test * fix: 🩹 amend review (pool -> manager) * feat: ✨ add tip tracking in transaction manager * docs: 💡 add permalink to Polkadot SDK's `TransactionStatus` * test: ✅ rename and fix transaction manager integration test * fix: ✅ single volunteer test * test: ✅ improve BSP volunteer for multiple files test * feat: ✨ Allow arbitrary strings for log level in tests * feat: 🔊 Improve logs of transactions adding nonce * fix: 🐛 add initial check when watching for tx success * fix: 🐛 check for the `ExtrinsicSuccess` event to ensure the new ext didn't fail * test: ✅ minor test fixes * fix: 🩹 Fix typos and names of tests * fix: 🐛 Update transaction status in `run()` cycle of blockchain service * fix: 🩹 Remove processing of transaction status in block import and block finalised * fix: 🐛 skip to correct block when testing MSP charge * fix: 🩹 Remove `only: true` flag from test * fix: 🩹 Move `tx_status_receiver` out of `actor` struct * fix: 🐛 Avoid cross-talk between updates from replaced transactions * test: ✅ fix invalid transaction test by dropping retry tx --------- Co-authored-by: Facundo Farall <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR solves the issue regarding nonce management and transactions being emitted with future nonces when reorgs or other issues happen, which bricked the blockchain service as from that point on no transaction sent by it would make it into a block.
This PR is a complete refactor of how the blockchain service handles transactions. Now, instead of relying on getting the
InBlocknotification for transactions it sent, the blockchain service has its own transaction pool that logs and keeps track of all status updates of all the transactions it sends.Not only this is useful for traceability, but with this plus some clever logic we manage to solve the nonce gap issue that was bricking the bcsv: now, dropped or invalid transactions get detected, its nonce gets cleared and the next transaction that the blockchain service has to send (or a
remarkif that takes too long) uses this now free nonce, making it so any other transactions that had a greater nonce get unstuck and make it into the pool.It's still a draft as there are three key TODOs to fix:
debuglog level, since the tests for this functionality rely on that.These three things are already in the works and should be ready soon. In the meantime, I'm opening this PR as a draft to facilitate the reviewing process.Done, marked as ready to review