Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Upgradable Earner Manager #86

Merged
merged 3 commits into from
Oct 30, 2024
Merged

feat: Upgradable Earner Manager #86

merged 3 commits into from
Oct 30, 2024

Conversation

deluca-mike
Copy link
Collaborator

@deluca-mike deluca-mike commented Oct 23, 2024

No description provided.

@deluca-mike deluca-mike added the enhancement New feature or request label Oct 23, 2024
@deluca-mike deluca-mike self-assigned this Oct 23, 2024
Copy link

github-actions bot commented Oct 23, 2024

Changes to gas cost

Generated at commit: 4b1b98eb4bdb00ca4bc491dfb434e34012bc2275, compared to commit: 8eecb792783f80364ea782efa1e50506b28e5bc1

🧾 Summary (20% most significant diffs)

Contract Method Avg (+/-) %
WrappedMTokenHarness balanceOf
claimFor
stopEarningFor(address[])
totalSupply
+79 ❌
+57,357 ❌
-3,403 ✅
-232 ✅
+11.95%
+181.69%
-9.64%
-19.46%
WrappedMToken balanceOf +38 ❌ +6.39%

Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
WrappedMTokenHarness 5,156,895 (+302,300) accruedYieldOf
balanceOf
balanceWithYieldOf
claimExcess
claimFor
currentIndex
decimals
disableEarning
enableEarning
excess
isEarning
lastIndexOf
mToken
migrationAdmin
registrar
setAccountOf(address,uint256)
setLastIndexOf
setPrincipalOfTotalEarningSupply
setTotalEarningSupply
setTotalNonEarningSupply
startEarningFor(address)
startEarningFor(address[])
stopEarningFor(address)
stopEarningFor(address[])
symbol
totalEarningSupply
totalNonEarningSupply
totalSupply
transfer
unwrap(address)
unwrap(address,uint256)
wrap(address)
wrap(address,uint256)
675 (+28)
634 (-27)
1,008 (+6)
42,993 (-44)
5,164 (-6)
701 (+22)
315 (+22)
8,685 (-80)
8,704 (-14)
2,128 (+22)
649 (+16)
637 (+16)
315 (+22)
315 (+22)
294 (-44)
5,176 (+50)
22,606 (+16)
2,583 (+22)
2,561 (+45)
2,538 (-23)
2,673 (+16)
2,778 (+22)
8,607 (-2,727)
14,926 (-3,395)
2,724 (+22)
436 (+22)
436 (+22)
643 (+22)
3,058 (+16)
3,114 (-38)
591 (-71)
11,705 (-6)
484 (+16)
+4.33%
-4.08%
+0.60%
-0.10%
-0.12%
+3.24%
+7.51%
-0.91%
-0.16%
+1.04%
+2.53%
+2.58%
+7.51%
+7.51%
-13.02%
+0.98%
+0.07%
+0.86%
+1.79%
-0.90%
+0.60%
+0.80%
-24.06%
-18.53%
+0.81%
+5.31%
+5.31%
+3.54%
+0.53%
-1.21%
-10.73%
-0.05%
+3.42%
1,401 (+23)
740 (+79)
1,777 (0)
42,993 (-44)
88,925 (+57,357)
1,311 (+22)
315 (+22)
22,045 (-83)
82,138 (-16)
4,628 (+22)
649 (+16)
637 (+16)
315 (+22)
315 (+22)
294 (-44)
24,603 (+50)
22,606 (+16)
15,391 (+105)
21,431 (-13)
20,812 (-23)
78,058 (-2,647)
63,311 (-2,223)
63,961 (-361)
31,915 (-3,403)
2,724 (+22)
926 (+92)
471 (+22)
960 (-232)
24,032 (-14)
53,152 (+7)
38,376 (+1,232)
51,273 (+26)
63,850 (+27)
+1.67%
+11.95%
0.00%
-0.10%
+181.69%
+1.71%
+7.51%
-0.38%
-0.02%
+0.48%
+2.53%
+2.58%
+7.51%
+7.51%
-13.02%
+0.20%
+0.07%
+0.69%
-0.06%
-0.11%
-3.28%
-3.39%
-0.56%
-9.64%
+0.81%
+11.03%
+4.90%
-19.46%
-0.06%
+0.01%
+3.32%
+0.05%
+0.04%
675 (+28)
634 (-27)
1,008 (+6)
42,993 (-44)
90,837 (+57,489)
1,117 (+22)
315 (+22)
30,967 (-84)
82,353 (-18)
4,628 (+22)
649 (+16)
637 (+16)
315 (+22)
315 (+22)
294 (-44)
25,076 (+50)
22,606 (+16)
22,483 (+22)
22,461 (+45)
22,438 (-23)
66,346 (-2,660)
70,233 (-3,355)
65,736 (-59)
31,915 (-3,403)
2,724 (+22)
436 (+22)
436 (+22)
643 (+22)
16,182 (+41)
54,686 (-38)
51,285 (-71)
46,734 (-6)
62,948 (+16)
+4.33%
-4.08%
+0.60%
-0.10%
+172.39%
+2.01%
+7.51%
-0.27%
-0.02%
+0.48%
+2.53%
+2.58%
+7.51%
+7.51%
-13.02%
+0.20%
+0.07%
+0.10%
+0.20%
-0.10%
-3.85%
-4.56%
-0.09%
-9.64%
+0.81%
+5.31%
+5.31%
+3.54%
+0.25%
-0.07%
-0.14%
-0.01%
+0.03%
2,675 (+28)
2,634 (+1,973)
2,646 (-6)
42,993 (-44)
134,878 (+84,430)
2,701 (+22)
315 (+22)
30,967 (-84)
82,353 (-18)
7,129 (+22)
649 (+16)
637 (+16)
315 (+22)
315 (+22)
294 (-44)
25,076 (+50)
22,606 (+16)
22,483 (+22)
22,461 (+45)
22,438 (-23)
91,846 (-2,660)
116,924 (-3,336)
65,736 (-59)
48,905 (-3,410)
2,724 (+22)
2,436 (+22)
2,436 (+22)
2,643 (+22)
77,117 (+41)
78,440 (+127)
76,735 (+106)
80,934 (-6)
96,433 (+16)
+1.06%
+298.49%
-0.23%
-0.10%
+167.36%
+0.82%
+7.51%
-0.27%
-0.02%
+0.31%
+2.53%
+2.58%
+7.51%
+7.51%
-13.02%
+0.20%
+0.07%
+0.10%
+0.20%
-0.10%
-2.81%
-2.77%
-0.09%
-6.52%
+0.81%
+0.91%
+0.91%
+0.84%
+0.05%
+0.16%
+0.14%
-0.01%
+0.02%
1,201 (0)
1,565 (+219)
300 (0)
100 (0)
106 (+4)
6 (0)
1 (0)
7 (0)
675 (+4)
200 (0)
202 (0)
111 (0)
1 (0)
1 (0)
1 (0)
782 (0)
1 (0)
239 (0)
343 (0)
778 (0)
201 (0)
3 (0)
102 (0)
2 (0)
1 (0)
587 (-98)
857 (+1)
107 (+100)
210 (0)
299 (0)
107 (0)
100 (0)
306 (0)
WrappedMToken 4,797,853 (+284,653) accruedYieldOf
balanceOf
claimExcess
claimFor
enableEarning
excess
migrate()
migrate(address)
startEarningFor
stopEarningFor
totalEarningSupply
transfer
unwrap(address)
unwrap(address,uint256)
wrap
630 (+6)
633 (+38)
19,886 (+57)
33,475 (+127)
82,336 (+17)
2,717 (0)
14,943 (+22)
9,629 (+16)
57,605 (-2,672)
51,910 (-4)
369 (-67)
52,834 (+218)
54,720 (+54)
54,177 (+64)
41,568 (+51)
+0.96%
+6.39%
+0.29%
+0.38%
+0.02%
0.00%
+0.15%
+0.17%
-4.43%
-0.01%
-15.37%
+0.41%
+0.10%
+0.12%
+0.12%
1,508 (0)
633 (+38)
32,885 (+75)
33,475 (+127)
82,336 (+17)
2,805 (+2)
14,943 (+22)
9,629 (+16)
64,565 (-2,672)
51,910 (-4)
369 (-67)
59,748 (+159)
54,720 (+54)
65,344 (+153)
42,466 (+51)
0.00%
+6.39%
+0.23%
+0.38%
+0.02%
+0.07%
+0.15%
+0.17%
-3.97%
-0.01%
-15.37%
+0.27%
+0.10%
+0.23%
+0.12%
2,268 (-6)
633 (+38)
32,885 (+75)
33,475 (+127)
82,336 (+17)
2,717 (0)
14,943 (+22)
9,629 (+16)
66,305 (-2,672)
51,910 (-4)
369 (-67)
53,097 (+218)
54,720 (+54)
65,226 (+152)
41,568 (+51)
-0.26%
+6.39%
+0.23%
+0.38%
+0.02%
0.00%
+0.15%
+0.17%
-3.87%
-0.01%
-15.37%
+0.41%
+0.10%
+0.23%
+0.12%
2,285 (-6)
633 (+38)
45,884 (+92)
33,475 (+127)
82,336 (+17)
4,015 (+35)
14,943 (+22)
9,629 (+16)
66,305 (-2,672)
51,910 (-4)
369 (-67)
73,315 (+41)
54,720 (+54)
76,747 (+241)
92,868 (+51)
-0.26%
+6.39%
+0.20%
+0.38%
+0.02%
+0.88%
+0.15%
+0.17%
-3.87%
-0.01%
-15.37%
+0.06%
+0.10%
+0.32%
+0.05%
30 (0)
232 (0)
2 (0)
1 (0)
3 (0)
219 (0)
1 (0)
1 (0)
5 (0)
1 (0)
18 (0)
3 (0)
1 (0)
4 (0)
204 (0)
MockRegistrar 180,239 (0) get
listContains
set
setListContains
405 (0)
636 (-2,000)
44,020 (-360)
22,512 (0)
0.00%
-75.87%
-0.81%
0.00%
2,361 (-37)
2,571 (-65)
44,319 (-61)
44,313 (0)
-1.54%
-2.47%
-0.14%
0.00%
2,405 (0)
2,636 (0)
44,392 (+12)
44,424 (0)
0.00%
0.00%
+0.03%
0.00%
2,405 (0)
2,636 (0)
44,392 (+12)
44,448 (+24)
0.00%
0.00%
+0.03%
+0.05%
1,052 (-255)
780 (-217)
67 (+66)
782 (-211)
MockM 312,057 (0) currentIndex
setBalanceOf
setCurrentIndex
356 (0)
24,203 (0)
23,723 (0)
0.00%
0.00%
0.00%
1,559 (+2)
43,930 (+1)
26,587 (+37)
+0.13%
+0.00%
+0.14%
2,356 (0)
44,211 (0)
26,547 (0)
0.00%
0.00%
0.00%
2,356 (0)
44,463 (0)
43,623 (0)
0.00%
0.00%
0.00%
2,978 (+11)
716 (0)
1,549 (+10)

