makerdao / spells-mainnet

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

PE-1119 #293

Closed hexonaut closed 1 year ago

hexonaut commented 1 year ago

Description

Contribution Checklist

Checklist

hexonaut commented 1 year ago

This is ready for review now. @gbalabasquer @megozu @The-Arbiter @rockyfour

rockyfour commented 1 year ago

Note that etherscan verification was done wIth GPL V3 instead of AGPL V3 (I think it's not a showstopper as the etherscan source code does have the correct license in the header).

hexonaut commented 1 year ago

In the interest of getting this deployed soon, can we keep all remaining review comments just to if any particular check or functionality is missing and not format, generalization of tests, etc.

gbalabasquer commented 1 year ago

The D3M sections looks good.

rockyfour commented 1 year ago

The cD3M part LGTM.

hexonaut commented 1 year ago

Two minor changes. LMK when you guys think it's good to deploy. Again please just structural issues and missing checks, etc.

@The-Arbiter take your time. Thorough review is important.

rockyfour commented 1 year ago

cd3m part still good to deploy

luke7211 commented 1 year ago

L2Spell Review

LGTM

image

hexonaut commented 1 year ago

Ok everything outstanding should be resolved now. Waiting for final confirmation before deploying @megozu @The-Arbiter

The-Arbiter commented 1 year ago

I forgot to put part of my deploy stage review so here it is:

Deployed Stage

Running 1 test for src/test/starknet.t.sol:StarknetTests
[PASS] testStarknet() (gas: 2716118)
Test result: ok. 1 passed; 0 failed; finished in 136.81s

Running 20 tests for src/DssSpell.t.sol:DssSpellTest
[PASS] testCastCost() (gas: 2454060)
[PASS] testDirectCompV2Integration() (gas: 2964797)
[PASS] testFailTooEarly() (gas: 401840)
[PASS] testFailTooLate() (gas: 401685)
[PASS] testFailWrongDay() (gas: 401686)
[PASS] testFail_notScheduled() (gas: 14169)
[PASS] testIlkClipper() (gas: 4227150)
[PASS] testMKRPayments() (gas: 2467692)
[PASS] testMedianizers() (gas: 2460832)
[PASS] testNewChainlogValues() (gas: 2578334)
[PASS] testNewIlkRegistryValues() (gas: 2476975)
[PASS] testOnTime() (gas: 2450515)
[PASS] testSpellIsCast_GENERAL() (gas: 29143581)
[PASS] test_auth() (gas: 9223336852501768144)
[PASS] test_auth_in_sources() (gas: 9223336852484776805)
[PASS] test_bytecode_matches() (gas: 5128272)
[PASS] test_chainlog_values() (gas: 8129997)
[PASS] test_chainlog_version_bump() (gas: 4455745)
[PASS] test_nextCastTime() (gas: 404835)
[PASS] test_use_eta() (gas: 337923)
Test result: ok. 20 passed; 0 failed; finished in 1714.34s