-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2023-08-18 Executive Spell #360
Conversation
@@ -110,16 +110,16 @@ contract Config { | |||
// | |||
// Values for all system configuration changes | |||
// | |||
afterSpell.line_offset = 750 * MILLION; // Offset between the global line against the sum of local lines | |||
afterSpell.pot_dsr = 8_00; // In basis points | |||
afterSpell.line_offset = 680 * MILLION; // Offset between the global line against the sum of local lines |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we are changing offset here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the global debt ceiling was changed in this spell. 100M down for CRVV1ETHSTETH-A
, 30M up for RWA002-A
. Btw, the test actually would pass without this change, as it checks if global line is anywhere between line_offset
and line_offset * 2
, but I've changed it to reflect overall decrease in the gap:
spells-mainnet/src/DssSpell.t.base.sol
Lines 692 to 693 in 71685d7
assertTrue(sums[0] + values.line_offset * RAD <= vat.Line(), "TestError/vat-Line-2"); | |
assertTrue(sums[0] + 2 * values.line_offset * RAD >= vat.Line(), "TestError/vat-Line-3"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clear, I think it would be good to have a line about gap in checklist
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't been really following the concrete numbers, but I think the 30M shouldn't be part of the gap change, as that 30M increment in RWA-2
has the corresponding of the global debt ceiling as well. However the -100M from CRVV1ETHSTETH-A
is correct as the global debt ceiling was reduced in that number when the local debt ceiling was already 0. So the reference value IMO (and just following the logic at a high level point of view) should be 650.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the reference value IMO (and just following the logic at a high level point of view) should be 650
Thaks for flagging this, we discussed it at GovOps yesterday and we will fix it in the PR for the upcoming spell
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to deploy. Checked:
- Tests passing
- Office hours off
- Tests are correct
- Wallet addresses updated
- Config changes are good
- Rate updates are good
Diff vs Goerli on spell matches except the following additions which have been checked:
- Description is good and hash matches
- Delegate wallet addresses are correct
- Double checked Spark addresses
- Curve stETH/ETH offboarding parameters are correct
- Delegate payments are correct
- D3M Housekeeping is correct
Good to deploy Mainnet Executive Spell Review ChecklistMainnet 2023-08-18Spell Actions (Per Exec Doc):
Development Stage
Running 2 tests for src/test/starknet.t.sol:StarknetTests
[PASS] testStarknet() (gas: 2153208)
[PASS] testStarknetSpell() (gas: 2346)
Test result: ok. 2 passed; 0 failed; finished in 58.60s
Running 21 tests for src/DssSpell.t.sol:DssSpellTest
[PASS] testAuth() (gas: 9223371487105614665)
[PASS] testAuthInSources() (gas: 9223371487099402314)
[PASS] testBytecodeMatches() (gas: 4234210)
[PASS] testCastCost() (gas: 1998557)
[PASS] testChainlogValues() (gas: 10835077)
[PASS] testChainlogVersionBump() (gas: 5611276)
[PASS] testContractSize() (gas: 8984)
[PASS] testDeployCost() (gas: 4214698)
[PASS] testFailNotScheduled() (gas: 14375)
[PASS] testFailTooEarly() (gas: 13607)
[PASS] testFailTooLate() (gas: 13584)
[PASS] testFailWrongDay() (gas: 13629)
[PASS] testGeneral() (gas: 39113710)
[PASS] testMKRPayments() (gas: 2131782)
[PASS] testNextCastTime() (gas: 353659)
[PASS] testOnTime() (gas: 1994262)
[PASS] testPSMs() (gas: 3357915)
[PASS] testSparkAdminTransfer() (gas: 2034842)
[PASS] testSparkSpellIsExecuted() (gas: 1997865)
[PASS] testUseEta() (gas: 352346)
[PASS] test_RWA002_Update() (gas: 2196746)
Test result: ok. 21 passed; 0 failed; finished in 963.16s |
The spell is deployed at https://etherscan.io/address/0x2f34BB0FE10BCb5652390FD0bA3Af7879BcA4b62#code |
Here is the simulation of casting it https://dashboard.tenderly.co/shared/fork/simulation/1977ded7-a885-46fd-a40e-1525ca5f3e4e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to submit. Checked:
- Spell code matches flattened repo
- Tests pass
- Spell address, timestamp and block number are good
- DssExecLib matches repo
- Archive matches
Good to submit Deployed Stage
|
Description
This PR implements executive spell planned for 2023-08-18.
Relevant goerli spell: makerdao/spells-goerli#222
Contribution Checklist
Checklist
officeHours
modifier override30 days
unless otherwise specified)Validate all addresses used are in changelog or knownETH_GAS_LIMIT="XXX" ETH_GAS_PRICE="YYY" make deploy
mainnet
contract on etherscanmake archive-spell
ormake date="YYYY-MM-DD" archive-spell
to make an archive directory and copyDssSpell.sol
,DssSpell.t.sol
,DssSpell.t.base.sol
, andDssSpellCollateralOnboarding.sol
squash and merge
this PR