add october 17th, 2024 spell and tests #55

0xShaito · 2024-10-09T10:55:03Z

[Mainnet] Onboard sUSDS into SparkLend as collateral-only
[Mainnet] Update sDAI oracle to use fixed DAI/USD at $1
[Mainnet] Onboard PT-sUSDe-26Dec2024 and PT-sUSDe-27Mar2025 to Morpho Spark DAI vault
[Mainnet] Delayed WBTC Changes

https://forum.sky.money/t/oct-3-2024-proposed-changes-to-spark-for-upcoming-spell/25293

github-actions · 2024-10-09T10:58:18Z

Spell Caster

Inspect the impact of spell execution on forked networks:

Spell	App URL	RPC URL
SparkEthereum_20241017	🎇 App	🌎 RPC

_{Deployed from b3ecb90 on 2024-10-14T15:41:30.370Z}

0xShaito · 2024-10-09T11:28:24Z

.github/workflows/test.yml

@@ -40,7 +40,7 @@ jobs:
          BASE_RPC_URL: ${{secrets.BASE_RPC_URL}}
          GOERLI_RPC_URL: ${{secrets.GOERLI_RPC_URL}}
        run: |
-          forge test -vvv
+          forge test -vvv --evm-version cancun


This flag was added based on this open issue in foundry.

We can remove it but we would need to remove the tests for the price of the pt token oracles.

lucas-manuel

Did initial review:

URLs LGTM
Addresses good except PT price feeds, still need to figure out where those came from
sUSDS onboarding LGTM (besides seeding)
sDAI oracle update LGTM
WBTC changes LGTM
First pass review of PTs (need to revisit)
First pass review of tests

src/proposals/20241017/SparkEthereum_20241017.t.sol

src/proposals/20241017/SparkEthereum_20241017.sol

0xteddybear

spell

couldn't understand the pendle price oracle setup, would like to do so before approving

