makerdao / spells-mainnet

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

2023-05-24: Keeper Network Streams, RWA014 onboarding #344

Closed 0xdecr1pto closed 1 year ago

0xdecr1pto commented 1 year ago

Description

Contribution Checklist

Checklist

0xdecr1pto commented 1 year ago

@hexonaut Can you please review the Spark GNO part and confirm that it's correct

hexonaut commented 1 year ago

@hexonaut Can you please review the Spark GNO part and confirm that it's correct

Reviewed and it is correct.

SidestreamSweatyPumpkin commented 1 year ago

Tests now pass


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

Running 37 tests for src/DssSpell.t.sol:DssSpellTest
[PASS] testAuth() (gas: 9223371487103698441)
[PASS] testAuthInSources() (gas: 9223371487099004440)
[PASS] testBytecodeMatches() (gas: 5355603)
[PASS] testCastCost() (gas: 5957515)
[PASS] testChainlogValues() (gas: 14382310)
[PASS] testChainlogVersionBump() (gas: 8410918)
[PASS] testContractSize() (gas: 8984)
[PASS] testDeployCost() (gas: 5333022)
[PASS] testDssCronPaymentAdaptersParams() (gas: 5981414)
[PASS] testFailNotScheduled() (gas: 14384)
[PASS] testFailRWA014_DRAW_ABOVE_LINE() (gas: 6368642)
[PASS] testFailRWA014_INTEGRATION_CURE_BEFORE_TELL() (gas: 6339328)
[PASS] testFailRWA014_OUTPUT_CONDUIT_PUSH_ABOVE_BALANCE() (gas: 6602165)
[PASS] testFailTooEarly() (gas: 417636)
[PASS] testFailTooLate() (gas: 417571)
[PASS] testFailWrongDay() (gas: 417640)
[PASS] testGeneral() (gas: 44314278)
[PASS] testNewChainlogValues() (gas: 6076433)
[PASS] testNewIlkRegistryValues() (gas: 5980413)
[PASS] testNextCastTime() (gas: 446343)
[PASS] testOnTime() (gas: 5945085)
[PASS] testPSMs() (gas: 7353662)
[PASS] testPayments() (gas: 6015751)
[PASS] testRWA014_CONTRACT_DEPLOYMENT_SETUP() (gas: 129551)
[PASS] testRWA014_INTEGRATION_BUMP() (gas: 5972711)
[PASS] testRWA014_INTEGRATION_CONDUITS_SETUP() (gas: 5996072)
[PASS] testRWA014_INTEGRATION_TELL() (gas: 5970820)
[PASS] testRWA014_INTEGRATION_TELL_CULL() (gas: 5978705)
[PASS] testRWA014_INTEGRATION_TELL_CURE_GOOD() (gas: 5969425)
[PASS] testRWA014_OPERATOR_LOCK_DRAW_CAGE() (gas: 6897294)
[PASS] testRWA014_PAUSE_PROXY_OWNS_RWA014_TOKEN_BEFORE_SPELL() (gas: 16486)
[PASS] testRWA014_SPELL_LOCK() (gas: 5961931)
[PASS] testRWA014_SPELL_LOCK_OPERATOR_DRAW_WIPE_FREE() (gas: 6476750)
[PASS] testSparkLendCollateralOnboarding() (gas: 6833479)
[PASS] testUseEta() (gas: 352537)
[PASS] testVestDAI() (gas: 6295436)
[PASS] testYankDAI() (gas: 5961990)
Test result: ok. 37 passed; 0 failed; finished in 1140.44s
SidestreamSweatyPumpkin commented 1 year ago

Can you add one test for Spark-interacted contracts that checks the core contract wards before and after spell execution? After this one I can calmly approve it.

e.g.

@The-Arbiter writes

No SubDao contract being interacted with is authed on a core contract like vat, etc. (script this eventually) This is correct for vat but I am not sure about anything else (wards script is broken right now)

so in this case i beleive we could write a test for these instead.

so the flow would go like ( in pseduo code )


for core-contract in corecontracts:
  ensure-subdao-contract-is-not-authed(core-contract)
  cast-spell()
  ensure-subdao-contract-is-not-authed(core-contract)
0xdecr1pto commented 1 year ago

Can you add one test for Spark-interacted contracts that checks the core contract wards before and after spell execution? After this one I can calmly approve it.

e.g.

@The-Arbiter writes

No SubDao contract being interacted with is authed on a core contract like vat, etc. (script this eventually) This is correct for vat but I am not sure about anything else (wards script is broken right now)

so in this case i beleive we could write a test for these instead.

so the flow would go like ( in pseduo code )

for core-contract in corecontracts:
  ensure-subdao-contract-is-not-authed(core-contract)
  cast-spell()
  ensure-subdao-contract-is-not-authed(core-contract)

This spell technically has no chance to set wards implicitly as soon as we don't usedelegatecall. Checking for wards is reviewer-specific action (Same as checking correct constructor args for deployed components, etc.)