makerdao / spells-mainnet

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

PE-965: Executive Spell #242

Closed gbalabasquer closed 2 years ago

gbalabasquer commented 2 years ago

Description

Contribution Checklist

Checklist

gbalabasquer commented 2 years ago

@talbaneth @naszam excepting for the missing hash this spell should be ready for reviewing.

talbaneth commented 2 years ago

@gbalabasquer looks good, only thing we might want to add are the forum polls links. Will have another quick read tomorrow with fresh eyes. Feel free to deploy beforehand if you want.

details:

Running 18 tests for src/DssSpell.t.sol:DssSpellTest [PASS] testFail_notScheduled() (gas: 7147) [PASS] test_auth_in_sources() (gas: 281473904952136) [PASS] testVestDAIFails() (gas: 1716895) [PASS] test_use_eta() (gas: 330145) [PASS] testOnTime() (gas: 1374223) [PASS] testPayments() (gas: 1400502) [PASS] testVestDAI() (gas: 1521107) [PASS] test_nextCastTime() (gas: 393457) [PASS] test_chainlog_values() (gas: 5657174) [PASS] testFailTooEarly() (gas: 370027) [PASS] test_bytecode_matches() (gas: 2128430) [PASS] test_chainlog_version_bump() (gas: 3141868) [PASS] test_auth() (gas: 281473913629769) [PASS] testSpellIsCast_GENERAL() (gas: 21304198) [PASS] testFailWrongDay() (gas: 370030) [PASS] testCastCost() (gas: 1375812) [PASS] testNewChainlogValues() (gas: 1382539) [PASS] testFailTooLate() (gas: 370007)

gbalabasquer commented 2 years ago

@gbalabasquer looks good, only thing we might want to add are the forum polls links. Will have another quick read tomorrow with fresh eyes. Feel free to deploy beforehand if you want.

details:

  • new vest contract code matches deployment.
  • new vest contract wards and params fine.
  • dai transfers and vest match the spreadsheet.
  • changelog changes ok
  • code covers the spreadsheet.
  • tests cover the code.
  • office hours on - ok.
  • tests pass

Running 18 tests for src/DssSpell.t.sol:DssSpellTest [PASS] testFail_notScheduled() (gas: 7147) [PASS] test_auth_in_sources() (gas: 281473904952136) [PASS] testVestDAIFails() (gas: 1716895) [PASS] test_use_eta() (gas: 330145) [PASS] testOnTime() (gas: 1374223) [PASS] testPayments() (gas: 1400502) [PASS] testVestDAI() (gas: 1521107) [PASS] test_nextCastTime() (gas: 393457) [PASS] test_chainlog_values() (gas: 5657174) [PASS] testFailTooEarly() (gas: 370027) [PASS] test_bytecode_matches() (gas: 2128430) [PASS] test_chainlog_version_bump() (gas: 3141868) [PASS] test_auth() (gas: 281473913629769) [PASS] testSpellIsCast_GENERAL() (gas: 21304198) [PASS] testFailWrongDay() (gas: 370030) [PASS] testCastCost() (gas: 1375812) [PASS] testNewChainlogValues() (gas: 1382539) [PASS] testFailTooLate() (gas: 370007)

Yeah Naz pointed me about the polls links but in general I do not add them as they are also part of the summary information. If you guys feel too strong about it I'll proceed adding them to the code.

talbaneth commented 2 years ago

@gbalabasquer looks good, only thing we might want to add are the forum polls links. Will have another quick read tomorrow with fresh eyes. Feel free to deploy beforehand if you want. details:

  • new vest contract code matches deployment.
  • new vest contract wards and params fine.
  • dai transfers and vest match the spreadsheet.
  • changelog changes ok
  • code covers the spreadsheet.
  • tests cover the code.
  • office hours on - ok.
  • tests pass

Running 18 tests for src/DssSpell.t.sol:DssSpellTest [PASS] testFail_notScheduled() (gas: 7147) [PASS] test_auth_in_sources() (gas: 281473904952136) [PASS] testVestDAIFails() (gas: 1716895) [PASS] test_use_eta() (gas: 330145) [PASS] testOnTime() (gas: 1374223) [PASS] testPayments() (gas: 1400502) [PASS] testVestDAI() (gas: 1521107) [PASS] test_nextCastTime() (gas: 393457) [PASS] test_chainlog_values() (gas: 5657174) [PASS] testFailTooEarly() (gas: 370027) [PASS] test_bytecode_matches() (gas: 2128430) [PASS] test_chainlog_version_bump() (gas: 3141868) [PASS] test_auth() (gas: 281473913629769) [PASS] testSpellIsCast_GENERAL() (gas: 21304198) [PASS] testFailWrongDay() (gas: 370030) [PASS] testCastCost() (gas: 1375812) [PASS] testNewChainlogValues() (gas: 1382539) [PASS] testFailTooLate() (gas: 370007)

Yeah Naz pointed me about the polls links but in general I do not add them as they are also part of the summary information. If you guys feel too strong about it I'll proceed adding them to the code.

I don't feel strong about it, can leave as is from my side.