Allow Safe to Own Itself

[nlordell]

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 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.

[nlordell]

The error duplicate owner is now consistently "GS204".

[nlordell]

Note that if we change the CompatibilityFallbackHandler to not use address(0) as the executor, then this test would fail:

diff --git a/contracts/handler/CompatibilityFallbackHandler.sol b/contracts/handler/CompatibilityFallbackHandler.sol
index 275797d..e652ad0 100644
--- a/contracts/handler/CompatibilityFallbackHandler.sol
+++ b/contracts/handler/CompatibilityFallbackHandler.sol
@@ -80,7 +80,7 @@ contract CompatibilityFallbackHandler is TokenCallbackHandler, ISignatureValidat
         } else {
             // We explicitly do not allow caller approved signatures for EIP-1271 to prevent unexpected behaviour. This
             // is done by setting the executor address to `0` which can never be an owner of the Safe.
-            safe.checkSignatures(address(0), messageHash, _signature);
+            safe.checkSignatures(msg.sender, messageHash, _signature);
         }
         return EIP1271_MAGIC_VALUE;
     }

  1) Safe
       checkSignatures
         should not allow trivial signatures when the Safe owns itself:
     AssertionError: Expected transaction to be reverted with reason 'GS025', but it didn't revert
      at async Context.<anonymous> (test/core/Safe.Signatures.spec.ts:511:13)

[nlordell]

Tagging with do not merge as I want this to be applied after the Safe v1.5.0 release.