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

WIP:: Custom method selector SVM (ABI SVM) #174

Merged
merged 40 commits into from
Feb 16, 2024
Merged

Conversation

filmakarov
Copy link
Collaborator

@filmakarov filmakarov commented Nov 24, 2023

Summary

Introduces the new Session Validation Module, which can validate an action, performed by userOp, based on:

  • Destination contract
  • Method selector
  • Rules for arguments of the method (==, !=, <, <=, >, >=). Every argument is a 32bytes word.

It allows a dApp to validate calls from a SmartAccount to any method of any contract.
This SVM can be a useful building block for custom flows via the Batched Session Router.

Detailed Document:
ABI SVM the universal Session Validation Module 2716292a4efe4b6b96d4910927ea3cca.pdf

Related Issue: #SMA-363

Change Type

  • New Feature

Checklist

  • My code follows this project's style guidelines
  • I've reviewed my own code
  • I've added comments for any hard-to-understand areas
  • I've updated the documentation if necessary
  • My changes generate no new warnings
  • I've added tests that prove my fix is effective or my feature works
  • All unit tests pass locally with my changes
  • Any dependent changes have been merged and published

@filmakarov filmakarov changed the base branch from main to develop November 24, 2023 17:20
@filmakarov filmakarov marked this pull request as draft November 30, 2023 15:25
@filmakarov filmakarov self-assigned this Dec 27, 2023
@livingrockrises
Copy link
Contributor

If the argument is address then how does rules like <= >= apply and be relevant

@livingrockrises
Copy link
Contributor

can you document more about how to add rules. I think we will hand hold people a lot understanding offset especially. and generating session key data accordingly cause this heavily depends on contract and method

* @return sessionKey address of the sessionKey that signed the userOp
* for example to store a list of allowed tokens or receivers
*/
function _validateSessionParams(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add more comments on solidity assembly blocks about what it achieves.
also would be great is constant numbers can be defined as constants or some offset constant then x:x+4 (depending on bytes)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added couple of constants, in other places just added comments not have constant+constant (which would add gas) instead of just hardcoded value

@livingrockrises
Copy link
Contributor

can sessionKeyData just be concat of

sessionKey,
permission.destContract,
permission.functionSelector,

and not providing anything else at all?

Copy link
Contributor

@livingrockrises livingrockrises left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review ii

@filmakarov
Copy link
Collaborator Author

can you document more about how to add rules. I think we will hand hold people a lot understanding offset especially. and generating session key data accordingly cause this heavily depends on contract and method

Yeah, it is explained here I think
https://www.notion.so/biconomy/ABI-SVM-the-universal-Session-Validation-Module-for-basic-use-cases-2716292a4efe4b6b96d4910927ea3cca
Looking forward to getting your feedback on this doc

@filmakarov
Copy link
Collaborator Author

If the argument is address then how does rules like <= >= apply and be relevant

technically any address is a uint as well. so technically it can be applied, but logically it rarely makes sense
but since we consider everything as bytes32 not address or uint, all the conditions can be applied to any arg

@filmakarov
Copy link
Collaborator Author

can sessionKeyData just be concat of

sessionKey,
permission.destContract,
permission.functionSelector,

and not providing anything else at all?

yes, added according test case

@livingrockrises
Copy link
Contributor

can you document more about how to add rules. I think we will hand hold people a lot understanding offset especially. and generating session key data accordingly cause this heavily depends on contract and method

Yeah, it is explained here I think https://www.notion.so/biconomy/ABI-SVM-the-universal-Session-Validation-Module-for-basic-use-cases-2716292a4efe4b6b96d4910927ea3cca Looking forward to getting your feedback on this doc

will provide feedback this week

@livingrockrises
Copy link
Contributor

can sessionKeyData just be concat of

sessionKey,
permission.destContract,
permission.functionSelector,

and not providing anything else at all?

yes, added according test case

can you link it here or on dm. sorry for trouble again

@filmakarov
Copy link
Collaborator Author

==== SELF-REVIEW AND INTERNAL REVIEWS ARE DONE ====

@filmakarov
Copy link
Collaborator Author

Linking the documentation in the PR Description

@filmakarov
Copy link
Collaborator Author

Passing to Security Auditors

@filmakarov filmakarov merged commit fbd8360 into develop Feb 16, 2024
2 checks passed
@filmakarov filmakarov deleted the feat/SMA-363-ABI-SVM branch February 16, 2024 10:52
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

Successfully merging this pull request may close these issues.

5 participants