Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimized validation pattern #575

Open
0xClandestine opened this issue May 26, 2023 · 3 comments
Open

Optimized validation pattern #575

0xClandestine opened this issue May 26, 2023 · 3 comments

Comments

@0xClandestine
Copy link

0xClandestine commented May 26, 2023

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
Copy link
Member

mmv08 commented May 26, 2023

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
Copy link
Author

0xClandestine commented May 26, 2023

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
Copy link
Member

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.

farreldarian pushed a commit to farreldarian/safe-contracts that referenced this issue Jan 14, 2024
* Add provider info

* Use sdk personal sign

* signOut with option

* Remove provider

* Sign safe transaction

* Test in gnosis chain

* Use ethers provider

* Update naming from Web3Auth to SafeAuth

* Renaming and persistent login

* Add tests

* set selected safe

* Add clearInit call in pack

* Remove signTransaction

* Fix typed Data sign

* Add test with ethers signer

* Added proposed transaction instead execute to check signature

* Throw error if the provider is not the web3auth one

* use SafeTx as typedData

* Metamask verify

* Update example sign and execute code

* sign transactions for testing purposes

* Remove @toruslabs/ethereum-controllers

* Publish

 - @safe-global/auth-kit@2.0.0-alpha.10

* Fix wrong chain in example app
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants