Named Static Cached PCVDeposit #379
Conversation
Co-authored-by: Joey <31974730+Joeysantoro@users.noreply.github.com>
Co-authored-by: Joey <31974730+Joeysantoro@users.noreply.github.com>
Co-authored-by: Joey <31974730+Joeysantoro@users.noreply.github.com>
Co-authored-by: Joey <31974730+Joeysantoro@users.noreply.github.com>
…protocol/fei-protocol-core into feat/named-static-cached-pcvdeposit
There was a problem hiding this comment.
Three main things:
-
see my idea for a uint256-type UUID for the set; if we implement this, we'll need to also ensure that the hash-params of the pcv deposit are immutable; alternatively we can just use a random UUID (but I do like the deterministic approach)
-
error messages should be readable in natural-language-style and understandable as to what exactly they check against
-
make sure that array index parameters are checked for out-of-bounds issues
…protocol/fei-protocol-core into feat/named-static-cached-pcvdeposit
…protocol/fei-protocol-core into feat/named-static-cached-pcvdeposit
…atic-cached-pcvdeposit-it
| balance = balance - updatePCVDeposit.usdAmount + usdAmount; | ||
| feiReportBalance = feiReportBalance - updatePCVDeposit.feiAmount + feiAmount; |
There was a problem hiding this comment.
nit: can store the intermediate sums in a local var to reuse
There was a problem hiding this comment.
We only use this value once so no need to store intermediately.
There was a problem hiding this comment.
moved to a cached system to save 2 SLOADs when emitting event
| // ----------- Helper methods to change state ----------- | ||
|
|
||
| /// @notice helper method to add a PCV deposit | ||
| function _addDeposit(DepositInfo memory newPCVDeposit) internal { |
There was a problem hiding this comment.
Should we store a mapping of names -> DepositInfo for lookups? Can also be used to ensure that no duplicate names are added to @ditchfieldcaleb 's earlier concerns
There was a problem hiding this comment.
We shouldn't need to think adversarially about deposit names as only OA and the governor can update the list and guardian can remove deposits instantly.
If we were to do this feature, the data structure would be name -> DepositInfo Index so that we aren't storing each struct and it's data twice.
…vdeposit-it Named Static Cached PCVDeposit IT
| uint256 newBalance = balance - updatePCVDeposit.usdAmount + usdAmount; | ||
| uint256 newFeiReportBalance = feiReportBalance - updatePCVDeposit.feiAmount + feiAmount; |
There was a problem hiding this comment.
Use oldBalance and oldFeiBalance to save another sload
…protocol/fei-protocol-core into feat/named-static-cached-pcvdeposit
…protocol/fei-protocol-core into feat/named-static-cached-pcvdeposit
No description provided.