Skip to content

fix(precompiles): use checked_sub for total_supply in burn - #480

Closed
forumevi wants to merge 1 commit into
circlefin:mainfrom
forumevi:fix/burn-total-supply-checked-sub
Closed

forumevi wants to merge 1 commit into
circlefin:mainfrom
forumevi:fix/burn-total-supply-checked-sub

Conversation

@forumevi

@forumevi forumevi commented Oct 5, 2026

Copy link
Copy Markdown

## Summary

In native_coin_authority.rs, the burn handler reads the current total_supply
from storage and then subtracts the burned amount using saturating_sub:

// before
&current_total_supply.saturating_sub(args.amount).to_be_bytes_vec()

The inline comment says "Underflow cannot happen due to the balance check", but
saturating_sub silently clamps to zero when an underflow does occur instead of
surfacing it as an error. This means that any bug or storage corruption that violates
the invariant (total_supply >= individual balance) would cause total_supply to
be written as 0 without any revert or observable signal — permanently corrupting
the global supply accounting.

## Fix

Replace saturating_sub with checked_sub and propagate a hard revert with
ERR_OVERFLOW on underflow, consistent with how mint already guards the
opposite direction:

// after
let new_total_supply = current_total_supply
    .checked_sub(args.amount)
    .ok_or_else(|| new_reverted_with_early_penalty(gas_counter, reservoir, ERR_OVERFLOW))?;

If the invariant holds (it always should on a correct chain), behaviour is
identical. If it ever does not hold, the transaction reverts loudly instead of
corrupting the supply silently.

## Tests

Two regression tests are added to native_coin_authority.rs:

  • burn_decrements_total_supply_correctly — verifies the happy-path decrement is exact.
  • burn_reverts_on_total_supply_underflow_instead_of_saturating — verifies that when total_supply is artificially set below the burn amount (simulating storage corruption), the call reverts with ERR_OVERFLOW rather than writing zero.

saturating_sub silently clamps total_supply to zero when an underflow
occurs (e.g. due to storage corruption or an invariant violation).
This makes a critical accounting error invisible at the call site.

Replace saturating_sub with checked_sub so that any underflow is
surfaced immediately as a hard revert with ERR_OVERFLOW, consistent
with how mint already guards against overflow.

The invariant (total_supply >= individual balance) is maintained by
construction, so this change has no effect on correct execution.

Also add two regression tests:
- burn_decrements_total_supply_correctly: verifies the happy-path
  decrement is exact.
- burn_reverts_on_total_supply_underflow_instead_of_saturating:
  verifies that a corrupted/zeroed total_supply triggers ERR_OVERFLOW
  instead of silently writing zero.
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Hi @forumevi,

Thank you for your interest in contributing to Arc Node.

This PR has been automatically closed because it does not reference a GitHub issue. All PRs must reference an existing issue using the format Closes: #XXX.

To contribute properly:

  1. Find an existing issue you'd like to work on, or open a new issue describing your proposed change
  2. Comment on the issue requesting assignment and wait for maintainer approval
  3. Only submit a PR after you have been assigned to the issue

Please see our CONTRIBUTING.md for more details.

@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

⚠️ Unsigned Commits Detected

The following commits are missing a verified signature:

  • c5a1ac0 by forumevi

How to fix: Sign your commits.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant