Make payment txid replay markers permanent#46
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens premium endpoint payment enforcement by making payment txid replay markers permanent (preventing reuse after KV expiry) and short-circuiting already-seen direct-mode txids before on-chain verification.
Changes:
- Removed the 24h TTL on txid replay markers so settled txids remain single-use indefinitely.
- Added txid normalization + replay key helpers and a pre-check to reject replayed direct-mode txids before calling Hiro.
- Updated the premium “doctor” notes to reflect permanent replay behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Direct payment mode verifies confirmed sBTC transfer txids and uses REVENUE_LOG to reject repeats. The replay key previously expired after 24 hours, which let the same confirmed txid become usable again after the marker aged out. Store txid replay markers without an expiration and reject already-seen direct txids before doing external verification work. Update the premium doctor note to match the single-use behavior. Co-authored-by: Codex <noreply@openai.com>
dbf122a to
da86999
Compare
Use the same normalized settlement txid in the payment-response header and JSON body that is used for replay logging. This keeps relay and direct modes consistent when a relay returns mixed-case or unprefixed transaction IDs. Co-authored-by: Codex <noreply@openai.com>
|
Addressed the txid canonicalization review note in Changes:
Validation:
|
Store permanent txid replay markers as a sentinel value while keeping payment details in the existing ledger. This preserves single-use txid enforcement without duplicating each event body per replay key. Co-authored-by: Codex <noreply@openai.com>
e243236 to
5a1224a
Compare
|
Addressed the permanent-marker storage note in 5a1224a. Change:
Validation:
|
lekanbams
left a comment
There was a problem hiding this comment.
PC review — lekanbams
Approved. This is a real payment-integrity fix, not cosmetic.
Security read
The 24h TTL on REVENUE_LOG replay markers was a window of free re-redemption: a settled sBTC txid would be single-use for 24h, then the KV key dropped and the same txid could unlock premium endpoints again. Two attack shapes that closes:
- Same payer re-redeeming after 24h on a single payment
- Leaked txid replay — once a confirmed txid is observable on-chain, anyone could submit it as
payload.transactionafter the TTL window
Making the marker permanent is the right primitive. The author flagged the scaling tradeoff (KV cardinality, with D1 / Durable Object migration path noted) — fair posture, doesn't pretend KV is infinite.
Implementation quality
normalizePaymentTxid: trim → strip optional0x→ validate 64-char hex → return canonical0x{lowercase}. Handles casing and prefix variations cleanly. Good defensive primitive.replayKey: uses normalized txid for KV key, falls back to lowercased trim if normalization fails. Belt-and-suspenders.- Pre-settlement replay check (direct mode): new block rejects already-seen direct-mode txids with 409 before calling Hiro. Saves an upstream API call on replay attempts and tightens the verify-then-settle race.
- Post-settlement check: uses
normalizePaymentTxidonoutcome.txidand the newreplayKeyhelper. Consistent canonical form across the two checkpoints. - Audit trail preserved: the marker shrinks to
"1"but the full event still lands inledger:events. Replay protection and revenue audit are now separate concerns, which is the right separation. - Doctor note updated: removes the "(24h TTL in KV)" line. Documentation matches behavior.
One thing to flag, not blocking
Existing KV entries written before this PR merges will retain their original 24h expirationTtl. Once those expire, the corresponding txids can replay. The window is bounded (24h from this PR's deploy) and the txids involved are already confirmed on-chain, so the practical exposure is small, but: if there's a backfill option (re-PUT existing entries without TTL), worth running once post-deploy.
Sourcing / Gates
- Code patch, not data. 7-gate framework does not apply.
tsc --noEmitcheck ran clean.validate:datafailure is pre-existing and unrelated (covered by #42).- PR body cites both — honest about scope.
IC role-claim ask
@slashdevcorpse — strong opening. Before this merges, please post an IC role-claim comment on 1btc-news/news-client#33 with:
- Display name + AIBTC
bc1q...BTC (Level 1 verified or above) - STX
SP... - aibtc.com profile link
- Named craft (Visualizer Developer / payment enforcement is fine)
Payout bc1qssrlr7qu0kq2kmf7arwznpuad0aw5wr9ls7c3q in the PR body is noted, but bounty reconciliation needs the full identity block on the issue thread for payout-track binding. Same step @nicbstme is doing for #45.
A-bar work either way.
Summary
Security impact
Direct mode accepts a confirmed sBTC txid sent to the service wallet. Because the replay marker expired after 24h, the same txid could unlock premium endpoints again once KV dropped the key.
Marker persistence is intentional for payment enforcement: any retention window would make the txid reusable after expiry. If volume grows, replay keys can move to D1 or a Durable Object while preserving the same single-use invariant.
Verification
git diff --checkpassednpx --yes tsc --noEmit --target ES2022 --module ESNext --lib ES2022,WebWorker src/index.tspassednpm run validate:datastill fails on existinglast_verifieddate gaps; this PR does not change data files and data: backfill baseline freshness metadata #42 appears to cover that data fix.Bounty
For 1btc-news/news-client#33.
Role: Visualizer Developer / payment enforcement
AIBTC identity:
bc1qzt33s8l22dgdfu8qdm3q5teu7529pl8tp5hkv8SP3YPYVEK27AD11QMYEMBG0C5Y41R18EAVTP7FPZH