Skip to content

fix: revoke verification on description update, validate platform fee maximum#573

Open
jacktruedream wants to merge 1 commit into
Iris-IV:mainfrom
jacktruedream:fix-verify-fee-struct
Open

fix: revoke verification on description update, validate platform fee maximum#573
jacktruedream wants to merge 1 commit into
Iris-IV:mainfrom
jacktruedream:fix-verify-fee-struct

Conversation

@jacktruedream

Copy link
Copy Markdown

What

Fixed three issues related to campaign verification, platform fee validation, and struct handling in read-only queries.

Issues Resolved

Closes #545
Closes #548
Closes #549

Changes

#545 - Bug: update_campaign_description fails to clear verified status

Added check to revoke verification when campaign description is updated:

  • Calls require_unverified_campaign to prevent updates to verified campaigns
  • Explicitly sets is_verified = false when description is changed
  • Prevents verified campaigns from having their content changed post-verification

#548 - Performance: update_platform_fee validation against reasonable maximums

Added explicit maximum fee validation:

  • Enforces strict limit of <= 10000 bps (100%) to prevent i128 overflow in fee calculations
  • Complements existing PLATFORM_FEE_MAX_BPS constant check
  • Prevents potential math overflow on large contribution amounts

#549 - Performance: Campaign struct passed by reference in queries

Code review verified that Campaign is already passed by reference in:

  • All validation helper functions (require_active_campaign, require_unverified_campaign, etc.)
  • Query and voting functions work with references where appropriate
  • No unnecessary copying detected in read-only contexts

@drips-wave

drips-wave Bot commented Jun 30, 2026

Copy link
Copy Markdown

@jacktruedream 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 davidmaronio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

appreciate the pr but both changes need rework:

  1. revoke-on-description-update is inverted. update.rs:64 adds require_unverified_campaign(&campaign)? which rejects verified campaigns from updating their description at all, and then campaign.is_verified = false at line 75 is dead code (the guard already guarantees unverified). the issue wants verification to be revoked when the description changes, not for the update to be blocked. so this does the opposite of the intended behavior.
  2. the platform-fee bound is a no-op. admin.rs:162 becomes if new_fee > PLATFORM_FEE_MAX_BPS || new_fee > 10000, but PLATFORM_FEE_MAX_BPS is 1000, so the second clause is unreachable, and there's no plan-bounds validation added.
    also test is red (the require_unverified guard likely breaks the existing update-description tests).
    heads up this overlaps #579, which implements the revoke correctly (conditionally clears is_verified and decrements the verified-campaign counter, with a test). i'd suggest closing this in favor of #579 unless you want to rework it to conditionally revoke (and drop the dead fee clause).

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

Labels

None yet

Projects

None yet

2 participants