PE-655 - 2021-09-03 Executive

[gbalabasquer]

Description

Contribution Checklist

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

Checklist

• [x] Every contract variable/method declared as public/external private/internal
• [x] Consider if this PR needs the officeHours modifier
• [x] Verify expiration (4 days monthly and 30 days for the rest)
• [x] Verify hash in the description matches here
• [x] Validate all addresses used are in changelog or known
• [x] Notify any external teams affected by the spell so they have the opportunity to review
• [x] Deploy spell ETH_GAS="XXX" ETH_GAS_PRICE="YYY" make deploy
• [x] Verify mainnet contract on etherscan
• [x] Change test to use mainnet spell address and deploy timestamp
• [x] Keep DssSpell.sol and DssSpell.t.sol the same, but make a copy in archive
• [x] squash and merge this PR

[julienmartinlevrai]

You can remove the expérimental pragma from the spell.

[julienmartinlevrai]

Also, please update the copyright in the spell and add it in the test.

[gbalabasquer]

You can remove the expérimental pragma from the spell.

Done

[gbalabasquer]

Also, please update the copyright in the spell and add it in the test.

Test is fine as it is. What do you mean with update in the spell?

[julienmartinlevrai]

Also, please update the copyright in the spell and add it in the test.

Test is fine as it is. What do you mean with update in the spell?

Currently it says "Copyright (C) 2021 Maker Ecosystem Growth Holdings, INC." in DssSpell and there is no copyright in the test

[gbalabasquer]

Ahh you mean the DAI Foundation copy. Ok good will update it. Let's leave the test as it is.

[godsflaw]

3 FAIL tests (figured I can run this faster than CI for you):

Running 21 tests for src/DssSpell.t.sol:DssSpellTest
[FAIL] testRWAUpdates()
[PASS] testFail_notScheduled() (gas: 7184)
[PASS] test_auth_in_sources() (gas: 281473904265292)
[PASS] test_use_eta() (gas: 320915)
[PASS] testOneTimePaymentDistributions() (gas: 3868216)
[PASS] testOnTime() (gas: 3851034)
[PASS] testVestDAI() (gas: 4050245)
[PASS] test_nextCastTime() (gas: 321810)
[PASS] testFailTooEarly() (gas: 1951)
[PASS] test_bytecode_matches() (gas: 3656671)
[PASS] test_auth() (gas: 281473907868726)
[FAIL] testSpellIsCast_GENERAL()
[PASS] testFailWrongDay() (gas: 1974)
[PASS] testFlash() (gas: 4224967)
[FAIL] testCollateralIntegrations()
[PASS] testCastCost() (gas: 3852317)
[PASS] testVestMKR() (gas: 4043292)
[PASS] testNewChainlogValues() (gas: 3857273)
[PASS] testNewIlkRegistryValues() (gas: 3852012)
[PASS] testFailTooLate() (gas: 1928)
[PASS] testRwaDrawUpToDebtCeiling() (gas: 1553509)

[gbalabasquer]

We need to remove the testRWAUpdates() test.

Yes already done, and I also commented testCollateralIntegrations. Just need to figuere it out what is happening with GENERAL, probably just the office hours check.

[godsflaw]

Running 19 tests for src/DssSpell.t.sol:DssSpellTest
[PASS] testFail_notScheduled() (gas: 7162)
[PASS] test_auth_in_sources() (gas: 281473904265292)
[PASS] test_use_eta() (gas: 320893)
[PASS] testOneTimePaymentDistributions() (gas: 3887932)
[PASS] testOnTime() (gas: 3870750)
[PASS] testVestDAI() (gas: 4069961)
[PASS] test_nextCastTime() (gas: 321810)
[PASS] testFailTooEarly() (gas: 3572)
[PASS] test_bytecode_matches() (gas: 3672181)
[PASS] test_auth() (gas: 281473907868726)
[PASS] testSpellIsCast_GENERAL() (gas: 12650679)
[PASS] testFailWrongDay() (gas: 3595)
[PASS] testFlash() (gas: 4244771)
[PASS] testCastCost() (gas: 3872033)
[PASS] testVestMKR() (gas: 4063008)
[PASS] testNewChainlogValues() (gas: 3876989)
[PASS] testNewIlkRegistryValues() (gas: 3871728)
[PASS] testFailTooLate() (gas: 3549)
[PASS] testRwaDrawUpToDebtCeiling() (gas: 1552302)

[naszam]

Certora DssVest Specs (with optimization on, 200 runs):

• DssVestMintable: Report

• DssVestSuckable: Report

To reproduce: https://github.com/makerdao/dss-vest/pull/40

[gbalabasquer]

      ├╴call 0x0fC8D4f2151453ca0cA56f07359049c8f07997Bd::res(9) (src/DssSpell.t.sol:2867)
      │  └╴← (1)
      ├╴log("Error: a == b not satisfied [uint]") (lib/ds-test/src/test.sol:124)
      ├╴log_named_uint("  Expected", 0) (lib/ds-test/src/test.sol:125)
      ├╴log_named_uint("    Actual", 1) (lib/ds-test/src/test.sol:126)```

Fixed

[julienmartinlevrai]

I just finished réviewing the code, LGTM

Now I am running the tests

[julienmartinlevrai]

dapp-test: rpc block: latest
Running 19 tests for src/DssSpell.t.sol:DssSpellTest
[PASS] testFail_notScheduled() (gas: 7237)
[PASS] test_auth_in_sources() (gas: 281473904256059)
[PASS] test_use_eta() (gas: 320751)
[PASS] testOneTimePaymentDistributions() (gas: 3910115)
[PASS] testOnTime() (gas: 3893765)
[PASS] testVestDAI() (gas: 4091147)
[PASS] test_nextCastTime() (gas: 321659)
[PASS] testFailTooEarly() (gas: 3545)
[PASS] test_bytecode_matches() (gas: 2769640)
[PASS] test_auth() (gas: 281473907795513)
[PASS] testSpellIsCast_GENERAL() (gas: 11720626)
[PASS] testFailWrongDay() (gas: 3677)
[PASS] testFlash() (gas: 4267505)
[PASS] testCastCost() (gas: 3895390)
[PASS] testVestMKR() (gas: 4083224)
[PASS] testNewChainlogValues() (gas: 3900244)
[PASS] testNewIlkRegistryValues() (gas: 3895220)
[PASS] testFailTooLate() (gas: 3919)
[PASS] testRwaDrawUpToDebtCeiling() (gas: 1542637)