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

Spell 2023-11-15 #235

Merged
merged 5 commits into from
Nov 16, 2023
Merged

Spell 2023-11-15 #235

merged 5 commits into from
Nov 16, 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 self-assigned this Nov 13, 2023
SidestreamSweatyPumpkin
SidestreamSweatyPumpkin suggested changes Nov 14, 2023
View reviewed changes
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.

Some minor comments about comments :)

src/DssSpell.sol Outdated
// address internal constant SPARK_SPELL = address(0);

function actions() public override {
// ---------- Trigger Spark Proxy Spell ----------
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Section title name
The exec sheet contains different title for the action: Spark Proxy-Spell

src/DssSpell.sol Outdated

// ---------- Housekeeping - GUSD & USDP - Add Jar & Conduit Contracts to Chainlog ----------
// Forum: https://forum.makerdao.com/t/proposed-housekeeping-items-upcoming-executive-spell-2023-11-01/22477
// ----- Spark D3M DC Increase -----
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Section title
The exec sheet contains different title for the section: Adjust Spark Protocol D3M Maximum Debt Ceiling

src/DssSpell.sol Outdated
Comment on lines 70 to 77
// Note: ignored on Goerli
// Forum: https://forum.makerdao.com/t/proposal-to-adjust-sparklend-parameters/22542
// Poll: https://vote.makerdao.com/polling/QmaBLbxP
// Poll: https://vote.makerdao.com/polling/QmZwRgr5
// Poll: https://vote.makerdao.com/polling/QmQPrHsm
// Poll: https://vote.makerdao.com/polling/QmRG9qUp
// Poll: https://vote.makerdao.com/polling/QmQjKpbU
// ProxyLike(SPARK_PROXY).exec(SPARK_SPELL, abi.encodeWithSignature("execute()"));
Copy link
Contributor

Choose a reason for hiding this comment

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

thought: Action title
The action titles are missing before the ProxyLike invocation on L77. Perhaps the batch of titles from the exec should be copied over here just like the poll comments.

src/DssSpell.sol Outdated
Comment on lines 22 to 25
// Note: ignored on Goerli
// interface ProxyLike {
// function exec(address target, bytes calldata args) external payable returns (bytes memory out);
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: Dead code
This code is not used, in previous spells sometimes it's included, sometimes not. I would prefer to remove it since:

  • validating and checking this has no point
  • info contained in the comment (code) is not relevant to the task at hand (which is perform actions on goerli that can be performed)

@0xdecr1pto 0xdecr1pto requested a review from 0xp3th1um November 14, 2023 16:58
@0xdecr1pto 0xdecr1pto marked this pull request as ready for review November 14, 2023 16:58
@SidestreamSweatyPumpkin
Copy link
Contributor

issue: October delegate compensation is missing
The exec sheet was updated and there's a new section here

Please, add the comment section to the source code

@0xp3th1um
Copy link
Collaborator

0xp3th1um commented Nov 15, 2023 •
edited
Loading

Good to deploy!

Goerli Executive Spell Review Checklist

Goerli 2023-11-15

Spell Actions (Per Exec Sheet):

⚠️ SKIP ON GOERLI Spark Proxy-Spell

  • Pass Spark Proxy Spelll at 0xDa69603384Ef825E52FD5B8bEF656ff62Fe19703

⚠️ SKIP ON GOERLI Gnosis Chain - Increase wstETH Supply Cap to 10,000 wstETH

⚠️ SKIP ON GOERLI Ethereum - Set DAI Market Maximum Loan-to-Value to Zero Percent

⚠️ SKIP ON GOERLI Ethereum - Reactivate WBTC and Optimize Parameters for Current Market Conditions

⚠️ SKIP ON GOERLI Ethereum - Increase rETH & wstETH Supply Caps

⚠️ SKIP ON GOERLI Ethereum & Gnosis Chain - Adjust ETH Market Interest Rate Models

Adjust Spark Protocol D3M Maximum Debt Ceiling

  • Increase the DIRECT-SPARK-DAI Maximum Debt Ceiling from 400 million DAI to 800 million DAI.

⚠️ SKIP ON GOERLI Launch Project Funds Transfer

  • Launch Project - 2200000.00 DAI - 0x3C5142F28567E6a0F172fd0BaaF1f2847f49D02F
  • Launch Project - 500.00 MKR - 0x3C5142F28567E6a0F172fd0BaaF1f2847f49D02F

Pass HVB Resolutions

  • Updated Standing Instructions to Escrow Agent - QmWVWXckY482WLTtCFv3x45DFioV1K8mfRM3FVrodqUDud
  • Approval of New Payment Instructions to Galaxy Digital Trading Cayman LLC - QmSbwqULr66CiCvNips93vwTrvoTe4i2rJVmho7QfmyqZG

⚠️ SKIP ON GOERLI Whistleblower Bounty

  • VeniceTree - 27.78 MKR - 0xCDDd2A697d472d1e8a0B1B188646c756d097b058

⚠️ SKIP ON GOERLI October Delegate Compensation

  • 0xDefensor - 41.67 MKR - 0x9542b441d65B6BF4dDdd3d4D2a66D8dCB9EE07a9
  • TRUE NAME - 41.67 MKR - 0x612F7924c367575a0Edf21333D96b15F1B345A5d
  • BONAPUBLICA - 41.67 MKR - 0x167c1a762B08D7e78dbF8f24e5C3f1Ab415021D3
  • Cloaky - 41.67 MKR - 0x869b6d5d8FA7f4FFdaCA4D23FFE0735c5eD1F818
  • Navigator - 40.33 MKR - 0x11406a9CC2e37425F15f920F494A51133ac93072
  • vigilant - 13.89 MKR - 0x2474937cB55500601BCCE9f4cb0A0A72Dc226F61
  • UPMaker - 13.89 MKR - 0xbB819DF169670DC71A16F58F55956FE642cc6BcD
  • PBG - 13.89 MKR - 0x8D4df847dB7FfE0B46AF084fE031F7691C6478c2
  • PALC - 13.44 MKR - 0x78Deac4F87BD8007b9cb56B8d53889ed5374e83A
  • BLUE - 12.97 MKR - 0xb6C09680D822F162449cdFB8248a7D3FC26Ec9Bf
  • JAG - 4.45 MKR - 0x58D1ec57E4294E4fe650D1CB12b96AE34349556f

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'
  • Ensure the comments inside the spell action are correct:
    • Every Section text from the Exec Sheet should be copied as comment to the spell code (above the code segment that implements the action)
      • is surrounded by the set of dashes (E.g. // ----- Section text -----)
    • Every Instruction text Exec Sheet should be:
      • copied to the spell code as // Instruction text
      • have newline above it
    • Every Reasoning URL and Authority URL from the Exec Sheet should be present under relevant section or instruction in the spell code (depending on which row the link is present)
      • Every Reasoning URL and Authority URL should have prefix derived from the url itself
        • // Executive Vote: if URL starts with https://vote.makerdao.com/executive/
        • // Poll: if URL starts with https://vote.makerdao.com/polling/
        • // Forum: if URL starts with https://forum.makerdao.com/t/
        • // MIP: if URL starts with https://mips.makerdao.com/mips/details/
    • If action in the spell doesn't have relevant instruction (e.g.: chainlog version bump), it should have explanation prefixed with // Note:
    • If an instruction can not be taken, it should have a comment under the instruction prefixed with // Note: (e.g.: // Note: Payments are skipped on goerli)
  • 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'
    • Library Checks
      ℹ️ Skipped
      • 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 Governance Facilitators) 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
    • Debt Ceiling changes (line)
      • Autoline (line) + Liquidation Oracle Price Bump (val)
        • Enable Autoline
          • ilk follows format "RWAXXX-A"
          • line (max debt ceiling)
          • gap
          • ttl
      • 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)
        • 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
    • Soft Liquidation (tell)
      • Call RwaLiquidationOracle.tell(ilk)
      • IF RWAXX_A_INPUT_CONDUIT is an instance of TinlakeMgr (it is a Centrifuge integration)
        • Call TinlakeMgr.tell() to prevent further TIN redemptions in the Centrifuge pool.
  • 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
      • Adjust Spark Protocol D3M Maximum Debt Ceiling
        ✅ testGeneral()
    • 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
      d63f8fb
    • 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 103 files with 0.8.16
