Add MultiCallSendOnly support to Safe{Wallet} guard#427
Add MultiCallSendOnly support to Safe{Wallet} guard#427
MultiCallSendOnly support to Safe{Wallet} guard#427Conversation
This reverts commit 307bb0a.
…stead of mapping from the hash to the expiry
…ta` and `getTransactionHash`
…is improves the UX of using the guard It still requires 2 transactions to be submitted. There's no way around that.
…, but we want to wear tinfoil
duncancmt
force-pushed
the
dcmt/upgrade-timelock-1.4.0-multicall
branch
from
73e1447 to
4a50090
Compare
🛡️ Immunefi PR ReviewsWe noticed that your project isn't set up for automatic code reviews. If you'd like this PR reviewed by the Immunefi team, you can request it manually using the link below: Once submitted, we'll take care of assigning a reviewer and follow up here. |
|
|
||
| uint256 threshold = _safe.getThreshold(); | ||
| if (threshold < _MINIMUM_THRESHOLD) { | ||
| revert ThresholdTooLow(threshold); |
There was a problem hiding this comment.
For a 3 of 5 safe, going through the resignation flow:
_getThresholdAfterResign returns 3 -> removeOwner executes -> 3 of 4 -> this check hits (4 - 3 < 2) -> ThresholdTooHigh
Similar for 4 of 6, etc.
Is this intentional? I assume we want to keep at least some buffer of signers in case keys are lost or inaccessible, but this might block removeOwners needed for cancel at common multisig configurations?
There was a problem hiding this comment.
Let's consider the reverse? What if that scenario were to occur, that the safe had an owner removed and became 3-of-4? How does this affect the recovery game in both the "leaked key" and the "malicious signer" cases?
Given that EIP-8141 is CFI for Hegota and it depends on 7997, compatibility is desirable.
2 participants
In response to #253 (comment)