feat: enforce approved rewards have a payment at the database#1679
Open
JoeriDijkstra wants to merge 2 commits into
Open
feat: enforce approved rewards have a payment at the database#1679JoeriDijkstra wants to merge 2 commits into
JoeriDijkstra wants to merge 2 commits into
Conversation
Add a CHECK constraint on fund_rewards (status <> 'approved' OR payment_id IS NOT NULL) so the invariant can't be broken by a future non-Multi path. Currently it is only guaranteed by the approval Multi in app code. The constraint is evaluated per-statement and Postgres cannot defer CHECKs, so the approval Multi had to stop transiently leaving a reward :approved with a null payment_id: create the payment bookkeeping entry first, then a single compare-and-swap sets status and payment_id together. The CAS still serializes concurrent approvals — a losing writer updates 0 rows and rolls its payment back with the transaction. The migration adds the constraint NOT VALID then VALIDATEs it separately, so the deploy never holds an exclusive lock across the validation scan. Addresses the "approved rewards must have payment_id" tech-debt ticket (nonblocker from PR #1503 review). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0179xMCWA3jhqdCpg6r28Y6g
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.
Add a CHECK constraint on fund_rewards (status <> 'approved' OR payment_id IS NOT NULL) so the invariant can't be broken by a future non-Multi path. Currently it is only guaranteed by the approval Multi in app code.
The constraint is evaluated per-statement and Postgres cannot defer CHECKs, so the approval Multi had to stop transiently leaving a reward :approved with a null payment_id: create the payment bookkeeping entry first, then a single compare-and-swap sets status and payment_id together. The CAS still serializes concurrent approvals — a losing writer updates 0 rows and rolls its payment back with the transaction.
The migration adds the constraint NOT VALID then VALIDATEs it separately, so the deploy never holds an exclusive lock across the validation scan.
Addresses the "approved rewards must have payment_id" tech-debt ticket (nonblocker from PR #1503 review).