diff --git a/contracts/domain/BosonConstants.sol b/contracts/domain/BosonConstants.sol index 2512c2bab..31664c4b9 100644 --- a/contracts/domain/BosonConstants.sol +++ b/contracts/domain/BosonConstants.sol @@ -159,6 +159,8 @@ string constant FEE_PERCENTAGE_INVALID = "Percentage representation must be less string constant VALUE_ZERO_NOT_ALLOWED = "Value must be greater than 0"; // EIP712Lib +string constant PROTOCOL_NAME = "Boson Protocol"; +string constant PROTOCOL_VERSION = "V2"; bytes32 constant EIP712_DOMAIN_TYPEHASH = keccak256( bytes("EIP712Domain(string name,string version,address verifyingContract,bytes32 salt)") ); diff --git a/contracts/mock/MockMetaTransactionsHandlerFacet.sol b/contracts/mock/MockMetaTransactionsHandlerFacet.sol index f85d7193e..ad11df563 100644 --- a/contracts/mock/MockMetaTransactionsHandlerFacet.sol +++ b/contracts/mock/MockMetaTransactionsHandlerFacet.sol @@ -18,4 +18,13 @@ contract MockMetaTransactionsHandlerFacet is ProtocolBase { protocolMetaTxInfo().isMetaTransaction = true; protocolMetaTxInfo().currentSenderAddress = _signerAddress; } + + /** + * @notice Sets the cached chain id value. + * + * @param _chainId - chain id + */ + function setCachedChainId(uint256 _chainId) public { + protocolMetaTxInfo().cachedChainId = _chainId; + } } diff --git a/contracts/protocol/facets/ConfigHandlerFacet.sol b/contracts/protocol/facets/ConfigHandlerFacet.sol index a39269a1b..a3af6a177 100644 --- a/contracts/protocol/facets/ConfigHandlerFacet.sol +++ b/contracts/protocol/facets/ConfigHandlerFacet.sol @@ -69,7 +69,8 @@ contract ConfigHandlerFacet is IBosonConfigHandler, ProtocolBase { // Initialize protocol meta-transaction config params ProtocolLib.ProtocolMetaTxInfo storage pmti = protocolMetaTxInfo(); - pmti.domainSeparator = EIP712Lib.domainSeparator("BosonProtocolDiamond", "V1"); + pmti.domainSeparator = EIP712Lib.buildDomainSeparator(PROTOCOL_NAME, PROTOCOL_VERSION); + pmti.cachedChainId = block.chainid; } /** diff --git a/contracts/protocol/libs/EIP712Lib.sol b/contracts/protocol/libs/EIP712Lib.sol index e03b0b50e..5b889fb7f 100644 --- a/contracts/protocol/libs/EIP712Lib.sol +++ b/contracts/protocol/libs/EIP712Lib.sol @@ -12,12 +12,15 @@ import { ProtocolLib } from "../libs/ProtocolLib.sol"; library EIP712Lib { /** * @notice Generates the domain separator hash. + * @dev Using the chainId as the salt enables the client to be active on one chain + * while a metatx is signed for a contract on another chain. That could happen if the client is, + * for instance, a metaverse scene that runs on one chain while the contracts it interacts with are deployed on another chain. * * @param _name - the name of the protocol * @param _version - The version of the protocol * @return the domain separator hash */ - function domainSeparator(string memory _name, string memory _version) internal view returns (bytes32) { + function buildDomainSeparator(string memory _name, string memory _version) internal view returns (bytes32) { return keccak256( abi.encode( @@ -49,7 +52,7 @@ library EIP712Lib { bytes32 _sigR, bytes32 _sigS, uint8 _sigV - ) internal view returns (bool) { + ) internal returns (bool) { // Ensure signature is unique // See https://github.com/OpenZeppelin/openzeppelin-contracts/blob/04695aecbd4d17dddfd55de766d10e3805d6f42f/contracts/cryptography/ECDSA.sol#63 require( @@ -64,12 +67,23 @@ library EIP712Lib { } /** - * @notice Gets the domain separator from storage. + * @notice Gets the domain separator from storage if matches with the chain id and diamond address, else, build new domain separator. * - * @return the domain separator from storage + * @return the domain separator */ - function getDomainSeparator() private view returns (bytes32) { - return ProtocolLib.protocolMetaTxInfo().domainSeparator; + function getDomainSeparator() private returns (bytes32) { + ProtocolLib.ProtocolMetaTxInfo storage pmti = ProtocolLib.protocolMetaTxInfo(); + uint256 cachedChainId = pmti.cachedChainId; + + if (block.chainid == cachedChainId) { + return pmti.domainSeparator; + } else { + bytes32 domainSeparator = buildDomainSeparator(PROTOCOL_NAME, PROTOCOL_VERSION); + pmti.domainSeparator = domainSeparator; + pmti.cachedChainId = block.chainid; + + return domainSeparator; + } } /** @@ -84,7 +98,7 @@ library EIP712Lib { * @param _messageHash - the message hash * @return the EIP712 compatible message hash */ - function toTypedMessageHash(bytes32 _messageHash) internal view returns (bytes32) { + function toTypedMessageHash(bytes32 _messageHash) internal returns (bytes32) { return keccak256(abi.encodePacked("\x19\x01", getDomainSeparator(), _messageHash)); } diff --git a/contracts/protocol/libs/ProtocolLib.sol b/contracts/protocol/libs/ProtocolLib.sol index f832c10b8..220de923f 100644 --- a/contracts/protocol/libs/ProtocolLib.sol +++ b/contracts/protocol/libs/ProtocolLib.sol @@ -202,6 +202,8 @@ library ProtocolLib { bytes32 domainSeparator; // address => nonce => nonce used indicator mapping(address => mapping(uint256 => bool)) usedNonce; + // The cached chain id + uint256 cachedChainId; // map function name to input type mapping(string => BosonTypes.MetaTxInputType) inputType; // map input type => hash info diff --git a/scripts/util/test-utils.js b/scripts/util/test-utils.js index b4301f0d9..698a1790b 100644 --- a/scripts/util/test-utils.js +++ b/scripts/util/test-utils.js @@ -120,8 +120,8 @@ async function prepareDataSignatureParameters( ]; const domainData = { - name: "BosonProtocolDiamond", - version: "V1", + name: "Boson Protocol", + version: "V2", verifyingContract: metaTransactionsHandlerAddress, salt: ethers.utils.hexZeroPad(ethers.BigNumber.from(31337).toHexString(), 32), //hardhat default chain id is 31337 }; diff --git a/test/protocol/MetaTransactionsHandlerTest.js b/test/protocol/MetaTransactionsHandlerTest.js index 0b545d75b..02d448e21 100644 --- a/test/protocol/MetaTransactionsHandlerTest.js +++ b/test/protocol/MetaTransactionsHandlerTest.js @@ -29,6 +29,8 @@ const { mockExchange, } = require("../utils/mock"); const { oneWeek, oneMonth } = require("../utils/constants"); +const { getSelectors, FacetCutAction } = require("../../scripts/util/diamond-utils.js"); + /** * Test the Boson Meta transactions Handler interface */ @@ -60,7 +62,8 @@ describe("IBosonMetaTransactionsHandler", function () { pauseHandler, bosonToken, support, - result; + result, + mockMetaTransactionsHandler; let metaTransactionsHandler, nonce, functionSignature; let seller, offerId, buyerId; let validOfferDetails, @@ -231,6 +234,43 @@ describe("IBosonMetaTransactionsHandler", function () { [bosonToken, mockToken] = await deployMockTokens(gasLimit, ["BosonToken", "Foreign20"]); }); + async function upgradeMetaTransactionsHandlerFacet() { + // Upgrade the ExchangeHandlerFacet functions + // DiamondCutFacet + const cutFacetViaDiamond = await ethers.getContractAt("DiamondCutFacet", protocolDiamond.address); + + // Deploy MockMetaTransactionsHandlerFacet + const MockMetaTransactionsHandlerFacet = await ethers.getContractFactory("MockMetaTransactionsHandlerFacet"); + const mockMetaTransactionsHandlerFacet = await MockMetaTransactionsHandlerFacet.deploy(); + await mockMetaTransactionsHandlerFacet.deployed(); + + // Define the facet cut + const facetCuts = [ + { + facetAddress: mockMetaTransactionsHandlerFacet.address, + action: FacetCutAction.Add, + functionSelectors: getSelectors(mockMetaTransactionsHandlerFacet), + }, + ]; + + // Send the DiamondCut transaction + const tx = await cutFacetViaDiamond + .connect(deployer) + .diamondCut(facetCuts, ethers.constants.AddressZero, "0x", { gasLimit }); + + // Wait for transaction to confirm + const receipt = await tx.wait(); + + // Be certain transaction was successful + assert.equal(receipt.status, 1, `Diamond upgrade failed: ${tx.hash}`); + + // Cast Diamond to MockMetaTransactionsHandlerFacet + mockMetaTransactionsHandler = await ethers.getContractAt( + "MockMetaTransactionsHandlerFacet", + protocolDiamond.address + ); + } + // Interface support (ERC-156 provided by ProtocolDiamond, others by deployed facets) context("📋 Interfaces", async function () { context("👉 supportsInterface()", async function () { @@ -422,6 +462,45 @@ describe("IBosonMetaTransactionsHandler", function () { assert.equal(result, expectedResult, "Nonce is unused"); }); + it("Should build a new domain separator if cachedChainId does not match with chain id used in signature", async function () { + await upgradeMetaTransactionsHandlerFacet(); + + // update the cached chain id + await mockMetaTransactionsHandler.setCachedChainId(123456); + + // Prepare the function signature for the facet function. + functionSignature = accountHandler.interface.encodeFunctionData("createSeller", [ + seller, + emptyAuthToken, + voucherInitValues, + ]); + + message.functionSignature = functionSignature; + + // Collect the signature components + let { r, s, v } = await prepareDataSignatureParameters( + operator, + customTransactionType, + "MetaTransaction", + message, + metaTransactionsHandler.address + ); + + // send a meta transaction, does not revert + await expect( + metaTransactionsHandler + .connect(deployer) + .executeMetaTransaction(operator.address, message.functionName, functionSignature, nonce, r, s, v) + ) + .to.emit(metaTransactionsHandler, "MetaTransactionExecuted") + .withArgs(operator.address, deployer.address, message.functionName, nonce); + + // Verify that nonce is used. Expect true. + let expectedResult = true; + result = await metaTransactionsHandler.connect(operator).isUsedNonce(deployer.address, nonce); + assert.equal(result, expectedResult, "Nonce is unused"); + }); + it("does not modify revert reasons", async function () { // Set seller as inactive seller.active = false;