Sort earners before migrator construction#135
Merged
PierrickGT merged 2 commits intoJun 22, 2026
Conversation
Changes to gas cost
🧾 Summary (20% most significant diffs)
Full diff report 👇
|
LCOV of commit
|
Drop the manual storage-to-memory copy loops in the integration test fixtures; assigning or passing the storage array to a memory parameter already performs the copy.
PierrickGT
approved these changes
Jun 22, 2026
PierrickGT
merged commit b873e34
into
proto-969-fix-remaining-chainsecurity-issues
5 checks passed
PierrickGT
added a commit
that referenced
this pull request
Jun 24, 2026
* fix(WrappedM): prevent frozen claim recipient from blocking stopEarningFor An earner could point their claim recipient at a frozen address so that the permissionless stopEarningFor() reverts inside _claim() -> _transfer() -> _revertIfFrozen(recipient_), keeping the account earning indefinitely and defeating the deauthorization mechanism. Both stopEarningFor() overloads now fall back to skipTransfer when the resolved claim recipient is frozen, mirroring the _beforeFreeze path: the yield is crystallized onto the account's own balance instead of being routed to the frozen recipient, so deauthorization always succeeds. * fix(WrappedM): enforce sorted unique earners in migrator list The WrappedMTokenMigratorV1 migration corrupts accounting if an address appears twice in the earners array, since the first pass overwrites the lastIndex slot with the computed principal. ListOfEarnersToMigrate now requires strictly ascending addresses, enforcing sorted order, uniqueness and non-zero in a single O(n) pass without the gas cost of an O(n^2) check. Integration test fixtures sort the in-memory earners copy before deploying the migrator, mirroring the production generation script. * chore(script): harden earners list generation Make the earners list generation safe to rely on for the v1->v2 migration: - Paginate holder fetches so earners are never silently truncated by the `first` cap, with a max-pages guard against a broken `skip`. - Build the whole file in memory and write once after every network succeeds; abort without writing on any error, and exit non-zero, so a partial run never mixes fresh and stale lists. - Throw on duplicate or zero earner addresses instead of relying solely on the on-chain check. - Fix the GraphQL response typing (drop the unsound cast), add a request timeout, centralize the chain enum, and drop the unused balance field. * Sort earners before migrator construction (#135) * fix(deploy): sort earners before migrator construction * refactor(test): pass earners storage array directly to migrator deploy Drop the manual storage-to-memory copy loops in the integration test fixtures; assigning or passing the storage array to a memory parameter already performs the copy. --------- Co-authored-by: Pierrick Turelier <pierrick@turelier.com> --------- Co-authored-by: Denali Marsh <denalimarsh@gmail.com>
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
2 participants
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.
Summary
Issue 016 in #134 added a strictly-ascending check to the
ListOfEarnersToMigrateconstructor, but the sorting it now requires was only applied to the generator script and the test fixtures, not to the production deploy path or the committedEarnersAddresses.sol. As written, the mainnet upgrade would revert at deploy time. This PR closes that gap.Problem
DeployUpgradeMainnetpassesEarnersAddresses.getEthereumEarners()straight into the migrator constructor. That committed list predates the sorting change and is not strictly ascending, sonew ListOfEarnersToMigrate(...)reverts withEarnersNotSortedOrUniqueand the deployment aborts. CI didn't surface it because the integration tests sort their fixtures at runtime and the upgrade test is mainnet-fork-gated.Changes
DeployBase._deployMigrator), so a deploy is robust to input ordering. The constructor's uniqueness and non-zero checks still apply.TestBase,Upgrade.t.sol) throughDeployBaseand drop the two duplicated_sortAddresseshelpers, so the tests now exercise the real deploy path.EarnersAddresses.solin place. Membership is unchanged: the address set is identical before/after (verified); only the Ethereum array's order moves.ListOfEarnersToMigratedirectly from each committed array, so an unsorted/duplicate/zero-address artifact now fails in default CI.