diff --git a/ethereum/contracts/bridge/Bridge.sol b/ethereum/contracts/bridge/Bridge.sol index ff42847ea..23be10361 100644 --- a/ethereum/contracts/bridge/Bridge.sol +++ b/ethereum/contracts/bridge/Bridge.sol @@ -309,11 +309,9 @@ contract Bridge is BridgeGovernance, ReentrancyGuard { fee: fee }); - bytes memory encoded = encodeTransfer(transfer); - sequence = wormhole().publishMessage{value: callValue}( nonce, - encoded, + encodeTransfer(transfer), finality() ); } @@ -333,7 +331,6 @@ contract Bridge is BridgeGovernance, ReentrancyGuard { uint32 nonce, bytes memory payload ) internal returns (uint64 sequence) { - BridgeStructs.TransferWithPayload memory transfer = BridgeStructs .TransferWithPayload({ payloadID: 3, @@ -346,14 +343,13 @@ contract Bridge is BridgeGovernance, ReentrancyGuard { payload: payload }); - bytes memory encoded = encodeTransferWithPayload(transfer); - sequence = wormhole().publishMessage{value: callValue}( nonce, - encoded, + encodeTransferWithPayload(transfer), finality() ); } + function updateWrapped(bytes memory encodedVm) external returns (address token) { (IWormhole.VM memory vm, bool valid, string memory reason) = wormhole().parseAndVerifyVM(encodedVm); @@ -578,11 +574,7 @@ contract Bridge is BridgeGovernance, ReentrancyGuard { function verifyBridgeVM(IWormhole.VM memory vm) internal view returns (bool){ require(!isFork(), "invalid fork"); - if (bridgeContracts(vm.emitterChainId) == vm.emitterAddress) { - return true; - } - - return false; + return bridgeContracts(vm.emitterChainId) == vm.emitterAddress; } function encodeAssetMeta(BridgeStructs.AssetMeta memory meta) public pure returns (bytes memory encoded) { diff --git a/ethereum/contracts/bridge/token/TokenImplementation.sol b/ethereum/contracts/bridge/token/TokenImplementation.sol index 12fd12ad0..f952ed7b7 100644 --- a/ethereum/contracts/bridge/token/TokenImplementation.sol +++ b/ethereum/contracts/bridge/token/TokenImplementation.sol @@ -7,6 +7,7 @@ import "./TokenState.sol"; import "@openzeppelin/contracts/access/Ownable.sol"; import "@openzeppelin/contracts/utils/Context.sol"; import "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol"; +import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; // Based on the OpenZepplin ERC20 implementation, licensed under MIT contract TokenImplementation is TokenState, Context { @@ -18,12 +19,33 @@ contract TokenImplementation is TokenState, Context { string memory symbol_, uint8 decimals_, uint64 sequence_, - address owner_, - uint16 chainId_, bytes32 nativeContract_ ) initializer public { + _initializeNativeToken( + name_, + symbol_, + decimals_, + sequence_, + owner_, + chainId_, + nativeContract_ + ); + + // initialize w/ EIP712 state variables for domain separator + _initializePermitStateIfNeeded(); + } + + function _initializeNativeToken( + string memory name_, + string memory symbol_, + uint8 decimals_, + uint64 sequence_, + address owner_, + uint16 chainId_, + bytes32 nativeContract_ + ) internal { _state.name = name_; _state.symbol = symbol_; _state.decimals = decimals_; @@ -35,6 +57,21 @@ contract TokenImplementation is TokenState, Context { _state.nativeContract = nativeContract_; } + function _initializePermitStateIfNeeded() internal { + if (!permitInitialized()) { + _state.hashedTokenChain = _hashedTokenChain(); + _state.hashedNativeContract = _hashedNativeContract(); + _state.hashedVersion = _hashedDomainVersion(); + _state.typeHash = _hashedDomainType(); + _state.cachedChainId = block.chainid; + _state.cachedDomainSeparator = _buildNativeDomainSeparator( + _state.typeHash, _state.hashedTokenChain, _state.hashedNativeContract, _state.hashedVersion + ); + _state.cachedThis = address(this); + _state.permitInitialized = true; + } + } + function name() public view returns (string memory) { return string(abi.encodePacked(_state.name)); } @@ -174,4 +211,108 @@ contract TokenImplementation is TokenState, Context { _; } + + /** + * @dev Returns the domain separator for the current chain. + */ + function _domainSeparatorV4() internal view returns (bytes32) { + if (address(this) == _state.cachedThis && block.chainid == _state.cachedChainId) { + return _state.cachedDomainSeparator; + } else { + return _buildNativeDomainSeparator( + _hashedDomainType(), + _hashedTokenChain(), + _hashedNativeContract(), + _hashedDomainVersion() + ); + } + } + + function _buildNativeDomainSeparator( + bytes32 typeHash, + bytes32 tokenChainHash, + bytes32 nativeContractHash, + bytes32 versionHash + ) internal view returns (bytes32) { + return keccak256(abi.encode(typeHash, tokenChainHash, nativeContractHash, versionHash, block.chainid, address(this))); + } + + /** + * @dev Given an already https://eips.ethereum.org/EIPS/eip-712#definition-of-hashstruct[hashed struct], this + * function returns the hash of the fully encoded EIP712 message for this domain. + * + * This hash can be used together with {ECDSA-recover} to obtain the signer of a message. For example: + * + * ```solidity + * bytes32 digest = _hashTypedDataV4(keccak256(abi.encode( + * keccak256("Mail(address to,string contents)"), + * mailTo, + * keccak256(bytes(mailContents)) + * ))); + * address signer = ECDSA.recover(digest, signature); + * ``` + */ + function _hashTypedDataV4(bytes32 structHash) internal view returns (bytes32) { + return ECDSA.toTypedDataHash(_domainSeparatorV4(), structHash); + } + + /** + * @dev See {IERC20Permit-permit}. + */ + function permit( + address owner_, + address spender_, + uint256 value_, + uint256 deadline_, + uint8 v_, + bytes32 r_, + bytes32 s_ + ) public { + // for those tokens that have been initialized before permit, we need to set + // the permit state variables if they have not been set before + _initializePermitStateIfNeeded(); + + // permit is only allowed before the signature's deadline + require(block.timestamp <= deadline_, "ERC20Permit: expired deadline"); + + bytes32 structHash = keccak256( + abi.encode(_hashedPermitType(), owner_, spender_, value_, _useNonce(owner_), deadline_) + ); + + bytes32 message = _hashTypedDataV4(structHash); + address signer = ECDSA.recover(message, v_, r_, s_); + + // if we cannot recover the token owner, signature is invalid + require(signer == owner_, "ERC20Permit: invalid signature"); + + _approve(owner_, spender_, value_); + } + + /** + * @dev See {IERC20Permit-DOMAIN_SEPARATOR}. + */ + // solhint-disable-next-line func-name-mixedcase + function DOMAIN_SEPARATOR() public view returns (bytes32) { + return _domainSeparatorV4(); + } + + function _hashedTokenChain() internal view returns (bytes32) { + return keccak256(abi.encodePacked(_state.chainId)); + } + + function _hashedNativeContract() internal view returns (bytes32) { + return keccak256(abi.encodePacked(_state.nativeContract)); + } + + function _hashedDomainVersion() internal pure returns (bytes32) { + return keccak256(bytes("1")); + } + + function _hashedDomainType() internal pure returns (bytes32) { + return keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); + } + + function _hashedPermitType() internal pure returns (bytes32) { + return keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"); + } } diff --git a/ethereum/contracts/bridge/token/TokenState.sol b/ethereum/contracts/bridge/token/TokenState.sol index cd58ce54a..93526a90d 100644 --- a/ethereum/contracts/bridge/token/TokenState.sol +++ b/ethereum/contracts/bridge/token/TokenState.sol @@ -3,6 +3,8 @@ pragma solidity ^0.8.0; +import "@openzeppelin/contracts/utils/Counters.sol"; + contract TokenStorage { struct State { string name; @@ -23,9 +25,47 @@ contract TokenStorage { uint16 chainId; bytes32 nativeContract; + + // EIP712 + // Cache the domain separator as an immutable value, but also store the chain id that it corresponds to, in order to + // invalidate the cached domain separator if the chain id changes. + bool permitInitialized; + bytes32 cachedDomainSeparator; + uint256 cachedChainId; + address cachedThis; + + bytes32 hashedTokenChain; + bytes32 hashedNativeContract; + bytes32 hashedVersion; + bytes32 typeHash; + + // ERC20Permit draft + mapping(address => Counters.Counter) nonces; } } contract TokenState { + using Counters for Counters.Counter; + TokenStorage.State _state; + + /** + * @dev See {IERC20Permit-nonces}. + */ + function nonces(address owner_) public view returns (uint256) { + return _state.nonces[owner_].current(); + } + + /** + * @dev "Consume a nonce": return the current value and increment. + */ + function _useNonce(address owner_) internal returns (uint256 current) { + Counters.Counter storage nonce = _state.nonces[owner_]; + current = nonce.current(); + nonce.increment(); + } + + function permitInitialized() public view returns (bool) { + return _state.permitInitialized; + } } \ No newline at end of file diff --git a/ethereum/forge-test/TokenImplementation.t.sol b/ethereum/forge-test/TokenImplementation.t.sol new file mode 100644 index 000000000..3395dedbf --- /dev/null +++ b/ethereum/forge-test/TokenImplementation.t.sol @@ -0,0 +1,345 @@ +// SPDX-License-Identifier: Apache 2 + +pragma solidity ^0.8.0; + +import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; +import "../contracts/bridge/token/TokenImplementation.sol"; +import "forge-std/Test.sol"; + +import "forge-std/console.sol"; + +contract TestTokenImplementation is TokenImplementation, Test { + uint256 constant SECP256K1_CURVE_ORDER = + 115792089237316195423570985008687907852837564279074904382605163141518161494337; + + struct InitiateParameters { + string name; + string symbol; + uint8 decimals; + uint64 sequence; + address owner; + uint16 chainId; + bytes32 nativeContract; + } + + function setupTestEnvironmentWithInitialize() public { + InitiateParameters memory init; + init.name = "Valuable Token"; + init.symbol = "VALU"; + init.decimals = 8; + init.sequence = 1; + init.owner = 0xDeaDbeefdEAdbeefdEadbEEFdeadbeEFdEaDbeeF; + init.chainId = 1; + init + .nativeContract = 0x1337133713371337133713371337133713371337133713371337133713371337; + + initialize( + init.name, + init.symbol, + init.decimals, + init.sequence, + init.owner, + init.chainId, + init.nativeContract + ); + } + + function setupTestEnvironmentWithOldInitialize() public { + InitiateParameters memory init; + init.name = "Old Valuable Token"; + init.symbol = "OLD"; + init.decimals = 8; + init.sequence = 1; + init.owner = 0xDeaDbeefdEAdbeefdEadbEEFdeadbeEFdEaDbeeF; + init.chainId = 1; + init + .nativeContract = 0x1337133713371337133713371337133713371337133713371337133713371337; + + _initializeNativeToken( + init.name, + init.symbol, + init.decimals, + init.sequence, + init.owner, + init.chainId, + init.nativeContract + ); + } + + function simulatePermitSignature( + bytes32 walletPrivateKey, + address spender, + uint256 amount, + uint256 deadline + ) + public + returns ( + address allower, + uint8 v, + bytes32 r, + bytes32 s + ) + { + // prepare signer allowing for tokens to be spent + uint256 sk = uint256(walletPrivateKey); + allower = vm.addr(sk); + + bytes32 PERMIT_TYPEHASH = keccak256( + "Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)" + ); + bytes32 structHash = keccak256( + abi.encode( + PERMIT_TYPEHASH, + allower, + spender, + amount, + nonces(allower), + deadline + ) + ); + + bytes32 message = ECDSA.toTypedDataHash(DOMAIN_SEPARATOR(), structHash); + (v, r, s) = vm.sign(sk, message); + } + + function testPermit( + bytes32 walletPrivateKey, + uint256 amount, + address spender + ) public { + vm.assume(walletPrivateKey != bytes32(0)); + vm.assume(uint256(walletPrivateKey) < SECP256K1_CURVE_ORDER); + vm.assume(spender != address(0)); + + // initialize TokenImplementation + setupTestEnvironmentWithInitialize(); + + // prepare signer allowing for tokens to be spent + uint256 deadline = 10; + ( + address allower, + uint8 v, + bytes32 r, + bytes32 s + ) = simulatePermitSignature( + walletPrivateKey, + spender, + amount, + deadline + ); + + // get allowance before calling permit + uint256 allowanceBefore = allowance(allower, spender); + + // set allowance with permit + permit(allower, spender, amount, deadline, v, r, s); + uint256 allowanceAfter = allowance(allower, spender); + + require( + allowanceAfter - allowanceBefore == amount, + "allowance incorrect" + ); + } + + function testFailPermitWithSameSignature( + bytes32 walletPrivateKey, + uint256 amount, + address spender + ) public { + vm.assume(walletPrivateKey != bytes32(0)); + vm.assume(uint256(walletPrivateKey) < SECP256K1_CURVE_ORDER); + vm.assume(spender != address(0)); + + // initialize TokenImplementation + setupTestEnvironmentWithInitialize(); + + // prepare signer allowing for tokens to be spent + uint256 deadline = 10; + ( + address allower, + uint8 v, + bytes32 r, + bytes32 s + ) = simulatePermitSignature( + walletPrivateKey, + spender, + amount, + deadline + ); + + // set allowance with permit + permit(allower, spender, amount, deadline, v, r, s); + + // try again... you shall not pass + // NOTE: using "testFail" instead of "test" because + // vm.expectRevert("ERC20Permit: invalid signature") does not work + permit(allower, spender, amount, deadline, v, r, s); + } + + function testFailPermitWithBadSignature( + bytes32 walletPrivateKey, + uint256 amount, + address spender + ) public { + vm.assume(walletPrivateKey != bytes32(0)); + vm.assume(uint256(walletPrivateKey) < SECP256K1_CURVE_ORDER); + vm.assume(spender != address(0)); + + // initialize TokenImplementation + setupTestEnvironmentWithInitialize(); + + // avoid overflow for this test + uint256 wrongAmount; + unchecked { + wrongAmount = amount + 1; // amount will never equal + } + + // prepare signer allowing for tokens to be spent + uint256 deadline = 10; + ( + address allower, + uint8 v, + bytes32 r, + bytes32 s + ) = simulatePermitSignature( + walletPrivateKey, + spender, + wrongAmount, + deadline + ); + + // you shall not pass! + // NOTE: using "testFail" instead of "test" because + // vm.expectRevert("ERC20Permit: invalid signature") does not work + permit(allower, spender, amount, deadline, v, r, s); + } + + function testPermitWithSignatureUsedAfterDeadline( + bytes32 walletPrivateKey, + uint256 amount, + address spender + ) public { + vm.assume(walletPrivateKey != bytes32(0)); + vm.assume(uint256(walletPrivateKey) < SECP256K1_CURVE_ORDER); + vm.assume(spender != address(0)); + + // initialize TokenImplementation + setupTestEnvironmentWithInitialize(); + + // prepare signer allowing for tokens to be spent + uint256 deadline = 10; + ( + address allower, + uint8 v, + bytes32 r, + bytes32 s + ) = simulatePermitSignature( + walletPrivateKey, + spender, + amount, + deadline + ); + + // waited too long + vm.warp(deadline + 1); + + // and fail + vm.expectRevert("ERC20Permit: expired deadline"); + permit(allower, spender, amount, deadline, v, r, s); + } + + function testInitializePermitState() public { + // initialize TokenImplementation as if it were the old implementation + setupTestEnvironmentWithOldInitialize(); + require(!permitInitialized(), "permit state should not be initialized"); + + // explicity call private method + _initializePermitStateIfNeeded(); + require(permitInitialized(), "permit state should be initialized"); + + // check permit state variables + require( + _state.hashedTokenChain == + keccak256(abi.encodePacked(_state.chainId)), + "_state.hashedTokenChain != expected" + ); + require( + _state.hashedNativeContract == + keccak256(abi.encodePacked(_state.nativeContract)), + "_state.hashedNativeContract != expected" + ); + require( + _state.hashedVersion == keccak256(bytes("1")), + "_state.hashedVersion != expected" + ); + require( + _state.typeHash == + keccak256( + "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)" + ), + "_state.typeHash != expected" + ); + require( + _state.cachedChainId == block.chainid, + "_state.cachedChainId != expected" + ); + require( + _state.cachedDomainSeparator == + keccak256( + abi.encode( + _hashedDomainType(), + _hashedTokenChain(), + _hashedNativeContract(), + _hashedDomainVersion(), + block.chainid, + address(this) + ) + ), + "_state.cachedDomainSeparator != expected" + ); + require( + _state.cachedThis == address(this), + "_state.cachedThis != expected" + ); + } + + function testPermitForPreviouslyDeployedImplementation( + bytes32 walletPrivateKey, + uint256 amount, + address spender + ) public { + vm.assume(walletPrivateKey != bytes32(0)); + vm.assume(uint256(walletPrivateKey) < SECP256K1_CURVE_ORDER); + vm.assume(spender != address(0)); + + // initialize TokenImplementation as if it were the old implementation + setupTestEnvironmentWithOldInitialize(); + + // prepare signer allowing for tokens to be spent + uint256 deadline = 10; + ( + address allower, + uint8 v, + bytes32 r, + bytes32 s + ) = simulatePermitSignature( + walletPrivateKey, + spender, + amount, + deadline + ); + + // get allowance before calling permit + uint256 allowanceBefore = allowance(allower, spender); + + // set allowance with permit + permit(allower, spender, amount, deadline, v, r, s); + + uint256 allowanceAfter = allowance(allower, spender); + + require( + allowanceAfter - allowanceBefore == amount, + "allowance incorrect" + ); + } +}