Market Reserve Automation #456

ElliotFriedman · 2025-01-09T08:35:01Z

No description provided.

Signed-off-by: Elliot <[email protected]>

…nto feat/market-reserve-automation

Signed-off-by: Elliot <[email protected]>

openzeppelin-code · 2025-01-09T08:59:58Z

Market Reserve Automation

Generated at commit: 40f3d9402eb9afad194666b6143422c905b352de

🚨 Report Summary

	Severity Level	Results
Contracts	Critical High Medium Low Note Total	3 5 0 12 45 65
Dependencies	Critical High Medium Low Note Total	0 0 0 0 0 0

For more details view the full report in OpenZeppelin Code Inspector

src/market/ReserveAutomation.sol

Signed-off-by: Elliot <[email protected]>

…om deployment scripts Signed-off-by: Elliot <[email protected]>

Signed-off-by: Elliot <[email protected]>

…ng and sale periods

Signed-off-by: Elliot <[email protected]>

… cycles and making purchases in each Signed-off-by: Elliot <[email protected]>

Signed-off-by: Elliot <[email protected]>

…nto feat/market-reserve-automation

Signed-off-by: Elliot <[email protected]>

anajuliabit · 2025-01-21T14:26:53Z

proposals/mips/mip-reserve-automation/reserveAutomationDeploy.sol

