Repository navigation
Conversation
ace6d8b to
17a16b4
Compare
d4a8a5c to
acc732a
Compare
17a16b4 to
ea115bd
Compare
500d36c to
6bc1044
Compare
89dd8c3 to
90eb4d0
Compare
6bc1044 to
3baa5b9
Compare
ueco-jb
left a comment
ueco-jb
left a comment
There was a problem hiding this comment.
The state classification is correct: held_authorization_nonce holds the pair for every state that could have submitted a nonce-consuming transaction, releases only JournalRejected and a close that supplied a matching, state-gated nonce acknowledgement, and the close capture, redelivery self-exclusion, and legacy-row rebuild all resolve to the safe holding direction. The gap is the concurrency the guard targets: the record-stage re-check narrows but does not close the two-delivery race, and several comments plus the SPEC state the one-holder property as absolute. Details inline, plus doc and test-coverage follow-ups.
90eb4d0 to
649992f
Compare
3baa5b9 to
4f2a618
Compare
649992f to
204a5da
Compare
4f2a618 to
9eea491
Compare
204a5da to
1fa060c
Compare
ea12771 to
58d5a50
Compare
|
@rain-marvin review |
|
🔎 Reviewing |
![rain-marvin[bot]](https://avatars.githubusercontent.com/in/5077017?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Opus 5.5 (Claude 1)
This PR stops two mints from holding the same (recipient, nonce) authorization pair. The orchestrator lets one pair land only once, but two mints with that pair would both match the landing and both complete, so one AP would get tokens once for shares journaled twice. The mint-authorization endpoint now returns 409 when another mint still holds the pair. It checks before the on-chain validation reads, and again under a static admission lock right before the AuthorizeMint send. After the send it confirms that mint_view shows the claim. A mint keeps its pair for its whole life, except after JournalRejected or a close that supplied the nonce acknowledgement. An expression index on mint_view serves the holder lookup, and Mint::SCHEMA_VERSION is now 5.
Overall the design is sound, and every earlier thread is addressed at this head. The endpoint is the only production path that records an authorization, so the admission lock closes the concurrent race between deliveries in this process. Two minor points remain, and neither blocks the merge. The first: the check added for a lost mint_view write reports the loss, but it does not keep the pair blocked, because the bot's retry gets a clean 200. The second: a journal rejection that lands at the same moment can cause a false lost-write error.
Two more points were raised but not posted. Holder lookup ignores network and orchestrator address, but nonces are random bytes32, so a collision across chains needs a deliberate reuse. An ordinary close of a mint that never signed a transaction keeps its pair, which is conservative, and the bot can pick a new nonce.
58d5a50 to
f1da65a
Compare
a2bd76e to
47ab2dd
Compare
f1da65a to
d7b1aec
Compare
47ab2dd to
98841be
Compare
|
@CodeRabbit review |
✅ Action performedReview finished.
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (7)
Included review availability: This review used your included allowance. 5 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 8 reviews per hour. WalkthroughThe mint model now tracks recipient/nonce pairs held across its lifecycle, and the mint view can find holders using an indexed candidate query. The authorization endpoint returns a conflict when another mint holds the pair, checks ownership before validation and again during serialized recording, and verifies that the recorded claim appears in the view. Tests cover concurrent deliveries, duplicate claims, state refusals, and nonce retention and release. Priority: ➖ Normal Unblocks: 2 PRs Merge Risk: 🔵 Low · up to This change blocks duplicate recipient/nonce mint authorizations and serializes recording so that concurrent deliveries cannot both claim a pair. A rare case remains. If a view write is lost and a nonce is reused before the service restarts, two mints could hold the same pair. The change is mergeable if the owner accepts that edge case. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
d7b1aec to
ff25a48
Compare
d73c8ff to
fa58c57
Compare
ff25a48 to
729d161
Compare
ueco-jb
left a comment
ueco-jb
left a comment
There was a problem hiding this comment.
record_under_admission now holds the process-wide AUTHORIZATION_ADMISSION lock from the record-stage holder read through the AuthorizeMint send and confirm_claim_visible. It is the only production sender of AuthorizeMint, Store::send writes mint_view before it returns, and SPEC limits the issuer to one process per store, so two mints can no longer both record one (recipient, nonce); concurrent_deliveries_of_one_pair_admit_exactly_one covers it. The identical-redelivery path, the releasing close through MintingFailed { NonceReplayUnresolved }, the "held by another mint" wording and the legacy-close and test docs are fixed.
fa58c57 to
a4d8951
Compare
00be7c9 to
afe9b85
Compare
a4d8951 to
028c7b6
Compare
agryaznov
left a comment
agryaznov
left a comment
There was a problem hiding this comment.
Looks good to me — the hold/release boundary is now an exhaustive match with the schema bump behind it, the admission lock covers exactly the read-then-commit it has to, and the recipient key comes from the aggregate rather than the request body. Nothing further from me.
Merge activity
|
3 participants
The mint-authorization endpoint now refuses a
(recipient, nonce)pair another mint already holds, returning409 Conflict. The orchestrator keysnonceUsedon the recipient across every token, so one pair produces at most one landing. Without this guard two same-sized mints for one recipient would both full-match that landing and both complete — one AP's tokens for shares journaled twice. The check runs before the on-chain validation RPCs (refusing a duplicate cheaply) and again, under a static admission lock, immediately before theAuthorizeMintsend (catching a pair claimed while the validation was in flight). A mint holds its pair for its whole lifecycle exceptJournalRejected(which never submitted) and an acknowledged close (where the operator verified the nonce is free on an independent chain view).Closes RAI-2620
Contributes to RAI-2601
Contributes to RAI-1215
Live effect: none, refusal path on the internal endpoint · Risk: medium (money path — new admission lock serializes concurrent deliveries for different mints at record stage, lost view write makes claim invisible until rebuild) · Ships: on merge
Decisions
Risks
Proof
held_authorization_nonceexhaustive match pins every state's hold/release decision (test coverage)Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.