[⠊] Solc 0.8.16 finished in 7.61s
Compiler run successful!

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

Running 17 tests for src/DssSpell.t.sol:DssSpellTest
[PASS] testAuth() (gas: 51607426)
[PASS] testAuthInSources() (gas: 7618791)
[PASS] testBytecodeMatches() (gas: 1431738)
[PASS] testCastCost() (gas: 414424)
[PASS] testChainlogValues() (gas: 8794428)
[PASS] testChainlogVersionBump() (gas: 3845515)
[PASS] testContractSize() (gas: 8962)
[PASS] testDeployCost() (gas: 1419962)
[PASS] testFailNotScheduled() (gas: 14397)
[PASS] testFailTooEarly() (gas: 13585)
[PASS] testFailTooLate() (gas: 13606)
[PASS] testFailWrongDay() (gas: 13585)
[PASS] testGeneral() (gas: 36829747)
[PASS] testNextCastTime() (gas: 364790)
[PASS] testOnTime() (gas: 410063)
[PASS] testPSMs() (gas: 1825244)
[PASS] testUseEta() (gas: 363541)
Test result: ok. 17 passed; 0 failed; 0 skipped; finished in 1004.02s

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

@SidestreamSweatyPumpkin
Copy link
Contributor

Good to deploy 👍

