Skip to content
This repository has been archived by the owner on Apr 22, 2024. It is now read-only.

2023-10-11 Goerli Spell #231

Merged
merged 13 commits into from
Oct 12, 2023
Merged

2023-10-11 Goerli Spell #231

merged 13 commits into from
Oct 12, 2023

Conversation

0xdecr1pto
Copy link
Collaborator

Description

Contribution Checklist

  • PR title starts with (PE-<TICKET_NUMBER>)
  • Code approved
  • Tests approved
  • CI Tests pass

Checklist

  • Every contract variable/method declared as public/external private/internal
  • Consider if this PR needs the officeHours modifier override
  • Verify expiration (30 days unless otherwise specified)
  • Verify hash in the description matches here
  • Validate all addresses used are in Goerli changelog or known
  • Notify any external teams affected by the spell so they have the opportunity to review
  • Deploy spell to Goerli ETH_GAS_LIMIT="XXX" ETH_GAS_PRICE="YYY" make deploy
  • Ensure contract is verified on Goerli etherscan
  • Change test to use Goerli spell address and deploy timestamp
  • Cast spell on Goerli make spell="0x-deployed-spell-address" cast-spell
  • Run make archive-spell or make date="YYYY-MM-DD" archive-spell to make an archive directory and copy DssSpell.sol, DssSpell.t.sol and DssSpell.t.base.sol
  • squash and merge this PR

@0xdecr1pto 0xdecr1pto marked this pull request as ready for review October 9, 2023 13:57
@0xdecr1pto 0xdecr1pto requested a review from amusingaxl October 9, 2023 13:57
Copy link
Collaborator

@amusingaxl amusingaxl left a comment

Choose a reason for hiding this comment

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

Goerli Executive Spell Review Checklist

TL;DR: missing RwaLiquidationOracle.bump for changed parameters in RWA007-A and RWA015-A and some nitpicks.

Goerli 2023-10-11

Spell Actions (Per Exec Sheet):

  • Non-Scope Defined Parameter Changes - WBTC DC-IAM Changes
    • Reduce the WBTC-A DC-IAM Target Available Debt (gap) from 10 million DAI to 2 million DAI.
    • Reduce the WBTC-B DC-IAM Target Available Debt (gap) from 5 million DAI to 2 million DAI.
    • Reduce the WBTC-C DC-IAM Target Available Debt (gap) from 10 million DAI to 2 million DAI.
  • Stability Fee Changes
    • Increase the ETH-A Stability Fee (SF) by 1.55%, from 3.70% to 5.25%.
    • Increase the ETH-B Stability Fee (SF) by 1.55%, from 4.20% to 5.75%.
    • Increase the ETH-C Stability Fee (SF) by 1.55%, from 3.45% to 5.00%.
    • Increase WBTC-A Stability Fee (SF) by 0.06%, from 5.8% to 5.86%
      ❌ Wrong comment in the code
    • Increase WBTC-B Stability Fee (SF) by 0.06%, from 5.8% to 6.36%
      ❌ Wrong comment in the code and invalid action in sheet
    • Increase WBTC-C Stability Fee (SF) by 0.06%, from 5.8% to 5.61%
      ❌ Wrong comment in the code and invalid action in sheet
  • Initial RETH-A Offboarding
    • Set rETH-A Debt Ceiling (line) to 0 (zero).
    • Remove RETH-A from Autoline.
  • Reconfiguring Andromeda RWA015-A
    • Set the Maximum Debt Ceiling (line) to 3 billion DAI.
  • Reconfiguring Clydesdale RWA007-A
    • Reactivate the Debt Ceiling Instant Access Module for this vault type.
    • Set the Target Available Debt (gap) to 50 million DAI.
    • Set the Ceiling Increase Cooldown (ttl) to 86400 (24 hours).
    • Set the Maximum Debt Ceiling (line) to 3 billion DAI.
  • Set up Governance Facilitator Streams ⚠️ Ignored on Goerli
    • ~~JanSky | 2023-10-01 00:00:00 to 2024-~~09-30 23:59:59 | 504,000.00 DAI | 0xf3F868534FAD48EF5a228Fe78669cf242745a755
    • VoteWizard | 2023-10-01 00:00:00 to 2024-09-30 23:59:59 | 504,000.00 DAI | 0x9E72629dF4fcaA2c2F5813FbbDc55064345431b1
    • JanSky | 2023-10-01 00:00:00 to 2024-09-30 23:59:59 | 216.00 MKR | 0xf3F868534FAD48EF5a228Fe78669cf242745a755
  • USDP-PSM Facilitation Incentives
    • DAO Resolution 1: Initial Setup (copy only)
    • Add dao_resolution parameter: QmWg43PNNGfEyXnTv1qN8dRXFJz5ZchrmZU8qH57Ki6D62
  • BA Labs MKR Distribution ⚠️ Ignored on Goerli
    • BA Labs - 175 MKR - 0x5d67d5B1fC7EF4bfF31967bE2D2d7b9323c1521c
  • AVC Member Compensation ⚠️ Ignored on Goerli
    • opensky - 20.85 MKR - 0x8e67ee3bbeb1743dc63093af493f67c3c23c6f04
    • DAI-Vinci - 12.51 MKR - 0x9ee47F0f82F1A6F45C4E1D25Ce95C321D8C8356a
    • IamMeeoh - 20.85 MKR - 0x47f7A5d8D27f259582097E1eE59a07a816982AE9
    • ACRE DAOs - 20.85 MKR - 0xBF9226345F601150F64Ea4fEaAE7E40530763cbd
    • Harmony - 20.85 MKR - 0xE20A2e231215e9b7Aa308463F1A7490b2ECE55D3
    • Res - 20.85 MKR - 0x8c5c8d76372954922400e4654AF7694e158AB784
    • seedlatam.eth - 20.85 MKR - 0x0087a081a9b430fd8f688c6ac5dd24421bfb060d
  • Delegate Compensation ⚠️ Ignored on Goerli
    • 0xDefensor - 41.67 MKR - 0x9542b441d65B6BF4dDdd3d4D2a66D8dCB9EE07a9
    • TRUE NAME - 41.67 MKR - 0x612F7924c367575a0Edf21333D96b15F1B345A5d
    • BONAPUBLICA - 41.67 MKR - 0x167c1a762B08D7e78dbF8f24e5C3f1Ab415021D3
    • Navigator - 41.67 MKR - 0x11406a9CC2e37425F15f920F494A51133ac93072
    • vigilant - 34.5 MKR - 0x2474937cB55500601BCCE9f4cb0A0A72Dc226F61
    • Cloaky - 21.06 MKR - 0x869b6d5d8FA7f4FFdaCA4D23FFE0735c5eD1F818
    • UPMaker - 13.89 MKR - 0xbB819DF169670DC71A16F58F55956FE642cc6BcD
    • PALC - 13.89 MKR - 0x78Deac4F87BD8007b9cb56B8d53889ed5374e83A
    • BLUE - 12.78 MKR - 0xb6C09680D822F162449cdFB8248a7D3FC26Ec9Bf
    • PBG - 6.48 MKR - 0x8D4df847dB7FfE0B46AF084fE031F7691C6478c2

