Skip to content
This repository has been archived by the owner on Nov 27, 2024. It is now read-only.

feat: Gasless Meta Transactions #208

Merged
merged 7 commits into from
Apr 10, 2023
Merged

Conversation

0xrajath
Copy link
Contributor

@0xrajath 0xrajath commented Apr 3, 2023

Motivation:

We need to add support for createActionBySig and also fix the user in castApprovalBySig and castDisapprovalBySig and add replay protection

Modifications:

  • createActionBySig
  • user check in castApprovalBySig and castDisapprovalBySig
  • added reason in castApprovalBySig and castDisapprovalBySig
  • createActionBySig test scaffold
  • Using nonces for replay protection

Result:

Support for Gasless transactions. Closes #49

@0xrajath 0xrajath self-assigned this Apr 3, 2023
src/VertexCore.sol Outdated Show resolved Hide resolved
@0xrajath
Copy link
Contributor Author

0xrajath commented Apr 3, 2023

Do we have any worry of censorable signed messages similar to https://eips.ethereum.org/EIPS/eip-2612#security-considerations? Should we have some concept of a deadline similar to this for the signature functions?

src/VertexCore.sol Outdated Show resolved Hide resolved
@0xrajath 0xrajath changed the title feat: Gasless Transactions feat: Gasless Meta Transactions Apr 3, 2023
@0xrajath
Copy link
Contributor Author

0xrajath commented Apr 3, 2023

As far as I can see there is no specific issue to being frontrun on these transactions here. If anyone thinks otherwise feel free to let me know what the issue might be.

src/VertexCore.sol Show resolved Hide resolved
src/VertexCore.sol Show resolved Hide resolved
src/VertexCore.sol Outdated Show resolved Hide resolved
src/VertexCore.sol Outdated Show resolved Hide resolved
src/VertexCore.sol Outdated Show resolved Hide resolved
src/VertexCore.sol Outdated Show resolved Hide resolved
src/VertexCore.sol Show resolved Hide resolved
@0xrajath
Copy link
Contributor Author

0xrajath commented Apr 6, 2023

Do we have any worry of censorable signed messages similar to https://eips.ethereum.org/EIPS/eip-2612#security-considerations? Should we have some concept of a deadline similar to this for the signature functions?

We have decided against using a deadline as it's unnecessary for the reasons listed here: ethereum/EIPs#2613

@0xrajath
Copy link
Contributor Author

0xrajath commented Apr 6, 2023

As far as I can see there is no specific issue to being frontrun on these transactions here. If anyone thinks otherwise feel free to let me know what the issue might be.

Confirmed that there is no speicific issue to being frontrun on these transactions here.

AustinGreen added a commit that referenced this pull request Apr 6, 2023
**Motivation:**

Close #187 

**Modifications:**

Completed the TODOs in VertexCore besides the signature functions that
will be merged in #208 and integration tests.

**Result:**

VertexCore test coverage will improve.
@0xrajath 0xrajath force-pushed the rajath/gasless-transactions-latest branch from 5626cd1 to 4049493 Compare April 6, 2023 22:26
@0xrajath 0xrajath force-pushed the rajath/gasless-transactions-latest branch from 4049493 to bf6b206 Compare April 10, 2023 20:28
@github-actions
Copy link

Coverage after merging rajath/gasless-transactions-latest into main will be

88.82%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   VertexAccount.sol100%100%100%100%
   VertexCore.sol86.79%85%78.26%89.15%207, 230–231, 231, 231–232, 320, 333–334, 334, 334–335, 370, 385–386, 386, 386–387, 539, 611, 615, 619, 621
   VertexFactory.sol100%100%100%100%
   VertexLens.sol100%100%100%100%
   VertexPolicy.sol87.40%77.78%90%88.61%142, 185–186, 188, 240–241, 246, 251–252, 271, 327, 336, 374
   VertexPolicyTokenURI.sol100%100%100%100%
   VertexStrategy.sol97.96%83.33%100%100%178
src/lib
   Checkpoints.sol79.68%72.22%94.44%80.87%105, 204, 216–217, 258–260, 260, 260–261, 263, 266, 294–295, 333–335, 337–339, 341–343, 345–347, 349–351, 353–355, 361–362, 380, 389, 53
   ERC721NonTransferableMinimalProxy.sol72.46%72.73%72.73%72.22%102, 104, 106, 118, 190, 88, 88, 88, 90, 90, 90, 92, 92, 92, 97, 99

Copy link
Contributor

@dd0sxx dd0sxx left a comment

Choose a reason for hiding this comment

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

Is there an issue to write test implementations? Looks good minus the lack of test coverage

@0xrajath
Copy link
Contributor Author

Is there an issue to write test implementations? Looks good minus the lack of test coverage

Yup. Just created one: #222. I'm currently working on this. Will link it to this issue when I raise the PR.

@0xrajath 0xrajath merged commit 647dc07 into main Apr 10, 2023
@0xrajath 0xrajath deleted the rajath/gasless-transactions-latest branch April 10, 2023 21:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: Gasless transactions for users
3 participants