diff --git a/src/ERC712Extended.sol b/src/ERC712Extended.sol index d450004..2fd559e 100644 --- a/src/ERC712Extended.sol +++ b/src/ERC712Extended.sol @@ -28,6 +28,9 @@ abstract contract ERC712Extended is IERC712Extended { /// @dev Initial EIP-712 domain separator set at deployment. bytes32 internal immutable _INITIAL_DOMAIN_SEPARATOR; + /// @dev Initial address of this contract set at deployment, used to detect proxy (delegatecall) context. + address internal immutable _INITIAL_THIS; + /// @dev The name of the contract (stored as a bytes32 instead of a string in order to be immutable). bytes32 internal immutable _name; @@ -41,6 +44,7 @@ abstract contract ERC712Extended is IERC712Extended { _name = Bytes32String.toBytes32(name_); _INITIAL_CHAIN_ID = block.chainid; + _INITIAL_THIS = address(this); _INITIAL_DOMAIN_SEPARATOR = _getDomainSeparator(); } @@ -74,7 +78,10 @@ abstract contract ERC712Extended is IERC712Extended { /// @inheritdoc IERC712 function DOMAIN_SEPARATOR() public view virtual returns (bytes32) { - return block.chainid == _INITIAL_CHAIN_ID ? _INITIAL_DOMAIN_SEPARATOR : _getDomainSeparator(); + return + (address(this) == _INITIAL_THIS && block.chainid == _INITIAL_CHAIN_ID) + ? _INITIAL_DOMAIN_SEPARATOR + : _getDomainSeparator(); } /* ============ Internal View/Pure Functions ============ */ diff --git a/test/ERC712ExtendedProxy.t.sol b/test/ERC712ExtendedProxy.t.sol new file mode 100644 index 0000000..a00506c --- /dev/null +++ b/test/ERC712ExtendedProxy.t.sol @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: UNLICENSED + +pragma solidity >=0.8.20 <0.9.0; + +import { Proxy } from "../src/Proxy.sol"; + +import { BaseERC712ExtendedTests } from "./base/BaseERC712Extended.t.sol"; + +import { ERC712ExtendedHarness } from "./utils/ERC712ExtendedHarness.sol"; +import { IERC712ExtendedHarness } from "./utils/IERC712ExtendedHarness.sol"; + +/// @dev Runs the full `BaseERC712ExtendedTests` suite against a non-upgradeable `ERC712Extended` deployed behind a +/// `Proxy`, ensuring the EIP-712 domain separator binds to the proxy (not the implementation) and is stable +/// across upgrades. +contract ERC712ExtendedProxyTests is BaseERC712ExtendedTests { + /// @dev `keccak256('eip1967.proxy.implementation') - 1`. + bytes32 internal constant _IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc; + + function setUp() public override { + super.setUp(); + + address implementation_ = address(new ERC712ExtendedHarness(_NAME)); + _erc712 = IERC712ExtendedHarness(address(new Proxy(implementation_))); + } + + /* ============ proxy domain separator ============ */ + function test_proxyDomainDiffersFromStandaloneImplementation() external { + address standaloneImplementation_ = address(new ERC712ExtendedHarness(_NAME)); + + assertEq(_erc712.DOMAIN_SEPARATOR(), _computeDomainSeparator(_NAME, block.chainid, address(_erc712))); + assertTrue( + _erc712.DOMAIN_SEPARATOR() != IERC712ExtendedHarness(standaloneImplementation_).DOMAIN_SEPARATOR(), + "proxy domain must differ from the standalone implementation" + ); + } + + function test_proxyDomainStableAcrossUpgrade() external { + bytes32 domainSeparatorBefore_ = _erc712.DOMAIN_SEPARATOR(); + + assertEq(domainSeparatorBefore_, _computeDomainSeparator(_NAME, block.chainid, address(_erc712))); + + address newImplementation_ = address(new ERC712ExtendedHarness(_NAME)); + vm.store(address(_erc712), _IMPLEMENTATION_SLOT, bytes32(uint256(uint160(newImplementation_)))); + + assertEq(_erc712.DOMAIN_SEPARATOR(), domainSeparatorBefore_, "domain must be stable across an upgrade"); + assertEq(_erc712.DOMAIN_SEPARATOR(), _computeDomainSeparator(_NAME, block.chainid, address(_erc712))); + } +}