Development Stage

  • Office Hours
    • OFF (On Goerli the deployed spell is cast with a pause delay of 60 sec.)
  • 30 days spell expiry in constructor (block.timestamp + 30 days)
  • No Exec Doc Hash on Goerli
  • Spell Description
    • Ensure description exactly matches 'Goerli Spell'
  • Local Environment Actions
    • Update Foundry by running foundryup
    • Reinstall libraries by running rm -r lib && git submodule update --init --recursive
      Submodule path 'lib/dss-exec-lib': checked out '69b658f35d8618272cd139dfc18c5713caf6b96b'
      Submodule path 'lib/dss-exec-lib/lib/dss-interfaces': checked out '9bfd7afadd1f8c217ef05850b2555691786286cb'
      Submodule path 'lib/dss-exec-lib/lib/forge-std': checked out '0aa99eb8456693c015350c5e6c4f442ebe912f77'
      Submodule path 'lib/dss-exec-lib/lib/forge-std/lib/ds-test': checked out 'cd98eff28324bfac652e63a239a60632a761790b'
      Submodule path 'lib/dss-test': checked out '7529fa1e043b9195f41549852d9c5bb0714eaa27'
      Submodule path 'lib/dss-test/lib/dss-interfaces': checked out '9bfd7afadd1f8c217ef05850b2555691786286cb'
      Submodule path 'lib/dss-test/lib/forge-std': checked out 'aea0b2685bebc883c09f5554d7fb481e85d0564d'
      Submodule path 'lib/dss-test/lib/forge-std/lib/ds-test': checked out 'cd98eff28324bfac652e63a239a60632a761790b'
      
  • Rate constants used are correct
    • Manual check 1: Calculate rates using make rates pct=<pct> (e.g. pct=0.75, for 0.75%)
    • Manual check 2: Compare rates used against IPFS rate list
    • Variable name conforms to X_PT_Y_Z_PCT_RATE (e.g. ZERO_PT_SEVEN_FIVE_PCT_RATE for 0.75%, SIX_PCT_RATE for 6.00%)
    • Visibility is internal
    • State mutability is constant
  • Constants Match
    • Precision unit constants used match their defined values
      • RAD = 10 ** 45
      • Visibility is internal
      • State mutability is constant
    • Math unit constants used match their defined values
      • MILLION = 10 ** 6
      • BILLION = 10 ** 9
      • Visibility is internal
      • State mutability is constant
  • String Constants
    • IPFS hashes or DAO resolutions
      • IPFS hashes and DAO resolutions match the exec doc
      • The variable is named dao_resolutions
      • if multiple dao resolutions are present
        • they are located in the same string constant and are comma separated
        • The comment above the the variable states: Comma-separated list of DAO resolutions IPFS hashes
    • Must be TOP LEVEL (or ds pause will reject)
    • Name of the variable matches purpose per EXEC DOC
    • Visibility is public
    • State mutability MUST BE constant
  • Core System Parameter Changes
  • Offboarding (Lerp mat)
    • 1st Stage Spell Actions
      • Remove Ilk from Autoline
      • Set Ilks Debt Ceilings to 0
      • Cache Ilks line to Reduce in the Global Debt Ceiling
      • Decrease Global Debt Ceiling by Total Amount of Offboarded Ilks line Cached
  • RWA Updates
    • Autoline (line) + Liquidation Oracle Price Bump (val)
      • Enable Autoline
        • ilk follows format "RWAXXX-A"
        • line (max debt ceiling)
        • gap
        • ttl
      • bump RwaLiquidationOracle with new computed increased price (val)
        ❌ Missing!
        • ensure val is set accordingly with autoline max debt ceiling (line)
        • val should enable DAI to be drawn over the loan period while taking into account the configured ink amount, interest rate and liquidation ratio (see below)
          • New val is calculated with line * [(1 + duty) ** years] * mat - rounded up - and makes sense in context of the rate mechanism. Minimum duration is usually in the Exec Doc of the spell with the RWAXXX ilk onboarding.
          • Comment explaining val formula (Debt ceiling * [ (1 + RWA stability fee ) ^ (minimum deal duration in years) ] * liquidation ratio) is present
          • Accompanying comment above bump line in format // XXM * 1.XX^X * X.XX as a WAD corresponding to the val calculation formula (e.g. // 15M * 1.03^2 * 1.00 as a WAD) is present along with the calculation formula on the line above
          • IF combining val of multiple RWA ilks being combined, val calculation is done once per ilk and added to make the total, with workings provided in code comments. The existing val value can be retrieved by calling read() on PIP_RWAXX and converting the result into decimal.
      • Poke spotter to pull in the new price
        ❌ Missing!
    • Debt Ceiling (line) + Liquidation Oracle Price Bump (val)
      • Increase Ilk Debt Ceiling (set DC + increase Global DC)
      • bump RwaLiquidationOracle with new computed increased price (val)
        ❌ Missing!
        • val should enable DAI to be drawn over the loan period while taking into account the configured ink amount, interest rate and liquidation ratio (see below)
          • New val is calculated with line * [(1 + duty) ** years] * mat - rounded up - and makes sense in context of the rate mechanism. Minimum duration is usually in the Exec Doc of the spell with the RWAXXX ilk onboarding.
          • Comment explaining val formula (Debt ceiling * [ (1 + RWA stability fee ) ^ (minimum deal duration in years) ] * liquidation ratio) is present
          • Accompanying comment above bump line in format // XXM * 1.XX^X * X.XX as a WAD corresponding to the val calculation formula (e.g. // 15M * 1.03^2 * 1.00 as a WAD) is present along with the calculation formula on the line above
          • IF combining val of multiple RWA ilks being combined, val calculation is done once per ilk and added to make the total, with workings provided in code comments. The existing val value can be retrieved by calling read() on PIP_RWAXX and converting the result into decimal.
      • Poke spotter to pull in the new price
        ❌ Missing!
  • Tests
    • Ensure each spell action has sufficient test coverage
      • Non-Scope Defined Parameter Changes - WBTC DC-IAM Changes ✅ testGeneral
      • Stability Fee Changes ✅ testGeneral
      • Initial RETH-A Offboarding ✅ testGeneral
      • Reconfiguring Andromeda RWA015-A ❌ Missing RwaLiquidationOracle.bump test
      • Reconfiguring Clydesdale RWA007-A ❌ Missing RwaLiquidationOracle.bump test
      • USDP-PSM Facilitation Incentives ⚠️ No test coverage. Maybe consider adding it.
    • Ensure every test function is declared as public if enabled or private if disabled
    • Ensure that the DssExecLib.address file is not being modified by the spell PR
    • Check all CI tests are passing as at the latest commit
      8e35e7f
    • Check all tests are passing locally using make test
      • Ensure that only ETH_RPC_URL is being used from env (i.e. no match, block or similar are active in your env)
Using DssExecLib at: 0x122F6c0Dcd898b4a07310E92c3eAE5D7Ce0c8bb6
[] Compiling...
[] Compiling 6 files with 0.8.16
[] Solc 0.8.16 finished in 1.82s
Compiler run successful!

Running 2 tests for src/test/starknet.t.sol:StarknetTests
[PASS] testStarknet() (gas: 1070878)
[PASS] testStarknetSpell() (gas: 2324)
Test result: ok. 2 passed; 0 failed; 0 skipped; finished in 55.89s

Running 17 tests for src/DssSpell.t.sol:DssSpellTest
[PASS] testAuth() (gas: 52095774)
[PASS] testAuthInSources() (gas: 8107139)
[PASS] testBytecodeMatches() (gas: 2029961)
[PASS] testCastCost() (gas: 911772)
[PASS] testChainlogValues() (gas: 9287776)
[PASS] testChainlogVersionBump() (gas: 4338863)
[PASS] testContractSize() (gas: 8962)
[PASS] testDeployCost() (gas: 2016529)
[PASS] testFailNotScheduled() (gas: 14397)
[PASS] testFailTooEarly() (gas: 13585)
[PASS] testFailTooLate() (gas: 13606)
[PASS] testFailWrongDay() (gas: 13585)
[PASS] testGeneral() (gas: 37783728)
[PASS] testNextCastTime() (gas: 364790)
[PASS] testOnTime() (gas: 907411)
[PASS] testPSMs() (gas: 2219522)
[PASS] testUseEta() (gas: 363541)
Test result: ok. 17 passed; 0 failed; 0 skipped; finished in 1087.21s
 
Ran 2 test suites: 19 tests passed, 0 failed, 0 skipped (19 total tests)

src/DssSpell.sol Outdated Show resolved Hide resolved
src/DssSpell.sol Outdated Show resolved Hide resolved
src/DssSpell.sol Outdated
// ---------- Reconfiguring Andromeda RWA015-A ----------
// Forum: https://forum.makerdao.com/t/poll-request-reconfiguring-rwa-allocator-vaults/22159
// Poll: https://vote.makerdao.com/polling/QmPoLbah

Copy link
Collaborator

Choose a reason for hiding this comment

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

Dangling whitespace:

Suggested change