the rest of the spell looks good to me, see checklist below (feedback of course welcome on if I'm missing any item)

tests

I only performed a superficial review of the tests, will give them another look tomorrow!

checklist

src/proposals/20241017/SparkEthereum_20241017.sol

0xteddybear · 2024-10-10T00:45:59Z

src/proposals/20241017/SparkEthereum_20241017.sol

+
+    address internal constant SDAI_PRICE_FEED = 0x0c0864837C7e65458aCD3C665222203217019436;
+
+    address internal constant PT_26DEC2024_PRICE_FEED = 0x81E5E28F33D314e9211885d6f0F4080E755e4595;


I see this oracle mocks a chainlink pricefeed which composeds with the newly-deployed pricefeed cited in the post

I verified the latter has a correct implementation & deploy params, however I don't understand what purpose it serves and can't find where Pendle published them 👉 👈

I looked deeper into this and it looks like the MorphoChainlinkOracleV2 is only scaling the return of the PendleSparkLinearDiscountOracle up to 36 decimals.

This strikes me as sub-optimal, since we could be either

using the oracle composition to consider both the discounted price of the PT and the market usd value of the collateral (although iirc risk already mentioned we are okay with assuming USDe value to always be 1)

using the PendleSparkLinearDiscountOracle directly, if it returned 36 decimals, and save an external call

this is a nitpick and should't block the PR, however I'd be glad to be corrected if I misunderstood any of the above 😁

src/proposals/20241017/SparkEthereum_20241017.sol

src/proposals/20241017/SparkEthereum_20241017.t.sol

0xteddybear

changeset looks good to me (checklist below)

only actionable (albeit minor and non-blocking) items are

set the evm version in foundry.toml to have consistency between ci and local test execution if we dont revert the changes to ci in the future
extra assertion on oracle prices after maturity to remain at 1

checklist:

0xteddybear · 2024-10-10T21:19:19Z

src/proposals/20241017/SparkEthereum_20241017.t.sol

+        assertEq(IMorphoChainlinkOracle(PT_26DEC2024_PRICE_FEED).price(), 1e36);
+
+        vm.warp(IMorphoPT(PT_SUSDE_27MAR2025).expiry());
+        assertEq(IMorphoChainlinkOracle(PT_27MAR2025_PRICE_FEED).price(), 1e36);


non-blocking, but a test ensuring that prices remain at one past maturity can't hurt

0xteddybear · 2024-10-10T23:51:35Z

src/proposals/20241017/SparkEthereum_20241017.sol

+
+    address internal constant SDAI_PRICE_FEED = 0x0c0864837C7e65458aCD3C665222203217019436;
+
+    address internal constant PT_26DEC2024_PRICE_FEED = 0x81E5E28F33D314e9211885d6f0F4080E755e4595;


I looked deeper into this and it looks like the MorphoChainlinkOracleV2 is only scaling the return of the PendleSparkLinearDiscountOracle up to 36 decimals.

This strikes me as sub-optimal, since we could be either

using the oracle composition to consider both the discounted price of the PT and the market usd value of the collateral (although iirc risk already mentioned we are okay with assuming USDe value to always be 1)

using the PendleSparkLinearDiscountOracle directly, if it returned 36 decimals, and save an external call

this is a nitpick and should't block the PR, however I'd be glad to be corrected if I misunderstood any of the above 😁

RedLikeRosesss

All questions were answered and the report is attached below.
17 Oct 2024 - Before Deployment.pdf

lucas-manuel · 2024-10-14T15:30:11Z

src/proposals/20241017/SparkEthereum_20241017.t.sol

+        // assertEq(ptUsde26DecPrice, 0.969105874238964993e36);
+        // assertEq(ptUsde27MarPrice, 0.908944818619989853e36);


Update to (commented out)

assertEq(ptUsde26DecPrice, 0.970544002092846271e36); assertEq(ptUsde27MarPrice, 0.910862322425164891e36);

And add a note // NOTE: Can run this test individually after commenting out these assertions with forge t --mt testMorphoVaults -vvv --evm-version cancun

0xteddybear

verified compiled smart contract is the same, with bytecode differences caused by the change in compiler version only

RedLikeRosesss

17 Oct 2024 - After Deployment.pdf
LGTM

0xteddybear · 2024-10-14T18:49:59Z

after looking into it in a bit more detail, I believe we should actually be forcing the evm version to cancun, since it's the spec under which both gnosis and mainnet will run the spells. If foundry chooses to use a different hardfork, it's actually reducing the usefulness of our tests. this also has the benefit of allowing all of the tests defined herein to pass, and a smaller spell bytecode

I propose we set evmVersion = 'cancun' in foundry.toml and set the payload back to 0x74b3D0E74f2711f30442536832D7fBCB0F42C195

I still can't explain why foundry is choosing to use anything other than cancun when not forced to, since solc version is pinned to 0.8.25, for which the default hardfork is cancun, and as reported in the issue linked in the diff: foundry-rs/foundry#6228 (comment) , foundry's execution environment defaults to the hardfork the contracts were compiled for.

since the spells are equivalent, this does not block execution and could be implemented in a subsequent PR if the timeline requires it

0xShaito · 2024-10-15T07:41:47Z

@0xteddybear even though I think that at some point we will be using cancun by default, the execution team doesn't want to make the change now and they want to use when it's fully default and supported for everything

0xShaito added 2 commits October 9, 2024 12:42

add october 17th, 2024 spell and tests

d933907

code linting

b21c923

code ordering

d8c0b81

0xShaito commented Oct 9, 2024

View reviewed changes

cleanup of unused code

e7ac840

0xShaito requested review from lucas-manuel, RedLikeRosesss and 0xteddybear October 9, 2024 11:46

0xShaito added the Priority: Medium label Oct 9, 2024

0xShaito self-assigned this Oct 9, 2024

0xShaito added Priority: High Status: Ready for Review and removed Priority: Medium labels Oct 9, 2024

lucas-manuel requested changes Oct 9, 2024

View reviewed changes

src/proposals/20241017/SparkEthereum_20241017.t.sol Outdated Show resolved Hide resolved

src/proposals/20241017/SparkEthereum_20241017.t.sol Outdated Show resolved Hide resolved

src/proposals/20241017/SparkEthereum_20241017.sol Outdated Show resolved Hide resolved

lucas-manuel added Status: Revisions Needed and removed Status: Ready for Review labels Oct 9, 2024

0xteddybear reviewed Oct 10, 2024

View reviewed changes

0xShaito added 2 commits October 10, 2024 09:02

remove susds transfer

b21cb89

update diff file

ed88f93

0xShaito added Status: Ready for Review and removed Status: Revisions Needed labels Oct 10, 2024

0xShaito added 2 commits October 10, 2024 21:41

test price increase for pendle token

2fc2bd8

deploy new spell with cancun flag

c26fa0f

lucas-manuel reviewed Oct 10, 2024

View reviewed changes

src/proposals/20241017/SparkEthereum_20241017.t.sol Outdated Show resolved Hide resolved

0xteddybear previously approved these changes Oct 11, 2024

View reviewed changes

price on maturity stays at 1

e0a8433

0xShaito dismissed 0xteddybear’s stale review via e0a8433 October 11, 2024 06:24

lucas-manuel previously approved these changes Oct 11, 2024

View reviewed changes

RedLikeRosesss previously approved these changes Oct 11, 2024

View reviewed changes

0xteddybear previously approved these changes Oct 11, 2024

View reviewed changes

evm version paris deployment

b0b0315

0xShaito dismissed stale reviews from 0xteddybear, RedLikeRosesss, and lucas-manuel via b0b0315 October 14, 2024 07:49

0xShaito added Priority: Critical and removed Priority: High labels Oct 14, 2024

lucas-manuel requested changes Oct 14, 2024

View reviewed changes

lucas-manuel added Status: Revisions Needed and removed Status: Ready for Review labels Oct 14, 2024

0xShaito added 2 commits October 14, 2024 17:37

update pt prices

2aa62de

better comment

b3ecb90

lucas-manuel approved these changes Oct 14, 2024

View reviewed changes

lucas-manuel added Status: Ready for Review and removed Status: Revisions Needed labels Oct 14, 2024

0xteddybear approved these changes Oct 14, 2024

View reviewed changes

RedLikeRosesss approved these changes Oct 14, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add october 17th, 2024 spell and tests #55

add october 17th, 2024 spell and tests #55

0xShaito commented Oct 9, 2024

github-actions bot commented Oct 9, 2024 •

edited

Loading

0xShaito Oct 9, 2024 •

edited

Loading

lucas-manuel left a comment

0xteddybear left a comment

0xteddybear Oct 10, 2024

0xteddybear Oct 10, 2024

0xteddybear left a comment •

edited

Loading

0xteddybear Oct 10, 2024

0xteddybear Oct 10, 2024

RedLikeRosesss left a comment

lucas-manuel Oct 14, 2024

0xteddybear left a comment •

edited

Loading

RedLikeRosesss left a comment

0xteddybear commented Oct 14, 2024

0xShaito commented Oct 15, 2024


		address internal constant SDAI_PRICE_FEED = 0x0c0864837C7e65458aCD3C665222203217019436;

		address internal constant PT_26DEC2024_PRICE_FEED = 0x81E5E28F33D314e9211885d6f0F4080E755e4595;

		// assertEq(ptUsde26DecPrice, 0.969105874238964993e36);
		// assertEq(ptUsde27MarPrice, 0.908944818619989853e36);

add october 17th, 2024 spell and tests #55

Are you sure you want to change the base?

add october 17th, 2024 spell and tests #55

Conversation

0xShaito commented Oct 9, 2024

github-actions bot commented Oct 9, 2024 • edited Loading

Spell Caster

0xShaito Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

lucas-manuel left a comment

Choose a reason for hiding this comment

0xteddybear left a comment

Choose a reason for hiding this comment

spell

tests

checklist

0xteddybear Oct 10, 2024

Choose a reason for hiding this comment

0xteddybear Oct 10, 2024

Choose a reason for hiding this comment

0xteddybear left a comment • edited Loading

Choose a reason for hiding this comment

0xteddybear Oct 10, 2024

Choose a reason for hiding this comment

0xteddybear Oct 10, 2024

Choose a reason for hiding this comment

RedLikeRosesss left a comment

Choose a reason for hiding this comment

lucas-manuel Oct 14, 2024

Choose a reason for hiding this comment

0xteddybear left a comment • edited Loading

Choose a reason for hiding this comment

RedLikeRosesss left a comment

Choose a reason for hiding this comment

0xteddybear commented Oct 14, 2024

0xShaito commented Oct 15, 2024

github-actions bot commented Oct 9, 2024 •

edited

Loading

0xShaito Oct 9, 2024 •

edited

Loading

0xteddybear left a comment •

edited

Loading

0xteddybear left a comment •

edited

Loading