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.
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
VIP-14 Morpho PCV Deposit #128
VIP-14 Morpho PCV Deposit #128
Changes from 6 commits
5ba0def
344cbc0
95368fe
85e18d2
fc92e6b
86ea05e
43ac3fb
c9ff1b1
38f7cdb
cf6c21a
c0ddda9
96c3b93
09a11e5
1b11bda
3a927a5
288fb42
6323a8e
f86be20
d72ddea
175b45c
02453fa
64177bb
41b1496
513d872
c231877
72101fa
979ce31
376e7d4
b3a1843
101415e
729fc01
64a50dd
4c0e890
6619e94
21d0bf7
1ce3129
248a693
97b9255
3358ab0
c56434f
649ec46
6de5056
e83c195
6ca9cc5
4999583
d2a78e0
cb8ccd0
1c06df3
79a6981
91a817b
8f6b3d6
397642b
28a501a
a2fcf0a
49e18cd
5a03f63
1a6b664
a71e3b3
bc52fa4
a96025a
2073f25
024edd3
3056222
457613d
42d92da
26be8a2
81cba10
0a90a64
ab9c93e
1fc86aa
72a4567
c8ca5d7
414b443
4d59814
52f3ef6
d1e2632
513e3a9
9c14d09
072b88d
a1e9b2f
8519129
d7c9f6c
ab72cd2
6cd4317
d2c285e
7fa0d40
0101e8d
f867262
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not read the state in
lastRecordedBalance
? it's freshly updated with _recordPNL() etcThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lastRecordedBalance is off by a hair because on deposit, you take a loss when compound rounds down in favor of their protocol, so you have to read balance instead to get the correct amount of balance in the system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This piece of code is written 4 times and could have been in an internal function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could refactor this into a helper function, but it's too late in the game to change code before a deployment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a withdraw all, you also can pass
type(uint256).max
as argument to avoid leaving dust on Morpho but as you're calling_recordPNL
just before it must be exactly the same.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing uint256 max to the withdraw function would revert as lastRecordedBalance has amount subtracted from it on line 309. This would cause an underflow and revert.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can save an sload here by using
_pcvOracle
instead