Delete pending cap when removing a market

[Jean-Grimal]

Fixes https://cantina.xyz/code/8409a0ce-6c21-4cc9-8ef2-bd77ce7425af/findings/ce072b25-742b-4a94-b430-4a24c445815b

[MathisGD]

to me we should more prevent any cap setting when the market is being removed

[MerlinEgalite]

to me we should more prevent any cap setting when the market is being removed

indeed it sounds good

[Rubilmax]

to me we should more prevent any cap setting when the market is being removed

There's one thing for sure is that this PR does not 100% fix the issue raised: even if the pending cap is deleted, it can be submitted again afterwards
Am I right?

In which case I believe reverting when removableAt != 0 in _setCap is better

[Rubilmax]

See above

[MerlinEgalite]

to me we should more prevent any cap setting when the market is being removed

There's one thing for sure is that this PR does not 100% fix the issue raised: even if the pending cap is deleted, it can be submitted again afterwards Am I right?

In which case I believe reverting when removableAt != 0 in _setCap is better

which would be fixed by #353 right?

[Jean-Grimal]

to me we should more prevent any cap setting when the market is being removed

There's one thing for sure is that this PR does not 100% fix the issue raised: even if the pending cap is deleted, it can be submitted again afterwards Am I right?
In which case I believe reverting when removableAt != 0 in _setCap is better

which would be fixed by #353 right?

I don't think so,

@MathisGD @Rubilmax should I make the change in this PR ?

[QGarchery]

[optional] I realize now that these changes (both config[id] and pendingCap[id]) may be difficult to follow offchain with events. This requires to do the difference of sets to know which markets were removed. Maybe emitting an event here would be nice for offchain purposes

[MerlinEgalite]

cc @julien-devatom

[QGarchery]

There's one thing for sure is that this PR does not 100% fix the issue raised: even if the pending cap is deleted, it can be submitted again afterwards
Am I right?

I thought it would need happen after the removal of the market (or otherwise it gets deleted), which sounds like a wanted behavior.

But actually it could happen that the pending cap is accepted first in 2 cases:

• the removal of the market is done slowly after its timelock, for example a couple of blocks slip by and the pending cap timelock is reached
• the timelock is reduced, and a new pending cap is submitted before the end of the removal of the market timelock

So in the end I agree that we should enforce stricter rules on the pending cap

[StErMi]

Should you also delete any pending cap proposal when the market is removed from the withdrawal queue?

[MerlinEgalite]

Should you also delete any pending cap proposal when the market is removed from the withdrawal queue?

I don't think so since pendingCap[id].validAt is checked before. If this timestamp is null then the pending cap is null as well.

[QGarchery]

Sounds good, it fixes this comment because now the pending cap prevents removal (which means there are no problem of ordering market removal and updating caps)

[Jean-Grimal]

LGTM