feat(deposit-address-service): announce settled withdrawals - #3711
Open
amateima wants to merge 2 commits into
Open
feat(deposit-address-service): announce settled withdrawals#3711amateima wants to merge 2 commits into
amateima wants to merge 2 commits into
Conversation
amateima
requested review from
bmzig,
dijanin-brat,
mrice32,
nicholaspai and
pxrl
as code owners
August 12, 2026 09:02
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73aa4955bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 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". |
amateima
force-pushed
the
feat/deposit-address-service-withdraw-publishing
branch
from
August 24, 2026 11:50
b006f38 to
ddf03a3
Compare
amateima
force-pushed
the
feat/deposit-address-service-withdraw-publishing
branch
from
August 27, 2026 17:19
ddf03a3 to
e82a19a
Compare
amateima
force-pushed
the
feat/deposit-address-service-withdraw-publishing
branch
from
September 3, 2026 22:19
e82a19a to
48edf93
Compare
pxrl
previously approved these changes
Sep 4, 2026
amateima
disabled the stack merge
September 7, 2026 13:21
amateima
force-pushed
the
feat/deposit-address-service-withdraw-publishing
branch
from
September 7, 2026 13:21
48edf93 to
cb93520
Compare
amateima
force-pushed
the
feat/deposit-address-service-withdraw-publishing
branch
from
September 7, 2026 15:10
cb93520 to
d26c4d0
Compare
Base automatically changed from
feat/deposit-address-service-v3-withdrawal
to
master
September 8, 2026 08:37
A withdrawal leaves no on-chain provenance event, so unlike a deposit it has to be
announced: the Pub/Sub `withdraw_executed` message is the only way the indexer learns
the refund settled. Reuses `buildWithdrawExecutedPayload` and the locked `{type, data}`
envelope verbatim.
The announcement is durable state rather than a step of the request that made it —
`withdrawLifecyclePublishedAt` on the `withdraw_executed` record. That is what makes a
dropped one recoverable, and it is why the terminal short-circuit is pierced in both
places: a settled-but-unannounced withdrawal takes the lock and publishes instead of
acknowledging. Piercing one alone would ACK before the retry could happen.
Publish, then stamp. The reverse order loses the announcement for good on any failure
between the two. A fresh withdrawal publishes from what `TransferStore` durably holds
rather than from what the request believes it just did, so the happy path runs the same
code a redelivery does — the recovery path being the one that cannot be exercised in
production. Recovery keys on the recorded state, never on the message's classification,
since a `correct_transfer` refunded below the minimum owes the same announcement; and it
re-fetches the receipt, because the payload's `logIndex` comes from scanning
`receipt.logs` and cannot be rebuilt from the record.
A publish failure preserves `withdraw_executed` unstamped and NACKs, so the redelivery
retries the publication alone and no path re-withdraws. A receipt with no settlement log
is ACK + warn: the funds moved correctly and no redelivery can conjure a log that is not
there. The polling bot instead catches, logs at `error` and never throws, so a dropped
publish is never replayed; closing that is the point of this change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ending record A redelivery that found `broadcast_pending` for a withdrawal resolved it, wrote `withdraw_executed` and ACKed without announcing — the exact permanent loss this change exists to prevent, reached from the one route that writes the terminal state rather than finds it. That is the case where the original request broadcast the refund and died before its receipt landed, so this delivery is both the first to observe the settlement and the last that could ever announce it. Resolution and announcement are now one function and no caller invokes the resolver directly, so a fourth call site cannot reintroduce the split. The deposit path pays only an extra read for that: `deposit_executed` never awaits an announcement. Reported in review of the pending-record branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
amateima
force-pushed
the
feat/deposit-address-service-withdraw-publishing
branch
from
September 8, 2026 08:41
d26c4d0 to
57b50e4
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Part of #3663 — PR 6 of the standalone deposit-address service. Stacked on #3709.
A withdrawal leaves no on-chain provenance event, so unlike a deposit it has to be announced: the
Pub/Sub
withdraw_executedmessage is the only way the indexer learns the refund settled. PR 5 recordswithdraw_executedand announces nothing, so every refund it makes today leaves the indexer row pending.buildWithdrawExecutedPayloadand the{ type, data }envelope locked byDepositAddressWithdrawConsumerare reused verbatim.buildDepositExecutedPayloadis not — the servicenever publishes deposits.
The structural change
The announcement is durable state, not a step of the request that made it —
withdrawLifecyclePublishedAton thewithdraw_executedrecord. That is what makes a dropped onerecoverable, and it is why the terminal short-circuit is pierced in both places (the pre-lock read in
createDepositHandlerand the post-lock re-read inprocessUnderLock): a settled-but-unannouncedwithdrawal takes the lock and publishes instead of acknowledging. Piercing one alone would ACK before the
retry could happen.
Recovery keys on the recorded state, never on the message's classification — a
correct_transferrefunded below the minimum owes the same announcement — and it re-fetches the receipt, because the
payload's
logIndexcomes from scanningreceipt.logsfor the settlement log and cannot be rebuilt fromthe record.
Ordering
Publish, then stamp. The reverse order loses the announcement for good on any failure between the
two; this order can at worst announce twice, which at-least-once delivery already implies. A fresh
withdrawal publishes from what
TransferStoredurably holds rather than from what the request believesit just did, so the happy path runs the same code a redelivery does — the recovery path being the one
that cannot be exercised in production.
withdraw_executedunstamped, NACK; the redelivery retries the publication onlywarn, left unstamped — the funds moved correctly and no redelivery can conjure a log that is not thereNo path here re-executes the withdrawal. The polling bot instead catches, logs at
errorand neverthrows, so a dropped publish is never replayed; closing that is the point of this change, so its swallow
is deliberately not copied.
Config
Mirrors the polling bot, reusing its variable names so both can run during migration:
ENABLE_DEPOSIT_ADDRESS_WITHDRAW_PUBLISHER,PUBSUB_GCP_PROJECT_ID,PUBSUB_DEPOSIT_ADDRESS_WITHDRAW_TOPIC. The gate on with either of the other two empty fails startup,as does a gate with no publisher behind it — announcing nothing is otherwise invisible until a refund
goes unannounced.
ENABLE_DEPOSIT_ADDRESS_DEPOSIT_PUBLISHERstays dead config. The publisher is injectedlike Redis, since
getGcpPubSubPublisheranswersundefinedunderRELAYER_TEST.Testing
DepositAddressService.deposit.tsgains the publisher in its default harness, so every pre-existingwithdraw test now exercises the announcement rather than passing because publishing was off. New cases
cover: the envelope and the stamp; publish throws ⇒ state preserved unstamped + NACK; redelivery of a
published withdrawal ACKs without republishing; redelivery of an unpublished one republishes and never
re-withdraws; the same for a below-minimum
correct_transfer; missing settlement log ⇒ ACK + warn; afailed terminal write announces nothing; no publisher configured; and a deposit is never announced.
Each new branch was verified red-first — the bug reintroduced and the test confirmed failing — for the
stamp/publish swap, both short-circuit pierces individually, gating recovery on classification, stamping
on the missing-log path, and announcing before the durable terminal write.
Review focus: the envelope still matches the locked consumer contract; a publication retry can never
re-withdraw.
🤖 Generated with Claude Code