From e20c81e1b54a0fd4dcf44234b470104781b36fb0 Mon Sep 17 00:00:00 2001 From: James Duncombe Date: Wed, 25 Oct 2023 18:16:42 +0100 Subject: [PATCH] wip --- contracts/common/AHasForwarder.sol | 62 +++++++++++++++++-- contracts/common/AHasMembers.sol | 5 +- .../marketplace/MarketplaceAccessFacet.sol | 8 +-- .../marketplace/MarketplaceInitFacet.sol | 3 +- contracts/paymaster/BasePaymaster.sol | 3 +- contracts/paymaster/PaymasterTopFacet.sol | 4 +- deploy/20_deployMarketplace.ts | 5 +- package.json | 2 +- tasks/marketplace.ts | 10 +-- .../MarketplaceAccessFacet.test.ts | 32 ++++++++-- .../MarketplaceAutomatonsFacet.test.ts | 1 - ...etplaceFastDeploymentRequestsFacet.test.ts | 1 - test/marketplace/MarketplaceInitFacet.test.ts | 4 +- .../MarketplaceTokenHoldersFacet.test.ts | 1 - test/marketplace/MarketplaceTopFacet.test.ts | 1 - yarn.lock | 11 ++-- 16 files changed, 110 insertions(+), 43 deletions(-) diff --git a/contracts/common/AHasForwarder.sol b/contracts/common/AHasForwarder.sol index 5b2c58e1..c9d3b364 100644 --- a/contracts/common/AHasForwarder.sol +++ b/contracts/common/AHasForwarder.sol @@ -1,22 +1,74 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.10; -import "../common/lib/LibHasForwarder.sol"; +import "./lib/LibHasForwarder.sol"; +// import "@opengsn/contracts/src/ERC2771Recipient.sol"; +import "@opengsn/contracts/src/interfaces/IERC2771Recipient.sol"; -import "@opengsn/contracts/src/ERC2771Recipient.sol"; +import "hardhat/console.sol"; /** * @title The Forwarder Smart Contract. * @notice The HasForwarder abstract contract is in charge of... */ -abstract contract AHasForwarder is ERC2771Recipient { +abstract contract AHasForwarder is IERC2771Recipient { /** - * @notice ERC2771Recipient stuff... - * FIX THE MODIFIER !!!!!!!!!!!!!!!!!!!!!! + * @notice ERC2771Recipient implementation. + * TODO: FIX MODIFIER */ function setTrustedForwarder(address _forwarderAddress) external { LibHasForwarder.Data storage ds = LibHasForwarder.data(); ds.forwarderAddress = _forwarderAddress; _setTrustedForwarder(_forwarderAddress); } + + /* + * Forwarder singleton we accept calls from + */ + address private _trustedForwarder; + + /** + * :warning: **Warning** :warning: The Forwarder can have a full control over your Recipient. Only trust verified Forwarder. + * @notice Method is not a required method to allow Recipients to trust multiple Forwarders. Not recommended yet. + * @return forwarder The address of the Forwarder contract that is being used. + */ + function getTrustedForwarder() public view virtual returns (address forwarder) { + return _trustedForwarder; + } + + function _setTrustedForwarder(address _forwarder) internal { + _trustedForwarder = _forwarder; + } + + /// @inheritdoc IERC2771Recipient + function isTrustedForwarder(address forwarder) public view virtual override returns (bool) { + return forwarder == _trustedForwarder; + } + + /// @inheritdoc IERC2771Recipient + function _msgSender() internal view virtual override returns (address ret) { + console.log("msg.sender %s", msg.sender); + console.log("original msg.data"); + console.logBytes(msg.data); + if (msg.data.length >= 20 && isTrustedForwarder(msg.sender)) { + // At this point we know that the sender is a trusted forwarder, + // so we trust that the last bytes of msg.data are the verified sender address. + // extract sender address from the end of msg.data + assembly { + ret := shr(96, calldataload(sub(calldatasize(), 20))) + } + console.log("last 20 bytes from msg.data %s", ret); + } else { + ret = msg.sender; + } + } + + /// @inheritdoc IERC2771Recipient + function _msgData() internal view virtual override returns (bytes calldata ret) { + if (msg.data.length >= 20 && isTrustedForwarder(msg.sender)) { + return msg.data[0:msg.data.length - 20]; + } else { + return msg.data; + } + } } diff --git a/contracts/common/AHasMembers.sol b/contracts/common/AHasMembers.sol index 5967df59..bbdf772c 100644 --- a/contracts/common/AHasMembers.sol +++ b/contracts/common/AHasMembers.sol @@ -4,13 +4,14 @@ pragma solidity 0.8.10; import "../lib/LibAddressSet.sol"; import "../lib/LibPaginate.sol"; import "../common/lib/LibHasMembers.sol"; +import "../common/AHasForwarder.sol"; import "../interfaces/ICustomErrors.sol"; /** * @title The Fast Smart Contract. * @notice The Fast Members abstract contract is in charge of keeping track of automaton accounts. */ -abstract contract AHasMembers { +abstract contract AHasMembers is AHasForwarder { using LibAddressSet for LibAddressSet.Data; /// Errors. @@ -94,7 +95,7 @@ abstract contract AHasMembers { * @notice Adds a member to the list of known members. * @param who is the address to be added. */ - function addMember(address who) external onlyMemberManager(msg.sender) onlyValidMember(who) { + function addMember(address who) external onlyMemberManager(_msgSender()) onlyValidMember(who) { // Add the member. LibHasMembers.data().memberSet.add(who, false); // Notify via callback. diff --git a/contracts/marketplace/MarketplaceAccessFacet.sol b/contracts/marketplace/MarketplaceAccessFacet.sol index f5983cb5..0475f0b0 100644 --- a/contracts/marketplace/MarketplaceAccessFacet.sol +++ b/contracts/marketplace/MarketplaceAccessFacet.sol @@ -56,12 +56,12 @@ contract MarketplaceAccessFacet is AMarketplaceFacet, AHasMembers, IHasActiveMem */ function memberAddedToFast(address member) external { // Verify that the given address is in fact a registered FAST contract. - if (!IssuerTopFacet(LibMarketplace.data().issuer).isFastRegistered(msg.sender)) { + if (!IssuerTopFacet(LibMarketplace.data().issuer).isFastRegistered(_msgSender())) { revert ICustomErrors.RequiresFastContractCaller(); } // Keep track of the member's FAST membership. // TODO: We don't throw until we've fixed the `marketplace.fastMemberships`. - LibMarketplaceAccess.data().fastMemberships[member].add(msg.sender, true); + LibMarketplaceAccess.data().fastMemberships[member].add(_msgSender(), true); } /** @@ -69,12 +69,12 @@ contract MarketplaceAccessFacet is AMarketplaceFacet, AHasMembers, IHasActiveMem * @param member The member for which a FAST membership has been removed. */ function memberRemovedFromFast(address member) external { - if (!IssuerTopFacet(LibMarketplace.data().issuer).isFastRegistered(msg.sender)) { + if (!IssuerTopFacet(LibMarketplace.data().issuer).isFastRegistered(_msgSender())) { revert ICustomErrors.RequiresFastContractCaller(); } // Remove the tracked membership. // TODO: We don't throw until we've fixed the `marketplace.fastMemberships`. - LibMarketplaceAccess.data().fastMemberships[member].remove(msg.sender, true); + LibMarketplaceAccess.data().fastMemberships[member].remove(_msgSender(), true); } /// IHasActiveMembers implementation. diff --git a/contracts/marketplace/MarketplaceInitFacet.sol b/contracts/marketplace/MarketplaceInitFacet.sol index 362c7e7e..af56c743 100644 --- a/contracts/marketplace/MarketplaceInitFacet.sol +++ b/contracts/marketplace/MarketplaceInitFacet.sol @@ -24,7 +24,6 @@ contract MarketplaceInitFacet is AMarketplaceFacet { struct InitializerParams { address issuer; - address trustedForwarder; } function initialize(InitializerParams calldata params) external onlyDeployer { @@ -68,6 +67,6 @@ contract MarketplaceInitFacet is AMarketplaceFacet { // ------------------------------------- // // Initialize forwarder storage. - LibHasForwarder.data().forwarderAddress = params.trustedForwarder; + LibHasForwarder.data().forwarderAddress = address(0); } } diff --git a/contracts/paymaster/BasePaymaster.sol b/contracts/paymaster/BasePaymaster.sol index 19881f1b..9c0dd091 100644 --- a/contracts/paymaster/BasePaymaster.sol +++ b/contracts/paymaster/BasePaymaster.sol @@ -7,10 +7,11 @@ pragma solidity 0.8.10; import "@openzeppelin/contracts/utils/introspection/ERC165.sol"; import "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol"; +import "@opengsn/contracts/src/utils/GsnTypes.sol"; import "@opengsn/contracts/src/interfaces/IPaymaster.sol"; import "@opengsn/contracts/src/interfaces/IRelayHub.sol"; -import "@opengsn/contracts/src/forwarder/IForwarder.sol"; import "@opengsn/contracts/src/utils/GsnEip712Library.sol"; +import "@opengsn/contracts/src/forwarder/IForwarder.sol"; /** * @notice An abstract base class to be inherited by a concrete Paymaster. diff --git a/contracts/paymaster/PaymasterTopFacet.sol b/contracts/paymaster/PaymasterTopFacet.sol index 10d10597..967f4b4b 100644 --- a/contracts/paymaster/PaymasterTopFacet.sol +++ b/contracts/paymaster/PaymasterTopFacet.sol @@ -31,7 +31,7 @@ contract PaymasterTopFacet is BasePaymaster { (context, success, gasUseWithoutPost, relayData); } - function versionPaymaster() external view override returns (string memory) { - return "3.0.0-beta.3+opengsn.accepteverything.ipaymaster"; + function versionPaymaster() external pure override returns (string memory) { + return "3.0.0-beta.9+opengsn.tokensphere.ipaymaster"; } } diff --git a/deploy/20_deployMarketplace.ts b/deploy/20_deployMarketplace.ts index dec68ce9..53cb67c9 100644 --- a/deploy/20_deployMarketplace.ts +++ b/deploy/20_deployMarketplace.ts @@ -6,10 +6,7 @@ import { deployments } from "hardhat"; const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { console.log("----------------------------------- 20_deployMarketplace"); - // TODO: From a lookup... - const trustedForwarderAddr = "0xB2b5841DBeF766d4b521221732F9B618fCf34A87"; - - await deployMarketplace(hre, (await deployments.get("Issuer")).address, trustedForwarderAddr); + await deployMarketplace(hre, (await deployments.get("Issuer")).address); }; func.tags = ["DeployMarketplace"]; export default func; diff --git a/package.json b/package.json index bbbddd6c..7bb4d8d6 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ }, "dependencies": { "@openzeppelin/contracts": "^4.9.2", - "@opengsn/contracts": "^3.0.0-beta.6", + "@opengsn/contracts": "^3.0.0-beta.9", "@typechain/ethers-v5": "^11.1.1", "@typechain/hardhat": "^9.0.0", "ethers": "^5.7.2", diff --git a/tasks/marketplace.ts b/tasks/marketplace.ts index 183ca455..c47440d3 100644 --- a/tasks/marketplace.ts +++ b/tasks/marketplace.ts @@ -11,9 +11,7 @@ interface MarketplaceDeployParams { } task("marketplace-deploy", "Deploys the main Marketplace contract").setAction( async (_params: MarketplaceDeployParams, hre) => { const { address: issuerAddr } = await hre.deployments.get("Issuer"); - // TODO: From a lookup... - const trustedForwarderAddr = "0xB2b5841DBeF766d4b521221732F9B618fCf34A87"; - await deployMarketplace(hre, issuerAddr, trustedForwarderAddr); + await deployMarketplace(hre, issuerAddr); } ); @@ -64,8 +62,7 @@ const MARKETPLACE_FACETS = [ const deployMarketplace = async ( hre: HardhatRuntimeEnvironment, - issuerAddr: string, - trustedForwarderAddr: string + issuerAddr: string ): Promise => { const { ethers, deployments, getNamedAccounts } = hre; const { diamond } = deployments; @@ -84,8 +81,7 @@ const deployMarketplace = async ( contract: "MarketplaceInitFacet", methodName: "initialize", args: [{ - issuer: issuerAddr, - trustedForwarder: trustedForwarderAddr + issuer: issuerAddr }], }, deterministicSalt: deploymentSalt(hre), diff --git a/test/marketplace/MarketplaceAccessFacet.test.ts b/test/marketplace/MarketplaceAccessFacet.test.ts index d826b8a9..dc65f87b 100644 --- a/test/marketplace/MarketplaceAccessFacet.test.ts +++ b/test/marketplace/MarketplaceAccessFacet.test.ts @@ -24,13 +24,15 @@ describe("MarketplaceAccessFacet", () => { alice: SignerWithAddress, bob: SignerWithAddress, rob: SignerWithAddress, - john: SignerWithAddress; + john: SignerWithAddress, + trustedForwarder: SignerWithAddress; let issuer: FakeContract, fast: FakeContract, marketplace: Marketplace, access: MarketplaceAccessFacet, - issuerMemberAccess: MarketplaceAccessFacet; + issuerMemberAccess: MarketplaceAccessFacet, + trustedForwarderAccess: MarketplaceAccessFacet; const marketplaceDeployFixture = deployments.createFixture( marketplaceFixtureFunc @@ -50,7 +52,7 @@ describe("MarketplaceAccessFacet", () => { before(async () => { // Keep track of a few signers. - [deployer, issuerMember, alice, bob, rob, john] = await ethers.getSigners(); + [deployer, issuerMember, alice, bob, rob, john, trustedForwarder] = await ethers.getSigners(); // Mock Issuer and Fast contracts. issuer = await smock.fake("Issuer"); fast = await smock.fake("Fast"); @@ -68,11 +70,11 @@ describe("MarketplaceAccessFacet", () => { marketplace.address ); issuerMemberAccess = access.connect(issuerMember); + trustedForwarderAccess = access.connect(trustedForwarder); }, }, initWith: { issuer: issuer.address, - trustedForwarder: ethers.constants.AddressZero, }, }); @@ -147,6 +149,28 @@ describe("MarketplaceAccessFacet", () => { await expect(subject).to.have.revertedWith(`RequiresMembersManager`); }); + it.only("accepts a trusted forwarder as the msg.sender", async () => { + // Set the trusted forwarder. + await issuerMemberAccess.setTrustedForwarder(trustedForwarder.address); + + console.log('forwarder', await issuerMemberAccess.getTrustedForwarder()); + console.log('issuer', issuerMember.address); + console.log('alice', alice.address); + + // Now add member... + // Don't do this at home kids... + let encodedFunctionCall = await trustedForwarderAccess.interface.encodeFunctionData("addMember", [alice.address]); + let data = `${encodedFunctionCall}${issuerMember.address.slice(2)}`; + + // Override the transaction data. + // Issuer really called this :D + await trustedForwarder.sendTransaction({ data: data, to: access.address }); + + // Check that Alice is a member. + const subject = await marketplace.isMember(alice.address); + expect(subject).to.eq(true); + }); + it("delegates to the Issuer for permission", async () => { await issuerMemberAccess.addMember(alice.address); expect(issuer.isMember).to.be.calledOnceWith(issuerMember.address); diff --git a/test/marketplace/MarketplaceAutomatonsFacet.test.ts b/test/marketplace/MarketplaceAutomatonsFacet.test.ts index 9775aaac..451b5a17 100644 --- a/test/marketplace/MarketplaceAutomatonsFacet.test.ts +++ b/test/marketplace/MarketplaceAutomatonsFacet.test.ts @@ -79,7 +79,6 @@ describe("MarketplaceAutomatonsFacet", () => { }, initWith: { issuer: issuer.address, - trustedForwarder: ethers.constants.AddressZero }, }); }); diff --git a/test/marketplace/MarketplaceFastDeploymentRequestsFacet.test.ts b/test/marketplace/MarketplaceFastDeploymentRequestsFacet.test.ts index e23de7f8..ea11af9d 100644 --- a/test/marketplace/MarketplaceFastDeploymentRequestsFacet.test.ts +++ b/test/marketplace/MarketplaceFastDeploymentRequestsFacet.test.ts @@ -63,7 +63,6 @@ describe("MarketplaceFastDeploymentRequestsFacet", () => { }, initWith: { issuer: issuer.address, - trustedForwarder: ethers.constants.AddressZero }, }); }); diff --git a/test/marketplace/MarketplaceInitFacet.test.ts b/test/marketplace/MarketplaceInitFacet.test.ts index 32695bb2..a8e912b7 100644 --- a/test/marketplace/MarketplaceInitFacet.test.ts +++ b/test/marketplace/MarketplaceInitFacet.test.ts @@ -48,7 +48,6 @@ describe("MarketplaceInitFacet", () => { }, initWith: { issuer: issuer.address, - trustedForwarder: ethers.constants.AddressZero }, }); }); @@ -65,8 +64,7 @@ describe("MarketplaceInitFacet", () => { DEPLOYER_FACTORY_COMMON.factory ); const subject = marketplaceInitAsItself.initialize({ - issuer: issuer.address, - trustedForwarder: ethers.constants.AddressZero + issuer: issuer.address }); await expect(subject).to.have.revertedWith("AlreadyInitialized"); diff --git a/test/marketplace/MarketplaceTokenHoldersFacet.test.ts b/test/marketplace/MarketplaceTokenHoldersFacet.test.ts index dca2e6bf..4c552915 100644 --- a/test/marketplace/MarketplaceTokenHoldersFacet.test.ts +++ b/test/marketplace/MarketplaceTokenHoldersFacet.test.ts @@ -52,7 +52,6 @@ describe("MarketplaceTokenHoldersFacet", () => { }, initWith: { issuer: issuer.address, - trustedForwarder: ethers.constants.AddressZero }, }); diff --git a/test/marketplace/MarketplaceTopFacet.test.ts b/test/marketplace/MarketplaceTopFacet.test.ts index 94a9e9a3..2e083048 100644 --- a/test/marketplace/MarketplaceTopFacet.test.ts +++ b/test/marketplace/MarketplaceTopFacet.test.ts @@ -38,7 +38,6 @@ describe("MarketplaceTopFacet", () => { }, initWith: { issuer: issuer.address, - trustedForwarder: ethers.constants.AddressZero }, }); diff --git a/yarn.lock b/yarn.lock index 58d4f8a7..623250dc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2045,12 +2045,15 @@ resolved "https://registry.yarnpkg.com/@nomiclabs/hardhat-waffle/-/hardhat-waffle-2.0.6.tgz#d11cb063a5f61a77806053e54009c40ddee49a54" integrity sha512-+Wz0hwmJGSI17B+BhU/qFRZ1l6/xMW82QGXE/Gi+WTmwgJrQefuBs1lIf7hzQ1hLk6hpkvb/zwcNkpVKRYTQYg== -"@opengsn/contracts@^3.0.0-beta.6": - version "3.0.0-beta.6" - resolved "https://registry.yarnpkg.com/@opengsn/contracts/-/contracts-3.0.0-beta.6.tgz#3455b7c249922a087422721689239a7d539d1138" - integrity sha512-zqd7BG339Uq5bfx40CwKXr2waaJqLN9qBVllcWhKBGnkZPKR59wVIPh2zTNzvLorndMRWB3TnOQxAr6tZKjwGQ== +"@opengsn/contracts@^3.0.0-beta.9": + version "3.0.0-beta.10" + resolved "https://registry.yarnpkg.com/@opengsn/contracts/-/contracts-3.0.0-beta.10.tgz#eaab77c696cd0ea5074b6875d71166439e37a28f" + integrity sha512-phnrtNmLO1dre2MniOcbNelNN5GnBMNsIDeFRHKrkFpcgwzf8PTCqfDCVT7pKGoQOwP526m6gDTV1Fj2Wz5SYw== dependencies: + "@ethersproject/abi" "^5.7.0" + "@ethersproject/providers" "^5.7.2" "@openzeppelin/contracts" "^4.2.0" + ethers "^5.7.2" "@openzeppelin/contracts@^4.2.0", "@openzeppelin/contracts@^4.9.2": version "4.9.3"