+                .InitParams({
+                    recipientAddress: holdingDeposit,
+                    wellToken: xWellProxy,
+                    reserveAsset: _addresses.getAddress(underlyingName),


This can be replaced with mToken.getUnderlying(), making the file more generic so we can reuse it for future deployments to other chains

sure those changes can be made, but Halborn won't be auditing the deployment script anyway so it's fine to leave as is for the Base deployment.

anajuliabit · 2025-01-21T14:27:06Z

proposals/mips/mip-reserve-automation/reserveAutomationDeploy.sol

based on the current repo folder structure I believe it's makes more sense to keep this file inside /script and rename to DeployReserveAutomation

chainink oracle, market and underlying address names could be placed in a json config file so we could easily deploy the reserve automation to any chain

anajuliabit · 2025-01-21T14:30:20Z

proposals/mips/mip-reserve-automation/reserveAutomationDeploy.sol

+
+    /// @notice array of mToken names to deploy automation for
+    function _getMTokens() internal pure returns (string[] memory) {
+        string[] memory tokens = new string[](12);


I would prefer to make the token list an argument for the deploy and validate functions, so we can reuse it to deploy on other chains

we should differentiate between what is a nice to have vs what is a must have for this audit. since they won't be reviewing deployment scripts this should be non blocking.

src/market/AutomationDeploy.sol

src/market/ERC20Mover.sol

src/market/ReserveAutomation.sol

Signed-off-by: Elliot <[email protected]>

src/market/ReserveAutomation.sol

anajuliabit · 2025-01-21T17:50:11Z

src/market/ReserveAutomation.sol

+        }
+
+        /// normalize decimals up to 18 if reserve asset has less than 18 decimals
+        if (reserveAssetDecimals != 18) {


as we have checks to make sure this is never > 18 we could do < instead

Suggested change

if (reserveAssetDecimals != 18) {

if (reserveAssetDecimals < 18) {

are you going to remediate this?

src/market/ReserveAutomation.sol

anajuliabit · 2025-01-21T18:06:35Z

src/market/ReserveAutomation.sol

+
+        /// if the reserve asset has non 18 decimals, shrink down the amount of
+        /// reserve asset received to the actual amount
+        if (reserveAssetDecimals != 18) {


as we have checks to make sure this is never > 18 we could do < instead

Suggested change

if (reserveAssetDecimals != 18) {

if (reserveAssetDecimals < 18) {

are you going to remediate this?

src/market/ReserveAutomation.sol

anajuliabit · 2025-01-21T18:08:37Z

src/market/ReserveAutomation.sol

+            ,
+            uint80 answeredInRound
+        ) = AggregatorV3Interface(oracleAddress).latestRoundData();
+        bool valid = price > 0 && answeredInRound >= roundId;


maybe also checks that updatedAt != 0

I think Gautlet is going to report this if we don't add the check

src/market/ReserveAutomation.sol

anajuliabit · 2025-01-21T18:18:34Z

src/market/ReserveAutomation.sol

+        if (
+            startPeriodTimestampCachedChainlinkPrice[startTime].wellPrice == 0
+        ) {
+            (int256 wellPrice, ) = getPriceAndDecimals(wellChainlinkFeed);
+            startPeriodTimestampCachedChainlinkPrice[startTime]
+                .wellPrice = wellPrice;
+
+            (int256 reservePrice, ) = getPriceAndDecimals(reserveChainlinkFeed);
+            startPeriodTimestampCachedChainlinkPrice[startTime]
+                .reservePrice = reservePrice;
+        }
+


if we move this above line 534 amountOut = getAmountReservesOut(amountWellIn); we don't need to call (int256 price, uint8 decimals) = getPriceAndDecimals(oracleAddress); in the getNormalizedFunction as we will be sure there is a cached price

nice optimization

add MWethDelegatorOwner

Signed-off-by: Elliot <[email protected]>

github-actions · 2025-01-25T20:02:02Z

Compiling 4 files with Solc 0.8.19
Solc 0.8.19 finished in 8.49ms
Compiler run successful!
Script ran successfully.
Gas used: 35208166

If you wish to simulate on-chain transactions pass a RPC URL.

anajuliabit · 2025-01-27T11:49:10Z

src/market/ReserveAutomation.sol

+        address oracleAddress,
+        int256 cachedPrice
+    ) internal view returns (uint256 normalizedPrice) {
+        (int256 price, uint8 decimals) = getPriceAndDecimals(oracleAddress);


Suggested change

(int256 price, uint8 decimals) = getPriceAndDecimals(oracleAddress);

anajuliabit · 2025-01-27T11:49:36Z

src/market/ReserveAutomation.sol

+        }
+
+        /// normalize decimals up to 18 if reserve asset has less than 18 decimals
+        if (reserveAssetDecimals != 18) {


are you going to remediate this?

anajuliabit · 2025-01-27T11:49:42Z

src/market/ReserveAutomation.sol

+
+        /// if the reserve asset has non 18 decimals, shrink down the amount of
+        /// reserve asset received to the actual amount
+        if (reserveAssetDecimals != 18) {


are you going to remediate this?

ElliotFriedman added 5 commits January 8, 2025 09:53

init: market automation

8231339

Signed-off-by: Elliot <[email protected]>

remove: reserve registry

25df713

Signed-off-by: Elliot <[email protected]>

add: auction delay and guardian

b19b3ac

Signed-off-by: Elliot <[email protected]>

test: auction delay and guardian

666b48d

Signed-off-by: Elliot <[email protected]>

remove unneeded check from constructor

8311c90

Signed-off-by: Elliot <[email protected]>

ElliotFriedman requested a review from anajuliabit January 9, 2025 08:35

ElliotFriedman added 2 commits January 9, 2025 00:35

Merge branch 'main' of github.com:moonwell-fi/moonwell-contracts-v2 i…

8abac49

…nto feat/market-reserve-automation

remove unused import

485cf8a

Signed-off-by: Elliot <[email protected]>

anajuliabit requested changes Jan 9, 2025

View reviewed changes

ElliotFriedman added 5 commits January 9, 2025 11:12

end -> start time natspec

f241b98

Signed-off-by: Elliot <[email protected]>

naming: clarify variable and function names

148d6e2

Signed-off-by: Elliot <[email protected]>

add: erc20 holding deposit integration test

b3b4db0

Signed-off-by: Elliot <[email protected]>

add: purchase failure tests + erc20 holding deposit tests

7671520

Signed-off-by: Elliot <[email protected]>

add: reserve automation integration tests

27bbc5d

Signed-off-by: Elliot <[email protected]>

ElliotFriedman requested a review from anajuliabit January 9, 2025 22:50

ElliotFriedman added 12 commits January 10, 2025 00:18

add: reserve automation deploy script

162c355

Signed-off-by: Elliot <[email protected]>

refactor init params

bf2fd20

Signed-off-by: Elliot <[email protected]>

refactor constructor params, add additional checks on deployment

6e33739

Signed-off-by: Elliot <[email protected]>

refactor: make validate and deploy function external to ease testing

310fb6b

Signed-off-by: Elliot <[email protected]>

make setup virtual for test

7498cdb

Signed-off-by: Elliot <[email protected]>

add tests for newly deployed reserve automation contracts directly fr…

fd39fb2

…om deployment scripts Signed-off-by: Elliot <[email protected]>

fix: change buffer updated time based on amount of buffer taken

e51aed2

Signed-off-by: Elliot <[email protected]>

add new integration test

9ca8c21

Signed-off-by: Elliot <[email protected]>

give mock erc20 token functionality

408375d

Signed-off-by: Elliot <[email protected]>

fix error message

56be0b6

Signed-off-by: Elliot <[email protected]>

fix test issues

b1f3034

Signed-off-by: Elliot <[email protected]>

increment timestamp to fix accrue interest error

6ecb3f6

Signed-off-by: Elliot <[email protected]>

ElliotFriedman marked this pull request as ready for review January 11, 2025 03:17

increase wait time accruing interest

0f56033

Signed-off-by: Elliot <[email protected]>

ElliotFriedman added 18 commits January 14, 2025 09:16

add new tests, make testSetup view

3f2bf19

Signed-off-by: Elliot <[email protected]>

remove sale start time check, allow guardian to cancel many times

06bdea2

Signed-off-by: Elliot <[email protected]>

checkpoint: add premium and discount logic + other changes like cachi…

0105662

…ng and sale periods

add _addReserves function

8ed09c0

add fuzz test around discount application throughout period

4dc46f4

fix discount, and start and end timestamps to be inclusive

de5f26b

fmt

376098d

fix failing tests

61d64b2

uncomment code, fix deployment

baa073f

Signed-off-by: Elliot <[email protected]>

add period start and end tests as well as chainlink price caching

671026c

Signed-off-by: Elliot <[email protected]>

check mToken and reserve asset match

635955c

Signed-off-by: Elliot <[email protected]>

organize import

23d4861

Signed-off-by: Elliot <[email protected]>

cancel auction sends all funds back to mToken by adding reserves

f85ea52

Signed-off-by: Elliot <[email protected]>

add tests around start and end time bounds, as well as moving through…

0eb75db

… cycles and making purchases in each Signed-off-by: Elliot <[email protected]>

add price caching test

81807c0

Signed-off-by: Elliot <[email protected]>

Merge branch 'main' of github.com:moonwell-fi/moonwell-contracts-v2 i…

6865e3c

…nto feat/market-reserve-automation

fix underlying mismatch

0e0e65d

Signed-off-by: Elliot <[email protected]>

add MWethDelegatorOwner

f5c8f84

Signed-off-by: Elliot <[email protected]>

anajuliabit requested changes Jan 21, 2025

View reviewed changes

anajuliabit reviewed Jan 21, 2025

View reviewed changes

src/market/ReserveAutomation.sol Show resolved Hide resolved

address feedback, make internal function private, name mapping variables

693779e

Signed-off-by: Elliot <[email protected]>

anajuliabit reviewed Jan 21, 2025

View reviewed changes

ElliotFriedman and others added 2 commits January 21, 2025 21:07

Merge pull request #466 from moonwell-fi/feat/mweth-owner

2057280

add MWethDelegatorOwner

address pr feedback

40f3d94

Signed-off-by: Elliot <[email protected]>

anajuliabit requested changes Jan 27, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Market Reserve Automation #456

Market Reserve Automation #456

ElliotFriedman commented Jan 9, 2025

openzeppelin-code bot commented Jan 9, 2025 •

edited

Loading

anajuliabit Jan 21, 2025

ElliotFriedman Jan 25, 2025

anajuliabit Jan 21, 2025

anajuliabit Jan 21, 2025

anajuliabit Jan 21, 2025

ElliotFriedman Jan 25, 2025

anajuliabit Jan 21, 2025

anajuliabit Jan 27, 2025

anajuliabit Jan 21, 2025

anajuliabit Jan 27, 2025

anajuliabit Jan 21, 2025

anajuliabit Jan 27, 2025

anajuliabit Jan 21, 2025

ElliotFriedman Jan 25, 2025

github-actions bot commented Jan 25, 2025

anajuliabit Jan 27, 2025

anajuliabit Jan 27, 2025

anajuliabit Jan 27, 2025

	if (reserveAssetDecimals != 18) {
	if (reserveAssetDecimals < 18) {

Market Reserve Automation #456

Are you sure you want to change the base?

Market Reserve Automation #456

Conversation

ElliotFriedman commented Jan 9, 2025

openzeppelin-code bot commented Jan 9, 2025 • edited Loading

Market Reserve Automation

🚨 Report Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 25, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openzeppelin-code bot commented Jan 9, 2025 •

edited

Loading