fix/35-payment-execute-send-duplicate-guard - #64
Open
Wetshakat wants to merge 1 commit into
Open
Conversation
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.
closed #35
fix-payment-execute-send-duplicate-guard
Creates payment_submissions (stellar_tx_hash PK, transaction_id, created_at) — the single-payment parallel of batch_submissions from migration 010.
One row per distinct signed XDR hash, inserted atomically before Horizon is called. The PRIMARY KEY is what actually closes the race; no check-then-
insert.
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
src/services/payment.rs
Three structural changes, all directly mirroring what batch.rs already does:
PaymentService::new gains pool: PgPool and network_passphrase: impl Into. These are the same two things BatchPaymentService already carried.
The hash is computed immediately after the empty-XDR check. An unparseable XDR is rejected here — before any DB write — exactly like the batch path.
This is verified by rejects_invalid_signed_xdr_before_touching_the_database.
. The first outcome is the authoritative one.
the row so ReconciliationService can find it by hash and converge it, same as it does for batches. Definitive Horizon rejections → Failed.
Unit tests added (all pass without a DB):
Integration tests added (#[ignore], require DATABASE_URL):
identical signed_xdr, asserts exactly one Ok and one Conflict, and exactly one payment_submissions row.
Conflict.
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
src/models/payment.rs
Added Clone to #[derive] on SendPaymentRequest — required by the concurrency test's req.clone(), and reasonable to have on a request type anyway.
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
src/routes/payments.rs
Updated both get_quote and send_payment handlers to pass state.pool.clone() and &state.config.stellar_network_passphrase to PaymentService::new,
matching the updated constructor.
▸ Credits: 3.06 • Time: 6m 37s
6% >
give me a pr message for what you did
Brings PaymentService::execute_send up to the same hardening standard as BatchPaymentService::execute_batch after #30. Three concrete gaps closed:
Duplicate-submission guard
Compute the canonical transaction hash locally (via the existing tx_hash::compute_transaction_hash) before touching the DB or Horizon, then atomically
claim it with INSERT INTO payment_submissions. A primary-key violation returns 409 Conflict immediately — the same primitive batch_submissions uses,
for the same reason: no TOCTOU gap.
Terminal-status guard
When transaction_id is supplied, reject with 409 Conflict if the existing record is already Completed or Failed. Previously a second call could
silently overwrite the recorded hash and status, discarding the authoritative outcome.
Ambiguous-failure classification
Connection failures and timeouts are now recorded as SubmittedUnconfirmed (not Failed), with the precomputed hash persisted on the row.
ReconciliationService can now recover stuck single-payment sends by hash lookup, exactly as it already does for batches since BatchPaymentService leaves transactions permanently 'pending' if the process crashes between leg insert and status update #30. Definitive Horizon
rejections remain Failed.
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Changes
of batch_submissions
(mirrors batch.rs); unit tests + two #[ignore] integration tests
Testing
Unit tests (no DB required) cover: empty XDR rejected before DB, unparseable XDR rejected before DB, Horizon rejection → Failed, connection failure →
SubmittedUnconfirmed.
Integration tests (cargo test -- --ignored, requires DATABASE_URL) cover: two concurrent sends with identical signed_xdr — exactly one succeeds, the
other gets Conflict; ambiguous submission → SubmittedUnconfirmed → retry blocked with Conflict.