feat(contract): implement immutable payment privacy semantics - #135
Conversation
MissingNullifierCommitment, MissingStealthDeliveryKey and PrivacyLevelMismatch for the enforceable payment-privacy semantics.
Store exactly one identity representation per privacy level: optional payer (Standard), hashed_wallet + stealth_delivery_key (Private), nullifier_commitment (Anonymous).
ProcessedNonceHash, UserEventTicketsHash and SpentNullifier keys plus the nonce-hash and nullifier helper functions.
Payment, ticket and refund events now emit a masked identity (full address, wallet hash or nullifier commitment) taken from the payment's own privacy level, never a raw address for Private/Anonymous.
PaymentParams carries the per-level privacy inputs; build_payment_record maps each level to exactly one identity representation and rejects cross-level material with PrivacyLevelMismatch; build_ticket and private_wallet_hash added.
Standard keys by raw address, Private by a wallet hash, Anonymous by its nullifier commitment so no wallet-linked value is read from a ledger key.
Persist the built payment/ticket via the privacy-aware paths, enforce nullifier uniqueness for Anonymous payments, and key nonce/ticket writes without ever storing a payer-derived value for Anonymous.
…ints pay_for_ticket accepts nullifier_commitment and stealth_delivery_key; the commitment and options variants pass the appropriate None values.
Refunds require an on-chain payer, so Anonymous/Private payments return RefundNotAllowed; the refund event derives its masked identity from the stored payment.
Resolve the ticket owner and refund recipient as optional and return RefundNotAllowed when no on-chain address exists.
…paths bind_email_commitment, list_ticket_for_resale and buy_resale_ticket compare against the now-optional payer/owner; only address-bound (Standard) records are eligible.
Signals that the cross-contract registration path cannot settle a Private or Anonymous payment.
Derive the event's configured privacy and reject Private/Anonymous paid registrations instead of silently settling them as Standard and storing the raw attendee. Addresses CodeRabbit review.
Match the extended pay_for_ticket signature; the event contract is the payer, so Standard with no privacy material is correct.
Covers per-level storage, field exclusivity, nullifier reuse, refund guards, immutability, emitted masked identity (asserted from env.events()), and balance-safety on rejected purchases (issue BuidlZone-Labs#117).
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughPayment privacy semantics now flow through payment records, tickets, storage, events, refunds, resale, and event registration. Standard, Private, and Anonymous payments use distinct identity material and validation rules. ChangesPayment privacy semantics
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant PaymentsContract
participant PaymentStorage
participant PaymentEvents
Client->>PaymentsContract: pay_for_ticket with privacy inputs
PaymentsContract->>PaymentStorage: validate and store privacy-specific identity
PaymentsContract->>PaymentEvents: emit masked payment and ticket events
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ethnum 1.5.2 fails to compile with E0512 (invalid mem::transmute) on newer stable rustc, breaking CI Build/Clippy/Test. 1.5.3 replaces the transmutes with safe construction. Lock-only change; semver-compatible with soroban-env-common.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
contracts/payments/src/lib.rs (1)
335-355: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winToken transfer executes before the new privacy-material validation and nullifier-uniqueness check.
try_transfer(unchanged, lines ~337-341) runs beforebuild_payment_record'sMissingNullifierCommitment/MissingStealthDeliveryKey/PrivacyLevelMismatchchecks and before thehas_nullifierdedup check. Every other validation in this function (nonce, amount, event status, ticket cap) already runs before the transfer — this is the one new check ordering that doesn't follow that pattern. Because Soroban reverts all state on anErrreturn, this isn't a fund-safety bug (confirmed bytest_anonymous_requires_commitment/test_private_requires_stealth_keyasserting unchanged balances), but it means a cross-contract token transfer call is executed and paid for even on invocations that are guaranteed to fail on privacy-material shape alone.Consider validating the privacy-material shape (the checks currently inside
build_payment_record) before thetry_transfercall, and moving the nullifier-uniqueness check up alongside the nonce dedup check.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contracts/payments/src/lib.rs` around lines 335 - 355, Reorder validation in the payment flow so privacy-material checks currently performed by build_payment_record and the nullifier uniqueness check occur before token_client.try_transfer. Preserve the existing validation behavior and ensure payment construction/storage still uses the validated values after the transfer succeeds.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@contracts/event/src/lib.rs`:
- Around line 845-855: Enforce the configured event privacy before any
registration, ticket, or payment state mutation: in contracts/event/src/lib.rs
lines 845-855, move the PrivacyLevel::Private/Anonymous rejection outside the
tier.price > 0 branch so free registrations are rejected too; in
contracts/event/src/lib.rs lines 1150-1151, add the same privacy gate before the
Standard payment/ticket flow in verify_and_attend. Keep Standard events
proceeding through their existing paths.
---
Nitpick comments:
In `@contracts/payments/src/lib.rs`:
- Around line 335-355: Reorder validation in the payment flow so
privacy-material checks currently performed by build_payment_record and the
nullifier uniqueness check occur before token_client.try_transfer. Preserve the
existing validation behavior and ensure payment construction/storage still uses
the validated values after the transfer succeeds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2dd33783-b85b-4d41-9463-d0387fcc56fd
📒 Files selected for processing (13)
contracts/event/src/errors.rscontracts/event/src/integration_tests.rscontracts/event/src/lib.rscontracts/payments/src/errors.rscontracts/payments/src/events.rscontracts/payments/src/lib.rscontracts/payments/src/multi_token_test.rscontracts/payments/src/receipt_commitment_test.rscontracts/payments/src/revenue_split_test.rscontracts/payments/src/storage.rscontracts/payments/src/test.rscontracts/payments/src/test_privacy_semantics.rscontracts/payments/src/types.rs
|
@Depo-dev kindly resolve the major security & privacy concern |
The privacy gate previously ran only for paid tiers in register_for_event, so free Private/Anonymous registrations still stored the raw attendee, and verify_and_attend had no gate at all. Extract a shared require_settleable_privacy helper and call it before any registration, ticket, or payment state is mutated in both flows. Addresses CodeRabbit review.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@contracts/event/src/test.rs`:
- Around line 742-754: The existing test only exercises paid tier 0 and does not
cover the free-registration privacy bypass. Add a separate Private or Anonymous
event scenario using the free-tier registration path, such as the appropriate
free-tier amount or configuration, and assert that registration is rejected with
EventError::PaymentPrivacyUnsupported without storing attendee or ticket state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cbd1541c-2cf7-42d7-81ff-f24475d9aa64
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
contracts/event/src/lib.rscontracts/event/src/test.rs
|
@Depo-dev let's work on some maintainability & code quality, pls address the minor change |
Add a price-0 Private-event case proving the free-registration path is rejected too; the existing paid-tier test would pass even with the old paid-only gate. Addresses CodeRabbit review.
2 participants
Summary
Implements enforceable on-chain semantics for the three payment privacy levels from the spec (Anonymous → Private → Standard), so the privacy slider maps to real contract behaviour instead of UI-only state. Rebased onto the latest
mainwith all merge conflicts resolved.Each
PaymentRecord/Ticketnow stores exactly one identity representation, chosen by its privacy level:Key behaviours
PrivacyLevelMismatch.RefundNotAllowed; settlement happens off-chain.register_for_eventenforces the event's configured payment privacy rather than silently downgrading Private/Anonymous events to Standard.CodeRabbit review items addressed
build_payment_record.register_for_event.MaskedAddress, not just stored state.Tests
New
test_privacy_semantics.rsplus updates covering field exclusivity, missing-field rejection, nullifier reuse, refund guards, immutability, emitted-event identity, and balance-safety on failed purchases. Full workspace suite green;cargo fmtandcargo clippy -D warningsclean.Known limitation
require_auth()runs for every privacy level because the token transfer needs wallet authorization, so the submitting account is visible in the Stellar transaction envelope regardless of level. Anonymous/Private privacy applies at the contract storage and event layer only; transaction-level anonymity would require a relayer / meta-transaction model, which is out of scope for this issue.Closes #117
Summary by CodeRabbit