Skip to content

fix(spammer): reject --mix weight totals that overflow u32 - #362

Closed
kutluhaneth46 wants to merge 1 commit into
circlefin:mainfrom
kutluhaneth46:fix/spammer-mix-weight-overflow
Closed

kutluhaneth46 wants to merge 1 commit into
circlefin:mainfrom
kutluhaneth46:fix/spammer-mix-weight-overflow

Conversation

@kutluhaneth46

Copy link
Copy Markdown
Contributor

Summary

Fixes #352.

--mix transfer=4294967295,legacy=1 parses successfully today because each weight is a valid u32, then panics in TxTypeMix::total_weight() during Config::validate() (attempt to add with overflow, exit 101).

Changes

  • Add TxTypeMix::checked_total_weight() using checked_add.
  • Reject overflowing aggregates in FromStr and Config::validate() with a clear --mix error.
  • Make select_tx_type consume the checked total so generation cannot rely on overflowing arithmetic.
  • Add regression tests for ordinary ratios, u32::MAX, multi-field overflow, and programmatic Config::validate() (error, not panic).

Test plan

  • cargo test -p spammer tx_type_mix_ (and related config overflow test)

TxTypeMix parsed each weight as u32 but summed them with wrapping/panic-prone
addition. Validate the aggregate with checked addition in FromStr and
Config::validate, and make generator selection consume the checked total so
invalid CLI mixes fail with a normal --mix error instead of overflowing.
@huklaa

huklaa commented Sep 6, 2026

Copy link
Copy Markdown

@kutluhaneth46, this appears to reopen the same #352 implementation scope that you previously closed in #355 after acknowledging that my public claim came first.

For clarity, the chronology is already documented in #355: I claimed #352 first with the exact scope (overflow-safe aggregate handling, configuration-boundary rejection, and focused regression coverage), and you later wrote that my claim was visible first, that #355 covered the same scope, and that you were closing it so I could complete the work I had claimed.

PR #362 again says "Fixes #352" and implements substantially that same scope: checked aggregate addition, rejection in "FromStr" / "Config::validate", and overflow regression tests. Reopening the same implementation under a new PR number after explicitly closing the earlier duplicate does not resolve the coordination issue.

Please close #362 as well, or explain what materially changed in the chronology/scope since your acknowledgement on #355. I am continuing the work I publicly claimed first, and I’d like us to keep the contribution history and coordination transparent for maintainers.

@kutluhaneth46

Copy link
Copy Markdown
Contributor Author

@huklaa You're right — thanks for catching this.

I had already closed #355 after acknowledging your earlier public claim on #352, and #362 should not have reopened that same implementation scope. Closing this PR now so you can finish the work you claimed first. Sorry for the noise.

@kutluhaneth46

Copy link
Copy Markdown
Contributor Author

Closing as duplicate of the claimed #352 scope (see #355 / @huklaa).

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.

Spammer should reject transaction mix weights whose total overflows

2 participants