safe-global / safe-smart-account

Safe allows secure management of blockchain assets.
https://safe.global
GNU Lesser General Public License v3.0
1.84k stars 907 forks source link

Optimized validation pattern #575

Closed 0xClandestine closed 2 months ago

0xClandestine commented 1 year ago

I have developed a validation pattern that can greatly reduce the gas associated with wallet deployment and usage. The concept behind this pattern is rather straightforward: instead of storing various parameters required for validation, such as the list of signers and the quorum, you only store a hash. This hash can be reconstructed by first recovering the list of signers and then hashing them together with any other variables that need to be validated, such as quorum.

To ensure proper reconstruction of the validation hash, it is crucial to provide the signatures in the exact order of the addresses that were hashed during the creation of the validation hash. Moreover, if a signer does not provide a signature, their address must be used as a replacement. This implies that whenever signatures are provided, they MUST ALWAYS be placed at the same index of signatures.

Gas is saved on deployment because only a single slot needs to be modified in order to initialize everything. Furthermore, only a two SLOADs are needed for transaction validation. 1 SLOAD for nonce, and 1 SLOAD for the validation hash as opposed to an SLOAD for each signer, and an additional SLOAD for the signers array length.

  bytes32 public signersAndQuorumHash;

  uint256 public nonce;

  function _verify(bytes32 digest, uint256 quorum, bytes[] calldata signatures)
      internal
      virtual
  {
      address[] memory signers = new address[](signatures.length);

      uint256 totalNonSigners;

      for (uint256 i; i < signatures.length; ++i) {
          if (signatures[i].length != 20) {
              signers[i] = ECDSA.recover(digest, signatures[i]);
          } else {
              signers[i] = address(bytes20(signatures[i]));

              ++totalNonSigners;
          }
      }

      // Assert the list of signers and quorum are correct.
      if (keccak256(abi.encodePacked(signers, quorum)) != signersAndQuorumHash) {
          revert VerificationFailed();
      }

      // Assert m-of-n of the signers have signed.
      if (signatures.length - totalNonSigners < quorum) {
          revert InsufficientSigners();
      }
  }

  function call(
      address target,
      uint256 value,
      uint256 deadline,
      uint256 quorum,
      bytes calldata data,
      bytes[] calldata signatures
  ) external payable virtual {
      unchecked {
          bytes32 digest = _hashTypedData(
              keccak256(abi.encode(CALL_TYPEHASH, target, value, data, deadline, nonce++))
          );

          _verify(digest, quorum, signatures);

          (bool success,) = target.call{value: value}(data);

          if (!success) revert ExecutionReverted();
      }
  }

Foundry says 77k gas to deploy my version via a clone factory and 45k for an ETH transfer when using 3 signers and with a quorum of 2.

PS: I understand these changes would require a redeployment, so I'm mostly here to bring awareness for the next iteration of Safe**.

Here's the example implementation.

mmv08 commented 1 year ago

That's an interesting approach, indeed! Would be interesting to check how much gas it'd use if it supported all types of signatures that Safe supports (on-chain approvals, eip-191, eip-712, eip1271)

PS: I understand these changes would require a redeployment, so I'm mostly here to bring awareness for the next iteration of Gnosis.

FYI, we spun out of Gnosis and are just Safe now 😉

0xClandestine commented 1 year ago

That's an interesting approach, indeed! Would be interesting to check how much gas it'd use if it supported all types of signatures that Safe supports (on-chain approvals, eip-191, eip-712, eip1271)

PS: I understand these changes would require a redeployment, so I'm mostly here to bring awareness for the next iteration of Gnosis.

FYI, we spun out of Gnosis and are just Safe now 😉

Thanks, I'm currently working on EIP-1271 support; in terms of additional overhead it should only be a conditional statement and external call. Will try to get some better gas comparisons out this week.

PS safe*** haha

EDIT: While I remember, this could also be built as a SafeModule, will get one out as well.

rmeissner commented 1 year ago

I build a similar contract in the past: https://github.com/rmeissner/stateless-vault/blob/main/packages/contracts/contracts/StatelessVault.sol#L185 (it uses MerkleProofs to optimize on the digest for Safes with many owners).

It is an interesting approach, but would require a major rewrite of the Safe including how data is stored. Also it would have an impact of on-chain interaction with the Safe that need to be evaluated.

mmv08 commented 2 months ago

Closing as not planned.