[Safe 1.5.0] Evaluate Posibility of moving `checkSignatures(bytes32, bytes, bytes)` into `Safe`

[nlordell]

Context / issue

Currently, the old checkSignatures(bytes32, bytes, bytes) function was moved to the fallback handler. It would be nice to keep it in the Safe contract for gas reasons, but we are running into code size limits. Evaluate if it is possible to save small amounts of code to make this possible.

Additional context

687

[mmv08]

I think we removed everything that could be removed. I don't think it's possible. Is it a significant overhead for the consumers to update to the new method?

[nlordell]

I have some tiny ideas that might add up to something meaningful:

---

Use uint256 for v internally. AFAIR, uintN will mask on every access (so each of the if branches) - if we keep it a uint256 until we need to cast it to a uint8, then we might save some bytes

https://github.com/safe-global/safe-contracts/blob/69caefcda788f2f6b0b154d50d010897560c8deb/contracts/Safe.sol#L316

---

Generally speaking, there are a few instances of bytes memory that may possible be converted to bytes calldata. Needs further analysis.

---

We have some array accesses that I think generate Solidity bounds checks, but aren’t needed. For example:

https://github.com/safe-global/safe-contracts/blob/69caefcda788f2f6b0b154d50d010897560c8deb/contracts/base/ModuleManager.sol#L167

---

Some view methods can move to the FallbackHandler implementation. For example:

https://github.com/safe-global/safe-contracts/blob/69caefcda788f2f6b0b154d50d010897560c8deb/contracts/common/StorageAccessible.sol#L17C59-L17C65

At an extreme, even the getOwners* and getModules* view methods can also be moved there.

---

Anyway - don’t know if we should do it or not, we just might be able to squeeze it in if we try really hard.

Is it a significant overhead for the consumers to update to the new method?

It is just annoying for modules like the 4337 one, where we would need to keep a v1.4.1 version and a v1.5.0 version (or pay the additional gas of going over the fallback handler).

[remedcu]

I tried moving the CompatibilityFallbackHandler:checkSignatures(bytes32,bytes memory,bytes memory) to Safe.sol and though it increased the bytecode size by around 400, it still is slightly less than the limit of 24KB.

Change made in Safe.sol:

    function checkSignatures(bytes32 dataHash, bytes memory signatures) public view {
        checkSignatures(dataHash, "", signatures);
    }

    function checkSignatures(bytes32 dataHash, bytes memory /* IGNORED */, bytes memory signatures) public view {
        // Load threshold to avoid multiple storage loads
        uint256 _threshold = threshold;
        // Check that a threshold is set
        require(_threshold > 0, "GS001");
        checkNSignatures(msg.sender, dataHash, signatures, _threshold);
    }

Updated size (using npx hardhat codesize):

Safe 23666 bytes (limit is 24576)
SafeL2 24508 bytes (limit is 24576)

I checked these changes on the last commit (using git rev-parse HEAD):

69caefcda788f2f6b0b154d50d010897560c8deb