From 82963eab737012a9e5d6e07c897bbd0859cb59e4 Mon Sep 17 00:00:00 2001 From: James Duncombe Date: Mon, 5 Feb 2024 16:00:11 +0000 Subject: [PATCH] Adds the context behaviour to MarketplaceAccessFacet. --- .../marketplace/MarketplaceAccessFacet.sol | 24 ++++- .../MarketplaceAccessFacet.test.ts | 91 ++++++++++++++++++- 2 files changed, 109 insertions(+), 6 deletions(-) diff --git a/contracts/marketplace/MarketplaceAccessFacet.sol b/contracts/marketplace/MarketplaceAccessFacet.sol index f5983cb5..60a7406e 100644 --- a/contracts/marketplace/MarketplaceAccessFacet.sol +++ b/contracts/marketplace/MarketplaceAccessFacet.sol @@ -4,6 +4,8 @@ pragma solidity 0.8.10; import "../lib/LibAddressSet.sol"; import "../lib/LibPaginate.sol"; import "../common/AHasMembers.sol"; +import "../common/AHasForwarder.sol"; +import "../common/AHasContext.sol"; import "../issuer/IssuerTopFacet.sol"; import "../interfaces/ICustomErrors.sol"; import "../interfaces/IHasActiveMembers.sol"; @@ -15,9 +17,21 @@ import "./MarketplaceAutomatonsFacet.sol"; * @title The Marketplace Smart Contract. * @notice The Marketplace Access facet is in charge of keeping track of marketplace members. */ -contract MarketplaceAccessFacet is AMarketplaceFacet, AHasMembers, IHasActiveMembers { +contract MarketplaceAccessFacet is AMarketplaceFacet, AHasMembers, AHasContext, IHasActiveMembers { using LibAddressSet for LibAddressSet.Data; + /// AHasContext implementation. + + // TODO: Could _isTrustedForwarder actually have it's default implementation point to + // IHasForwarder(address(this)).isTrustedForwarder(forwarder) or similar? + function _isTrustedForwarder(address forwarder) internal view override(AHasContext) returns (bool) { + return AHasForwarder(address(this)).isTrustedForwarder(forwarder); + } + + function _msgSender() internal view override(AHasMembers, AHasContext) returns (address) { + return AHasContext._msgSender(); + } + /// AHasMembers implementation. function isMembersManager(address who) internal view override(AHasMembers) returns (bool) { @@ -56,12 +70,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 +83,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/test/marketplace/MarketplaceAccessFacet.test.ts b/test/marketplace/MarketplaceAccessFacet.test.ts index 0287bff0..14fa1e21 100644 --- a/test/marketplace/MarketplaceAccessFacet.test.ts +++ b/test/marketplace/MarketplaceAccessFacet.test.ts @@ -13,6 +13,7 @@ import { Issuer, Marketplace, } from "../../typechain/hardhat-diamond-abi/HardhatDiamondABI.sol"; +import { IForwarder } from "../../typechain"; chai.use(solidity); chai.use(smock.matchers); @@ -29,6 +30,7 @@ describe("MarketplaceAccessFacet", () => { let issuer: FakeContract, fast: FakeContract, marketplace: Marketplace, + forwarder: FakeContract, access: MarketplaceAccessFacet, issuerMemberAccess: MarketplaceAccessFacet; @@ -48,12 +50,19 @@ describe("MarketplaceAccessFacet", () => { issuer.isFastRegistered.returns(false); }; + const resetIForwarderMock = () => { + forwarder.supportsInterface.reset(); + forwarder.supportsInterface.whenCalledWith(/* IForwarder */ "0x25e23e64").returns(true); + forwarder.supportsInterface.returns(false); + } + before(async () => { // Keep track of a few signers. [deployer, issuerMember, alice, bob, rob, john] = await ethers.getSigners(); - // Mock Issuer and Fast contracts. + // Mock Issuer, Fast and IForwarder contracts. issuer = await smock.fake("Issuer"); fast = await smock.fake("Fast"); + forwarder = await smock.fake("IForwarder"); }); beforeEach(async () => { @@ -77,6 +86,30 @@ describe("MarketplaceAccessFacet", () => { resetIssuerMock(); resetFastMock(); + resetIForwarderMock(); + }); + + describe("AHasContext implementation", () => { + describe("_isTrustedForwarder", () => { + it("returns true if the address is a trusted forwarder", async () => { + // Set the trusted forwarder. + forwarder.supportsInterface.reset(); + forwarder.supportsInterface.whenCalledWith(/* IForwarder */ "0x25e23e64").returns(true); + forwarder.supportsInterface.returns(false); + + await marketplace.connect(issuerMember).setTrustedForwarder(forwarder.address); + + // isTrustedForwarder() should delegate to _isTrustedForwarder(). + const subject = await marketplace.connect(issuerMember).isTrustedForwarder(forwarder.address); + expect(subject).to.eq(true); + }); + }); + + describe("_msgSender", () => { + it("returns the original msg.sender", async () => { + // Call a function on the Marketplace contract that's sponsored. + }); + }); }); describe("IHasMembers", () => { @@ -171,6 +204,34 @@ describe("MarketplaceAccessFacet", () => { }); it("calls back onMemberAdded"); + + it("is callable by a trusted forwarder", async () => { + // Set the trusted forwarder. + await marketplace.connect(issuerMember).setTrustedForwarder(forwarder.address); + + // Impersonate the trusted forwarder contract. + const accessAsForwarder = await impersonateContract(access, forwarder.address); + + // Build the data to call addMember() on the Marketplace contract. + // Pack the issuerMember address at the end - this is sponsored callers address. + const encodedFunctionCall = await access.interface.encodeFunctionData("addMember", [alice.address]); + const data = ethers.utils.solidityPack( + ["bytes", "address"], + [encodedFunctionCall, issuerMember.address] + ); + + // As the forwarder send the packed transaction. + await accessAsForwarder.signer.sendTransaction( + { + data: data, + to: access.address, + } + ); + + // Check that Alice is a member. + const subject = await marketplace.isMember(alice.address); + expect(subject).to.eq(true); + }); }); describe("removeMember", () => { @@ -224,6 +285,34 @@ describe("MarketplaceAccessFacet", () => { }); it("calls back onMemberRemoved"); + + it("is callable by a trusted forwarder", async () => { + // Set the trusted forwarder. + await marketplace.connect(issuerMember).setTrustedForwarder(forwarder.address); + + // Impersonate the trusted forwarder contract. + const accessAsForwarder = await impersonateContract(access, forwarder.address); + + // Build the data to call removeMember() on the Marketplace contract. + // Pack the issuerMember address at the end - this is sponsored callers address. + const encodedFunctionCall = await access.interface.encodeFunctionData("removeMember", [alice.address]); + const data = ethers.utils.solidityPack( + ["bytes", "address"], + [encodedFunctionCall, issuerMember.address] + ); + + // As the forwarder send the packed transaction. + await accessAsForwarder.signer.sendTransaction( + { + data: data, + to: access.address, + } + ); + + // Check that Alice is not member. + const subject = await marketplace.isMember(alice.address); + expect(subject).to.eq(false); + }); }); describe("onMemberAdded", () => {