Skip to content

fix(governance-token): panic on mint(amount <= 0), matching transfer/burn - #773

Open
kragent66-glitch wants to merge 1 commit into
ThinkLikeAFounder:mainfrom
kragent66-glitch:fix/760-mint-amount-validation
Open

fix(governance-token): panic on mint(amount <= 0), matching transfer/burn#773
kragent66-glitch wants to merge 1 commit into
ThinkLikeAFounder:mainfrom
kragent66-glitch:fix/760-mint-amount-validation

Conversation

@kragent66-glitch

Copy link
Copy Markdown

Fixes #760.

Problem

governance-token::mint had no input validation on amount, while the sibling functions transfer and burn in the same file both panic on amount <= 0. A non-positive amount would:

  • Pass current_supply.checked_add(amount) (silently decreasing TotalSupply when amount < 0)
  • Decrement the recipient's balance via balance + amount
  • Skew delegated-power bookkeeping in the same wrong direction

Only the admin can call mint, so this isn't externally exploitable by an attacker — but a buggy or compromised admin flow would corrupt global token accounting with no panic to flag it, and the inconsistency with transfer/burn is itself a real defect.

Approach (minimal)

  • Add at the top of mint, immediately after the admin check:
    if amount <= 0 {
        panic!("invalid amount");
    }
  • Use the wording from transfer ("invalid amount") rather than burn's ("burn amount must be positive") since mint is the most generic token-mutating function of the three.
  • Two regression tests in test.rs: test_mint_zero_amount_rejected and test_mint_negative_amount_rejected, both #[should_panic(expected = "invalid amount")].

This is independent of #670, which added a supply overflow guard via checked_add. checked_add(-N) on a non-overflowing current_supply still succeeds and silently decreases supply; the input guard that #760 calls for was not added by that PR.

Verification

  • cargo test -p pulsar-governance-token --lib19 passed; 0 failed, including both new tests.
  • Sabotage check: removed the guard, re-ran tests. The two new tests FAILED (test_mint_negative_amount_rejected, test_mint_zero_amount_rejected); all other mint tests still passed. Restored the guard → all 19 pass.
  • Tested with rustc 1.95.0. CI uses dtolnay/rust-toolchain@stable and cargo test --workspace.
  • Diffs are limited to contracts/governance-token/src/lib.rs (+4 lines: guard) and contracts/governance-token/src/test.rs (+20 lines: 2 new tests) plus the two matching test snapshot JSON files.

Known issues / notes

  • No drive-by formatting or refactoring. The diff is purely the missing guard + regression tests.
  • Cargo.lock is not tracked in this repo (gitignored) so no lockfile churn is included in the PR.

…burn

Closes ThinkLikeAFounder#760

mint accepted zero and negative amounts because it had no input
validation, while sibling functions transfer and burn both panic on
amount <= 0. A non-positive amount would let current_supply.checked_add
succeed (silently decreasing supply), corrupt the recipient's balance,
and skew delegated power bookkeeping.

Add the same if amount <= 0 { panic!("invalid amount") } guard that
transfer uses, placed after the admin check and before the supply
arithmetic. Cover it with two regression tests for amount=0 and
amount=-1; both fail on pre-fix code (verified via sabotage check) and
pass with the fix.
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.

governance-token: mint() has no amount validation, unlike transfer()/burn()

1 participant