Sherlock-110: revert add settleable

[mattcushman]

Description of change

High level

• see hyh - Debt write off can be prohibited by HPB depositor by continuously allocating settlement blocking dust deposits in the higher buckets sherlock-audit/2023-04-ajna-judging#110
  • do not allow adding quote tokens into the pool if there's auction to settle
  • Pool.addQuoteToken revert with AuctionNotCleared custom error

Contract size

Pre Change

============ Deployment Bytecode Sizes ============
  ERC721Pool               -  24,477B  (99.59%)
  ERC20Pool                -  23,719B  (96.51%)

Post Change

============ Deployment Bytecode Sizes ============
  ERC721Pool               -  24,490B  (99.65%)
  ERC20Pool                -  23,732B  (96.56%)

Gas usage

Pre Change

| addQuoteToken                        | 122500          | 179774 | 164363 | 715163  | 60004   |

Post Change

| addQuoteToken                        | 122895          | 180303 | 164758 | 719558  | 60004   |

[mattcushman]

lgtm

[ith-harvey]

The links that summarize the issue are dead, please copy and paste the issue into the description

[grandizzy]

The links that summarize the issue are dead, please copy and paste the issue into the description

Will update to new judge repo which is permanent

[ith-harvey]

I consider this change a mitigation to the issue rather than the fix, which is fine. I'm 51% no change 49% fine with this change. As was detailed in the issue why would a lender perform a dusting to protect a position that they can't withdraw? The incentives don't seem significant enough to warrant a lender performing this dusting...

As a lender who is performing the dusting I see two very simple workarounds:

• dust with addCollateral(), performs the same protections
• call addQuoteToken() on the 71st hour of the auction (before settle is callable) , this mitigation doesn't protect against.

Approving PR because it suffices as a soft mitigation and code looks correct. Again happy to leave code as is and not merge this PR because I don't see sufficient incentives for lender to perform dusting, nor do I see this mitigation as entirely solving the issue.

[grandizzy]

LGTM

[ith-harvey]

I consider this change a mitigation to the issue rather than the fix, which is fine. I'm 51% no change 49% fine with this change. As was detailed in the issue why would a lender perform a dusting to protect a position that they can't withdraw? The incentives don't seem significant enough to warrant a lender performing this dusting...

As a lender who is performing the dusting I see two very simple workarounds:

* dust with `addCollateral()`, performs the same protections

* call `addQuoteToken()` on the 71st hour of the auction (before settle is callable) , this mitigation doesn't protect against.

Approving PR because it suffices as a soft mitigation and code looks correct. Again happy to leave code as is and not merge this PR because I don't see sufficient incentives for lender to perform dusting, nor do I see this mitigation as entirely solving the issue.

I stand corrected, addCollateral() doesn't effect the HPB so that would be ineffective to reproduce the attack.

[MikeHathaway]

LGTM

[EdNoepel]

Unrelated to this change, I was surprised to learn we don't allow lenders to remove collateral when an auction is settleable. LGTM.

[dmitriia]

Looks ok