Skip to content

fix(contract): enforce maximum campaign deadline extension#569

Open
MerlinTheWhiz wants to merge 4 commits into
Iris-IV:mainfrom
MerlinTheWhiz:fix/max-extension-days-validation
Open

fix(contract): enforce maximum campaign deadline extension#569
MerlinTheWhiz wants to merge 4 commits into
Iris-IV:mainfrom
MerlinTheWhiz:fix/max-extension-days-validation

Conversation

@MerlinTheWhiz

@MerlinTheWhiz MerlinTheWhiz commented Jun 30, 2026

Copy link
Copy Markdown

📌 Description

Adds CAMPAIGN_EXTENSION_MAX_DAYS — a non-configurable absolute cap of 365 days on total campaign duration (from start time to extended deadline) in extend_campaign_deadline. This acts as a defense-in-depth safety net, preventing the deadline from being pushed beyond 365 days even if admin-configured category duration caps are set to the maximum.

Previously, the category duration cap (default 365 days, configurable by admin up to 365) was the only guard on total campaign duration during extension. The new constant provides a hard-coded ceiling that cannot be overridden, isolating the extension logic from admin governance of category caps and preventing funds from being locked indefinitely.

🔗 Related Issues

Closes #558
Closes #547

🧪 Changes Made

  • Bug fix
  • New feature
  • Refactor
  • Documentation update

✅ Checklist

  • Code compiles successfully
  • Tests added/updated and passing
  • Linting passes (no warnings/errors)
  • Documentation updated (if required)
  • No breaking changes (or clearly documented)

⚠️ Breaking Changes

None. The new check is strictly additive — it only rejects extensions that were already rejected by the existing category cap check (both use 365 days as the default cap). All 274 existing tests pass unchanged.

📸 Screenshots (if applicable)

N/A

🧩 Additional Notes

The constant is placed alongside the existing validation constants in src/lib.rs and is referenced directly in src/campaigns/update.rs without passing through admin-configurable storage, ensuring the 365-day ceiling is always enforced.

@drips-wave

drips-wave Bot commented Jun 30, 2026

Copy link
Copy Markdown

@MerlinTheWhiz 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! 🚀

Learn more about application limits

@davidmaronio

Copy link
Copy Markdown
Contributor

the fix is correct, update.rs:118-120 caps the total extended duration (new_deadline - start_time) at CAMPAIGN_EXTENSION_MAX_DAYS=365, checked against total duration which is the right basis, inclusive bound (>365 rejects, 365 ok, no off-by-one), and the test at test_deadline_ext.rs:250 covers both sides. fmt/clippy/test are green. one note: it's largely redundant with the existing category_cap check on line 114 (get_category_duration_cap defaults to 365), so it only adds value as a hard ceiling if an admin sets a per-category cap above 365, harmless defense-in-depth. it's just behind main, rebase and it's good.

heads up on consolidation: this change is also carried verbatim inside your #571 and #572 (they stack, #569 in #571 in #572). we should land one of the three and close the others rather than merge overlapping copies, see my notes there.

@MerlinTheWhiz

MerlinTheWhiz commented Jul 4, 2026

Copy link
Copy Markdown
Author

@davidmaronio Yh I worked on all three PRs, so it's like a chain branch. Each was checked out of the other with this being the parent/origin. So the right thing to do is to merge in order of PR raised starting from this one

I just rebased and I'm getting failed checks, prolly because you merged a PR with failed check to the project. But I'll confirm to check if it's from my changes

@MerlinTheWhiz

Copy link
Copy Markdown
Author

@davidmaronio As I suspected, the failed checks weren't from my changes. You can merge now

@MerlinTheWhiz

Copy link
Copy Markdown
Author

Hi @davidmaronio this is still up

@davidmaronio

Copy link
Copy Markdown
Contributor

your actual change (the extension cap in update.rs/lib.rs/test) is still fine, the new red is from how the branch got updated: you merged main in via a merge commit and it came out half-resolved. fmt/clippy/test now fail on artifacts that aren't in your diff:

  • admin.rs:91 error[E0255] set_emergency_pause_signers is defined multiple times (the merge duplicated it), delete the duplicate.
  • a cascade of error[E0063] "missing fields token and uses_milestones in CreateCampaignParams" across the test helpers/files, main added those two fields and the merge didn't update the call-sites.
    cleanest fix is to rebase instead of merge: git fetch origin && git rebase origin/main && git push --force-with-lease, resolve the admin.rs duplicate + update the CreateCampaignParams call-sites during the rebase. the merge commit is also what will trip commit-message validation, so rebasing fixes that too.

@davidmaronio

Copy link
Copy Markdown
Contributor

update: main was actually broken this whole time, not your pr. PR #574 (per-campaign tokens / milestone withdrawals / emergency pause) got merged on 07-01 while its CI was red and it didn't compile, so every branch that rebased or merged main inherited those exact errors (the duplicate set_emergency_pause_signers, the CreateCampaignParams missing-fields cascade, iter_mut, etc). that wasn't your merge, it was the base. i've reverted #574 in #584 and main is green again. so please rebase onto the current main and those inherited errors will clear: git fetch origin && git rebase origin/main && git push --force-with-lease. (#574's feature will be re-submitted properly.) sorry for the earlier note pinning it on your rebase.

@MerlinTheWhiz

Copy link
Copy Markdown
Author

@davidmaronio Alright then, please be on standby so you can review and merge once I'm done. You've not really being responsive, so I don't know when next you'll be active.

@MerlinTheWhiz

MerlinTheWhiz commented Jul 5, 2026

Copy link
Copy Markdown
Author

@davidmaronio You can review and merge this now. Let me work on the rest.

@MerlinTheWhiz

Copy link
Copy Markdown
Author

@davidmaronio This is still up

@MerlinTheWhiz

Copy link
Copy Markdown
Author

@davidmaronio Hey, this is still up.

1 similar comment
@MerlinTheWhiz

Copy link
Copy Markdown
Author

@davidmaronio Hey, this is still up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Security] No max boundary on extension days in extend_campaign_deadline [Security] No max boundary on extension days in extend_campaign_deadline

2 participants