Conversation
Solana indexes transactions by account, so querying a relay's fillStatus PDA returns all program events from any transaction touching that PDA - including other relays' events from batched fills. relayFillStatus() and findFillEvent() previously consumed those events unfiltered, so a batched fill could trip findFillEvent's uniqueness assertion or resolve another relay's event into a false Filled/RequestedSlowFill status. Thread the relay data through to event reconstruction and resolve events via queryEventsForRelay(), which associates each event to its relay by relay data hash. queryDerivedAddressEvents() is removed; association is now part of the query contract and unassociated PDA queries are no longer expressible. Co-authored-by: nicholaspai <npai.nyc@gmail.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Removing Three live call sites:
These are CCTP events on non-SvmSpoke programs, so The PR body already names this — "unassociated PDA queries are no longer expressible" — I just don't think that consequence is intended, because it's a capability in use. The deprecation added in #1512 seems like the right landing spot: keep the method deprecated, migrate the relayer's CCTP paths onto whatever the generic replacement is, then remove it. Removing it in the same stack that deprecates it means the deprecation never appears in a published version for anyone to react to. Worth noting the CCTP path also breaks twice earlier in the stack independently of this removal — The fill-status half of the change is right, to be clear — associating by relay data hash is the correct fix and the |
|
This supersedes #1489, which fixes the same bug. md0x's #1489 ("fix(svm): validate fill-status event ownership", open since 2026-07-17) addresses the identical issue — unrelated events from a transaction that merely touches the queried PDA being attributed to that relay. Its approach is to recompute the fillStatus PDA from each candidate event and compare against the queried PDA; this PR compares relay data hashes directly. Same guarantee, since the PDA is derived from the hash, but comparing hashes is strictly cheaper — no So I'd close #1489 in favour of this, rather than leaving two open fixes for one bug. Worth a heads-up to md0x either way, and #1490 from the same batch still needs sequencing against this stack (noted on #1507). |
|
This also halves the RPC work in The old code mapped
The PR body currently reads as purely a correctness fix, which undersells it — the batched-fill scenario it's guarding against is also the scenario where the old double-sweep was most expensive. |
…eneric derived-address query The deletion claimed zero downstream callers, but the relayer queries CCTP events (TokenMessengerMinter DepositForBurn, MessageTransmitter) through this method with plain string event names. Reinstate it in generic form - eventName: string, returning RawEventWithData[] - since non-SvmSpoke events carry no typed names, and document that results are transaction-scoped, so callers must associate events themselves (queryEventsForRelay does this for SvmSpoke relay events). Add a regression test in the relayer's call pattern.
A fill takes precedence over a slow fill request irrespective of event ordering (UMIP-179): a relay cannot be filled twice and a slow fill request cannot follow a fill, so any FilledRelay event is authoritative. The previous sort-by-slot-and-pop picked whichever event landed later, but slot order is not a reliable discriminator - both events can land in the same slot, where sort order is unstable. Adds a regression test with the request in a later slot than the fill.
Reinis-FRP
left a comment
Reinis-FRP
left a comment
There was a problem hiding this comment.
LGTM - relay-specific event association and monotonic fill precedence correctly prevent cross-relay and same-slot status misattribution.
3 participants
Solana indexes transactions by account, so querying a relay's fillStatus
PDA returns all program events from any transaction touching that PDA -
including other relays' events from batched fills. relayFillStatus() and
findFillEvent() previously consumed those events unfiltered, so a
batched fill could trip findFillEvent's uniqueness assertion or resolve
another relay's event into a false Filled/RequestedSlowFill status.
Thread the relay data through to event reconstruction and resolve events
via queryEventsForRelay(), which associates each event to its relay by
relay data hash. The SpokePool paths no longer issue unassociated PDA
queries; queryDerivedAddressEvents() remains for non-SvmSpoke programs
(the relayer's CCTP flows), documented as requiring caller-side
association.
Commitment is caller-selectable (default confirmed) and applies to the
signature listing and transaction reads alike. Concurrent transaction
reads are bounded by the provider's rate-limiting queue
(RateLimitedSolanaRpcFactory), mirroring the EVM provider layer. A
caller-supplied fillStatus PDA is only a transaction locator:
association is by relay data hash, so a stale or incorrect PDA can only
yield missing events, never another relay's.