Goerli 2023-11-15

Spell Actions (Per Exec Sheet):

  • SKIP ON GOERLI Spark Proxy-Spell
    • Gnosis Chain - Increase wstETH Supply Cap to 10,000 wstETH
    • Ethereum - Set DAI Market Maximum Loan-to-Value to Zero Percent
    • Ethereum - Reactivate WBTC and Optimize Parameters for Current Market Conditions
    • Ethereum - Increase rETH & wstETH Supply Caps
    • Ethereum & Gnosis Chain - Adjust ETH Market Interest Rate Models
  • Adjust Spark Protocol D3M Maximum Debt Ceiling
    • Increase the DIRECT-SPARK-DAI Maximum Debt Ceiling from 400 million DAI to 800 million DAI.
  • SKIP ON GOERLI Launch Project Funds Transfer
    • Launch Project - 2200000.00 DAI - 0x3C5142F28567E6a0F172fd0BaaF1f2847f49D02F
    • Launch Project - 500.00 MKR - 0x3C5142F28567E6a0F172fd0BaaF1f2847f49D02F
  • Pass HVB Resolutions
    • Updated Standing Instructions to Escrow Agent - QmWVWXckY482WLTtCFv3x45DFioV1K8mfRM3FVrodqUDud
    • Approval of New Payment Instructions to Galaxy Digital Trading Cayman LLC - QmSbwqULr66CiCvNips93vwTrvoTe4i2rJVmho7QfmyqZG
  • SKIP ON GOERLI Whistleblower Bounty
    • VeniceTree - 27.78 MKR - 0xCDDd2A697d472d1e8a0B1B188646c756d097b058
  • SKIP ON GOERLI October Delegate Compensation
    • 0xDefensor - 41.67 MKR - 0x9542b441d65B6BF4dDdd3d4D2a66D8dCB9EE07a9
    • TRUE NAME - 41.67 MKR - 0x612F7924c367575a0Edf21333D96b15F1B345A5d
    • BONAPUBLICA - 41.67 MKR - 0x167c1a762B08D7e78dbF8f24e5C3f1Ab415021D3
    • Cloaky - 41.67 MKR - 0x869b6d5d8FA7f4FFdaCA4D23FFE0735c5eD1F818
    • Navigator - 40.33 MKR - 0x11406a9CC2e37425F15f920F494A51133ac93072
    • vigilant - 13.89 MKR - 0x2474937cB55500601BCCE9f4cb0A0A72Dc226F61
    • UPMaker - 13.89 MKR - 0xbB819DF169670DC71A16F58F55956FE642cc6BcD
    • PBG - 13.89 MKR - 0x8D4df847dB7FfE0B46AF084fE031F7691C6478c2
    • PALC - 13.44 MKR - 0x78Deac4F87BD8007b9cb56B8d53889ed5374e83A
    • BLUE - 12.97 MKR - 0xb6C09680D822F162449cdFB8248a7D3FC26Ec9Bf
    • JAG - 4.45 MKR - 0x58D1ec57E4294E4fe650D1CB12b96AE34349556f

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'
  • Ensure the comments inside the spell action are correct:
    • Every Section text from the Exec Sheet should be copied as comment to the spell code (above the code segment that implements the action)
      • is surrounded by the set of dashes (E.g. // ----- Section text -----)
    • Every Instruction text Exec Sheet should be:
      • copied to the spell code as // Instruction text
      • have newline above it
    • Every Reasoning URL and Authority URL from the Exec Sheet should be present under relevant section or instruction in the spell code (depending on which row the link is present)
      • Every Reasoning URL and Authority URL should have prefix derived from the url itself
        • // Executive Vote: if URL starts with https://vote.makerdao.com/executive/
        • // Poll: if URL starts with https://vote.makerdao.com/polling/
        • // Forum: if URL starts with https://forum.makerdao.com/t/
        • // MIP: if URL starts with https://mips.makerdao.com/mips/details/
    • If action in the spell doesn't have relevant instruction (e.g.: chainlog version bump), it should have explanation prefixed with // Note:
    • If an instruction can not be taken, it should have a comment under the instruction prefixed with // Note: (e.g.: // Note: Payments are skipped on goerli)
  • 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'
    • Library Checks
      • dss-exec-lib
        • If submodule upgrades are present make sure dss-exec-lib is synced as well
          • not present
        • (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 Governance Facilitators) 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
    • Debt Ceiling changes (line)
      • Autoline (line) + Liquidation Oracle Price Bump (val)
        • Enable Autoline
          • ilk follows format "RWAXXX-A"
          • line (max debt ceiling)
          • gap
          • ttl
      • 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)
        • 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
    • Soft Liquidation (tell)
      • Call RwaLiquidationOracle.tell(ilk)
      • IF RWAXX_A_INPUT_CONDUIT is an instance of TinlakeMgr (it is a Centrifuge integration)
        • Call TinlakeMgr.tell() to prevent further TIN redemptions in the Centrifuge pool.
  • Payments
    • Payments are not crafted or reviewed on Goerli (comments may be present)
  • SubDAO content
    🚧 NOTE: won't be crafted on goerli since SPARK has dropped goerli support
    • 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
      • not tested: resolutions
      • config file tests the debt ceiling
    • 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
      fe4a3d1
    • 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...
