feat: principal-based account

[deluca-mike]

• principal-based earner account, instead of last index
• auto and manual migration for existing earners

[github-actions]

LCOV of commit b6e8c41 during Forge Coverage #537

Summary coverage rate:
  lines......: 98.4% (250 of 254 lines)
  functions..: 98.2% (56 of 57 functions)
  branches...: 88.0% (22 of 25 branches)

Files changed coverage rate:
                       |Lines       |Functions  |Branches    
  Filename             |Rate     Num|Rate    Num|Rate     Num
  ===========================================================
  src/WrappedMToken.sol|23.1%    225|41.2%    51|    -      0

[toninorair]

why the was a round up in previous version? Bug? round down makes more sense

[deluca-mike]

Needed to round up because individual account principals were not stored and thus could not be accumulated in a fixed way, so principalOfTotalEarningSupply needed to be rounded up (inflated) to ensure excess was conservative (deflated).

Now that each earner has a fixed computeed earning principal, totalEarningPrincipal can simply be added to or subtracted from with these individual account earning principals. It is an exact accumulation.

[toninorair]

a lot of checks: this one should have already covered principal https://github.com/m0-foundation/wrapped-m-token/pull/102/files#diff-82f33536a281f203780891834e2f2453a322bed686e89ea9b7171a2d74fc10b9R442-R448

[deluca-mike]

That link does not seem to work so I do not know what you are referring to (but I assume it is the uint112 principal_ = UIntMath.min112 line in _subtractEarningAmount). In any case, the UIntMath.min240 and UIntMath.min112 here just prevent underflows without the _subtractTotalEarningSupply function needing to be aware of how its callers operate or what they check. It is a safety measure, especially if this function may be used again later down the road. It is minimal gas for that safety.

But yes, technically it would be impossible to decrement totalEarningSupply or totalEarningPrincipal below 0 given any accounts' amount_ or principal_, since both those totals are already established totals.

We can remove the checks if you prefer.

[toninorair]

balanceWithUnclaimedYieldOf or even delete this function

[deluca-mike]

I will handle fucntion renames later on as:

• it is outside the scope of this PR
• they would be applicable and useful even if this pr is not merged
• the rebase(s) would potentially be too tedious at this point

[toninorair]

optimization mentioned in TODO?

[deluca-mike]

In a future PR, if needed at all. May depend on other PRs as well.

[toninorair]

great work, things to discuss - simplifying migration strategy / code

by looping over all wM accounts in Migrator contract etc

[PierrickGT]

Is this check still relevant?
We are performing no operation once we enter this condition.

[deluca-mike]

Yes, because the event is emitted, and it may break indexers. Plus, you should not be able to transfer yourself a billion dollars if you don't have it.

[deluca-mike]

great work, things to discuss - simplifying migration strategy / code by looping over all wM accounts in Migrator contract etc

While it could work, it would mean that the earners list cannot change from the time the Migrator is deployed to the time it is used, which is not that trivial.

First the new implementation and migrator are deployed, then the proposals are created to enable the migrator, then they are passed, then they are executed, then the migration can be performed. In that time, other proposals to modify the earners list may come in, which can create gaps in that batch account migration.

The "as needed" migration is impossible to break.

However, if we did do batch migration (and could guarantee completeness), it would:

• remove the account migration code from the implementation and interface
• remove the "ugly" EarningState enums
• make future PRs and abstracted branches have a smaller diff

Its worth noting that trying to migrate a non-earner results in a no-op, so it is fine. But skipping over an earner and not migrating them is a critical error, and the MIgrate contract process only happens once and cannot be repeated.