fix(queue): reject duplicate per-identity enrollment (Closes #99) - #166
Open
Fabiana1504 wants to merge 1 commit into
Open
fix(queue): reject duplicate per-identity enrollment (Closes #99)#166Fabiana1504 wants to merge 1 commit into
Fabiana1504 wants to merge 1 commit into
Conversation
enroll_position() created a new position for any identity without checking whether that identity already held one, so a single identity could occupy multiple slots and break the one-position-per-identity fairness guarantee. Feature (issue Stellar-Deejah#99): - Per-identity index: storage key ("identity_pos", identity) -> u32 position id, mirroring the existing ("pos", id) tuple layout so it never collides. - enroll_position(): panics "identity_already_enrolled" if the identity already holds a position; writes the index after creating the position. - cancel_position(): removes the index so a cancelled identity can re-enroll. total_enrolled is unaffected (counts next_id - 1, incl. cancelled positions). - get_position_by_identity(env, identity) -> Option<u32> added to the Queue trait and implemented (O(1) lookup instead of unbounded iteration). - 4 tests: duplicate enroll panics, distinct identities succeed, index cleared on cancel (+ re-enroll), get_position_by_identity after enroll and after cancel. - SDK QueueClient.getPositionByIdentity() read method. Concurrency: each enroll_position() is a full transaction; Soroban serializes transactions at ledger level and storage writes are atomic within a tx, so the check-then-write index update cannot race. Pre-existing fixes required for `cargo test -p lineproof-queue` to compile & pass (the test target was broken on main by an earlier merge, unrelated to Stellar-Deejah#99): - test.rs: removed a duplicate `use` import and a duplicate `name:` field; fixed `|&id|` closures to `|id|` (soroban Vec::iter yields owned values). - initialize(): removed an extend_ttl() on the contract address (a persistent key that never exists) which panicked Storage/MissingValue and broke ~every test. - Made expire_position()/expire_positions_batch() `pub` so the generated client exposes them (tests call client.expire_position(...)). - Two expire tests used advance(&admin, &0) (a no-op that never transitions) so expire panicked on state; switched them to close(), a valid state for expiry. Result: cargo test -p lineproof-queue -> 29 passed; 0 failed. Closes Stellar-Deejah#99. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Someone is attempting to deploy a commit to the Deejah Team on Vercel. A member of the Team first needs to authorize it. |
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.
Closes #99.
Adds a per-identity duplicate guard to the queue contract so a single identity can no longer occupy multiple positions (the one-position-per-identity fairness guarantee).
Feature (issue #99)
("identity_pos", identity) -> u32(the position id), mirroring the existing("pos", id)tuple layout so it never collides with position or scalar keys.enroll_position()— checks the index before creating a position and panics with"identity_already_enrolled"on a duplicate; writes the index entry after the position is created.cancel_position()— removes the index entry, so a cancelled identity may re-enroll.total_enrolledis unaffected (it countsnext_id - 1, including cancelled positions).get_position_by_identity(env, identity) -> Option<u32>— added to theQueuetrait and implemented; O(1) lookup instead of unbounded iteration.get_position_by_identityafter enroll and after cancel.QueueClient.getPositionByIdentity()read method (mirrorsgetPosition).Why this key format & concurrency: the
("identity_pos", identity)tuple matches the contract's existing("pos", id)convention, namespacing the entry via aSymboldiscriminator. Eachenroll_position()is a full transaction; Soroban serializes transactions at the ledger level and storage writes are atomic within a transaction, so the check-then-write index update cannot race.Pre-existing fixes (required to build/run the tests)
cargo test -p lineproof-queuedid not compile onmain(an earlier merge left the test target broken). These were unrelated to #99 but necessary to satisfy the "tests pass" criterion:test.rs: removed a duplicateuseimport and a duplicatename:struct field; changed|&id|closures to|id|(sorobanVec::iter()yields owned values).initialize(): removed anextend_ttl()on the contract address as a persistent key that never exists — it panicked withStorage/MissingValueand broke nearly every test.expire_position()/expire_positions_batch()pubso the generated client exposes them (their tests callclient.expire_position(...)).advance(&admin, &0)(a no-op that never transitions state), soexpirepanicked; switched them toclose(), a valid state for expiry.Testing
cargo test -p lineproof-queue→ 29 passed; 0 failed. SDKqueue.tstypechecks clean. (Auto-generatedtest_snapshots/are gitignored, so not included.)Closes #99.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com