From 2de74a66c9642bfcd8e91912e72636c71dec694f Mon Sep 17 00:00:00 2001 From: Erik Tierney Date: Thu, 2 Feb 2023 13:29:34 -0500 Subject: [PATCH 1/2] Add test to make sure DOMAIN_SEPARATOR slot stays at 15 --- test/helpers/storageSlots.behavior.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/helpers/storageSlots.behavior.ts b/test/helpers/storageSlots.behavior.ts index 89de422cd..504387b4e 100644 --- a/test/helpers/storageSlots.behavior.ts +++ b/test/helpers/storageSlots.behavior.ts @@ -4,6 +4,8 @@ import { FiatTokenProxyInstance } from "../../@types/generated"; const FiatTokenProxy = artifacts.require("FiatTokenProxy"); const FiatTokenV1 = artifacts.require("FiatTokenV1"); const FiatTokenV1_1 = artifacts.require("FiatTokenV1_1"); +const FiatTokenV2 = artifacts.require("FiatTokenV2"); +const FiatTokenV2_1 = artifacts.require("FiatTokenV2_1"); export function usesOriginalStorageSlotPositions< T extends Truffle.ContractInstance @@ -35,10 +37,12 @@ export function usesOriginalStorageSlotPositions< alice, bob, charlie, + lostAndFound, ] = accounts; let fiatToken: T; let proxy: FiatTokenProxyInstance; + let domainSeparator : String; beforeEach(async () => { fiatToken = await Contract.new(); @@ -72,6 +76,15 @@ export function usesOriginalStorageSlotPositions< from: owner, }); } + if (version >= 2) { + const proxyAsFiatTokenV2 = await FiatTokenV2.at(proxy.address); + await proxyAsFiatTokenV2.initializeV2(name); + domainSeparator = await proxyAsFiatTokenV2.DOMAIN_SEPARATOR(); + } + if (version >= 2.1) { + const proxyAsFiatTokenV2_1 = await FiatTokenV2_1.at(proxy.address); + await proxyAsFiatTokenV2_1.initializeV2_1(lostAndFound); + } }); it("retains original storage slots 0 through 13", async () => { @@ -132,6 +145,12 @@ export function usesOriginalStorageSlotPositions< expect(parseAddress(slot)).to.equal(rescuer); }); } + if (version >= 2) { + it("retains slot 15 for DOMAIN_SEPARATOR", async()=> { + const slot = await readSlot(proxy.address, 15); + expect("0x" + slot).to.equal(domainSeparator); + }); + } it("retains original storage slots for blacklisted mapping", async () => { // blacklisted[alice] From fd9fe83641ad76bbe47f7a5b01f56335c1c0a27b Mon Sep 17 00:00:00 2001 From: Erik Tierney Date: Fri, 10 Mar 2023 09:33:49 -0500 Subject: [PATCH 2/2] Override DOMAIN_SEPARATOR to detect when chainid has changed --- contracts/v2/EIP2612.sol | 2 +- contracts/v2/EIP3009.sol | 6 +-- contracts/v2/EIP712Domain.sol | 13 ++++++- contracts/v2/FiatTokenV2.sol | 2 +- contracts/v2/FiatTokenV2_2.sol | 67 ++++++++++++++++++++++++++++++++++ test/v2/FiatTokenV2_2.test.ts | 43 ++++++++++++++++++++++ 6 files changed, 127 insertions(+), 6 deletions(-) create mode 100644 contracts/v2/FiatTokenV2_2.sol create mode 100644 test/v2/FiatTokenV2_2.test.ts diff --git a/contracts/v2/EIP2612.sol b/contracts/v2/EIP2612.sol index 615e39a72..9a9b90a75 100644 --- a/contracts/v2/EIP2612.sol +++ b/contracts/v2/EIP2612.sol @@ -78,7 +78,7 @@ abstract contract EIP2612 is AbstractFiatTokenV2, EIP712Domain { deadline ); require( - EIP712.recover(DOMAIN_SEPARATOR, v, r, s, data) == owner, + EIP712.recover(_domainSeparator(), v, r, s, data) == owner, "EIP2612: invalid signature" ); diff --git a/contracts/v2/EIP3009.sol b/contracts/v2/EIP3009.sol index ff83e0141..eeaa3c13d 100644 --- a/contracts/v2/EIP3009.sol +++ b/contracts/v2/EIP3009.sol @@ -109,7 +109,7 @@ abstract contract EIP3009 is AbstractFiatTokenV2, EIP712Domain { nonce ); require( - EIP712.recover(DOMAIN_SEPARATOR, v, r, s, data) == from, + EIP712.recover(_domainSeparator(), v, r, s, data) == from, "FiatTokenV2: invalid signature" ); @@ -155,7 +155,7 @@ abstract contract EIP3009 is AbstractFiatTokenV2, EIP712Domain { nonce ); require( - EIP712.recover(DOMAIN_SEPARATOR, v, r, s, data) == from, + EIP712.recover(_domainSeparator(), v, r, s, data) == from, "FiatTokenV2: invalid signature" ); @@ -186,7 +186,7 @@ abstract contract EIP3009 is AbstractFiatTokenV2, EIP712Domain { nonce ); require( - EIP712.recover(DOMAIN_SEPARATOR, v, r, s, data) == authorizer, + EIP712.recover(_domainSeparator(), v, r, s, data) == authorizer, "FiatTokenV2: invalid signature" ); diff --git a/contracts/v2/EIP712Domain.sol b/contracts/v2/EIP712Domain.sol index 2b9907348..805d65aa1 100644 --- a/contracts/v2/EIP712Domain.sol +++ b/contracts/v2/EIP712Domain.sol @@ -28,8 +28,19 @@ pragma solidity 0.6.12; * @title EIP712 Domain */ contract EIP712Domain { + + // was originally DOMAIN_SEPARATOR + // but that has been moved to a method so we can override it in V2_2+ + bytes32 internal _CACHED_DOMAIN_SEPARATOR; + /** * @dev EIP712 Domain Separator */ - bytes32 public DOMAIN_SEPARATOR; + function DOMAIN_SEPARATOR() external view returns (bytes32) { + return _domainSeparator(); + } + + function _domainSeparator() internal virtual view returns (bytes32) { + return _CACHED_DOMAIN_SEPARATOR; + } } diff --git a/contracts/v2/FiatTokenV2.sol b/contracts/v2/FiatTokenV2.sol index 5535df3ba..fc6b4a910 100644 --- a/contracts/v2/FiatTokenV2.sol +++ b/contracts/v2/FiatTokenV2.sol @@ -46,7 +46,7 @@ contract FiatTokenV2 is FiatTokenV1_1, EIP3009, EIP2612 { // solhint-disable-next-line reason-string require(initialized && _initializedVersion == 0); name = newName; - DOMAIN_SEPARATOR = EIP712.makeDomainSeparator(newName, "2"); + _CACHED_DOMAIN_SEPARATOR = EIP712.makeDomainSeparator(newName, "2"); _initializedVersion = 1; } diff --git a/contracts/v2/FiatTokenV2_2.sol b/contracts/v2/FiatTokenV2_2.sol new file mode 100644 index 000000000..e1c4d201a --- /dev/null +++ b/contracts/v2/FiatTokenV2_2.sol @@ -0,0 +1,67 @@ +/** + * SPDX-License-Identifier: MIT + * + * Copyright (c) 2018-2023 CENTRE SECZ + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +pragma solidity 0.6.12; + +import { FiatTokenV2_1 } from "./FiatTokenV2_1.sol"; +import { EIP712 } from "../util/EIP712.sol"; + +// solhint-disable func-name-mixedcase + +/** + * @title FiatToken V2.1 + * @notice ERC20 Token backed by fiat reserves, version 2.1 + */ +contract FiatTokenV2_2 is FiatTokenV2_1 { + + uint256 private _CACHED_CHAIN_ID; + + /** + * @notice Initialize v2.2 + */ + function initializeV2_2() external { + // solhint-disable-next-line reason-string + require(_initializedVersion == 2); + + _CACHED_CHAIN_ID = chainid(); + + _initializedVersion = 3; + } + + function chainid() internal view returns (uint256) { + uint256 chainId; + assembly { + chainId := chainid() + } + return chainId; + } + + function _domainSeparator() internal override view returns (bytes32) { + if (_CACHED_CHAIN_ID == chainid()) { + return _CACHED_DOMAIN_SEPARATOR; + } else { + return EIP712.makeDomainSeparator(name, "2"); + } + } +} diff --git a/test/v2/FiatTokenV2_2.test.ts b/test/v2/FiatTokenV2_2.test.ts new file mode 100644 index 000000000..09b0a8475 --- /dev/null +++ b/test/v2/FiatTokenV2_2.test.ts @@ -0,0 +1,43 @@ +import { FiatTokenV22Instance } from "../../@types/generated"; +import { expectRevert } from "../helpers"; +import { behavesLikeFiatTokenV2 } from "./FiatTokenV2.test"; + +const FiatTokenV2_2 = artifacts.require("FiatTokenV2_2"); + +contract("FiatTokenV2_2", (accounts) => { + const fiatTokenOwner = accounts[9]; + let fiatToken: FiatTokenV22Instance; + + beforeEach(async () => { + fiatToken = await FiatTokenV2_2.new(); + await fiatToken.initialize( + "USD Coin", + "USDC", + "USD", + 6, + fiatTokenOwner, + fiatTokenOwner, + fiatTokenOwner, + fiatTokenOwner + ); + await fiatToken.initializeV2("USD Coin", { from: fiatTokenOwner }); + const [, , lostAndFound] = accounts; + await fiatToken.initializeV2_1(lostAndFound); + await fiatToken.initializeV2_2(); + }); + + behavesLikeFiatTokenV2(accounts, () => fiatToken, fiatTokenOwner); + + describe("initializeV2_2", () => { + const [, user, lostAndFound] = accounts; + + beforeEach(async () => { + }); + + it("disallows calling initializeV2_2 twice", async () => { + await expectRevert( + fiatToken.initializeV2_2() + ); + }); + }); +});