No files changed, compilation skipped

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

Running 17 tests for src/DssSpell.t.sol:DssSpellTest
[PASS] testAuth() (gas: 51607426)
[PASS] testAuthInSources() (gas: 7618791)
[PASS] testBytecodeMatches() (gas: 1431738)
[PASS] testCastCost() (gas: 414424)
[PASS] testChainlogValues() (gas: 8794428)
[PASS] testChainlogVersionBump() (gas: 3845515)
[PASS] testContractSize() (gas: 8962)
[PASS] testDeployCost() (gas: 1419962)
[PASS] testFailNotScheduled() (gas: 14397)
[PASS] testFailTooEarly() (gas: 13585)
[PASS] testFailTooLate() (gas: 13606)
[PASS] testFailWrongDay() (gas: 13585)
[PASS] testGeneral() (gas: 36829747)
[PASS] testNextCastTime() (gas: 364790)
[PASS] testOnTime() (gas: 410063)
[PASS] testPSMs() (gas: 1825244)
[PASS] testUseEta() (gas: 363541)
Test result: ok. 17 passed; 0 failed; 0 skipped; finished in 832.25s

Ran 2 test suites: 19 tests passed, 0 failed, 0 skipped (19 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
      NOTE 🚧: london version on etherscan, but it's fine according to the previous PRs
  • 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
      ab4414e
    • 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 5 files with 0.8.16
[⠔] Solc 0.8.16 finished in 1.51s
Compiler run successful!

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

Running 17 tests for src/DssSpell.t.sol:DssSpellTest
[PASS] testAuth() (gas: 51607426)
[PASS] testAuthInSources() (gas: 7618791)
[PASS] testBytecodeMatches() (gas: 1431738)
[PASS] testCastCost() (gas: 414424)
[PASS] testChainlogValues() (gas: 8794428)
[PASS] testChainlogVersionBump() (gas: 3845515)
[PASS] testContractSize() (gas: 8962)
[PASS] testDeployCost() (gas: 1419962)
[PASS] testFailNotScheduled() (gas: 14397)
[PASS] testFailTooEarly() (gas: 13585)
[PASS] testFailTooLate() (gas: 13606)
[PASS] testFailWrongDay() (gas: 13585)
[PASS] testGeneral() (gas: 36831844)
[PASS] testNextCastTime() (gas: 364790)
[PASS] testOnTime() (gas: 410063)
[PASS] testPSMs() (gas: 1825244)
[PASS] testUseEta() (gas: 363541)
Test result: ok. 17 passed; 0 failed; 0 skipped; finished in 830.37s

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

@0xp3th1um
Copy link
Collaborator

TLDR: 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)
    • ⚠️ Part of the verified code is different as in previous spells: part of the DssExecLib is replaced {}
  • 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
        ⚠️ The script gives a wrong result on Mac, checked manually below
      • deployed_spell_block matches deployment block number
        ⚠️ The script gives a wrong result on Mac, checked manually below
    • 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)
      • ℹ️ There is current mismatch as in previous spells.
  • 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
      ab4414e
    • 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...
No files changed, compilation skipped

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


Running 17 tests for src/DssSpell.t.sol:DssSpellTest
[PASS] testAuth() (gas: 51607426)
[PASS] testAuthInSources() (gas: 7618791)
[PASS] testBytecodeMatches() (gas: 1431738)
[PASS] testCastCost() (gas: 414424)
[PASS] testChainlogValues() (gas: 8794428)
[PASS] testChainlogVersionBump() (gas: 3845515)
[PASS] testContractSize() (gas: 8962)
[PASS] testDeployCost() (gas: 1419962)
[PASS] testFailNotScheduled() (gas: 14397)
[PASS] testFailTooEarly() (gas: 13585)
[PASS] testFailTooLate() (gas: 13606)
[PASS] testFailWrongDay() (gas: 13585)
[PASS] testGeneral() (gas: 36831844)
[PASS] testNextCastTime() (gas: 364790)
[PASS] testOnTime() (gas: 410063)
[PASS] testPSMs() (gas: 1825244)
[PASS] testUseEta() (gas: 363541)
Test result: ok. 17 passed; 0 failed; 0 skipped; finished in 2050.32s

0xp3th1um
0xp3th1um approved these changes Nov 15, 2023
View reviewed changes
Copy link
Collaborator

@0xp3th1um 0xp3th1um left a comment

Choose a reason for hiding this comment

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

Cast and Merge Stage

SidestreamSweatyPumpkin
SidestreamSweatyPumpkin approved these changes Nov 16, 2023
View reviewed changes
Copy link
Contributor

@SidestreamSweatyPumpkin SidestreamSweatyPumpkin left a comment •
edited
Loading

Choose a reason for hiding this comment

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

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 8b80f0e into master Nov 16, 2023
2 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@0xp3th1um 0xp3th1um 0xp3th1um approved these changes

@SidestreamSweatyPumpkin SidestreamSweatyPumpkin SidestreamSweatyPumpkin approved these changes

Assignees

@0xdecr1pto 0xdecr1pto

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@0xdecr1pto @SidestreamSweatyPumpkin @0xp3th1um