Skip to content

Simplify tx watch ready-queue lookup and log decode failures#593

Merged
n13 merged 1 commit into
mainfrom
tx-watch-update
Jun 12, 2026
Merged

Simplify tx watch ready-queue lookup and log decode failures#593
n13 merged 1 commit into
mainfrom
tx-watch-update

Conversation

@n13

@n13 n13 commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Remove the fallback scan over the ready queue in TxWatch; rely solely on pool.ready_transaction() for lookup
  • Add warn-level logging when stripping the length prefix or decoding a ready-queue transaction fails, instead of silently skipping

@n13

n13 commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator Author

Review

Verdict: LGTM (evaluation only, not a formal approval)

What I checked

  • The removed fallback was dead code. The node pins TransactionPoolType::SingleState (node/src/service.rs:625). In the single-state pool, ready_transaction() is a direct hash lookup into the validated pool's ready set (validated_pool().ready_by_hash), and ready() iterates that same set — so the linear scan could never find a transaction the direct lookup missed. Even under the fork-aware pool, both methods read the same most-recent view (view_store.ready() / view_store.ready_transaction()), so the conclusion holds regardless of pool type.
  • Warn-level logs on decode failure replace silent continues — decode failures here would indicate a real bug (a ready-pool extrinsic that doesn't decode as UncheckedExtrinsic), so surfacing them is right. The not-in-ready-queue case correctly stays at trace, since that's an expected race with block inclusion.
  • Local verification: branch builds and all 17 txwatch tests pass.

Nit (non-blocking)

  • Commit message is "minor tx watch update" — the PR title is the better description; worth squash-merging with the title.
  • Build & Test Matrix was still running at review time; format and clippy are green.

@n13
n13 merged commit b62fe5e into main Jun 12, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants