Features: Gasless transactions. #89

jamesduncombe · 2023-10-19T15:42:33Z

🚧 WIP 🚧

Known issues to fix:

~~Actually securing the functions... modifiers etc~~ 💥
~~Working out how to deal with testing locally (the RelayHub etc).~~
~~Fixing and static stubbed values (contract addresses etc).~~
~~Tests in general.~~
supportsInterface forced by IPaymaster interface... but this is on the facet. 💥

OpenGSN ref: https://docs.opengsn.org/contracts/#paying-for-your-user-s-meta-transaction
BasePaymaster.sol: https://github.com/opengsn/gsn/blob/master/packages/contracts/src/BasePaymaster.sol

Notes

Currently includes...

AHasContext - Implementation for _isTrustedForwarder, _msgSender() and _msgData(). Used to override base classes.
AHasForwarder - Implementation of the Forwardable behaviour. Should be inherited from when adding a "Forwardable" facet. Adds setters, getters, events and errors.
LibHasForwarder - Diamond storage for AHasForwarder.
AHasMembers - updated to point to _msgSender(), provides default implementation which points to msg.sender - i.e. it's not aware of anything forwardable etc.
Concrete facet example: MarketplaceAccessFacet - Inherits from AHasMembers and AHasContext.
Concrete facet example: MarketplaceForwardableFacet - Inherits from AHasForwarder and AHasContext.

MarketplaceAccessFacet example:

contract MarketplaceAccessFacet is AMarketplaceFacet, AHasMembers, AHasContext, IHasActiveMembers {

...

  /// AHasContext implementation.

  // Delegate to Forwardable Marketplace facet.
  function _isTrustedForwarder(address forwarder) internal view override(AHasContext) returns (bool) {
    return AHasForwarder(address(this)).isTrustedForwarder(forwarder);
  }
  
  // Delegate to default implementation.
  function _msgSender() internal view override(AHasMembers, AHasContext) returns (address) {
    return AHasContext._msgSender();
  }

...
}

shortcut-integration · 2023-10-19T15:42:37Z

This pull request has been linked to Shortcut Story #9185: Gasless transactions.

codecov · 2023-10-23T17:31:33Z

Codecov Report

Attention: Patch coverage is 91.79104% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 79.03%. Comparing base (fd3cf39) to head (588a177).
Report is 7 commits behind head on main.

Files	Patch %	Lines
contracts/common/AHasContext.sol	55.55%	4 Missing ⚠️
contracts/common/AHasGovernors.sol	50.00%	2 Missing ⚠️
contracts/fast/FastForwardableFacet.sol	0.00%	2 Missing ⚠️
contracts/paymaster/PaymasterTopFacet.sol	95.65%	2 Missing ⚠️
contracts/common/AHasForwarder.sol	92.30%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            main      #89       +/-   ##
==========================================
+ Coverage   7.52%   79.03%   +71.50%     
==========================================
  Files         47       56        +9     
  Lines        797     1116      +319     
  Branches     210      234       +24     
==========================================
+ Hits          60      882      +822     
+ Misses       737      234      -503

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

hickscorp · 2023-10-24T10:58:35Z

contracts/interfaces/IERC165.sol

-}
+// Directly import IERC165 from OpenZeppelin contracts.
+// Solves an issue with 2 conflicting definitions of IERC165.
+import {IERC165} from "@openzeppelin/contracts/interfaces/IERC165.sol";


Very nice reusing something from libs here.

hickscorp · 2023-10-24T10:59:05Z

contracts/common/lib/LibHasForwarder.sol

+  // The current version of the storage.
+  uint16 internal constant STORAGE_VERSION = 1;
+  // This is `keccak256('HasForwarder.storage.Main')`.
+  bytes32 internal constant STORAGE_SLOT = 0xa9930c2ffa1b605b0243ba36b3020146bcba5a29c05a711f5ca7c705a8e851ca;


hickscorp · 2023-10-24T11:00:48Z

contracts/marketplace/MarketplaceInitFacet.sol

+    // ------------------------------------- //
+
+    // Initialize forwarder storage.
+    LibHasForwarder.data().forwarderAddress = params.trustedForwarder;


I don't know how this will work in production.
Adding to the initialize function will work locally or when deploying the Smart Contract from scratch...
But in production you can't really call initialize again.
Maybe what you could do is add a function to the InitFacet such as migrateV1ToV2 and add the code there?

Then your initialize method could call migrateV1ToV2, and in production we would simply call migrateV1ToV2 once the facet is upgraded.

This leaves with the modifiers question. migrateV1ToV2 should probably allow for only the deployer to call it? I think it would cover both cases... I'm not 100% sure on this one.

hickscorp · 2023-10-24T11:02:42Z