fix: immediately set Canceled status when cancelling a Sent/Submitted tx

[shaunwackerly]

Summary

• Cancelling a Sent or Submitted EVM transaction via DELETE /relayers/{id}/transactions/{tx_id} returned {"success":true} but the transaction kept appearing in subsequent ?status=Sent queries.
• Root cause: cancel_transaction for non-Pending transactions set is_canceled=true and queued a noop resubmit job, but left status unchanged in the TransactionUpdateRequest. The DB status stayed Sent/Submitted until the noop mined on-chain — at which point it transitioned to Confirmed, not Canceled.

Changes

src/domain/transaction/evm/evm_transaction.rs

• In cancel_transaction, the TransactionUpdateRequest built from prepare_noop_update_request now includes status: Some(TransactionStatus::Canceled). The noop is still submitted on-chain to unblock the nonce, but the relayer treats the transaction as terminal immediately.
• Test assertion updated from Submitted → Canceled to match the new behaviour.

No changes to status.rs are needed: is_final_state already includes Canceled (so check_transaction_status short-circuits correctly), and handle_status_impl already routes Canceled to handle_final_state (so a stale status-check job cannot re-open the transaction).

Relationship to PR #792

PR #792 fixes the ?status= filter so queries return the correct set of transactions. This PR is the complementary fix ensuring Sent/Submitted transactions actually transition to Canceled immediately after a cancel call, so they disappear from active-status views without waiting for on-chain confirmation of the noop.

The two PRs are independent and can be merged in either order.

Test plan

• DELETE /relayers/{id}/transactions/{tx_id} on a Sent transaction returns {"success":true}.
• Immediately after, GET /relayers/{id}/transactions/{tx_id} shows status: canceled.
• The transaction no longer appears in GET /relayers/{id}/transactions?status=Sent.
• Cancelling a Pending transaction still works (sets Canceled directly, no noop).
• Cancelling a Confirmed/Failed/Canceled transaction returns an error.
• All existing unit tests pass (cargo test --lib).

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Transaction cancellation now immediately updates the status to "Canceled" when replacing a submitted transaction, providing users with accurate status feedback instead of displaying outdated statuses.

[github-actions]

Thank you for your contribution to OpenZeppelin Relayer. Before being able to integrate those changes, we would like you to sign our Contributor License Agreement. You can sign the CLA by just posting a Pull Request Comment with the sentence below. Thanks.

---

I confirm that I have read and hereby agree to the OpenZeppelin Contributor License Agreement

---

_{You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.}

[coderabbitai]

Need the big picture first? Review this PR in Change Stack to see what changed before going file by file.

Walkthrough

This PR updates the transaction cancellation behavior in the EVM transaction domain. The cancel_transaction function now immediately sets the transaction status to Canceled when replacing a non-Pending transaction with a NOOP update, rather than leaving it as Submitted. The test for this scenario is updated to verify the new expected behavior.

Changes

Transaction Cancellation

Layer / File(s)	Summary
Cancel transaction status update src/domain/transaction/evm/evm_transaction.rs	When canceling a non-Pending transaction, the code now makes the prepared NOOP update mutable and explicitly sets update.status to TransactionStatus::Canceled before persisting. The corresponding test assertion is updated to expect Canceled status immediately.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A transaction crossed the line,
But now we cancel it just fine—
Status flips to Canceled state,
No more pending at the gate! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely summarizes the main change: setting Canceled status immediately when cancelling Sent/Submitted transactions.
Description check	✅ Passed	The PR description includes a comprehensive Summary section explaining the bug and fix, detailed Changes explaining the code modifications, and a thorough Test plan section covering all test scenarios.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/domain/transaction/evm/evm_transaction.rs (1)

1263-1282: ⚠️ Potential issue | 🔴 Critical

Fix cancellation NOOP submission: resubmit is skipped when status is set to Canceled

cancel_transaction sets update.status = Some(TransactionStatus::Canceled) and persists it before enqueueing the resubmit job. Later, the resubmit handler reloads the transaction and calls resubmit_transaction, which returns Ok(tx) unless status is Sent or Submitted—so the NOOP replacement will never be signed/submitted on-chain (despite the comment stating it is).

• Suggested fixes:
  • Relax the resubmit_transaction status gate for NOOP transactions (e.g., allow Canceled when is_noop(&evm_data)), or
  • Route the cancellation NOOP through a dedicated submission path/job that bypasses the Sent/Submitted guard.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/domain/transaction/evm/evm_transaction.rs` around lines 1263 - 1282, The
cancel path sets update.status = Some(TransactionStatus::Canceled) and persists
before calling send_transaction_resubmit_job, which causes resubmit_transaction
to skip NOOP submission because it only proceeds for Sent/Submitted states; fix
by allowing NOOP replacements to bypass that guard: update resubmit_transaction
to treat transactions with is_noop(&evm_data) as eligible even when status ==
TransactionStatus::Canceled (i.e., relax the Sent/Submitted check to include
Canceled for NOOPs), or alternatively add a dedicated submission path (e.g.,
send_noop_resubmit_job / handle_noop_resubmit) that is enqueued from
cancel_transaction and invokes the signing/submission logic without the
Sent/Submitted gate; touch prepare_noop_update_request,
send_transaction_resubmit_job, and resubmit_transaction (and the is_noop helper)
to implement the chosen approach.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/domain/transaction/evm/evm_transaction.rs`:
- Around line 2240-2241: The test currently asserts the transaction status is
Canceled but never verifies the async NOOP submission; update the test that
mocks produce_submit_transaction_job to instead run the queued job and assert
the resubmission behavior: invoke the job processor or call resubmit_transaction
for the cancelled_tx (referencing produce_submit_transaction_job and
resubmit_transaction) after cancelling a Submitted transaction and then assert
whether the NOOP was submitted on-chain or that resubmit_transaction skipped
submission due to TransactionStatus::Canceled (check cancelled_tx or resulting
on-chain submission record/state) so the test covers both cancellation and the
queued resubmit handling.

---

Outside diff comments:
In `@src/domain/transaction/evm/evm_transaction.rs`:
- Around line 1263-1282: The cancel path sets update.status =
Some(TransactionStatus::Canceled) and persists before calling
send_transaction_resubmit_job, which causes resubmit_transaction to skip NOOP
submission because it only proceeds for Sent/Submitted states; fix by allowing
NOOP replacements to bypass that guard: update resubmit_transaction to treat
transactions with is_noop(&evm_data) as eligible even when status ==
TransactionStatus::Canceled (i.e., relax the Sent/Submitted check to include
Canceled for NOOPs), or alternatively add a dedicated submission path (e.g.,
send_noop_resubmit_job / handle_noop_resubmit) that is enqueued from
cancel_transaction and invokes the signing/submission logic without the
Sent/Submitted gate; touch prepare_noop_update_request,
send_transaction_resubmit_job, and resubmit_transaction (and the is_noop helper)
to implement the chosen approach.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6f1bffca-440e-4f52-ada2-6f7f28b56e5b

📥 Commits

Reviewing files that changed from the base of the PR and between 345b895 and de4a903.

📒 Files selected for processing (1)

• src/domain/transaction/evm/evm_transaction.rs

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Test doesn't verify NOOP on-chain submission behavior.

The test assertion and comment claim the NOOP is submitted asynchronously, but the test only mocks produce_submit_transaction_job (line 2179) without verifying the actual resubmission logic. When the queued job runs in production, resubmit_transaction will skip the transaction due to its Canceled status (see previous comment on lines 1263-1282).

Consider adding an integration test that:

1. Cancels a Submitted transaction
2. Processes the queued resubmit job
3. Verifies the NOOP is actually submitted on-chain (or that resubmission is correctly skipped if that's the intended behavior)

This would catch the status-guard issue identified in the previous comment.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/domain/transaction/evm/evm_transaction.rs` around lines 2240 - 2241, The
test currently asserts the transaction status is Canceled but never verifies the
async NOOP submission; update the test that mocks produce_submit_transaction_job
to instead run the queued job and assert the resubmission behavior: invoke the
job processor or call resubmit_transaction for the cancelled_tx (referencing
produce_submit_transaction_job and resubmit_transaction) after cancelling a
Submitted transaction and then assert whether the NOOP was submitted on-chain or
that resubmit_transaction skipped submission due to TransactionStatus::Canceled
(check cancelled_tx or resulting on-chain submission record/state) so the test
covers both cancellation and the queued resubmit handling.

[zeljkoX]

Thank you for the contribution.

I have a concern that this change may break the noop replacement rather than fix the status.

The cancel flow submits the noop through the resubmit job, and resubmit_transaction only proceeds when the transaction is in
Sent or Submitted status. By setting the status to Canceled before that job runs, the guard rejects the transaction and the noop is never broadcast. As a result, the original transaction may still mine while the relayer reports Canceled. Since
Canceled is also a final state, the status checker stops reconciling the transaction, so this is never corrected.

I would suggest a different approach:

1. Revert the status change so the transaction stays Submitted while the noop is in flight and is broadcast correctly.
2. Update the finalization logic so that, when is_canceled is set and the mined transaction is a noop, the transaction is finalized as Canceled instead of Confirmed.

To do this safely, the decision should be based on the transaction that was actually mined rather than the stored data, since network_data is overwritten with the noop on cancel. The finalization path already identifies the mined hash, and we can use the existing is_noop check on its on-chain data to distinguish the two cases.

This will not change list responses while the transaction is in flight, as it remains Submitted until the noop mines. If we want earlier visibility, we could additionally expose the is_canceled field in the response.