feat: implemented auto-refund fallback mechanism on expired campaigns - #563
feat: implemented auto-refund fallback mechanism on expired campaigns#563opratem wants to merge 3 commits into
Conversation
|
@opratem Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
Warning Review limit reached
Next review available in: 52 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 (2)
📝 WalkthroughWalkthroughAdds a new ChangesCampaign Funding
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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 (5)
contracts/campaign-funding/src/lib.rs (5)
442-452: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDoc lists an error the function cannot return.
claim_fundsenforces creator identity viarequire_auth(), soError::Unauthorizedis never emitted (and appears unused across the contract). Drop it from the doc or remove the variant.🤖 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/campaign-funding/src/lib.rs` around lines 442 - 452, Update the `claim_funds` documentation to remove the `Error::Unauthorized` entry from its `# Errors` list, since authorization is enforced through `require_auth()` rather than that error variant. Leave the documented `CampaignNotSuccessful` and `AlreadyClaimed` errors unchanged.
211-225: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueMove
admin.require_auth()before state inspection.Minor ordering nit: authorization currently runs after the
AlreadyInitialized/FeeTooHighchecks. Not exploitable here (both checks are read-only), but auth-first is the safer convention.🤖 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/campaign-funding/src/lib.rs` around lines 211 - 225, The initialize function should authenticate the caller before inspecting initialization state or validating the fee rate. Move admin.require_auth() to the beginning of initialize, before the DataKey::Admin check, while preserving the existing validation and storage behavior.
1318-1352: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest name doesn't match what is asserted.
test_refund_multiple_contributors_all_refundedsets up three contributors onid(which reachesmin_targetand therefore can never be refunded) and then only refunds a single contributor on a second campaign. Either refund and assert all three contributors on a genuinely failed campaign, or rename to reflect the single-refund scenario.🤖 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/campaign-funding/src/lib.rs` around lines 1318 - 1352, The test test_refund_multiple_contributors_all_refunded does not match its single-contributor assertion. Rename it to describe the id2 single-refund scenario, or revise the setup and assertions to refund and verify all contributors on a genuinely failed campaign; preserve the existing refund behavior being tested.
1161-1177: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThis test does not actually verify permissionlessness.
The comment says "Called with no auth mocking", but
env.mock_all_auths()is active at Line 1166, so the test would pass even iftrigger_expiryrequired auth. Drop the mock (or useenv.set_auths(&[])) so the assertion has meaning.♻️ Proposed change
fn test_trigger_expiry_permissionless() { // A random third party (neither creator nor contributor) can call // trigger_expiry — the function requires no auth. let env = Env::default(); env.mock_all_auths(); set_time(&env, 1_000); let (_, client, _, _) = setup_contract(&env); let creator = Address::generate(&env); let token = Address::generate(&env); let id = client.create_campaign(&creator, &token, &10_000, &5_000, &2_000); set_time(&env, 3_000); - // Called with no auth mocking — just default env. + // Clear mocked auths so a missing auth requirement would fail the call. + env.set_auths(&[]); client.trigger_expiry(&id); assert_eq!(client.get_campaign(&id).status, CampaignStatus::Failed); }🤖 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/campaign-funding/src/lib.rs` around lines 1161 - 1177, Remove the active authorization mocking from test_trigger_expiry_permissionless, or explicitly clear authorizations with env.set_auths(&[]) before calling trigger_expiry. Preserve the existing setup and status assertion so the test genuinely verifies the call succeeds without authorization.
549-586: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
get_fee_collectorpanics with an untyped host error when uninitialized.Line 585's bare
unwrap()produces a missing-value host error instead ofError::NotInitialized, inconsistent with the rest of the surface.♻️ Proposed change
pub fn get_fee_collector(env: Env) -> Address { env.storage() .instance() .get(&DataKey::FeeCollector) - .unwrap() + .unwrap_or_else(|| panic_with_error!(&env, Error::NotInitialized)) }🤖 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/campaign-funding/src/lib.rs` around lines 549 - 586, Update get_fee_collector to handle an absent FeeCollector value explicitly and return the contract’s Error::NotInitialized error instead of using a bare unwrap. Preserve returning the stored Address when the value is initialized and match the established error behavior for uninitialized state.
🤖 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/campaign-funding/src/lib.rs`:
- Around line 1023-1032: Update test_contribute_to_nonexistent_campaign to
assert the numeric contract error emitted for CampaignNotFound rather than the
enum variant name CampaignNotActive. Use the corresponding CampaignNotFound
discriminant from the contract error enum and match the full host panic string
if required by the test framework.
---
Nitpick comments:
In `@contracts/campaign-funding/src/lib.rs`:
- Around line 442-452: Update the `claim_funds` documentation to remove the
`Error::Unauthorized` entry from its `# Errors` list, since authorization is
enforced through `require_auth()` rather than that error variant. Leave the
documented `CampaignNotSuccessful` and `AlreadyClaimed` errors unchanged.
- Around line 211-225: The initialize function should authenticate the caller
before inspecting initialization state or validating the fee rate. Move
admin.require_auth() to the beginning of initialize, before the DataKey::Admin
check, while preserving the existing validation and storage behavior.
- Around line 1318-1352: The test test_refund_multiple_contributors_all_refunded
does not match its single-contributor assertion. Rename it to describe the id2
single-refund scenario, or revise the setup and assertions to refund and verify
all contributors on a genuinely failed campaign; preserve the existing refund
behavior being tested.
- Around line 1161-1177: Remove the active authorization mocking from
test_trigger_expiry_permissionless, or explicitly clear authorizations with
env.set_auths(&[]) before calling trigger_expiry. Preserve the existing setup
and status assertion so the test genuinely verifies the call succeeds without
authorization.
- Around line 549-586: Update get_fee_collector to handle an absent FeeCollector
value explicitly and return the contract’s Error::NotInitialized error instead
of using a bare unwrap. Preserve returning the stored Address when the value is
initialized and match the established error behavior for uninitialized 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: 2ef6457b-1a14-4876-a954-e0f24437c153
📒 Files selected for processing (42)
contracts/Cargo.tomlcontracts/campaign-funding/Cargo.tomlcontracts/campaign-funding/src/lib.rscontracts/campaign-funding/test_snapshots/tests/test_claim_funds_double_claim.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_claim_funds_on_active_campaign.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_claim_funds_on_failed_campaign.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_claim_funds_success.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_claim_funds_zero_fee.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_contribute_accumulates.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_contribute_after_deadline.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_contribute_auto_succeed_on_hard_cap.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_contribute_exceeds_hard_cap.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_contribute_multiple_contributors.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_contribute_success.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_contribute_to_nonexistent_campaign.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_contribute_zero_amount.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_create_campaign_deadline_in_past.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_create_campaign_ids_increment.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_create_campaign_min_target_exceeds_target.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_create_campaign_not_initialized.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_create_campaign_success.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_create_campaign_zero_min_target.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_create_campaign_zero_target.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_fee_calculation_precision.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_initialize_fee_too_high.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_initialize_success.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_initialize_twice_fails.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_refund_double_refund_prevented.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_refund_multiple_contributors_all_refunded.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_refund_no_contribution.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_refund_on_active_campaign.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_refund_on_successful_campaign.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_refund_success.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_set_fee_collector.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_set_fee_rate.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_set_fee_rate_too_high.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_trigger_expiry_already_resolved.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_trigger_expiry_before_deadline.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_trigger_expiry_permissionless.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_trigger_expiry_sets_failed_when_target_not_met.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_trigger_expiry_sets_successful_when_target_met.1.jsoncontracts/campaign-funding/test_snapshots/tests/test_trigger_expiry_with_zero_contributions_fails.1.json
|
dont forget to offramp using https://stellar.fundable.finance/offramp its fast, free and p2p rates |
1 similar comment
|
dont forget to offramp using https://stellar.fundable.finance/offramp its fast, free and p2p rates |
|
conflicts resolved |
2 participants
Description
Implements the complete auto-refund fallback mechanism for the
campaign-fundingSoroban smart contract. When a campaign's deadline passes without meeting its minimum funding target, any caller can trigger expiry to mark the campaign as failed, after which contributors can individually reclaim their full escrowed contributions.Related Issue
Fixes #507
Type of Change
Changes Made
Implemented
trigger_expiry()function — permissionless deadline expiration trigger:Activestate (CampaignNotActiveerror if not)ledger.timestamp() >= deadline(DeadlineNotReachederror if not)Successfuliftotal_raised >= min_target, otherwiseFailedCampaignStatusChangedevent on every transitionrequire_auth()— intentionally callable by anyone (contributor, bot, third party)Implemented
refund()function — per-contributor escrow recovery:contributor.require_auth()Failedcampaigns (CampaignNotFailederror otherwise)RefundIssuedeventExtended
Errorenum with typed codes:DeadlineNotReached = 9CampaignNotFailed = 10NoContributionFound = 12CampaignNotSuccessful = 11,AlreadyClaimed = 13,TargetExceeded = 16(supporting codes)Storage TTL management —
extend_ttl(LEDGER_THRESHOLD, LEDGER_BUMP)applied to all persistent reads and writes (~30/31 day windows at 5 s/ledger)Full Rust doc comments on all public functions (
trigger_expiry,refund,create_campaign,contribute,claim_funds, and all query/admin setters)Testing
trigger_expiryandrefund:trigger_expiry(6 tests): setsSuccessfulwhen target met, setsFailedwhen target not met, handles zero contributions, rejects calls before deadline, rejects already-resolved campaigns, confirms permissionless caller worksrefund(6 tests): full refund success, multiple contributors all refunded, rejects active campaigns, rejects successful campaigns, rejects callers with no contribution, prevents double-refundtest_snapshots/Function Signatures
Checklist
Screenshots
N/A — smart contract, no UI changes
Additional Notes
trigger_expiryis deliberately permissionless. Requiring auth from the creator would allow a failing campaign creator to block refunds indefinitely by simply not calling it. Making it open ensures contributors can always recover their funds after a failed campaign, even without creator cooperation.Summary by CodeRabbit
New Features
Tests