Remove internal match order validity guard by sehyunc · Pull Request #1420 · renegade-fi/renegade

sehyunc · 2026-03-28T00:00:51Z

Summary

replace the stubbed order_still_valid path with fresh database reads after settlement failure
preserve the exact v1 post-failure guarantees instead of adding order-book checks that v1 did not use here
add focused unit coverage for removed, changed, depleted, busy-queue, and unchanged order rows

V1 Validity Criteria

In v1, wallet_still_valid re-read two things from the database after settlement failure:

the wallet row
the wallet's serial task queue length

It rejected continuing when:

the wallet nullifier changed
- justification: the stored wallet row no longer matched the wallet value used to choose the attempted match
the wallet's serial task queue length was non-zero
- justification: another task had already claimed that wallet for a state change

V2 Validity Criteria

V2 does not have a wallet-level nullifier for Ring 0 order freshness, so the equivalent guard is order-centric. order_still_valid now re-reads these database-backed values after settlement failure:

the order_id -> account_id mapping
the account's serial task queue length
the stored Order
the stored matchable amount

It rejects continuing when:

the order_id -> account_id mapping is gone
- justification: the database can no longer locate the order that produced the attempted match
the account's serial task queue length is non-zero
- justification: this is the direct v1 queue check
the stored Order row is gone
- justification: the order was removed from the account index
the stored Order differs from the Order value used to choose the attempted match
- justification: this is the v2 replacement for the v1 wallet-nullifier freshness check
the stored matchable amount is zero
- justification: the order is still stored, but it is no longer fillable
the stored matchable amount differs from the matchable amount used to choose the attempted match
- justification: the balance-backed capitalization changed, so the attempted match was computed on stale inputs

Explicitly Excluded

network-order ready_for_match
- justification: this value existed in v1, but wallet_still_valid did not read it. It belongs to the order-book / gossip path, while this guard is intentionally limited to the same database checks v1 used in its post-failure validity method.

Testing

cargo test -p matching-engine-worker order_still_valid -- --nocapture

Notes

I also tried to run the local Ring 0 harness from the writable worktree to verify the old Re-implement order still valid check warning disappears, but the fresh relayer startup failed before serving traffic with error parsing command line args: "Invalid byte 45, offset 5." in /tmp/relayer-output-internal-match-guard.log, so I could not complete the repro-path verification in this session.

claude · 2026-03-28T00:01:10Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

claude · 2026-03-28T00:55:07Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

claude · 2026-03-28T00:56:50Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

claude · 2026-03-28T00:57:38Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

Add queue-length checks before attempting internal matches: 1. At the top of run_internal_matching_engine(): bail if the account's serial task queue is non-empty. This mirrors v1's should_run_matching_engine() which prevented re-entering the matching engine unless the wallet's serial queue was confirmed empty. 2. In try_settle_match(): bail if the counterparty account's serial task queue is non-empty, avoiding a settlement attempt that would be rejected with 'serial preemption not allowed'. Together with PR #1420 (order_still_valid post-failure guard), this closes the gap between v1 and v2 internal match retry behavior.

- Remove v1 references from code comments - Consolidate 5 unit tests to 2 (order removed, unchanged) - Remove unused imports

claude · 2026-03-31T22:19:42Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

Add queue-length checks before attempting internal matches: 1. At the top of run_internal_matching_engine(): bail if the account's serial task queue is non-empty. This mirrors v1's should_run_matching_engine() which prevented re-entering the matching engine unless the wallet's serial queue was confirmed empty. 2. In try_settle_match(): bail if the counterparty account's serial task queue is non-empty, avoiding a settlement attempt that would be rejected with 'serial preemption not allowed'. Together with PR #1420 (order_still_valid post-failure guard), this closes the gap between v1 and v2 internal match retry behavior.

akirillo

as discussed, the order validity check seems irrelevant now that we rely on the post-task hook model for reinvoking the matching engine.

as we saw, the matching engine run will pull a fresh view of the order before attempting a match, so there's no need for a staleness check in the engine's hot path.

we can just log if the spawned match task fails.

