safe-global / safe-smart-account / 16191017663

94%

push

github

Allow Safe to Own Itself (#998)

This change was originally proposed by @akshay-ap 

This PR reverts an old change (See #229 and #259) and allows the Safe to
once again own itself. This is a step towards 7702 support (#997),
whereby we want the EOA that delegates to a Safe to be able to sign
transactions for itself.

The restriction was originally put in place because `execTransaction`
would call `isValidSignature` on itself (which used to be implemented on
the Safe itself and not as part of the compatibility fallback handler),
where the `msg.sender` would be the Safe itself, and allow for trivial
"approved hash" signatures (through

[this](https://github.com/safe-global/safe-smart-account/blob/36db12a14/contracts/GnosisSafe.sol#L244-L245)
mechanism).

This is no longer possible since v1.3.0, as the `isValidSignature` call
would be handled by the fallback handler, meaning that the `msg.sender`
that can trivially sign "approve hash" signatures is no longer the Safe
itself but the configured fallback handler. Since #866 and starting in
v1.5.0 the `CompatibilityFallbackHandler` is implemented to disallow
"approve hash" signatures regardless of the caller, further eliminating
the requirement for the restriction.

### Increasing Threshold Concerns

One concern with this change, and removing the restriction on having the
Safe own itself, is that the Safe cannot sign for itself, so you can
potentially block access to the account completely, by requiring the
Safe "self-owner" to be included as part of the signing threshold. For
example, if you have a n/n Safe (so `threshold == ownerCount`) and one
of the owners is the Safe itself, then the Safe will not be able to ever
produce a valid signature.

304 of 336 branches covered (90.48%)

Branch coverage included in aggregate %.

3 of 3 new or added lines in 1 file covered. (100.0%)

449 of 461 relevant lines covered (97.4%)

109.79 hits per line