src/DssSpell.sol Outdated Show resolved Hide resolved
src/DssSpell.sol Outdated Show resolved Hide resolved
src/DssSpell.sol Outdated Show resolved Hide resolved
src/DssSpell.sol Outdated Show resolved Hide resolved
src/DssSpell.sol Outdated Show resolved Hide resolved
src/DssSpell.sol Outdated
// Set DC-IAM Line (max DC) to 0 (zero).
(,,,uint256 line,) = VatLike(MCD_VAT).ilks("RETH-A");
DssExecLib.removeIlkFromAutoLine("RETH-A");
DssExecLib.decreaseIlkDebtCeiling("RETH-A", line / RAD, true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
DssExecLib.decreaseIlkDebtCeiling("RETH-A", line / RAD, true);
DssExecLib.decreaseIlkDebtCeiling("RETH-A", line / RAD, /* global = */ true);

src/DssSpell.sol Outdated Show resolved Hide resolved
Copy link
Contributor

@SidestreamSweatyPumpkin SidestreamSweatyPumpkin left a comment

Choose a reason for hiding this comment

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

Goerli Executive Spell Review Checklist

Goerli 2023-10-11

Spell Actions (Per Exec Sheet):

  • Non-Scope Defined Parameter Changes - WBTC DC-IAM Changes
    • Reduce the WBTC-A DC-IAM Target Available Debt from 10 million DAI to 2 million DAI.
    • Reduce the WBTC-B DC-IAM Target Available Debt from 5 million DAI to 2 million DAI.
    • Reduce the WBTC-C DC-IAM Target Available Debt from 10 million DAI to 2 million DAI.
  • Stability Fee Changes
    • Increase the ETH-A Stability Fee (SF) by 1.55%, from 3.70% to 5.25%.
    • Increase the ETH-B Stability Fee (SF) by 1.55%, from 4.20% to 5.75%.
    • Increase the ETH-C Stability Fee (SF) by 1.55%, from 3.45% to 5.00%.
    • Increase WBTC-A Stability Fee (SF) by 0.06%, from 5.8% to 5.86%
    • Increase WBTC-B Stability Fee (SF) by 0.06%, from 6.30% to 6.36%.
    • Increase WBTC-C Stability Fee (SF) by 0.06%, from 5.55% to 5.61%.
  • Initial RETH-A Offboarding
    • Set rETH-A Debt Ceiling (line) to 0 (zero).
    • Remove RETH-A from Autoline.
  • Reconfiguring Andromeda RWA015-A
    • Set the Maximum Debt Ceiling (line) to 3 billion DAI.
  • Reconfiguring Clydesdale RWA007-A
    • Reactivate the Debt Ceiling Instant Access Module for this vault type.
    • Set the Target Available Debt (gap) to 50 million DAI.
    • Set the Ceiling Increase Cooldown (ttl) to 86400 (24 hours).
    • Set the Maximum Debt Ceiling (line) to 3 billion DAI.
  • SKIP ON GOERLI (comments present) : Set up Governance Facilitator Streams
    • JanSky | 2023-10-01 00:00:00 to 2024-09-30 23:59:59 | 504,000.00 DAI | 0xf3F868534FAD48EF5a228Fe78669cf242745a755
    • VoteWizard | 2023-10-01 00:00:00 to 2024-09-30 23:59:59 | 504,000.00 DAI | 0x9E72629dF4fcaA2c2F5813FbbDc55064345431b1
    • JanSky | 2023-10-01 00:00:00 to 2024-09-30 23:59:59 | 216.00 MKR | 0xf3F868534FAD48EF5a228Fe78669cf242745a755
    • VoteWizard | 2023-10-01 00:00:00 to 2024-09-30 23:59:59 | 216.00 MKR | 0x9E72629dF4fcaA2c2F5813FbbDc55064345431b1
  • USDP-PSM Facilitation Incentives
    • Add dao_resolution parameter: QmWg43PNNGfEyXnTv1qN8dRXFJz5ZchrmZU8qH57Ki6D62
  • SKIP ON GOERLI (comments present): BA Labs MKR Distribution
    • BA Labs - 175 MKR - 0x5d67d5B1fC7EF4bfF31967bE2D2d7b9323c1521c
  • SKIP ON GOERLI (comments present): AVC Member Compensation
    • opensky - 20.85 MKR - 0x8e67ee3bbeb1743dc63093af493f67c3c23c6f04
    • DAI-Vinci - 12.51 MKR - 0x9ee47F0f82F1A6F45C4E1D25Ce95C321D8C8356a
    • IamMeeoh - 20.85 MKR - 0x47f7A5d8D27f259582097E1eE59a07a816982AE9
    • ACRE DAOs - 20.85 MKR - 0xBF9226345F601150F64Ea4fEaAE7E40530763cbd
    • Harmony - 20.85 MKR - 0xE20A2e231215e9b7Aa308463F1A7490b2ECE55D3
    • Res - 20.85 MKR - 0x8c5c8d76372954922400e4654AF7694e158AB784
    • seedlatam.eth - 20.85 MKR - 0x0087a081a9b430fd8f688c6ac5dd24421bfb060d
  • SKIP ON GOERLI (comments not present, not critical) :Delegate Compensation
    • 0xDefensor - 41.67 MKR - 0x9542b441d65B6BF4dDdd3d4D2a66D8dCB9EE07a9
    • TRUE NAME - 41.67 MKR - 0x612F7924c367575a0Edf21333D96b15F1B345A5d
    • BONAPUBLICA - 41.67 MKR - 0x167c1a762B08D7e78dbF8f24e5C3f1Ab415021D3
    • Navigator - 41.67 MKR - 0x11406a9CC2e37425F15f920F494A51133ac93072
    • vigilant - 34.5 MKR - 0x2474937cB55500601BCCE9f4cb0A0A72Dc226F61
    • Cloaky - 21.06 MKR - 0x869b6d5d8FA7f4FFdaCA4D23FFE0735c5eD1F818
    • UPMaker - 13.89 MKR - 0xbB819DF169670DC71A16F58F55956FE642cc6BcD
    • PALC - 13.89 MKR - 0x78Deac4F87BD8007b9cb56B8d53889ed5374e83A
    • BLUE - 12.78 MKR - 0xb6C09680D822F162449cdFB8248a7D3FC26Ec9Bf
    • PBG - 6.48 MKR - 0x8D4df847dB7FfE0B46AF084fE031F7691C6478c2

Development Stage

  • Office Hours
    • OFF (On Goerli the deployed spell is cast with a pause delay of 60 sec.)
  • 30 days spell expiry in constructor (block.timestamp + 30 days)
  • No Exec Doc Hash on Goerli
  • Spell Description
    • Ensure description exactly matches 'Goerli Spell'
  • Local Environment Actions
    • Update Foundry by running foundryup
    • Reinstall libraries by running rm -r lib && git submodule update --init --recursive
      Submodule path 'lib/dss-exec-lib': checked out '69b658f35d8618272cd139dfc18c5713caf6b96b'
      Submodule path 'lib/dss-exec-lib/lib/dss-interfaces': checked out '9bfd7afadd1f8c217ef05850b25556917
      86286cb'
      Submodule path 'lib/dss-exec-lib/lib/forge-std': checked out '0aa99eb8456693c015350c5e6c4f442ebe912f
      77'
      Submodule path 'lib/dss-exec-lib/lib/forge-std/lib/ds-test': checked out 'cd98eff28324bfac652e63a239
      a60632a761790b'
      Submodule path 'lib/dss-test': checked out '7529fa1e043b9195f41549852d9c5bb0714eaa27'
      Submodule path 'lib/dss-test/lib/dss-interfaces': checked out '9bfd7afadd1f8c217ef05850b255569178628
      6cb'
      Submodule path 'lib/dss-test/lib/forge-std': checked out 'aea0b2685bebc883c09f5554d7fb481e85d0564d'
      Submodule path 'lib/dss-test/lib/forge-std/lib/ds-test': checked out 'cd98eff28324bfac652e63a239a606
      32a761790b'
      
  • Rate constants used are correct
    • Manual check 1: Calculate rates using make rates pct=<pct> (e.g. pct=0.75, for 0.75%)
    • Manual check 2: Compare rates used against IPFS rate list
    • Variable name conforms to X_PT_Y_Z_PCT_RATE (e.g. ZERO_PT_SEVEN_FIVE_PCT_RATE for 0.75%, SIX_PCT_RATE for 6.00%)
    • Visibility is internal
    • State mutability is constant
  • Constants Match
    • Precision unit constants used match their defined values
      • WAD = 10 ** 18
      • RAY = 10 ** 27
      • RAD = 10 ** 45
      • Visibility is internal
      • State mutability is constant
    • Math unit constants used match their defined values
      • HUNDRED = 10 ** 2
      • THOUSAND = 10 ** 3
      • MILLION = 10 ** 6
      • BILLION = 10 ** 9
      • Visibility is internal
      • State mutability is constant
      • (Non-critical) Ensure they match with config
  • Address Declaration
    • Address Constants
      • Address is not in the ChainLog
      • Variable name matches corresponding entry in an addresses.sol file
      • Address value matches corresponding entry in an addresses.sol file
      • Visibility is internal
      • State mutability is constant
    • Address Immutables
      • Use the DssExecLib Core Address Helpers instead of getChangelogAddress() whenever possible (e.g. DssExecLib.vat())
      • Follow pattern address internal immutable KEY_NAME = DssExecLib.getChangelogAddress(KEY_NAME)
      • Variable name matches ChainLog key name
        • Exceptions must follow archive patterns (e.g. MKR for MCD_GOV)
      • Instantiate as address type
        • Exceptions must follow archive patterns (e.g. GemLike for MKR)
      • Visibility is internal
      • State mutability is immutable
  • String Constants
    • IPFS hashes or DAO resolutions
      • IPFS hashes and DAO resolutions match the exec doc
      • The variable is named dao_resolutions
      • if multiple dao resolutions are present
        • they are located in the same string constant and are comma separated
        • The comment above the the variable states: Comma-separated list of DAO resolutions IPFS hashes
    • Must be TOP LEVEL (or ds pause will reject)
    • Name of the variable matches purpose per EXEC DOC
    • Visibility is public
    • State mutability MUST BE constant
  • Deployed Contracts (not yet on chainlog or new to chainlog)
    • Verified on etherscan
    • Optimizations match Repo
    • GNU AGPLv3 license
    • Constructor args ok (e.g. vat, dai, dog, ...)
    • Wards ok (pause proxy relied, deployer denied)
      • MCD_ESM is already relied / being relied in this spell (as approved by GovAlpha) in order to allow de-authing the pause proxy during Emergency Shutdown, via denyProxy.
    • Matches corresponding github source code (i.e. diffcheck via vscode code --diff etherscan.sol github.sol)
    • Ensure deployer address is included into addresses_deployers.sol (to keep up to date)
  • [.] Core System Parameter Changes
  • Debt Ceiling Changes
  • Onboarding (insert relevant checklists inline here)
  • Offboarding (Lerp mat)
    • 1st Stage Spell Actions
      • Remove Ilk from Autoline
      • Set Ilks Debt Ceilings to 0
      • Cache Ilks line to Reduce in the Global Debt Ceiling
      • Decrease Global Debt Ceiling by Total Amount of Offboarded Ilks line Cached
    • 2nd Stage Spell Actions
      • Set Ilk Liquidation Penalty chop to 0
      • Set Keeper Incentive Flat Rate tip to 0
      • Check IF chip is required to be adjusted as well
      • Use DssExecLib.linearInterpolation
        • name Format matches "XXX-A Offboarding"
        • target matches spotter
        • ilk Format matches "XXX-A"
        • what matches mat
        • startTime matches block.timestamp
        • start matches Var CURRENT_XXX_A_MAT
        • end matches Var TARGET_XXX_A_MAT (Match Exec Sheet & Risk Computations)
          • Check IF Target mat Covers All Remaining Vaults CR times Risk Multiplier Factor
        • duration matches Exec Sheet
  • RWA Updates
    • doc (Using the _updateDoc helper or otherwise)
      • init the RwaLiquidationOracle to reset the doc
      • Sanity Check pip must be set (not the zero address)
      • ilk follows format "RWAXXX-A"
      • val price ignored (0) if init has already been called
      • doc new legal document (IPFS HASH) matches Doc (or Forum Post)
      • tau parameter used is the old tau value
    • Autoline (line) + Liquidation Oracle Price Bump (val)
      • Enable Autoline
        • ilk follows format "RWAXXX-A"
        • line (max debt ceiling)
        • gap
        • ttl
      • bump RwaLiquidationOracle with new computed increased price (val)
        • ensure val is set accordingly with autoline max debt ceiling (line)
        • val should enable DAI to be drawn over the loan period while taking into account the configured ink amount, interest rate and liquidation ratio (see below)
          • New val is calculated with line * [(1 + duty) ** years] * mat - rounded up - and makes sense in context of the rate mechanism. Minimum duration is usually in the Exec Doc of the spell with the RWAXXX ilk onboarding.
          • Comment explaining val formula (Debt ceiling * [ (1 + RWA stability fee ) ^ (minimum deal duration in years) ] * liquidation ratio) is present
          • Accompanying comment above bump line in format // XXM * 1.XX^X * X.XX as a WAD corresponding to the val calculation formula (e.g. // 15M * 1.03^2 * 1.00 as a WAD) is present along with the calculation formula on the line above
            • SF is zero so should probably replace the formula with the comment like this
          • IF combining val of multiple RWA ilks being combined, val calculation is done once per ilk and added to make the total, with workings provided in code comments. The existing val value can be retrieved by calling read() on PIP_RWAXX and converting the result into decimal.
      • Poke spotter to pull in the new price
    • Debt Ceiling (line) + Liquidation Oracle Price Bump (val)
      • Increase Ilk Debt Ceiling (set DC + increase Global DC)
      • bump RwaLiquidationOracle with new computed increased price (val)
        • val should enable DAI to be drawn over the loan period while taking into account the configured ink amount, interest rate and liquidation ratio (see below)
          • New val is calculated with line * [(1 + duty) ** years] * mat - rounded up - and makes sense in context of the rate mechanism. Minimum duration is usually in the Exec Doc of the spell with the RWAXXX ilk onboarding.
          • Comment explaining val formula (Debt ceiling * [ (1 + RWA stability fee ) ^ (minimum deal duration in years) ] * liquidation ratio) is present
          • Accompanying comment above bump line in format // XXM * 1.XX^X * X.XX as a WAD corresponding to the val calculation formula (e.g. // 15M * 1.03^2 * 1.00 as a WAD) is present along with the calculation formula on the line above
          • IF combining val of multiple RWA ilks being combined, val calculation is done once per ilk and added to make the total, with workings provided in code comments. The existing val value can be retrieved by calling read() on PIP_RWAXX and converting the result into decimal.
      • Poke spotter to pull in the new price
  • Payments
    • Payments are not crafted or reviewed on Goerli (comments may be present)
  • SubDAO Content
    • SubDAO SubProxy spell execution
      • SubDAO spell address matches Exec Sheet (comment or otherwise)
      • IF SubDAO spell deployer is a smart contract (e.g. multisig or factory)
        • Ensure the deployer address is in addresses_deployers.sol as an entry
      • Executed using ProxyLike(SUBDAO_PROXY).exec(SUBDAO_SPELL, abi.encodeWithSignature("execute()"));
      • Execution is NOT delegate call
      • Reviewer Note: Gas cost may be very high as SubDAO spells execute within the main cast execution. (Also note that low level call gas estimation is not done by our scripts)
    • Maker Core (main spell) SubDAO actions (i.e. operate in Pause Proxy DelegateCall context)
      • No SubDAO contract being interacted with is authed on a core contract like vat, etc. (Check comprehensively where the risk is high)
      • SubDAO contract licensing and optimizations generally do not matter (except where they pose a security risk)
      • SubDAO contracts and all libraries / dependencies are verified (Blocking if not true)
      • Upgradable SubDAO contracts
        • Any upgradable contracts have the PAUSE_PROXY as their admin (i.e. the party that can upgrade)
          • Any upgradable SubDAO contracts with an admin that is not PAUSE_PROXY are not authed on any core contracts (Blocking)
      • All SubDAO content addresses (i.e. provided contract addresses or EOAs) present in the Maker Core spell are present in the Exec Sheet and are correct. SubDAO addresses being authed or given any permissions and addresses being called must be confirmed by the SubDAO spell team.
      • SubDAO actions match Exec Sheet (only where inline with main spell code) and do not affect core contracts
      • Core contract knock-on actions (such as offboarding or setting DC to 0) are present in the Exec Sheet and match the code
      • External calls for SubDAO content are NOT delegate call
      • Code does not have untoward behavior within the scope of Maker Core Contracts (e.g. up to the SubDAO proxy)
  • External Contracts Calls (Not SubDAOs, e.g. Starknet)
    • Target Contract don't block spell execution
    • External call is NOT delegate call
    • Target Contract doesn't have permissions on the Vat
    • Target Contract doesn't do anything untoward (e.g. interacting with unsafe contracts)
    • Contracts deployed via CREATE2 (e.g. if it looks like a vanity address) do not have selfdestruct in their code
    • MCD Pause Proxy doesn't give any approvals
    • All possible actions of the Target Contract are documented
    • Target contract is not upgradable
    • Target Contract is included in the ChainLog
    • Test Coverage is comprehensive
  • ChainLog
    • Increment ChainLog version based on update type
      • Major -> New Vat (++.0.0)
      • Minor -> Core Module (DSS) Update (e.g. Flapper) (0.++.0)
      • Patch -> Collateral addition or addition/modification (0.0.++)
  • addresses_goerli.sol matches spell code
  • Ensure every spell variable is declared as public/internal
  • Ensure every spell variable is used in the spell (no unused variables)
    • RAD not used ❌
  • Spell actions match the corresponding Exec Sheet
  • Tests
    • Ensure each spell action has sufficient test coverage
      • Non-Scope defined parameter changes
      • SF changes
      • RETH-A offboarding stage 1
      • Adjustment of RWA007
      • Adjustment of RWA015
    • Ensure every test function is declared as public if enabled or private if disabled
    • Ensure that the DssExecLib.address file is not being modified by the spell PR
    • Check all CI tests are passing as at the latest commit
      499b7b5
    • Check all tests are passing locally using make test
      • Ensure that only ETH_RPC_URL is being used from env (i.e. no match, block or similar are active in your env)
      ./scripts/test-dssspell-forge.sh no-match="" match="" block=""
      Using DssExecLib at: 0x122F6c0Dcd898b4a07310E92c3eAE5D7Ce0c8bb6
      [⠘] Compiling...
      [⠒] Compiling 6 files with 0.8.16
      [⠒] Solc 0.8.16 finished in 1.73s
      Compiler run successful!
      
      Running 2 tests for src/test/starknet.t.sol:StarknetTests
      [PASS] testStarknet() (gas: 1164166)
      [PASS] testStarknetSpell() (gas: 2324)
      Test result: ok. 2 passed; 0 failed; 0 skipped; finished in 280.62s
      
      Running 19 tests for src/DssSpell.t.sol:DssSpellTest
      [PASS] testAuth() (gas: 52179062)
      [PASS] testAuthInSources() (gas: 8190427)
      [PASS] testBytecodeMatches() (gas: 2284491)
      [PASS] testCastCost() (gas: 1005060)
      [PASS] testChainlogValues() (gas: 9381042)
      [PASS] testChainlogVersionBump() (gas: 4432173)
      [PASS] testContractSize() (gas: 8984)
      [PASS] testDeployCost() (gas: 2270357)
      [PASS] testFailNotScheduled() (gas: 14375)
      [PASS] testFailTooEarly() (gas: 13607)
      [PASS] testFailTooLate() (gas: 13606)
      [PASS] testFailWrongDay() (gas: 13585)
      [PASS] testGeneral() (gas: 38118126)
      [PASS] testNextCastTime() (gas: 364790)
      [PASS] testOnTime() (gas: 1000699)
      [PASS] testPSMs() (gas: 2287352)
      [PASS] testUseEta() (gas: 363519)
      [PASS] test_RWA007_Update() (gas: 1225671)
      [PASS] test_RWA015_Update() (gas: 1193674)
      Test result: ok. 19 passed; 0 failed; 0 skipped; finished in 854.06s
      
      Ran 2 test suites: 21 tests passed, 0 failed, 0 skipped (21 total tests)
      

src/DssSpell.sol Outdated Show resolved Hide resolved
src/DssSpell.sol Outdated Show resolved Hide resolved
src/DssSpell.sol Outdated Show resolved Hide resolved
src/DssSpell.sol Outdated
(,,,uint256 line,) = VatLike(MCD_VAT).ilks("RETH-A");
DssExecLib.removeIlkFromAutoLine("RETH-A");
DssExecLib.setValue(MCD_VAT, "RETH-A", "line", 0);
// NOTE: decreasing globalline suniglow level API because of precision loss when using DssExec
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// NOTE: decreasing globalline suniglow level API because of precision loss when using DssExec
// NOTE: decreasing global line using the low level API because of precision loss when using DssExecLib

Copy link
Collaborator

@amusingaxl amusingaxl left a comment

Choose a reason for hiding this comment

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

❌ Test coverage for RWA liquidation is not good.
It is trying to draw 0 after spell being executed.

src/DssSpell.t.sol Show resolved Hide resolved
src/DssSpell.t.sol Show resolved Hide resolved
src/DssSpell.t.sol Show resolved Hide resolved
src/DssSpell.t.sol Show resolved Hide resolved
amusingaxl
amusingaxl previously approved these changes Oct 10, 2023
Copy link
Collaborator

@amusingaxl amusingaxl left a comment

Choose a reason for hiding this comment

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

Good to deploy :shipit:

Goerli Executive Spell Review Checklist

Goerli 2023-10-11

Spell Actions (Per Exec Sheet):

  • Non-Scope Defined Parameter Changes - WBTC DC-IAM Changes
    • Reduce the WBTC-A DC-IAM Target Available Debt (gap) from 10 million DAI to 2 million DAI.
    • Reduce the WBTC-B DC-IAM Target Available Debt (gap) from 5 million DAI to 2 million DAI.
    • Reduce the WBTC-C DC-IAM Target Available Debt (gap) from 10 million DAI to 2 million DAI.
  • Stability Fee Changes
    • Increase the ETH-A Stability Fee (SF) by 1.55%, from 3.70% to 5.25%.
    • Increase the ETH-B Stability Fee (SF) by 1.55%, from 4.20% to 5.75%.
    • Increase the ETH-C Stability Fee (SF) by 1.55%, from 3.45% to 5.00%.
    • Increase WBTC-A Stability Fee (SF) by 0.06%, from 5.8% to 5.86%
    • Increase WBTC-B Stability Fee (SF) by 0.06%, from 5.8% to 6.36%
    • Increase WBTC-C Stability Fee (SF) by 0.06%, from 5.8% to 5.61%
  • Initial RETH-A Offboarding
    • Set rETH-A Debt Ceiling (line) to 0 (zero).
    • Remove RETH-A from Autoline.
  • Reconfiguring Andromeda RWA015-A
    • Set the Maximum Debt Ceiling (line) to 3 billion DAI.
  • Reconfiguring Clydesdale RWA007-A
    • Reactivate the Debt Ceiling Instant Access Module for this vault type.
    • Set the Target Available Debt (gap) to 50 million DAI.
    • Set the Ceiling Increase Cooldown (ttl) to 86400 (24 hours).
    • Set the Maximum Debt Ceiling (line) to 3 billion DAI.
  • Set up Governance Facilitator Streams ⚠️ Ignored on Goerli
    • ~~JanSky | 2023-10-01 00:00:00 to 2024-~~09-30 23:59:59 | 504,000.00 DAI | 0xf3F868534FAD48EF5a228Fe78669cf242745a755
    • VoteWizard | 2023-10-01 00:00:00 to 2024-09-30 23:59:59 | 504,000.00 DAI | 0x9E72629dF4fcaA2c2F5813FbbDc55064345431b1
    • JanSky | 2023-10-01 00:00:00 to 2024-09-30 23:59:59 | 216.00 MKR | 0xf3F868534FAD48EF5a228Fe78669cf242745a755
  • USDP-PSM Facilitation Incentives
    • DAO Resolution 1: Initial Setup (copy only)
    • Add dao_resolution parameter: QmWg43PNNGfEyXnTv1qN8dRXFJz5ZchrmZU8qH57Ki6D62
  • BA Labs MKR Distribution ⚠️ Ignored on Goerli
    • BA Labs - 175 MKR - 0x5d67d5B1fC7EF4bfF31967bE2D2d7b9323c1521c
  • AVC Member Compensation ⚠️ Ignored on Goerli
    • opensky - 20.85 MKR - 0x8e67ee3bbeb1743dc63093af493f67c3c23c6f04
    • DAI-Vinci - 12.51 MKR - 0x9ee47F0f82F1A6F45C4E1D25Ce95C321D8C8356a
    • IamMeeoh - 20.85 MKR - 0x47f7A5d8D27f259582097E1eE59a07a816982AE9
    • ACRE DAOs - 20.85 MKR - 0xBF9226345F601150F64Ea4fEaAE7E40530763cbd
    • Harmony - 20.85 MKR - 0xE20A2e231215e9b7Aa308463F1A7490b2ECE55D3
    • Res - 20.85 MKR - 0x8c5c8d76372954922400e4654AF7694e158AB784
    • seedlatam.eth - 20.85 MKR - 0x0087a081a9b430fd8f688c6ac5dd24421bfb060d
  • Delegate Compensation ⚠️ Ignored on Goerli
    • 0xDefensor - 41.67 MKR - 0x9542b441d65B6BF4dDdd3d4D2a66D8dCB9EE07a9
    • TRUE NAME - 41.67 MKR - 0x612F7924c367575a0Edf21333D96b15F1B345A5d
    • BONAPUBLICA - 41.67 MKR - 0x167c1a762B08D7e78dbF8f24e5C3f1Ab415021D3
    • Navigator - 41.67 MKR - 0x11406a9CC2e37425F15f920F494A51133ac93072
    • vigilant - 34.5 MKR - 0x2474937cB55500601BCCE9f4cb0A0A72Dc226F61
    • Cloaky - 21.06 MKR - 0x869b6d5d8FA7f4FFdaCA4D23FFE0735c5eD1F818
    • UPMaker - 13.89 MKR - 0xbB819DF169670DC71A16F58F55956FE642cc6BcD
    • PALC - 13.89 MKR - 0x78Deac4F87BD8007b9cb56B8d53889ed5374e83A
    • BLUE - 12.78 MKR - 0xb6C09680D822F162449cdFB8248a7D3FC26Ec9Bf
    • PBG - 6.48 MKR - 0x8D4df847dB7FfE0B46AF084fE031F7691C6478c2

Development Stage

  • Office Hours
    • OFF (On Goerli the deployed spell is cast with a pause delay of 60 sec.)
  • 30 days spell expiry in constructor (block.timestamp + 30 days)
  • No Exec Doc Hash on Goerli
  • Spell Description
    • Ensure description exactly matches 'Goerli Spell'
  • Local Environment Actions
    • Update Foundry by running foundryup
    • Reinstall libraries by running rm -r lib && git submodule update --init --recursive
      Insert checked out submodule paths here
      
    • Library Checks
      • dss-exec-lib
        • If submodule upgrades are present make sure dss-exec-lib is synced as well
        • (Non-critical) Submodule hash matches the latest release version (NOTE: dss-exec-lib as installed locally will use GitHub code more recent than the 0.0.9 release until the 0.0.10 release)
      • dss-test
  • Rate constants used are correct
    • Manual check 1: Calculate rates using make rates pct=<pct> (e.g. pct=0.75, for 0.75%)
    • Manual check 2: Compare rates used against IPFS rate list
    • Variable name conforms to X_PT_Y_Z_PCT_RATE (e.g. ZERO_PT_SEVEN_FIVE_PCT_RATE for 0.75%, SIX_PCT_RATE for 6.00%)
    • Visibility is internal
    • State mutability is constant
  • Constants Match
    • Precision unit constants used match their defined values
      • WAD = 10 ** 18
      • RAY = 10 ** 27
      • RAD = 10 ** 45
      • Visibility is internal
      • State mutability is constant
      • (Non-critical) Ensure they match with ds-math and the Numerical Ranges
    • Math unit constants used match their defined values
      • HUNDRED = 10 ** 2
      • THOUSAND = 10 ** 3
      • MILLION = 10 ** 6
      • BILLION = 10 ** 9
      • Visibility is internal
      • State mutability is constant
      • (Non-critical) Ensure they match with config
  • Address Declaration
    • Address Constants
      • Address is not in the ChainLog
      • Variable name matches corresponding entry in an addresses.sol file
      • Address value matches corresponding entry in an addresses.sol file
      • Visibility is internal
      • State mutability is constant
    • Address Immutables
      • Use the DssExecLib Core Address Helpers instead of getChangelogAddress() whenever possible (e.g. DssExecLib.vat())
      • Follow pattern address internal immutable KEY_NAME = DssExecLib.getChangelogAddress(KEY_NAME)
      • Variable name matches ChainLog key name
        • Exceptions must follow archive patterns (e.g. MKR for MCD_GOV)
      • Instantiate as address type
        • Exceptions must follow archive patterns (e.g. GemLike for MKR)
      • Visibility is internal
      • State mutability is immutable
  • String Constants
    • IPFS hashes or DAO resolutions
      • IPFS hashes and DAO resolutions match the exec doc
      • The variable is named dao_resolutions
      • if multiple dao resolutions are present
        • they are located in the same string constant and are comma separated
        • The comment above the the variable states: Comma-separated list of DAO resolutions IPFS hashes
    • Must be TOP LEVEL (or ds pause will reject)
    • Name of the variable matches purpose per EXEC DOC
    • Visibility is public
    • State mutability MUST BE constant
  • Deployed Contracts (not yet on chainlog or new to chainlog)
    • Verified on etherscan
    • Optimizations match Repo
    • GNU AGPLv3 license
    • Constructor args ok (e.g. vat, dai, dog, ...)
    • Wards ok (pause proxy relied, deployer denied)
      • MCD_ESM is already relied / being relied in this spell (as approved by GovAlpha) in order to allow de-authing the pause proxy during Emergency Shutdown, via denyProxy.
    • Matches corresponding github source code (i.e. diffcheck via vscode code --diff etherscan.sol github.sol)
    • Ensure deployer address is included into addresses_deployers.sol (to keep up to date)
  • Core System Parameter Changes
  • Debt Ceiling Changes
  • Onboarding (insert relevant checklists inline here)
  • Offboarding (Lerp mat)
    • 1st Stage Spell Actions
      • Remove Ilk from Autoline
      • Set Ilks Debt Ceilings to 0
      • Cache Ilks line to Reduce in the Global Debt Ceiling
      • Decrease Global Debt Ceiling by Total Amount of Offboarded Ilks line Cached
    • 2nd Stage Spell Actions
      • Set Ilk Liquidation Penalty chop to 0
      • Set Keeper Incentive Flat Rate tip to 0
      • Check IF chip is required to be adjusted as well
      • Use DssExecLib.linearInterpolation
        • name Format matches "XXX-A Offboarding"
        • target matches spotter
        • ilk Format matches "XXX-A"
        • what matches mat
        • startTime matches block.timestamp
        • start matches Var CURRENT_XXX_A_MAT
        • end matches Var TARGET_XXX_A_MAT (Match Exec Sheet & Risk Computations)
          • Check IF Target mat Covers All Remaining Vaults CR times Risk Multiplier Factor
        • duration matches Exec Sheet
  • RWA Updates
    • doc (Using the _updateDoc helper or otherwise)
      • init the RwaLiquidationOracle to reset the doc
      • Sanity Check pip must be set (not the zero address)
      • ilk follows format "RWAXXX-A"
      • val price ignored (0) if init has already been called
      • doc new legal document (IPFS HASH) matches Doc (or Forum Post)
      • tau parameter used is the old tau value
    • Autoline (line) + Liquidation Oracle Price Bump (val)
      • Enable Autoline
        • ilk follows format "RWAXXX-A"
        • line (max debt ceiling)
        • gap
        • ttl
      • bump RwaLiquidationOracle with new computed increased price (val)
        • ensure val is set accordingly with autoline max debt ceiling (line)
        • val should enable DAI to be drawn over the loan period while taking into account the configured ink amount, interest rate and liquidation ratio (see below)
          • New val is calculated with line * [(1 + duty) ** years] * mat - rounded up - and makes sense in context of the rate mechanism. Minimum duration is usually in the Exec Doc of the spell with the RWAXXX ilk onboarding.
          • Comment explaining val formula (Debt ceiling * [ (1 + RWA stability fee ) ^ (minimum deal duration in years) ] * liquidation ratio) is present
          • Accompanying comment above bump line in format // XXM * 1.XX^X * X.XX as a WAD corresponding to the val calculation formula (e.g. // 15M * 1.03^2 * 1.00 as a WAD) is present along with the calculation formula on the line above
          • IF combining val of multiple RWA ilks being combined, val calculation is done once per ilk and added to make the total, with workings provided in code comments. The existing val value can be retrieved by calling read() on PIP_RWAXX and converting the result into decimal.
      • Poke spotter to pull in the new price
    • Debt Ceiling (line) + Liquidation Oracle Price Bump (val)
      • Increase Ilk Debt Ceiling (set DC + increase Global DC)
      • bump RwaLiquidationOracle with new computed increased price (val)
        • val should enable DAI to be drawn over the loan period while taking into account the configured ink amount, interest rate and liquidation ratio (see below)
          • New val is calculated with line * [(1 + duty) ** years] * mat - rounded up - and makes sense in context of the rate mechanism. Minimum duration is usually in the Exec Doc of the spell with the RWAXXX ilk onboarding.
          • Comment explaining val formula (Debt ceiling * [ (1 + RWA stability fee ) ^ (minimum deal duration in years) ] * liquidation ratio) is present
          • Accompanying comment above bump line in format // XXM * 1.XX^X * X.XX as a WAD corresponding to the val calculation formula (e.g. // 15M * 1.03^2 * 1.00 as a WAD) is present along with the calculation formula on the line above
          • IF combining val of multiple RWA ilks being combined, val calculation is done once per ilk and added to make the total, with workings provided in code comments. The existing val value can be retrieved by calling read() on PIP_RWAXX and converting the result into decimal.
      • Poke spotter to pull in the new price
  • Payments
    • Payments are not crafted or reviewed on Goerli (comments may be present)
  • SubDAO Content
    • SubDAO SubProxy spell execution
      • SubDAO spell address matches Exec Sheet (comment or otherwise)
      • IF SubDAO spell deployer is a smart contract (e.g. multisig or factory)
        • Ensure the deployer address is in addresses_deployers.sol as an entry
      • Executed using ProxyLike(SUBDAO_PROXY).exec(SUBDAO_SPELL, abi.encodeWithSignature("execute()"));
      • Execution is NOT delegate call
      • Reviewer Note: Gas cost may be very high as SubDAO spells execute within the main cast execution. (Also note that low level call gas estimation is not done by our scripts)
    • Maker Core (main spell) SubDAO actions (i.e. operate in Pause Proxy DelegateCall context)
      • No SubDAO contract being interacted with is authed on a core contract like vat, etc. (Check comprehensively where the risk is high)
      • SubDAO contract licensing and optimizations generally do not matter (except where they pose a security risk)
      • SubDAO contracts and all libraries / dependencies are verified (Blocking if not true)
      • Upgradable SubDAO contracts
        • Any upgradable contracts have the PAUSE_PROXY as their admin (i.e. the party that can upgrade)
          • Any upgradable SubDAO contracts with an admin that is not PAUSE_PROXY are not authed on any core contracts (Blocking)
      • All SubDAO content addresses (i.e. provided contract addresses or EOAs) present in the Maker Core spell are present in the Exec Sheet and are correct. SubDAO addresses being authed or given any permissions and addresses being called must be confirmed by the SubDAO spell team.
      • SubDAO actions match Exec Sheet (only where inline with main spell code) and do not affect core contracts
      • Core contract knock-on actions (such as offboarding or setting DC to 0) are present in the Exec Sheet and match the code
      • External calls for SubDAO content are NOT delegate call
      • Code does not have untoward behavior within the scope of Maker Core Contracts (e.g. up to the SubDAO proxy)
  • External Contracts Calls (Not SubDAOs, e.g. Starknet)
    • Target Contract don't block spell execution
    • External call is NOT delegate call
    • Target Contract doesn't have permissions on the Vat
    • Target Contract doesn't do anything untoward (e.g. interacting with unsafe contracts)
    • Contracts deployed via CREATE2 (e.g. if it looks like a vanity address) do not have selfdestruct in their code
    • MCD Pause Proxy doesn't give any approvals
    • All possible actions of the Target Contract are documented
    • Target contract is not upgradable
    • Target Contract is included in the ChainLog
    • Test Coverage is comprehensive
  • ChainLog
    • Increment ChainLog version based on update type
      • Major -> New Vat (++.0.0)
      • Minor -> Core Module (DSS) Update (e.g. Flapper) (0.++.0)
      • Patch -> Collateral addition or addition/modification (0.0.++)
  • addresses_goerli.sol matches spell code
  • Ensure every spell variable is declared as public/internal
  • Ensure every spell variable is used in the spell (no unused variables)
  • Spell actions match the corresponding Exec Sheet
  • Tests
    • Ensure each spell action has sufficient test coverage
      • Non-Scope Defined Parameter Changes - WBTC DC-IAM Changes
        testGeneral
      • Stability Fee Changes
        testGeneral
      • Initial RETH-A Offboarding
        testGeneral
      • Reconfiguring Andromeda RWA015-A
        testGeneral / :white_check_mark: test_RWA015_Update
      • Reconfiguring Clydesdale RWA007-A
        testGeneral / :white_check_mark: test_RWA0007_Update
      • Set up Governance Facilitator Streams ⚠️ Ignored on Goerli
      • USDP-PSM Facilitation Incentives ⚠️ No test coverage, not mandatory. Maybe consider adding it.
    • Ensure every test function is declared as public if enabled or private if disabled
    • Ensure that the DssExecLib.address file is not being modified by the spell PR
    • Check all CI tests are passing as at the latest commit
      5fbcea5
    • Check all tests are passing locally using make test
      • Ensure that only ETH_RPC_URL is being used from env (i.e. no match, block or similar are active in your env)
Running 2 tests for src/test/starknet.t.sol:StarknetTests
[PASS] testStarknet() (gas: 1163896)
[PASS] testStarknetSpell() (gas: 2324)
Test result: ok. 2 passed; 0 failed; 0 skipped; finished in 33.12s

Running 19 tests for src/DssSpell.t.sol:DssSpellTest
[PASS] testAuth() (gas: 52178792)
[PASS] testAuthInSources() (gas: 8190157)
[PASS] testBytecodeMatches() (gas: 2289317)
[PASS] testCastCost() (gas: 1004790)
[PASS] testChainlogValues() (gas: 9380772)
[PASS] testChainlogVersionBump() (gas: 4431903)
[PASS] testContractSize() (gas: 8984)
[PASS] testDeployCost() (gas: 2275165)
[PASS] testFailNotScheduled() (gas: 14375)
[PASS] testFailTooEarly() (gas: 13607)
[PASS] testFailTooLate() (gas: 13606)
[PASS] testFailWrongDay() (gas: 13585)
[PASS] testGeneral() (gas: 38122659)
[PASS] testNextCastTime() (gas: 364790)
[PASS] testOnTime() (gas: 1000429)
[PASS] testPSMs() (gas: 2287136)
[PASS] testUseEta() (gas: 363519)
[PASS] test_RWA007_Update() (gas: 1225706)
[PASS] test_RWA015_Update() (gas: 1193749)
Test result: ok. 19 passed; 0 failed; 0 skipped; finished in 1091.67s
 
Ran 2 test suites: 21 tests passed, 0 failed, 0 skipped (21 total tests)

@SidestreamSweatyPumpkin
Copy link
Contributor

Good to deploy

Local test rerun:

./scripts/test-dssspell-forge.sh no-match="" match="" block=""
Using DssExecLib at: 0x122F6c0Dcd898b4a07310E92c3eAE5D7Ce0c8bb6
[⠔] Compiling...
No files changed, compilation skipped

Running 2 tests for src/test/starknet.t.sol:StarknetTests
[PASS] testStarknet() (gas: 1163551)
[PASS] testStarknetSpell() (gas: 2324)
Test result: ok. 2 passed; 0 failed; 0 skipped; finished in 261.73s

Running 19 tests for src/DssSpell.t.sol:DssSpellTest
[PASS] testAuth() (gas: 52178447)
[PASS] testAuthInSources() (gas: 8189812)
[PASS] testBytecodeMatches() (gas: 2289317)
[PASS] testCastCost() (gas: 1004445)
[PASS] testChainlogValues() (gas: 9380427)
[PASS] testChainlogVersionBump() (gas: 4431558)
[PASS] testContractSize() (gas: 8984)
[PASS] testDeployCost() (gas: 2275165)
[PASS] testFailNotScheduled() (gas: 14375)
[PASS] testFailTooEarly() (gas: 13607)
[PASS] testFailTooLate() (gas: 13606)
[PASS] testFailWrongDay() (gas: 13585)
[PASS] testGeneral() (gas: 38122314)
[PASS] testNextCastTime() (gas: 364790)
[PASS] testOnTime() (gas: 1000084)
[PASS] testPSMs() (gas: 2286860)
[PASS] testUseEta() (gas: 363519)
[PASS] test_RWA007_Update() (gas: 1253158)
[PASS] test_RWA015_Update() (gas: 1255198)
Test result: ok. 19 passed; 0 failed; 0 skipped; finished in 788.73s

Ran 2 test suites: 21 tests passed, 0 failed, 0 skipped (21 total tests)

@SidestreamSweatyPumpkin
Copy link
Contributor

Good to cast

Deployed Stage

  • Deployed Spell is Verified
    • Optimization Enabled: No
    • Other Settings: default evmVersion, GNU AGPLv3 license
  • Deployed Spell Code matches GitHub
    • diffcheck etherscan source against spell PR (via make diff-deployed-spell)
  • Deployed Spell Etherscan Checks
    • automated checks via make check-deployed-spell
      • verified
      • license type matches
      • solc version matches
      • optimizations are disabled
      • dss-exec-lib library address used (under 'Libraries Used') matches the hardcoded local DssExecLib.address file
        • Check again that the PR did not modify the DssExecLib.address file (e.g. look under the 'Files Changed' PR tab, etc.)
      • deployed_spell_created matches deployment timestamp
      • deployed_spell_block matches deployment block number
    • manual checks
      • Ensure make deploy-info tx=<tx> matches config
        • deployed_spell_created timestamp
        • deployed_spell_block block number
      • Ensure Etherscan Libraries Used matches DssExecLib Latest Release
      • (For your tests to be accurate) git submodule hash matches dss-exec-lib latest release's tag commit and inspect diffs if doesn't match to ensure expected behaviour (Currently Non-Critical pending the next DssExecLib release, double check that the ExecLib used by the contract matches the latest release)
  • Archive matches src
    • make diff-archive-spell for current date or or date="YYYY-MM-DD" make diff-archive-spell (date as per cast timestamp)
    • Ensure date corresponds to target Exec Sheet date
  • Tests
    • Ensure that the DssExecLib.address file is not being modified by the spell PR
    • Check all CI tests are passing as at the latest commit
      d931479
    • Check all tests are passing locally using make test
      • Ensure that only ETH_RPC_URL is being used from env (i.e. no match, block or similar are active in your env)
./scripts/test-dssspell-forge.sh no-match="" match="" block=""
Using DssExecLib at: 0x122F6c0Dcd898b4a07310E92c3eAE5D7Ce0c8bb6
[⠔] Compiling...
[⠊] Compiling 4 files with 0.8.16
[⠒] Solc 0.8.16 finished in 1.53s
Compiler run successful!

Running 2 tests for src/test/starknet.t.sol:StarknetTests
[PASS] testStarknet() (gas: 1164126)
[PASS] testStarknetSpell() (gas: 2324)
Test result: ok. 2 passed; 0 failed; 0 skipped; finished in 256.92s

Running 19 tests for src/DssSpell.t.sol:DssSpellTest
[PASS] testAuth() (gas: 52179022)
[PASS] testAuthInSources() (gas: 8190387)
[PASS] testBytecodeMatches() (gas: 2289317)
[PASS] testCastCost() (gas: 1005020)
[PASS] testChainlogValues() (gas: 9381002)
[PASS] testChainlogVersionBump() (gas: 4432133)
[PASS] testContractSize() (gas: 8984)
[PASS] testDeployCost() (gas: 2275165)
[PASS] testFailNotScheduled() (gas: 14375)
[PASS] testFailTooEarly() (gas: 13607)
[PASS] testFailTooLate() (gas: 13606)
[PASS] testFailWrongDay() (gas: 13585)
[PASS] testGeneral() (gas: 38124986)
[PASS] testNextCastTime() (gas: 364790)
[PASS] testOnTime() (gas: 1000659)
[PASS] testPSMs() (gas: 2287320)
[PASS] testUseEta() (gas: 363519)
[PASS] test_RWA007_Update() (gas: 1253043)
[PASS] test_RWA015_Update() (gas: 1255543)
Test result: ok. 19 passed; 0 failed; 0 skipped; finished in 784.33s

Ran 2 test suites: 21 tests passed, 0 failed, 0 skipped (21 total tests)

@amusingaxl
Copy link
Collaborator

amusingaxl commented Oct 12, 2023

Good to cast.

Deployed Stage

  • Deployed Spell is Verified
    • Optimization Enabled: No
    • Other Settings: default evmVersion, GNU AGPLv3 license
  • Deployed Spell Code matches GitHub
    • diffcheck etherscan source against spell PR (via make diff-deployed-spell)
  • Deployed Spell Etherscan Checks
    • automated checks via make check-deployed-spell
      • verified
      • license type matches
      • solc version matches
      • optimizations are disabled
      • dss-exec-lib library address used (under 'Libraries Used') matches the hardcoded local DssExecLib.address file
        • Check again that the PR did not modify the DssExecLib.address file (e.g. look under the 'Files Changed' PR tab, etc.)
      • deployed_spell_created matches deployment timestamp
      • deployed_spell_block matches deployment block number
    • manual checks
      • Ensure make deploy-info tx=<tx> matches config
        • deployed_spell_created timestamp
        • deployed_spell_block block number
      • Ensure Etherscan Libraries Used matches DssExecLib Latest Release
      • (For your tests to be accurate) git submodule hash matches dss-exec-lib latest release's tag commit and inspect diffs if doesn't match to ensure expected behaviour (Currently Non-Critical pending the next DssExecLib release, double check that the ExecLib used by the contract matches the latest release)
  • Archive matches src
    • make diff-archive-spell for current date or or date="YYYY-MM-DD" make diff-archive-spell (date as per cast timestamp)
    • Ensure date corresponds to target Exec Sheet date
  • Tests
    • Ensure that the DssExecLib.address file is not being modified by the spell PR
    • Check all CI tests are passing as at the latest commit
      d931479
    • Check all tests are passing locally using make test
      • Ensure that only ETH_RPC_URL is being used from env (i.e. no match, block or similar are active in your env)
Using DssExecLib at: 0x122F6c0Dcd898b4a07310E92c3eAE5D7Ce0c8bb6
[] Compiling...
[] Compiling 4 files with 0.8.16
[] Solc 0.8.16 finished in 1.47s
Compiler run successful!

Running 2 tests for src/test/starknet.t.sol:StarknetTests
[PASS] testStarknet() (gas: 1163896)
[PASS] testStarknetSpell() (gas: 2324)
Test result: ok. 2 passed; 0 failed; 0 skipped; finished in 32.86s

Running 17 tests for src/DssSpell.t.sol:DssSpellTest
[PASS] testBytecodeMatches() (gas: 2289317)
[PASS] testCastCost() (gas: 1004790)
[PASS] testChainlogValues() (gas: 9380772)
[PASS] testChainlogVersionBump() (gas: 4431903)
[PASS] testContractSize() (gas: 8984)
[PASS] testDeployCost() (gas: 2275165)
[PASS] testFailNotScheduled() (gas: 14375)
[PASS] testFailTooEarly() (gas: 13607)
[PASS] testFailTooLate() (gas: 13606)
[PASS] testFailWrongDay() (gas: 13585)
[PASS] testGeneral() (gas: 38124756)
[PASS] testNextCastTime() (gas: 364790)
[PASS] testOnTime() (gas: 1000429)
[PASS] testPSMs() (gas: 2287136)
[PASS] testUseEta() (gas: 363519)
[PASS] test_RWA007_Update() (gas: 1253043)
[PASS] test_RWA015_Update() (gas: 1254968)
Test result: ok. 17 passed; 0 failed; 0 skipped; finished in 358.20s

Ran 2 test suites: 19 tests passed, 0 failed, 0 skipped (19 total tests)

@SidestreamSweatyPumpkin
Copy link
Contributor

SidestreamSweatyPumpkin commented Oct 12, 2023

Cast and Merge Stage

  • Spell is Cast
    • Check Cast Trace (via EthTx)
      • Ensure no reverts are present that block execution
        • Inspect low level call reverts if expected
      • Ensure all actions are executed and no out-of-gas errors are present
  • Ensure that no commits or changes have occurred since the spell was deployed and archived
  • Approve spell PR for merge via 'Approve' review option

Copy link
Collaborator

@amusingaxl amusingaxl left a comment

Choose a reason for hiding this comment

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

Good to merge 🥇

Cast and Merge Stage

  • Spell is Cast
    • Check Cast Trace (via EthTx)
      • Ensure no reverts are present that block execution
        • Inspect low level call reverts if expected
      • Ensure all actions are executed and no out-of-gas errors are present
  • Ensure that no commits or changes have occurred since the spell was deployed and archived
  • Approve spell PR for merge via 'Approve' review option

@0xdecr1pto 0xdecr1pto merged commit 4681055 into master Oct 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants