Fix #555: Prevent accept_campaign_transfer to same creator#580
Conversation
|
@Banx17 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! 🚀 |
davidmaronio
left a comment
There was a problem hiding this comment.
the actual fix is correct, the if pending == old_creator { return Err(InvalidNewOwner) } guard in transfer.rs + its test are exactly right. the problem is scope, this pr is branched off the #581 milestone branch so it drags in ~20 files of that feature (MaybeToken, uses_milestones, verify_milestone rewrite, types.rs/storage.rs/admin.rs), which overlaps #581 heavily and will clobber on merge. please rebase onto clean main so the diff is just the ~4-line transfer guard + test. also fix clippy (unused var admin, and the u32->u32 unnecessary cast) and run cargo fmt.
Overview
This PR addresses issue #555 where
accept_campaign_transferallowed transferring a campaign back to the same creator.While
initiate_campaign_transferalready has checks, adding it toaccept_campaign_transferensures full defense-in-depth and prevents wasted ledger space and compute.Changes Made
pending == old_creatorcheck inaccept_campaign_transferinsidesrc/campaigns/transfer.rsto prevent a transfer to the current owner.test_accept_campaign_transfer_same_creatortest case tosrc/tests/test_campaign_update.rsto verify the fix works as intended.Validation
[npm run lint ✅ completed (existing repo warnings only)
npm run typecheck ✅ passed
npm test -- src/config/tests/config.test.ts --runInBand ✅ passed (9/9)
npm run build ✅ passed]
Fixes #555