The internal settlement path already refreshes both accounts and re-enqueues\nfresh matching engine runs via failure hooks, so the post-failure\norder_still_valid branch was no longer guarding a meaningful retry\nloop. Removing it keeps failure handling in one place and drops the\ntest-only scaffolding that existed solely to exercise that branch.\n\nConstraint: Internal settlement failure already triggers refresh-account then fresh matching-engine runs\nRejected: Keep order_still_valid as a defensive readback | duplicated recovery logic without gating an inline retry\nConfidence: high\nScope-risk: narrow\nReversibility: clean\nDirective: If inline retry logic is reintroduced in the matching engine, reevaluate whether a local freshness gate is needed there\nTested: cargo fmt --all; cargo test -p matching-engine-worker --lib\nNot-tested: live Base Sepolia harness repro from this worktree

sehyunc · 2026-04-03T00:19:55Z

Closing this out: I removed the order_still_valid path from this PR.

After tracing the execution model, the extra validity check no longer seems necessary. On internal settlement failure, the system already recovers through hooks:

run_internal_matching_engine waits for the settlement task
failed settlement runs SettleInternalMatchTask::failure_hooks()
that enqueues RefreshAccountHook for both accounts
RefreshAccountTask refreshes state
its success hook enqueues fresh RunMatchingEngineHook jobs
those new matching-engine runs re-read current state before matching again

So we already have a fresh-state retry path after failure. order_still_valid was not guarding a real inline retry loop anymore; it was just adding extra reads and branching before return.

I also removed the guard-specific test scaffolding and dependency additions, since they only existed to support that branch.

Verification:

cargo fmt --all
cargo test -p matching-engine-worker --lib

claude · 2026-04-03T00:20:05Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

sehyunc and others added 2 commits March 27, 2026 17:00

Add internal match order validity guard

0a95c1f

Format Rust code using rustfmt

4e40879

Tighten validity guard to v1 semantics

998acd6

Clarify validity guard wording

f7dd50e

Tighten validity guard wording further

c352264

sehyunc added a commit that referenced this pull request Mar 31, 2026

Merge PR #1420: order validity guard

6debcc2

sehyunc mentioned this pull request Mar 31, 2026

Add pre-match queue guard for internal matching engine (T2) #1424

Closed

Clean up comments and consolidate tests

d9c9b20

- Remove v1 references from code comments - Consolidate 5 unit tests to 2 (order removed, unchanged) - Remove unused imports

sehyunc requested a review from akirillo April 1, 2026 18:01

akirillo reviewed Apr 1, 2026

View reviewed changes

sehyunc changed the title ~~Add internal match order validity guard~~ Remove internal match order validity guard Apr 3, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove internal match order validity guard#1420

Remove internal match order validity guard#1420
sehyunc wants to merge 7 commits intomainfrom
codex/implement-internal-match-order-validity-guard

sehyunc commented Mar 28, 2026 •

edited

Loading

Uh oh!

claude bot commented Mar 28, 2026 •

edited

Loading

Uh oh!

claude bot commented Mar 28, 2026 •

edited

Loading

Uh oh!

claude bot commented Mar 28, 2026 •

edited

Loading

Uh oh!

claude bot commented Mar 28, 2026 •

edited

Loading

Uh oh!

claude bot commented Mar 31, 2026 •

edited

Loading

Uh oh!

akirillo left a comment

Uh oh!

sehyunc commented Apr 3, 2026

Uh oh!

claude bot commented Apr 3, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

sehyunc commented Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

V1 Validity Criteria

V2 Validity Criteria

Explicitly Excluded

Testing

Notes

Uh oh!

claude bot commented Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

akirillo left a comment

Choose a reason for hiding this comment

Uh oh!

sehyunc commented Apr 3, 2026

Uh oh!

claude bot commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sehyunc commented Mar 28, 2026 •

edited

Loading

claude bot commented Mar 28, 2026 •

edited

Loading

claude bot commented Mar 28, 2026 •

edited

Loading

claude bot commented Mar 28, 2026 •

edited

Loading

claude bot commented Mar 28, 2026 •

edited

Loading

claude bot commented Mar 31, 2026 •

edited

Loading

claude bot commented Apr 3, 2026 •

edited

Loading