2023-05-10: Param Changes + Vesting setup + StarkNet Bridge Deposit Limit

[0xdecr1pto]

Description

Contribution Checklist

• [ ] PR title starts with (PE-<TICKET_NUMBER>)
• [ ] 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

[The-Arbiter]

CI is not passing and a binary file (swap file) has been added to the the diff.

Please delete these and fix the CI and I will continue with the review

[The-Arbiter]

As an update on above, tests passed for pre-deploy, so that is good.

Using DssExecLib at: 0x8De6DDbCd5053d32292AAA0D2105A32d108484a6
[⠃] Compiling...
[⠰] Compiling 4 files with 0.8.16
[⠰] Solc 0.8.16 finished in 2.51s
Compiler run successful

Running 2 tests for src/test/starknet.t.sol:StarknetTests
[PASS] testStarknet() (gas: 1493033)
[PASS] testStarknetSpell() (gas: 2346)
Test result: ok. 2 passed; 0 failed; finished in 77.87s

Running 23 tests for src/DssSpell.t.sol:DssSpellTest
[PASS] testAuth() (gas: 9223371487102514043)
[PASS] testAuthInSources() (gas: 9223371487099067340)
[PASS] testBytecodeMatches() (gas: 2732050)
[PASS] testCastCost() (gas: 1349404)
[PASS] testChainlogValues() (gas: 9649320)
[PASS] testChainlogVersionBump() (gas: 4745761)
[PASS] testContractSize() (gas: 8962)
[PASS] testDeployCost() (gas: 2716759)
[PASS] testFailNotScheduled() (gas: 14398)
[PASS] testFailTooEarly() (gas: 13585)
[PASS] testFailTooLate() (gas: 13562)
[PASS] testFailWrongDay() (gas: 13541)
[PASS] testGeneral() (gas: 36858950)
[PASS] testNewChainlogValues() (gas: 1354653)
[PASS] testNextCastTime() (gas: 353681)
[PASS] testOnTime() (gas: 1345088)
[PASS] testPSMs() (gas: 2828177)
[PASS] testPayments() (gas: 1416076)
[PASS] testUseEta() (gas: 352302)
[PASS] testVestDAI() (gas: 1527920)
[PASS] testVestMKR() (gas: 1501965)
[PASS] testYankDAI() (gas: 1361971)
[PASS] testYankMKR() (gas: 1416494)
Test result: ok. 23 passed; 0 failed; finished in 1538.19s

[amusingaxl]

• [x] Deployed Spell is Verified
  • [x] Optimization Enabled: No
  • [x] Other Settings: default evmVersion, GNU AGPLv3 license
• [x] Deployed Spell Code matches GitHub
  • [x] diffcheck etherscan source against spell PR (via make diff-deployed-spell)
• [x] Deployed Spell Etherscan Checks
  • [x] automated checks via make check-deployed-spell
  • [x] verified
  • [x] license type matches
  • [x] solc version matches
  • [x] optimizations are disabled
  • [x] dss-exec-lib library address used (under 'Libraries Used') matches the hardcoded local DssExecLib.address file
  • [x] deployed_spell_created matches deployment timestamp
  • [x] deployed_spell_block matches deployment block number
  • [x] manual checks
    • [x] Ensure make deploy-info tx=<tx> matches config
    • [x] deployed_spell_created timestamp
    • [x] deployed_spell_block block number
    • [x] Ensure Etherscan Libraries Used matches DssExecLib Latest Release
    • [ ] git submodule hash matches dss-exec-lib latest release's tag commit and inspect diffs if it doesn't match to ensure expected behaviour
• [x] Archive matches src
  • [x] make diff-archive-spell for current date or date="YYYY-MM-DD" make diff-archive-spell (date as per target exec date)
  • [x] ensure date corresponds to target exec date
• [ ] Local Tests and CI PASS

Running 2 tests for src/test/starknet.t.sol:StarknetTests
[PASS] testStarknet() (gas: 1493033)
[PASS] testStarknetSpell() (gas: 2346)
Test result: ok. 2 passed; 0 failed; finished in 77.87s

Running 23 tests for src/DssSpell.t.sol:DssSpellTest
[PASS] testAuth() (gas: 9223371487102514043)
[PASS] testAuthInSources() (gas: 9223371487099067340)
[PASS] testBytecodeMatches() (gas: 2732050)
[PASS] testCastCost() (gas: 1349404)
[PASS] testChainlogValues() (gas: 9649320)
[PASS] testChainlogVersionBump() (gas: 4745761)
[PASS] testContractSize() (gas: 8962)
[PASS] testDeployCost() (gas: 2716759)
[PASS] testFailNotScheduled() (gas: 14398)
[PASS] testFailTooEarly() (gas: 13585)
[PASS] testFailTooLate() (gas: 13562)
[PASS] testFailWrongDay() (gas: 13541)
[PASS] testGeneral() (gas: 36858950)
[PASS] testNewChainlogValues() (gas: 1354653)
[PASS] testNextCastTime() (gas: 353681)
[PASS] testOnTime() (gas: 1345088)
[PASS] testPSMs() (gas: 2828177)
[PASS] testPayments() (gas: 1416076)
[PASS] testUseEta() (gas: 352302)
[PASS] testVestDAI() (gas: 1527920)
[PASS] testVestMKR() (gas: 1501965)
[PASS] testYankDAI() (gas: 1361971)
[PASS] testYankMKR() (gas: 1416494)

Waiting on CI.

[The-Arbiter]

@amusingaxl the tests you have posted are literally a copy of my passing tests from earlier.

Running tests locally - with a remote node or local node - is not a joke and doesn't mean copying other people's results and claiming that your tests pass. Having two reviewers follow the procedure properly is how we ensure security and consistency of the spell process.

Approval for this spell requires two reviewers having passing tests in addition to CI.

Please amend your test results with your own passing results showing that your local tests pass after deploy and archive were completed.

[amusingaxl]

@amusingaxl the tests you have posted are literally a copy of my passing tests from earlier.

Running tests locally - with a remote node or local node - is not a joke and doesn't mean copying other people's results and claiming that your tests pass. Having two reviewers follow the procedure properly is how we ensure security and consistency of the spell process.

Not sure what happened, but was most likely a clipboard fuck up... Sorry! I copied your previous comment to compare against mine, and I guess I pasted the wrong content in here after diffing.

Here's my actual test report:

Running 2 tests for src/test/starknet.t.sol:StarknetTests
[PASS] testStarknet() (gas: 1492401)
[PASS] testStarknetSpell() (gas: 2346)
Test result: ok. 2 passed; 0 failed; finished in 36.85s

Running 23 tests for src/DssSpell.t.sol:DssSpellTest
[PASS] testAuth() (gas: 9223371487102514043)
[PASS] testAuthInSources() (gas: 9223371487099067340)
[PASS] testBytecodeMatches() (gas: 2732050)
[PASS] testCastCost() (gas: 1348772)
[PASS] testChainlogValues() (gas: 9648688)
[PASS] testChainlogVersionBump() (gas: 4745129)
[PASS] testContractSize() (gas: 8962)
[PASS] testDeployCost() (gas: 2716759)
[PASS] testFailNotScheduled() (gas: 14398)
[PASS] testFailTooEarly() (gas: 13585)
[PASS] testFailTooLate() (gas: 13562)
[PASS] testFailWrongDay() (gas: 13541)
[PASS] testGeneral() (gas: 36860415)
[PASS] testNewChainlogValues() (gas: 1354021)
[PASS] testNextCastTime() (gas: 353681)
[PASS] testOnTime() (gas: 1344456)
[PASS] testPSMs() (gas: 2827672)
[PASS] testPayments() (gas: 1415214)
[PASS] testUseEta() (gas: 352302)
[PASS] testVestDAI() (gas: 1527415)
[PASS] testVestMKR() (gas: 1501333)
[PASS] testYankDAI() (gas: 1361339)
[PASS] testYankMKR() (gas: 1415862)
Test result: ok. 23 passed; 0 failed; finished in 1115.92s

BTW, half of the tests from my report have the same gas amount as yours:

I also ran the tests once more to compare the results and everything matches exactly, except compilation and execution times and the gas amount of one of the tests.