search

makerdao / spells-mainnet

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

Incorrect gap in DssSpell.t.base.sol #362

Open ghost opened 10 months ago

ghost commented 10 months ago 
          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.

_Originally posted by @sunbreak1211 in https://github.com/makerdao/spells-mainnet/pull/360#discussion_r1298345077_

ghost commented 10 months ago

@SidestreamColdMelon @0xdecr1pto for attention

0xdecr1pto commented 10 months ago

@SidestreamColdMelon @amusingaxl @sunbreak1211 I found that we can't decrease the offset for 30m, because this test starts failing

assertTrue(sums[0] + 2 * values.line_offset * RAD >= vat.Line(), "TestError/vat-Line-3");

sum of ilks = 5784999296615681261561073747403742780250307433758853657 (in RAY) vat.Line() = 7119856657556728658157855938384546615580869737094691855 (in RAY)

the diff between is 1_334 m

So we can decrease offset for 13m max to 668m

Not sure what was wrong probably offset was messed up at some point

amusingaxl commented 10 months ago

Honestly, I believe we could set Line to a ridiculously high value and forget about it. Individual lines for ilks already do the job of controlling this. Furthermore, with the new Allocation System, SubDAOs will have 1T debt ceilings managed by AutoLine, so this global Line will make even less sense.

sunbreak1211 commented 10 months ago

I personally think the same, Line should be set to max_uint256 and forget about these issues if not gonna be used as a lower value than the sum of ilk lines. Some devs believed it was another security restriction and was worth to keep it but I could never really appreciate the purpose. Btw take into account the 1T is what the oracle + gem supply will allow as max debt. However the debt ceiling will be probably much less (using or not autoline).