Fix/platform fee upper bound

[MerlinTheWhiz]

📌 Description

Add a hard technical invariant (PLATFORM_FEE_ABSOLUTE_MAX_BPS = 10000) that prevents update_platform_fee, init, and set_campaign_fee_override from accepting fee values > 10000 bps (100%). Without this bound, the withdraw fee computation ceil(amount * fee / 10000) could overflow i128 when fee > 10000 on large amounts. The existing business cap (PLATFORM_FEE_MAX_BPS = 1000 / 10%) is preserved; the new check acts as defense-in-depth against future changes to that constant.

🔗 Related Issues

Closes #559

🧪 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 absolute max (10000 bps) is wider than the existing business cap (1000 bps), so no previously-allowed fee is rejected.

📸 Screenshots (if applicable)

N/A

🧩 Additional Notes

Three entry points gained the invariant check: init, update_platform_fee, and set_campaign_fee_override. All 274 existing tests pass without modification.

[drips-wave]

@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]

the intent is right but the added checks don't actually do anything, so this can't land as-is:

1. in admin.rs (init ~L30, update_platform_fee ~L165, set_campaign_fee_override ~L187) you add if fee > PLATFORM_FEE_ABSOLUTE_MAX_BPS (10000) { Err } right before the existing if fee > PLATFORM_FEE_MAX_BPS (1000) { Err }. any value over 10000 is already over 1000, so the pre-existing check rejects it first with the same error, the new branch is unreachable dead code.
2. the title says "bounds" but there's no lower bound added either (fee = 0 is still allowed), so neither bound is meaningfully changed.
3. this also re-includes all of #569's deadline-extension change (same update.rs/lib.rs/test), so it duplicates that pr.
   either make the checks real (e.g. add a sensible lower bound, or a per-campaign override ceiling that's actually below the global max), or close this in favor of #572 which already carries these same hunks. fmt/clippy/test are green, the issue is purely that the change is a no-op.

[MerlinTheWhiz]

@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 #569

I just rebased and I'm getting failed checks, and it's because you merged a PR with failed check to the project.

The redundant checks have been removed too. You can review and merge now.

[MerlinTheWhiz]

Hi @davidmaronio this is still up

[davidmaronio]

thanks for removing the redundant check. one thing to confirm though, the way it was removed changes the business rule: you replaced the fee > PLATFORM_FEE_MAX_BPS checks (the meaningful ~10% cap, the old tests rejected 1001 and 5000) with fee > PLATFORM_FEE_ABSOLUTE_MAX_BPS (=10000 = 100%), and updated the tests to only reject 10001. so the effective upper bound went from 10% to 100%, and there's still no lower bound. is raising the cap to 100% actually intended? if the goal was just to add a hard ceiling, keep the 1000-bps cap and add the 10000 one as a separate guard rather than replacing it.
separately, ci is red for an unrelated reason: you merged main in via a merge commit and it left the same admin.rs:91 duplicate set_emergency_pause_signers + CreateCampaignParams missing-fields breakage as the other prs. please rebase instead of merge (git fetch origin && git rebase origin/main && git push --force-with-lease).

[davidmaronio]

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]

@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]

@davidmaronio Yh I misunderstood what you said. I now know you meant I should remove the absolute max I added because it is a dead code.

But the confusion actually came from the task requirement. I just went back to my initial implementation on this task and realised that was where it was coming from:

I saw there was already an upper bound implementation (<=1000bps), and I thought the task meant to add that (<=10000) like an extra guard on that or a form of documentation. So putting that was actually what you said in the task, so removing it basically means I didn't do anything. Hence why I thought you meant to remove what was in the code initially, the <=1000, and preserve mine that was the task instruction.

Now I'll add the former back and preserve mine too.

[MerlinTheWhiz]

@davidmaronio Also for the lower bound you keep mentioning, no lower bound is needed becuase u32 enforces non-negative, and a zero fee is an intentional business case (e.g., charity campaigns or fee waivers), already tested and expected behavior.

Unless you explicitly want it, then you can tell me what value you have in mind.

[MerlinTheWhiz]

@davidmaronio But knowing how rarely you come online to respond, I'l leave that out and focus on the upper bound. Please if you're requesting for any change after these, please remain online to review immediately so you can merge.

[MerlinTheWhiz]

@davidmaronio I've implemented it. All done, and all green. Please review and merge.

[MerlinTheWhiz]

@davidmaronio This is still up

[MerlinTheWhiz]

@davidmaronio Hey, this is still up.