Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2023-08-18 Executive Spell #360

Merged
merged 12 commits into from
Aug 17, 2023
Merged

2023-08-18 Executive Spell #360

merged 12 commits into from
Aug 17, 2023

Conversation

SidestreamColdMelon
Copy link
Contributor

@SidestreamColdMelon SidestreamColdMelon commented Aug 10, 2023

Description

This PR implements executive spell planned for 2023-08-18.
Relevant goerli spell: makerdao/spells-goerli#222

Contribution Checklist

  • 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 changelog or known
  • Notify any external teams affected by the spell so they have the opportunity to review
  • Deploy spell ETH_GAS_LIMIT="XXX" ETH_GAS_PRICE="YYY" make deploy
  • Verify mainnet contract on etherscan
  • Change test to use mainnet spell address and deploy timestamp
  • Run make archive-spell or make date="YYYY-MM-DD" archive-spell to make an archive directory and copy DssSpell.sol, DssSpell.t.sol, DssSpell.t.base.sol, and DssSpellCollateralOnboarding.sol
  • squash and merge this PR

@SidestreamColdMelon SidestreamColdMelon self-assigned this Aug 10, 2023
@SidestreamColdMelon SidestreamColdMelon marked this pull request as ready for review August 15, 2023 18:38
@@ -110,16 +110,16 @@ contract Config {
//
// Values for all system configuration changes
//
afterSpell.line_offset = 750 * MILLION; // Offset between the global line against the sum of local lines
afterSpell.pot_dsr = 8_00; // In basis points
afterSpell.line_offset = 680 * MILLION; // Offset between the global line against the sum of local lines
Copy link
Contributor

Choose a reason for hiding this comment

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

Why we are changing offset here?

Copy link
Contributor Author

@SidestreamColdMelon SidestreamColdMelon Aug 16, 2023

Choose a reason for hiding this comment

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

Because the global debt ceiling was changed in this spell. 100M down for CRVV1ETHSTETH-A, 30M up for RWA002-A. Btw, the test actually would pass without this change, as it checks if global line is anywhere between line_offset and line_offset * 2, but I've changed it to reflect overall decrease in the gap:

assertTrue(sums[0] + values.line_offset * RAD <= vat.Line(), "TestError/vat-Line-2");
assertTrue(sums[0] + 2 * values.line_offset * RAD >= vat.Line(), "TestError/vat-Line-3");

Copy link
Contributor

Choose a reason for hiding this comment

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

Clear, I think it would be good to have a line about gap in checklist

Copy link
Contributor

Choose a reason for hiding this comment

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

Haven't been really following the concrete numbers, but I think the 30M shouldn't be part of the gap change, as that 30M increment in RWA-2 has the corresponding of the global debt ceiling as well. However the -100M from CRVV1ETHSTETH-A is correct as the global debt ceiling was reduced in that number when the local debt ceiling was already 0. So the reference value IMO (and just following the logic at a high level point of view) should be 650.

Copy link

Choose a reason for hiding this comment

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

So the reference value IMO (and just following the logic at a high level point of view) should be 650

Thaks for flagging this, we discussed it at GovOps yesterday and we will fix it in the PR for the upcoming spell

hexonaut
hexonaut previously approved these changes Aug 16, 2023
Copy link
Contributor

@hexonaut hexonaut 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. Checked:

  • Tests passing
  • Office hours off
  • Tests are correct
  • Wallet addresses updated
  • Config changes are good
  • Rate updates are good

Diff vs Goerli on spell matches except the following additions which have been checked:

  • Description is good and hash matches
  • Delegate wallet addresses are correct
  • Double checked Spark addresses
  • Curve stETH/ETH offboarding parameters are correct
  • Delegate payments are correct
  • D3M Housekeeping is correct

@0xdecr1pto
Copy link
Contributor

0xdecr1pto commented Aug 16, 2023

Good to deploy

Mainnet Executive Spell Review Checklist

Mainnet 2023-08-18

Spell Actions (Per Exec Doc):

  • EDSR Update
  • DSR-based Stability Fee Updates
  • Smart Burn Engine Parameter Updates
  • Non-DSR Related Parameter Changes
  • CRVV1ETHSTETH-A 2nd Stage Offboarding
  • Aligned Delegate Compensation for July 2023
  • Old D3M Parameter Housekeeping
  • New Silver Parameter Changes
  • DAO Resolution for BlockTower Andromeda
  • Transfer Spark Proxy Admin Controls
  • Trigger Spark Proxy Spell

Development Stage

  • Office Hours
    • OFF
    • Matches Exec Doc
  • 30 days spell expiry in constructor (block.timestamp + 30 days)
  • Exec Doc Hash
  • Spell Description
    • Description follows the format TARGET_DATE MakerDAO Executive Spell | Hash: EXEC_DOC_HASH)
    • Target date in description matches the Exec Doc target date
    • Exec Doc Hash in description matches your locally generated Exec Doc Hash
    • Accompanying comment above spell description
      • Comment follows the format // Hash: cast keccak -- "$(wget 'EXEC_DOC_URL' -q -O - 2>/dev/null)"
      • Exec Doc URL in comment matches your Raw Exec Doc URL
      • Exec Doc URL in comment refers to the 'Community' GitHub repo
  • Local Environment Actions
    • Update Foundry by running foundryup
    • Reinstall libraries
      • Remove libraries by deleting the lib folder
      • Install libraries using 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 '4ad127cf53eeaddfb7b8ad56dd4b13e57d6a0067'
        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'
        
    • Dependency checks
      • dss-exec-lib
        • if submodule upgrades are present make sure dss-exec-lib is synced as well
        • git submodule hash (run git submodule status) matches the latest release version or newer (NOTE: dss-exec-lib as installed locally will use GitHub code more recent than the 0.0.9 release)
      • dss-test
  • Interface Checks
    • dss-interfaces
      • used in the current spell
      • cleanup previous ones
      • ensure only single import layout is used (e.g. import "dss-interfaces/dss/VatAbstract.sol";)
    • Static Interfaces
      • ensure they match dss-interfaces (Where there is a mismatch, use cast interface as the source of truth)
      • check on-chain interface of deployed contract via cast interface <contract_address> to ensure correctness
      • interface naming style should match with Like suffix (e.g. VatLike), with some exceptions
      • ensure they only list used functions in spell code
  • Rate constants used are correct
    • Manual check 1: using make rates pct=<pct> (e.g. pct=0.75, for 0.75%)
      • 3.58 -> 1000000001115362602336059074
      • 3.33 -> 1000000001038735548426731741
      • 4.08 -> 1000000001268063427242299977
      • 5.00 -> 1000000001547125957863212448
      • 5.25 -> 1000000001622535724756171269
      • 5.55 -> 1000000001712791360746325100
      • 5.80 -> 1000000001787808646832390371
      • 6.30 -> 1000000001937312893803622469
      • 7.00 -> 1000000002145441671308778766
    • Manual check 2: Compare against IPFS
    • Variable name conforms to X_PT_Y_Z_PCT_RATE (e.g. ZERO_PT_SEVEN_FIVE_PCT_RATE for 0.75%)
    • Variable visibility declared as internal
    • State mutability declared as constant
  • Constants Match
    • Precision unit constants used match their defined values
      • RAY = 10 ** 27
      • RAD = 10 ** 45
      • Variable visibility declared as internal
      • State mutability declared as constant
      • Ensure they match with ds-math and the Numerical Ranges
    • Math unit constants used match their defined values
      • THOUSAND = 10 ** 3
      • MILLION = 10 ** 6
      • BILLION = 10 ** 9
      • Variable visibility declared as internal
      • State mutability declared as constant
      • Ensure they match with config
  • Core System Parameter Changes
    • Stability Fee jug.ilk.duty (setIlkStabilityFee)
      • ETH-A -> 3.58% // Increase ETH-A SF by 0.14% from 3.44% to 3.58%
      • ETH-B -> 4.08% // Increase ETH-B SF by 0.14% from 3.94%% to 4.08%
      • ETH-C -> 3.33% // Increase ETH-C SF by 0.14% from 3.19% to 3.33%
      • WSTETH-A -> 5.25% // Increase WSTETH-A SF by 1.81% from 3.44% to 5.25%
      • WSTETH-B -> 5.00% // Increase WSTETH-B SF by 1.81% from 3.19% to 5.00%
      • RETH-A -> 5.25% // Increase RETH-A SF by 1.81% from 3.44% to 5.25%
      • WBTC-A -> 5.80% // Increase WBTC-A SF by 0.11% from 5.69% to 5.80%
      • WBTC-B -> 6.30% // Increase WBTC-B SF by 0.11% from 6.19% to 6.30%
      • WBTC-C -> 5.55$ // Increase WBTC-C SF by 0.11% from 5.44% to 5.55%
    • Dai Savings Rate pot.dsr (setDSR)
      • Double check that PCT_RATE is correct
        • Check manually via make rates pct=<pct> (e.g. pct=0.75, for 0.75%)
          • 5.00 -> 1000000001547125957863212448
        • Compare against IPFS
  • Debt Ceiling Changes
    • Autoline Changes
      • setIlkAutoLineDebtCeiling
        • WSTETH-A -> 750 * MILLION // Increase WSTETH-A line by 250 million DAI from 500 million DAI to 750 million DAI (no change to gap or ttl)
        • RETH-A -> 75 * MILLION // Increase RETH-A line by 25 million DAI from 50 million DAI to 75 million DAI
      • setIlkAutoLineParameters
        • ilk: STETH-B
        • line: 1 * BILLION // Increase WSTETH-B line by 500 million DAi from 500 million DAI to 1 billion DAI
        • gap: 45 * MILLION // Increase WSTETH-B gap by 15 million DAI from 30 million DAI to 45 million DAI
        • ttl: 12 hours // Reduce WSTETH-B ttl by 14,400 seconds from 57,600 seconds to 43,200 seconds
  • Smart Burn Engine Parameter
    • VOW.bump -> 20_000 DAI (RAD precision used) // Increase vow.bump by 15,000 DAI from 5,000 DAI to 20,000 DAI
    • FLAP.hop -> 6_308// Increase hop by 4,731 seconds from 1,577 seconds to 6,308 seconds
  • Offboarding (CRVV1ETHSTETH-A)
    • 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
        Set chip to 0
      • Reduce Global Debt Ceiling by 100 million DAI to account for offboarded collateral
      • Set Liquidation Ratio (mat) to 10,000%
      • Update collateral price to propagate the changes
  • RWA Updates (RWA002-A)
    • Debt Ceiling (line) + Liquidation Oracle Price Bump (val)
      • Stability Fee jug.ilk.duty // Increase RWA002-A Stability Fee by 3.5% from 3.5% to 7%
      • Liquidation Ratio spotter.ilk.mat // Reduce Liquidation Ratio by 5% from 105% to 100%
      • Debt Ceiling (line) + Liquidation Oracle Price Bump (val)
        • Increase Ilk Debt Ceiling (set DC + increase Global DC) // Increase RWA002-A Debt Ceiling by 30 million DAI from 20 million DAI to 50 million DAI
        • 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.

            We use only one ilk

        • Poke spotter to pull in the new price
  • Payments
    • MKR transfers
      • Recipient addresses match Exec Doc
      • Transfer values match Exec Doc
      • Follows archive patterns
  • SubDAO Content
    • SubDAO SubProxy spell execution
      • SubDAO spell address matches Exec Sheet (comment or otherwise)
      • SubDAO spell deployer is purely an EOA
        • Ensure the deployer address is in addresses_deployers.sol as a comment
      • 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)
      • SubDAO actions match Exec Sheet (only where inline with main spell code) and do not affect core contracts
      • 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)
  • ChainLog
    • Increment ChainLog version based on update type (Not needed)
  • addresses_mainnet.sol matches spell code
  • Ensure every spell variable is declared as public/internal
  • Ensure immutable visibility is only used when fetching addresses from the ChainLog via DssExecLib.getChangelogAddress and constant is used instead for static addresses
    • Fetch addresses as type address and wrap with Like suffix interfaces inline (when making calls) unless archive patterns permit otherwise (Such as MKR)
    • Use the DssExecLib Core Address Helpers where possible (e.g. DssExecLib.vat())
    • Where addresses are fetched from the ChainLog, the variable name must match the value of the ChainLog key for that address (e.g. MCD_VAT rather than vat), except where the archive pattern differs from this pattern (e.g. MKR)
  • Spell actions match the corresponding Exec Doc
  • Tests
    • Ensure each spell action has sufficient test coverage
      • New Silver Parameter Changes -> test_RWA002_Update()
    • 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
      bcf4cef
    • 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: 2153208)
