sujithsomraaj / lifi-emergency-pause-facet-v1.2.0

1 day review - 1 / 09
0 stars 0 forks source link

Add batch remove function #8

Open sujithsomraaj opened 2 months ago

sujithsomraaj commented 2 months ago

Context: EmergencyPauseFacet.sol

Description: The removeFacet function allows the pauser (or) owner to pause a specific facet address passed in. However, in some emergency scenarios, there might be a reason to remove multiple facets simultaneously, which can now be batched through a multi-sig. However, an additional batch remove function could be helpful if the pauser wallet is an EOA.

Recommendation: Consider adding a new batchRemoveFacet function that simultaneously accepts and pauses multiple facets.

LI.FI: Thanks for pointing it out but I think we dont need this. If one facet is affected, we will remove it. If several facets are affected, we will pause the diamond and unpause it with the list of facets that should be removed.

Researcher: Acknowledged.

0xDEnYO commented 2 months ago

Thanks for pointing it out but I think we dont need this. If one facet is affected: we will remove it If several facets are affected, we will pause the diamond and unpause it with the list of facets that should be removed.

The scenario is unlikely imho. Also more facets affected means more complexity in which case we should pause the diamond.

I would leave this as as but will wait for Ed's feedback before I close this issue.

maxklenk commented 1 month ago

I agree with Daniel, the scenario is unlikely and we have it covered with the default pause flow. If really multiple facets are affected pausing everything will be safer and gives us time to review all facets before reenabling anything.

ezynda3 commented 1 month ago

Agreed with Daniel and Max