feat: multi-organizer revenue split and co-host wallet management - #131
Conversation
Replaces the single-organizer payout assumption with configurable revenue splits and co-host escrow handling (closes BuidlZone-Labs#122). - Event creation accepts revenue_splits: Vec<(Address, u32)> (basis points), validated to 1-5 recipients summing to 10000, no duplicates/zeros, with the primary organizer pinned at index 0. Splits are set once and immutable. - The split is synced to the payments contract, which settles each event once (deducting the platform fee first) and lets every recipient withdraw their allocated share independently via withdraw_split. Rounding dust accrues to the primary organizer so the full net is always distributed. - Primary organizer can flag a compromised co-host wallet; the flagged share is held in escrow and cannot be withdrawn. Admin resolves a dispute by either releasing the share to the recipient or reassigning it to the primary. - Legacy single-organizer withdrawal paths are rejected for split events to prevent double payout. Adds 17 payments unit tests and 3 event integration tests covering the happy path, fee ordering, independent withdrawals, the delay/dispute windows, split validation, immutability, flagging, and dispute resolution. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@josephchimebuka pls kindly resolve merge conflict, so i can complete review |
|
@josephchimebuka hello??? |
Resolve conflicts in error enums and payments storage keys by keeping both postponement and revenue-split additions with non-overlapping codes. Add revenue_splits to anon-claims test fixtures from upstream. Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds configurable revenue splits to event creation and settlement. The event contract validates and stores split data, syncs it to payments, and exposes split-aware actions. The payments contract adds split storage, settlement, withdrawals, flagging, and dispute resolution. Tests cover the new flows and legacy-path restrictions. ChangesMulti-organizer Revenue Split Feature
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Resolve conflicts with postponement, anon-claims, and merged PR BuidlZone-Labs#132 (zkEmail commitments). Keep both revenue-split and commitment APIs with non-overlapping PaymentError codes (35–42). Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
contracts/payments/src/lib.rs (1)
1721-1737: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winMirror the primary-organizer invariant in payments.
flag_cohosttreatssplits[0]as the admin, butsync_revenue_splitsnever checks that recipient against the stored event organizer. Since this contract holds the funds, validate index 0 before storing splits.Proposed hardening
+ let config = storage::get_event_config(&env, &event_id) + .ok_or(PaymentError::InvalidOrganizer)?; let mut normalized: soroban_sdk::Vec<RevenueSplit> = soroban_sdk::Vec::new(&env); let mut total: u32 = 0; for i in 0..len { let (recipient, bps) = splits.get(i).ok_or(PaymentError::InvalidSplitConfig)?; + if i == 0 && recipient != config.organizer { + return Err(PaymentError::InvalidSplitConfig); + }🤖 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 1721 - 1737, The revenue split normalization in sync_revenue_splits currently validates duplicates and bps totals but never enforces that splits[0] matches the stored event organizer, which flag_cohost assumes as the admin. Update sync_revenue_splits to check the first split’s recipient against the event organizer before pushing into normalized, using the existing event lookup/state in contracts/payments/src/lib.rs and preserving the current InvalidSplitConfig error path on mismatch.contracts/payments/src/revenue_split_test.rs (1)
523-567: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the refund-before-split-withdraw cancellation case.
This test only covers split withdrawal before attendee refunds. Add the reverse order so the test proves the remaining organizer/co-host share is still withdrawable after
claim_refund.🤖 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/revenue_split_test.rs` around lines 523 - 567, The cancellation coverage in test_cancelled_split_event_distributes_only_withdrawable_ratio only verifies withdrawals before attendee refunds; add the reverse order in the same test or a companion test so it exercises `claim_refund` first and then `withdraw_split`. Use the existing `test_cancelled_split_event_distributes_only_withdrawable_ratio`, `client.claim_refund`, and `client.withdraw_split` flow to confirm the organizer/cohost share still remains withdrawable after refunds, and assert the final balances reflect the same 50% distributable split.
🤖 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/payments/src/events.rs`:
- Around line 276-303: The FlaggedShareResolved event currently uses two
different identifiers for the same event in the topic and payload, so make them
canonical and consistent. Update the FlaggedShareResolved contractevent topic
and the event_type field in emit_flagged_share_resolved/event_type(...) to use
the same identifier everywhere, and keep the struct, publisher, and consumers
aligned on that single name.
In `@contracts/payments/src/lib.rs`:
- Around line 377-386: The split settlement path in the payout flow is only
using currently held payments, so cancelled events can lose track of funds after
`claim_refund` marks payments `Refunded`. Update the logic around
`collect_held_payments_for_token` and the split-withdraw calculation to use a
cancellation snapshot or payment records that still account for partially
refunded cancellation claims, rather than only unreleased balances. Make sure
the settlement path for cancelled split events can still derive the
organizer/co-host withdrawable amount even after attendee refunds have been
claimed.
- Around line 400-419: The full-settlement path in the payments release flow
clears token revenue but leaves aggregate event revenue unchanged, so update the
withdrawable_ratio_bps == 10_000 branch in the settlement logic to also clear
EventRevenue. Use the same storage helpers already used in the partial branch,
and keep the change localized around the payment release loop and the
storage::set_event_token_revenue / storage::set_event_revenue calls.
In `@contracts/payments/src/storage.rs`:
- Around line 712-779: Refresh TTL on read for split-related storage helpers so
persisted state does not expire after long-lived inactivity. Update the read
paths in get_splits, get_split_settlement, get_split_withdrawn, and
is_split_flagged to extend the TTL for their corresponding DataKey entries,
using the same TTL_THRESHOLD and TTL_BUMP pattern already used in set_splits,
set_split_settlement, set_split_withdrawn, and set_split_flagged. Keep the
behavior localized to these helper functions so has_splits and ensure_no_splits
continue to work correctly for EventSplits and SplitFlagged state.
---
Nitpick comments:
In `@contracts/payments/src/lib.rs`:
- Around line 1721-1737: The revenue split normalization in sync_revenue_splits
currently validates duplicates and bps totals but never enforces that splits[0]
matches the stored event organizer, which flag_cohost assumes as the admin.
Update sync_revenue_splits to check the first split’s recipient against the
event organizer before pushing into normalized, using the existing event
lookup/state in contracts/payments/src/lib.rs and preserving the current
InvalidSplitConfig error path on mismatch.
In `@contracts/payments/src/revenue_split_test.rs`:
- Around line 523-567: The cancellation coverage in
test_cancelled_split_event_distributes_only_withdrawable_ratio only verifies
withdrawals before attendee refunds; add the reverse order in the same test or a
companion test so it exercises `claim_refund` first and then `withdraw_split`.
Use the existing
`test_cancelled_split_event_distributes_only_withdrawable_ratio`,
`client.claim_refund`, and `client.withdraw_split` flow to confirm the
organizer/cohost share still remains withdrawable after refunds, and assert the
final balances reflect the same 50% distributable split.
🪄 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: 6db5afdc-d39d-4cc1-86fa-71d07776c151
📒 Files selected for processing (14)
contracts/event/src/errors.rscontracts/event/src/integration_tests.rscontracts/event/src/lib.rscontracts/event/src/test.rscontracts/event/src/test_anon_claims.rscontracts/event/src/test_claims.rscontracts/event/src/test_privacy.rscontracts/event/src/types.rscontracts/payments/src/errors.rscontracts/payments/src/events.rscontracts/payments/src/lib.rscontracts/payments/src/revenue_split_test.rscontracts/payments/src/storage.rscontracts/payments/src/types.rs
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/payments/src/errors.rs`:
- Around line 55-60: The PaymentError discriminants for CommitmentAlreadySet and
CommitmentNotAllowed must remain stable; do not renumber them when splitting the
new errors. Update the PaymentError enum in the errors definition so the new
commitment-related variants are appended after the existing 42 value, or
otherwise explicitly preserve the original numeric values, and verify any
match/serialization logic still maps the same variants to the same ABI codes.
🪄 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: fef22867-5188-47b6-8a06-e1a7cd51be77
📒 Files selected for processing (4)
contracts/payments/src/errors.rscontracts/payments/src/events.rscontracts/payments/src/lib.rscontracts/payments/src/types.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- contracts/payments/src/events.rs
- contracts/payments/src/types.rs
- contracts/payments/src/lib.rs
|
@josephchimebuka pls resolve coderabbit requested changes, and then ensure CI Format check passes |
…-Labs#122) - Align FlaggedShareResolved event topic with payload name - Settle cancelled split events from payment records after refunds - Clear aggregate EventRevenue on full split settlement - Refresh split storage TTL on reads - Require split index 0 to match event organizer - Add test for split withdrawal after attendee refund - Run cargo fmt Co-authored-by: Cursor <cursoragent@cursor.com>
Linked issue
Closes #122
What this PR does
Events are no longer locked to a single organizer wallet. An event can now be created with a revenue split — up to five recipients, each with a basis-point allocation summing to 10000. After the platform fee is deducted, each recipient withdraws their own share independently from the payments contract; no single party can drain the others' funds. The primary organizer (split index 0) keeps full admin rights and can flag a compromised co-host wallet, which freezes that recipient's share in escrow until an admin resolves the dispute by either releasing it to the recipient or reassigning it to the primary organizer.
Change type
Storage impact
CreateEventParams.revenue_splits/Event.revenue_splitsVec<(Address, u32)>DataKey::EventSplits(Symbol)(payments)Vec<RevenueSplit>DataKey::SplitSettlement(Symbol)(payments)SplitSettlementDataKey::SplitWithdrawn(Symbol, Address)(payments)i128DataKey::SplitFlagged(Symbol, Address)(payments)boolIs this a breaking storage change?
revenue_splitsfield is set on every newCreateEventParams(emptyVecfor the single-organizer case).On-chain vs. off-chain behaviour
ensure_split_settled;PlatformFeeCollectedemitted onceSplitWithdrawnguards double-withdrawal;RevenueWithdrawnemitted per recipientCohostFlagged/FlaggedShareResolvedemittedsync_revenue_splitsrejects any re-set; no setter existsCross-contract impact
PaymentsContract::sync_revenue_splits(event_contract, event_id, splits), called by the event contract duringcreate_event.event-contract(create_eventnow syncs the split;withdraw_splitandflag_cohostproxy to payments).Security checklist
sync_revenue_splitsrequires the linked event contract;withdraw_splitrequires the recipient;flag_cohostrequires the primary organizer;resolve_flagged_sharerequires the payments admin.event_end_ledger + withdrawal_delay + admin_extension; cancelled events respect the dispute window and the time-based withdrawable ratio (mirrorswithdraw).withdraw/withdraw_token/withdraw_all_tokens/withdraw_revenue/release_if_expiredare rejected for split events, preventing double payout.Test coverage
New tests added (payments —
revenue_split_test.rs):test_sync_revenue_splits_stores_and_reads_back(Address, u32)test_sync_revenue_splits_rejects_bad_sum/_more_than_five/_duplicate_and_zerotest_revenue_splits_are_immutable_once_settest_sync_revenue_splits_rejects_foreign_callertest_platform_fee_deducted_before_split_and_independent_withdrawalstest_withdraw_split_rejects_double_withdraw_and_non_recipienttest_withdraw_split_respects_withdrawal_delaytest_rounding_dust_accrues_to_primary_organizertest_only_primary_can_flag_and_cannot_flag_selftest_flagged_cohost_share_held_in_escrowtest_resolve_flag_release_to_recipient_allows_withdrawaltest_resolve_flag_reassign_to_primary_pays_primary_and_blocks_recipienttest_resolve_requires_flagged_recipienttest_legacy_withdraw_paths_rejected_for_split_eventstest_cancelled_split_event_distributes_only_withdrawable_ratioNew tests added (event —
integration_tests.rs):test_event_split_end_to_end_distribution,test_event_rejects_invalid_split_configurations,test_event_flag_cohost_through_front_door.Test count: 20 new, 18 updated (existing
CreateEventParamsliterals), 180 total passing (event 77, payments 103).cargo fmt --checkandcargo clippy -D warningsclean;cargo build --releasesucceeds.Acceptance criteria sign-off
revenue_splits: Vec<(Address, u32)>summing to 10000 —validate_revenue_splitsinevent/lib.rs;test_event_split_end_to_end_distribution.test_sync_revenue_splits_rejects_more_than_five.withdraw_split;test_platform_fee_deducted_before_split_and_independent_withdrawals.flag_cohostis primary-only; event admin ops keyed offevent.organizer.sync_revenue_splitsrejects re-set;test_revenue_splits_are_immutable_once_set.ensure_split_settleddeducts fee before computing shares; asserted in the fee test.flag_cohost+resolve_flagged_share; flagging/escrow/resolution tests.What this PR deliberately does NOT cover
withdraw.Reviewer focus areas
ensure_split_settledinpayments/lib.rs— fee-first ordering and the full vs. partial (cancelled) accounting againstvalidate_revenue_invariant.recipient_share— primary absorbs rounding dust; confirm sum of shares equals net.ensure_no_splitsguards on every legacy withdrawal path.Made with Cursor
Summary by CodeRabbit