[PASS] testStarknetSpell() (gas: 2346)
Test result: ok. 2 passed; 0 failed; finished in 58.60s

Running 21 tests for src/DssSpell.t.sol:DssSpellTest
[PASS] testAuth() (gas: 9223371487105614665)
[PASS] testAuthInSources() (gas: 9223371487099402314)
[PASS] testBytecodeMatches() (gas: 4234210)
[PASS] testCastCost() (gas: 1998557)
[PASS] testChainlogValues() (gas: 10835077)
[PASS] testChainlogVersionBump() (gas: 5611276)
[PASS] testContractSize() (gas: 8984)
[PASS] testDeployCost() (gas: 4214698)
[PASS] testFailNotScheduled() (gas: 14375)
[PASS] testFailTooEarly() (gas: 13607)
[PASS] testFailTooLate() (gas: 13584)
[PASS] testFailWrongDay() (gas: 13629)
[PASS] testGeneral() (gas: 39113710)
[PASS] testMKRPayments() (gas: 2131782)
[PASS] testNextCastTime() (gas: 353659)
[PASS] testOnTime() (gas: 1994262)
[PASS] testPSMs() (gas: 3357915)
[PASS] testSparkAdminTransfer() (gas: 2034842)
[PASS] testSparkSpellIsExecuted() (gas: 1997865)
[PASS] testUseEta() (gas: 352346)
[PASS] test_RWA002_Update() (gas: 2196746)
Test result: ok. 21 passed; 0 failed; finished in 963.16s

@SidestreamColdMelon
Copy link
Contributor Author

@SidestreamColdMelon
Copy link
Contributor Author

Copy link
Contributor

@hexonaut hexonaut 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 submit. Checked:

  • Spell code matches flattened repo
  • Tests pass
  • Spell address, timestamp and block number are good
  • DssExecLib matches repo
  • Archive matches

@0xdecr1pto
Copy link
Contributor

Good to submit

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 target Exec Doc date)
    • Ensure date corresponds to target Exec Doc 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
      4f3d23d
    • 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)

@SidestreamColdMelon SidestreamColdMelon merged commit 5890c64 into master Aug 17, 2023
3 checks passed
@SidestreamColdMelon SidestreamColdMelon deleted the 2023-08-18 branch August 17, 2023 17:40
@ghost ghost mentioned this pull request Aug 23, 2023
@SidestreamSweatyPumpkin SidestreamSweatyPumpkin mentioned this pull request Aug 29, 2023
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants