makerdao / spells-mainnet

Staging repo for MakerDAO weekly executive spells
GNU Affero General Public License v3.0
107 stars 43 forks source link

PE-1047: Wednesday August 3rd 2022 Executive Spell #265

Closed julienmartinlevrai closed 1 year ago

julienmartinlevrai commented 1 year ago

Description

Contribution Checklist

Checklist

gbalabasquer commented 1 year ago

@julienmartinlevrai we should clean up DssSpellCollateral.sol file. This is another negative side of having a different file, we just missed to clean it up as not easy to detect we are leaving code there. Edit: And I know we are not importing it but better to clean it so it doesn't remain that code in the archive of this week.

julienmartinlevrai commented 1 year ago

@julienmartinlevrai we should clean up DssSpellCollateral.sol file. This is another negative side of having a different file, we just missed to clean it up as not easy to detect we are leaving code there. Edit: And I know we are not importing it but better to clean it so it doesn't remain that code in the archive of this week.

Done. I think we should let CES manage that file however they want.

brianmcmichael commented 1 year ago

So far so good. Just waiting on copy hash and manager address (if GovAlpha adds it)

gbalabasquer commented 1 year ago

Running 16 tests for src/DssSpell.t.sol:DssSpellTest [PASS] testCastCost() (gas: 640377) [PASS] testFailTooEarly() (gas: 12706) [PASS] testFailTooLate() (gas: 12683) [PASS] testFailWrongDay() (gas: 12728) [PASS] testFail_notScheduled() (gas: 14139) [PASS] testOnTime() (gas: 631483) [PASS] testSpellIsCast_GENERAL() (gas: 22789451) [PASS] testVestDAI() (gas: 741317) [PASS] test_RWA009Draw() (gas: 644578) [PASS] test_auth() (gas: 9223336852496191121) [PASS] test_auth_in_sources() (gas: 9223336852484764849) [PASS] test_bytecode_matches() (gas: 1252056) [PASS] test_chainlog_values() (gas: 5919298) [PASS] test_chainlog_version_bump() (gas: 3166363) [PASS] test_nextCastTime() (gas: 338945) [PASS] test_use_eta() (gas: 337755) Test result: ok. 16 passed; 0 failed; finished in 1284.97s



Just waiting the hash and comment are added and get defined if the new vesting will go with a manager.
The rest looks good.
gbalabasquer commented 1 year ago
julienmartinlevrai commented 1 year ago
  • Hash is correct
  • Missing to add mgr address to the new vest or remove comment if not happening
  • We are missing a test for checking the vest id 8 is correctly yanked.

@gbalabasquer done

gbalabasquer commented 1 year ago

Changes look good. As soon as my local tests finish running will give the approval for deploying.

julienmartinlevrai commented 1 year ago

spell deployed at 0x6682133cCFbc8f87da51904d95241d243f7260Bf