fix: Handle community vote rejection and expiry in check_vote_threshold#9
fix: Handle community vote rejection and expiry in check_vote_threshold#90xdevcollins merged 2 commits intomainfrom
Conversation
check_vote_threshold only handled the approval path — if voters chose "Reject" or the voting period expired without quorum, the campaign was stuck in Submitted forever. Now it compares approve vs reject vote counts when threshold is reached, and checks session expiry when it isn't, returning the campaign to Draft in both rejection scenarios. Adds local copies of VotingSession/VoteOption types for cross-contract deserialization, CampaignVoteRejected event, and three new test cases. Closes #3 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
Disabled knowledge base sources:
📝 Walkthrough🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
contracts/crowdfund_registry/src/storage/mod.rs (1)
3-46: Prefer a shared schema for these governance types.These local copies now have to stay byte-for-byte aligned with
contracts/governance_voting/src/storage/mod.rs; a field or variant drift in either contract will turnget_session/get_optioninto runtime deserialization failures. Moving them intoboundless_typesor adding a compatibility test would make this much safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/crowdfund_registry/src/storage/mod.rs` around lines 3 - 46, The duplicated governance types (VoteContext, VoteStatus, VoteOption, VotingSession) must be centralized to avoid byte-layout drift: extract these structs/enums from contracts/crowdfund_registry/src/storage/mod.rs into the shared crate (e.g., boundless_types) and replace the local definitions with imports; update references (e.g., VotingSession, VoteOption, VoteContext, VoteStatus and any callers like get_session/get_option) to use the shared types, bump the shared crate version in Cargo.toml, and add a compatibility unit test that serializes/deserializes sessions/options to ensure the on-chain layout remains stable across both contracts.contracts/crowdfund_registry/src/tests/mod.rs (1)
682-684: Assert the specific error here.
assert!(result.is_err())will also pass on unrelated failures, so this test doesn't actually prove the active-session branch returnsVoteThresholdNotMet. Matching the contract error explicitly would make the coverage much tighter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/crowdfund_registry/src/tests/mod.rs` around lines 682 - 684, Replace the broad assertion that try_check_vote_threshold(&cid) returned any error with an explicit match against the contract's specific error variant (VoteThresholdNotMet) so the test verifies the active-session branch; update the assertion around t.client.try_check_vote_threshold(&cid) to unwrap the Err and assert it equals/contains the VoteThresholdNotMet variant (or use pattern matching on the returned Result to assert Err(ContractError::VoteThresholdNotMet) or equivalent) to make the test precise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@contracts/crowdfund_registry/src/contract.rs`:
- Around line 454-515: The current branch collapses ties into rejection; change
the post-threshold logic around approve_votes/reject_votes so you handle three
cases: if approve_votes > reject_votes then set campaign.status =
CampaignStatus::Campaigning, persist and publish CampaignValidated; else if
reject_votes > approve_votes then set campaign.status = CampaignStatus::Draft,
clear vote_session_id, persist and publish CampaignVoteRejected; else (tie)
leave campaign.status and vote_session_id unchanged (do not persist) and return
Err(CrowdfundError::VoteThresholdNotMet) so the session stays open until the tie
breaks or the session expires; update the code around
approve_option/reject_option, approve_votes, reject_votes, campaign.status,
CampaignValidated and CampaignVoteRejected accordingly.
In `@contracts/crowdfund_registry/src/events/mod.rs`:
- Around line 139-144: The CampaignVoteRejected event currently only carries id;
update the CampaignVoteRejected struct to include a rejection reason (e.g., an
enum or string field like `reason`) so consumers can distinguish why it was
rejected, then update all emitters in check_vote_threshold() and any publishers
in contracts/crowdfund_registry/src/contract.rs to populate that field with
distinct values (e.g., "reject_majority" vs "expired_without_approval"); ensure
the new field is annotated appropriately (e.g., not marked as #[topic] unless
intended) and update any handlers/tests that parse CampaignVoteRejected
accordingly.
---
Nitpick comments:
In `@contracts/crowdfund_registry/src/storage/mod.rs`:
- Around line 3-46: The duplicated governance types (VoteContext, VoteStatus,
VoteOption, VotingSession) must be centralized to avoid byte-layout drift:
extract these structs/enums from contracts/crowdfund_registry/src/storage/mod.rs
into the shared crate (e.g., boundless_types) and replace the local definitions
with imports; update references (e.g., VotingSession, VoteOption, VoteContext,
VoteStatus and any callers like get_session/get_option) to use the shared types,
bump the shared crate version in Cargo.toml, and add a compatibility unit test
that serializes/deserializes sessions/options to ensure the on-chain layout
remains stable across both contracts.
In `@contracts/crowdfund_registry/src/tests/mod.rs`:
- Around line 682-684: Replace the broad assertion that
try_check_vote_threshold(&cid) returned any error with an explicit match against
the contract's specific error variant (VoteThresholdNotMet) so the test verifies
the active-session branch; update the assertion around
t.client.try_check_vote_threshold(&cid) to unwrap the Err and assert it
equals/contains the VoteThresholdNotMet variant (or use pattern matching on the
returned Result to assert Err(ContractError::VoteThresholdNotMet) or equivalent)
to make the test precise.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f02389a9-7eea-43cd-9354-55e7857c9e0d
📒 Files selected for processing (7)
contracts/crowdfund_registry/src/contract.rscontracts/crowdfund_registry/src/events/mod.rscontracts/crowdfund_registry/src/storage/mod.rscontracts/crowdfund_registry/src/tests/mod.rscontracts/crowdfund_registry/test_snapshots/tests/test_vote_expired_without_quorum_returns_to_draft.1.jsoncontracts/crowdfund_registry/test_snapshots/tests/test_vote_reject_returns_to_draft.1.jsoncontracts/crowdfund_registry/test_snapshots/tests/test_vote_threshold_not_met_while_active.1.json
…tion logic for vote majority and expiration
Summary
check_vote_thresholdnow compares approve vs reject vote counts when threshold is reached — reject majority sends campaign back toDraftDraftinstead of being stuck foreverCampaignVoteRejectedevent for indexer integrationVotingSession/VoteOption/VoteStatustypes for cross-contract deserializationNone) on rejectionCloses #3
Test plan
test_vote_reject_returns_to_draft— voter votes "Reject" (option 1) → threshold reached → campaign returns to Draft with session clearedtest_vote_expired_without_quorum_returns_to_draft— threshold=5, only 1 vote cast, time expires → campaign returns to Drafttest_vote_threshold_not_met_while_active— voting still open, threshold not met → returnsVoteThresholdNotMeterror (existing behavior preserved)test_governance_flow— existing approve path still works correctlySummary by CodeRabbit
New Features
Bug Fixes
Tests