Repository navigation
Resend B2B webhook updates when a deposit or account returns to an earlier state - #1437
Merged
Merged
Conversation
DEPOSIT_UPDATED and ACCOUNT_UPDATED named each event after the snapshot hash alone. A deposit or account that came back to an earlier snapshot (a waiting reason flipping back after one failed quote, a second suspend and reactivate) reused that snapshot's event id, so the outbox dropped it as a duplicate and the partner was left on the stale state. Both cases showed up on the sandbox. The id now also covers the row's updated_at; a re-emit after a crash before the marker write keeps its id.
✅ Deploy Preview for vortex-sandbox ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for vrtx-dashboard canceled.
|
✅ Deploy Preview for vortexfi canceled.
|
A failed swap writes only its execution row and drops out of conversions[], so a deposit already converting could return to an earlier snapshot at the same updated_at and reuse that event id. The previous marker tells those returns apart, and a re-emit after a crash before the marker write still sees the same marker and keeps its id. The crash test now restores a real earlier marker; its instance update was a no-op, so it passed with any id.
The DEPOSIT_UPDATED marker was stamped with the clock at its write. A write that landed between the pass's read and that marker, such as the webhook inbox moving a deposit to returned, ended up older than the marker, so the settled deposit was never re-evaluated and the partner stayed on the stale snapshot. Stamping the row version that was evaluated re-queues it on the next pass.
Reverting migration 066 refuses while any KYC case holds verification state, and the 066 test leaves such cases behind. Whenever bun ran it before the migrator suite, the revert test failed. Resetting first makes the suite independent of file order.
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
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.
Why
DEPOSIT_UPDATEDandACCOUNT_UPDATEDnamed each event after a hash of the snapshot alone, and the snapshots carry no timestamp. When a deposit or account came back to a snapshot it had shown before, the new event got the earlier event's id. The outbox dropped it as a duplicate (ignoreDuplicateson(webhook_id, event_id)), and our docs tell partners to skip aneventIdthey already processed, so the partner was left on the stale state.Both cases showed up in the live sandbox tests:
below_floor→no_route→below_floor. The keeper keepswaiting.sinceacross reasons, so the last snapshot was byte-identical to the first, and the partner's last update saidno_routefor ten minutes.ACCOUNT_UPDATEDevents were delivered, so a partner could treat a live client as suspended, or a suspended one as live.What changes
The id suffix is now the first 16 hex of SHA-256 over the previous snapshot hash sent, the new snapshot hash and the row's
updated_at. A write to the row movesupdated_at, so a repeated state gets a fresh id. A change that touches only another table leavesupdated_atalone (a failed swap drops out ofconversions[]), but then the snapshot it replaces differs, so two such returns still get different ids. A re-emit after a crash between the enqueue and the marker write sees the same marker and row and keeps its id, so the outbox still absorbs it. Change detection is untouched (the storedlifecycle_event_hashis still the plain snapshot hash), so deploying this re-sends nothing.The same pass also stamped its marker (
lifecycle_event_at) with the clock at the write. A write that landed between the pass's read and that marker, such as the webhook inbox moving a deposit toreturned, ended up older than the marker, so the settled deposit was never re-evaluated and the partner never got its last update. The marker now records the row version that was evaluated, so such a write re-queues the deposit on the next pass.Compatibility
The payloads and the documented id shape (
deposit-updated:<depositId>:<16 hex>,account-updated:<accountId>:<16 hex>) stay the same; only the ids of new events differ. The partner docs (page 07), the shared JSDoc and security spec invariant 16 now say the id is unique per change rather than per snapshot. Wire-contract snapshot unchanged.Tests
returnedon the next pass.The id tests fail without the fix, the crash test fails if the id is not stable across the re-emit, and the settle test fails with the clock-stamped marker (all checked by mutation).
manager-events.test.tspasses (17), and lint, verify, typecheck and wire-contract are clean.CI:
migrator.test.tsnow resets the test database first. The 066 migration test leaves KYC cases with verification state behind, which made reverting 066 fail whenever bun ran that file first (also seen on other open PRs).