Skip to content

Commit

Permalink
wip
Browse files Browse the repository at this point in the history
  • Loading branch information
jamesduncombe committed Nov 1, 2023
1 parent b26eb3b commit 973c85b
Show file tree
Hide file tree
Showing 9 changed files with 120 additions and 19 deletions.
2 changes: 1 addition & 1 deletion contracts/common/AHasAutomatons.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ pragma solidity 0.8.10;

import "../lib/LibAddressSet.sol";
import "../lib/LibPaginate.sol";
import "../common/lib/LibHasAutomatons.sol";
import "./lib/LibHasAutomatons.sol";

/**
* @title The Fast Smart Contract.
Expand Down
43 changes: 39 additions & 4 deletions contracts/common/AHasForwarder.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,55 @@
pragma solidity 0.8.10;

import "./lib/LibHasForwarder.sol";

import "@opengsn/contracts/src/ERC2771Recipient.sol";

/**
* @title The Forwarder Smart Contract.
* @notice The HasForwarder abstract contract is in charge of...
* @title The Forwarder behaviour abstract contract.
* @notice The AHasForwarder abstract contract is in charge of adding the Forwarder
* functionality to any contract inheriting from it.
*/
abstract contract AHasForwarder is ERC2771Recipient {
/// Errors.

/**
* @notice Happens when a function is called by a non forwarder manager.
* @param who is the address that called the function.
*/
error RequiresForwarderManager(address who);

/// Events.

/**
* @notice Emited when a forwarder is set on an implementing contract.
* @param forwarderAddress is the address of the trusted forwarder.
*/
event ForwarderChanged(address forwarderAddress);

/**
* @notice ERC2771Recipient implementation.
* TODO: FIX MODIFIER
* @param _forwarderAddress the forwarder address.
*/
function setTrustedForwarder(address _forwarderAddress) external {
function setTrustedForwarder(address _forwarderAddress) external onlyForwarderManager {
LibHasForwarder.Data storage ds = LibHasForwarder.data();
ds.forwarderAddress = _forwarderAddress;
_setTrustedForwarder(_forwarderAddress);

emit ForwarderChanged(_forwarderAddress);
}

/**
* @notice Checks whether the given address is a forwarder manager or not.
* @dev Must be implemented by the inheriting contract.
* @param who is the address to test.
*/
function isValidForwarderManager(address who) internal view virtual returns (bool);

/// Modifiers.

/// @notice Ensures that a method can only be called by the forwarder manager.
modifier onlyForwarderManager() virtual {
if (!isValidForwarderManager(msg.sender)) revert RequiresForwarderManager(msg.sender);
_;
}
}
2 changes: 1 addition & 1 deletion contracts/common/AHasGovernors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ pragma solidity 0.8.10;

import "../lib/LibAddressSet.sol";
import "../lib/LibPaginate.sol";
import "../common/lib/LibHasGovernors.sol";
import "../interfaces/ICustomErrors.sol";
import "./lib/LibHasGovernors.sol";

/**
* @title The Fast Smart Contract.
Expand Down
8 changes: 5 additions & 3 deletions contracts/common/AHasMembers.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ pragma solidity 0.8.10;

import "../lib/LibAddressSet.sol";
import "../lib/LibPaginate.sol";
import "../common/lib/LibHasMembers.sol";
import "../interfaces/ICustomErrors.sol";
import "./lib/LibHasMembers.sol";

/**
* @title The Fast Smart Contract.
Expand All @@ -13,6 +13,8 @@ import "../interfaces/ICustomErrors.sol";
abstract contract AHasMembers {
using LibAddressSet for LibAddressSet.Data;

function _msgSender() internal view virtual returns (address);

/// Errors.

/// @notice Happens when a function is called by an address that is not a members manager.
Expand Down Expand Up @@ -94,7 +96,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.
Expand All @@ -109,7 +111,7 @@ abstract contract AHasMembers {
* @notice Requires that the caller is a member of this Issuer.
* @notice Emits a `AHasMembers.MemberRemoved` event.
*/
function removeMember(address member) external onlyMemberManager(msg.sender) {
function removeMember(address member) external onlyMemberManager(_msgSender()) {
// Notify via callback.
onMemberRemoved(member);
// Remove member.
Expand Down
15 changes: 14 additions & 1 deletion contracts/fast/FastAccessFacet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import "./FastAutomatonsFacet.sol";
* @notice The FAST Access facet is the source of truth when it comes to
* permissioning and ACLs within a given FAST.
*/
contract FastAccessFacet is AFastFacet, AHasGovernors, AHasMembers {
contract FastAccessFacet is AFastFacet, AHasGovernors, AHasMembers, AHasForwarder {
using LibAddressSet for LibAddressSet.Data;
/// Structs.

Expand Down Expand Up @@ -61,8 +61,21 @@ contract FastAccessFacet is AFastFacet, AHasGovernors, AHasMembers {
FastFrontendFacet(address(this)).emitDetailsChanged();
}

/// AHasForwarder implementation.

// For now the forwarder manager is an issuer.
function isValidForwarderManager(address who) internal view override(AHasForwarder) returns (bool) {
return AHasMembers(this).isMember(who);
}

/// AHasMembers implementation.

// Horrible override...
// Delegates to ERC2771Recipient.
function _msgSender() internal view override(AHasMembers, ERC2771Recipient) returns (address) {
return super._msgSender();
}

function isMembersManager(address who) internal view override(AHasMembers) returns (bool) {
return
// The current contract should be able to manage its own members.
Expand Down
1 change: 1 addition & 0 deletions contracts/interfaces/ICustomErrors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ interface ICustomErrors {
error RequiresFastGovernorship(address who);
error RequiresFastMemberCaller();
error RequiresFastMembership(address who);
error RequiresForwarderManager(address who);
error RequiresGovernorsManager(address who);
error RequiresIssuerMemberCaller();
error RequiresIssuerMemberOrIssuerCaller();
Expand Down
16 changes: 15 additions & 1 deletion contracts/issuer/IssuerAccessFacet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import "../lib/LibAddressSet.sol";
import "../lib/LibPaginate.sol";
import "../lib/LibHelpers.sol";
import "../common/AHasMembers.sol";
import "../common/AHasForwarder.sol";
import "../interfaces/ICustomErrors.sol";
import "../fast/FastTopFacet.sol";
import "../fast/FastTokenFacet.sol";
Expand All @@ -13,11 +14,24 @@ import "./lib/LibIssuerAccess.sol";
import "./lib/IIssuerEvents.sol";
import "../issuer/IssuerTopFacet.sol";

contract IssuerAccessFacet is AIssuerFacet, AHasMembers {
contract IssuerAccessFacet is AIssuerFacet, AHasMembers, AHasForwarder {
using LibAddressSet for LibAddressSet.Data;

/// AHasForwarder implementation.

// For now the forwarder manager is an issuer.
function isValidForwarderManager(address who) internal view override(AHasForwarder) returns (bool) {
return AHasMembers(this).isMember(who);
}

/// AHasMembers implementation.

// Horrible override...
// Delegates to ERC2771Recipient.
function _msgSender() internal view override(AHasMembers, ERC2771Recipient) returns (address) {
return super._msgSender();
}

function isMembersManager(address who) internal view override(AHasMembers) returns (bool) {
return AHasMembers(this).isMember(who);
}
Expand Down
24 changes: 19 additions & 5 deletions contracts/marketplace/MarketplaceAccessFacet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pragma solidity 0.8.10;
import "../lib/LibAddressSet.sol";
import "../lib/LibPaginate.sol";
import "../common/AHasMembers.sol";
import "../common/AHasForwarder.sol";
import "../issuer/IssuerTopFacet.sol";
import "../interfaces/ICustomErrors.sol";
import "../interfaces/IHasActiveMembers.sol";
Expand All @@ -15,11 +16,24 @@ 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, AHasForwarder, IHasActiveMembers {
using LibAddressSet for LibAddressSet.Data;

/// AHasForwarder implementation.

// For now the forwarder manager is an issuer.
function isValidForwarderManager(address who) internal view override(AHasForwarder) returns (bool) {
return _isIssuerMember(who);
}

/// AHasMembers implementation.

// Horrible override...
// Delegates to ERC2771Recipient.
function _msgSender() internal view override(AHasMembers, ERC2771Recipient) returns (address) {
return super._msgSender();
}

function isMembersManager(address who) internal view override(AHasMembers) returns (bool) {
return _isIssuerMember(who) || AHasAutomatons(address(this)).automatonCan(who, MARKETPLACE_PRIVILEGE_MANAGE_MEMBERS);
}
Expand Down Expand Up @@ -56,25 +70,25 @@ 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);
}

/**
* @notice Callback from FAST contracts allowing the Marketplace contract to keep track of FAST memberships.
* @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.
Expand Down
28 changes: 25 additions & 3 deletions test/marketplace/MarketplaceAccessFacet.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,16 @@ describe("MarketplaceAccessFacet", () => {
alice: SignerWithAddress,
bob: SignerWithAddress,
rob: SignerWithAddress,
john: SignerWithAddress;
john: SignerWithAddress,
trustedForwarder: SignerWithAddress;

let issuer: FakeContract<Issuer>,
fast: FakeContract<Fast>,
marketplace: Marketplace,
access: MarketplaceAccessFacet,
issuerMemberAccess: MarketplaceAccessFacet;
issuerMemberAccess: MarketplaceAccessFacet,
trustedForwarderAccess: MarketplaceAccessFacet,
deployerAccess: MarketplaceAccessFacet;

const marketplaceDeployFixture = deployments.createFixture(
marketplaceFixtureFunc
Expand All @@ -50,7 +53,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");
Expand All @@ -68,6 +71,8 @@ describe("MarketplaceAccessFacet", () => {
marketplace.address
);
issuerMemberAccess = access.connect(issuerMember);
deployerAccess = access.connect(deployer);
trustedForwarderAccess = access.connect(trustedForwarder);
},
},
initWith: {
Expand Down Expand Up @@ -146,6 +151,23 @@ describe("MarketplaceAccessFacet", () => {
await expect(subject).to.have.revertedWith(`RequiresMembersManager`);
});

it("accepts a trusted forwarder as the _msgSender()", async () => {
// Set the trusted forwarder.
await deployerAccess.setTrustedForwarder(trustedForwarder.address);

// Now add member...
const encodedFunctionCall = await trustedForwarderAccess.interface.encodeFunctionData("addMember", [alice.address]);
const data = `${encodedFunctionCall}${issuerMember.address.slice(2)}`;

// Override the transaction data.
// Should now appear from the Issuer.
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);
Expand Down

0 comments on commit 973c85b

Please sign in to comment.