Copy link

github-actions bot commented Oct 23, 2024

LCOV of commit 596cd79 during Forge Coverage #396

Summary coverage rate:
  lines......: 98.1% (259 of 264 lines)
  functions..: 95.9% (70 of 73 functions)
  branches...: 83.9% (47 of 56 branches)

Files changed coverage rate:
                                 |Lines       |Functions  |Branches    
  Filename                       |Rate     Num|Rate    Num|Rate     Num
  =====================================================================
  src/EarnerManager.sol          | 100%     63| 100%    15|95.0%     20
  src/WrappedMToken.sol          |98.8%    169|95.7%    47|90.0%     30
  src/WrappedMTokenMigratorV1.sol| 0.0%      3| 0.0%     1|    -      0

@deluca-mike deluca-mike force-pushed the feat/segregated-funds branch from 8897271 to 17e958f Compare October 23, 2024 19:59
src/EarnerManager.sol Outdated Show resolved Hide resolved
src/EarnerManager.sol Outdated Show resolved Hide resolved
src/EarnerManager.sol Outdated Show resolved Hide resolved
src/EarnerManager.sol Outdated Show resolved Hide resolved
src/WrappedMToken.sol Outdated Show resolved Hide resolved
src/WrappedMToken.sol Outdated Show resolved Hide resolved
src/WrappedMToken.sol Outdated Show resolved Hide resolved
src/WrappedMToken.sol Outdated Show resolved Hide resolved
script/DeployBase.sol Outdated Show resolved Hide resolved
script/DeployBase.sol Outdated Show resolved Hide resolved
src/EarnerManager.sol Outdated Show resolved Hide resolved
src/interfaces/IEarnerManager.sol Outdated Show resolved Hide resolved
src/interfaces/IEarnerManager.sol Outdated Show resolved Hide resolved
src/interfaces/IEarnerManager.sol Outdated Show resolved Hide resolved
src/interfaces/IEarnerManager.sol Outdated Show resolved Hide resolved
src/EarnerManager.sol Outdated Show resolved Hide resolved
src/EarnerManager.sol Outdated Show resolved Hide resolved
@deluca-mike deluca-mike force-pushed the feat/segregated-funds branch from ab3194a to e22f3fb Compare October 25, 2024 20:41
Copy link
Contributor

@toninorair toninorair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!
One remaining logical puzzle to think through (left comments):

  • situation when address was admin owner first, but later was also added to TTG is not clear.
  • admin won't be able to release this account and will get % of fee

@deluca-mike
Copy link
Collaborator Author

deluca-mike commented Oct 29, 2024

  • admin won't be able to release this account and will get % of fee

An admin will not get a fee for an account added to the registrar because the earner manager returns a zero fee rate and a zero admin for any account that's already in the registrar, regardless if it was originally added via an admin.

src/WrappedMToken.sol Outdated Show resolved Hide resolved
@deluca-mike deluca-mike force-pushed the feat/segregated-funds branch from 91e7f83 to 596cd79 Compare October 30, 2024 20:05
@deluca-mike deluca-mike merged commit 7e3edd3 into main Oct 30, 2024
4 checks passed
@deluca-mike deluca-mike deleted the feat/segregated-funds branch October 30, 2024 20:20
deluca-mike added a commit that referenced this pull request Jan 4, 2025
* feat: Upgradable Earner Manager
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants