fix(token): make the whale cap getter honest and surface the revoke-admin trade-off - #445
Merged
Merged
Conversation
Once revoke_admin is called the max-balance-per-account cap stops being enforced, but max_balance_per_account kept returning the stored percentage and the dashboard presented it as an active limit. Make the getter return None once the contract is locked, give up (not enforce) in the transfer/mint path when no admin exists, and name the trade-off in the dashboard. Closes soropad#406 Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com>
|
@khaylebfortune 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! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #406
Summary
revoke_admindeactivates the max-balance-per-account (whale) cap as an intentional side effect of removing the admin role — mint/transfer stop enforcing it so the token stays fully transferable (the fix shipped for #323). But nothing surfaced that consequence.max_balance_per_account()kept returning the stored percentage,PolicyCardpresented the cap as a standing rule, andTokenStatusBannerframed immutability purely as a trust win. Holders were told the token is both immutable and whale-capped, when revoking admin is exactly what turned the cap off.This PR makes every surface tell the truth:
Contract (
contracts/token/src/lib.rs)max_balance_per_account()now returnsNoneonce the contract is locked, even if a percentage was stored, so the getter never reports a limit that is not being enforced. Doc comment updated to say so._enforce_max_balance_per_account()now actually gives up when the admin is absent. The code read the admin as anOptionbut still enforced the cap for non-admin recipients after revoke — contradicting the documented behavior. It now uses thelet Some(admin) = … else { return }pattern: oncerevoke_adminremoves the admin, the cap is no longer enforced and holders may exceed it.Test (
contracts/token/src/lib.rs)test_non_admin_can_exceed_cap_after_revoke: sets a 10% cap, verifies a non-admin transfer beyond the cap is rejected while it is active, then callsrevoke_admin, verifiesmax_balance_per_account()returnsNone, and verifies the same non-admin can now accumulate past the former cap — locking in the semantic as intended, not accidental.test_holder_actions_still_work_after_revoke, which was failing onmasterfor exactly this reason (the cap still fired after revoke).Frontend
PolicyCardnow takes alockedprop and renders the whale cap as inactive when admin has been revoked, with a yellow warning explaining that revoking admin control turned the cap off and a single holder can now accumulate past it (or, when no cap was set, that the cap can no longer be enforced). An inactive cap is no longer shown in the active/blue styling.TokenStatusBannerimmutability row now names the trade-off instead of describing end-of-admin purely as a "strong trust signal": supply/admin permissions can no longer change, but any whale-protection cap is also off.Testing
cargo test -p soroban-token: 116 passed, including the new test and the previously-failingtest_holder_actions_still_work_after_revoke.master(stale#[should_panic(expected = "…")]strings vs. the typedTokenErrorpanics) and are unrelated to this change.cargo fmt --checkandcargo clippyproduce only pre-existing toolchain-version diffs/warnings already present onmaster; my changes add no new ones.tsc --noEmitandjestshow the same pre-existing failures onmaster; my touched files introduce no new type errors or test failures.Files changed
contracts/token/src/lib.rscontracts/token/test_snapshots/test/test_non_admin_can_exceed_cap_after_revoke.1.jsonfrontend/app/dashboard/[contractId]/components/AdminPanel.tsxfrontend/app/dashboard/[contractId]/components/admin/PolicyCard.tsxfrontend/components/TokenStatusBanner.tsx