5afe / safe-core-protocol-specs

Safe{Core} Protocol is an open, modular framework to make smart accounts secure, portable, and composable.
GNU General Public License v3.0
67 stars 13 forks source link

Update specs for Signature validator #44

Closed akshay-ap closed 11 months ago

akshay-ap commented 11 months ago

Fixes #43

This PR gives details of validation of EIP712 signatures only.

Changes in PR:

akshay-ap commented 11 months ago

Following use cases are not covered for me (will document this also in the attached issue):

  • How to handle multiple signature validators
  • How does this relate to 4337 (as there is also signature validation necessary)

Added details on handling multiple signatures validators Regarding 4337, it hasn't been considered so far yet. We have a followup task to be covered in next iterations.

akshay-ap commented 11 months ago

There are still a lot of open questions.

I think it would also be helpful to create a diagram that put this manager into context with the existing manager. As mentioned before for me this should all happen within the same contract, but would be nice to document considerations for this (might be more on the implementation side).

I would also propose to restructure this spec section. To start with after the general motivation that is already there I would start with explaining the general flow supported by the validation sequence diagram.

After that a short recap of the most important parts of ERC-712 (domain) and the mapping based on the domain could be explained. Here also the encoding of the signature should be outlined, as this is essential for the validation flow.

Then maybe have a section on the validator hooks and outline how these can improve security and why this is important.

Once all the components have been explained it can be explained how these are combined in the manager.

Only after that I would have a section with the Solidity interfaces.

This commit addresses most of the points mentioned