diff --git a/contracts/BridgingManager.sol b/contracts/BridgingManager.sol index abffef4d..8468ef7b 100644 --- a/contracts/BridgingManager.sol +++ b/contracts/BridgingManager.sol @@ -3,11 +3,11 @@ pragma solidity 0.8.10; -import {AccessControl} from "@openzeppelin/contracts/access/AccessControl.sol"; +import {AccessControlEnumerable} from "@openzeppelin/contracts/access/AccessControlEnumerable.sol"; /// @author psirex /// @notice Contains administrative methods to retrieve and control the state of the bridging -contract BridgingManager is AccessControl { +contract BridgingManager is AccessControlEnumerable { /// @dev Stores the state of the bridging /// @param isInitialized Shows whether the contract is initialized or not /// @param isDepositsEnabled Stores the state of the deposits diff --git a/contracts/lisk/stubs/AccountStub.sol b/contracts/lisk/stubs/AccountStub.sol new file mode 100644 index 00000000..e306b06c --- /dev/null +++ b/contracts/lisk/stubs/AccountStub.sol @@ -0,0 +1,35 @@ +// SPDX-FileCopyrightText: 2022 Lido +// SPDX-License-Identifier: GPL-3.0 + +pragma solidity 0.8.10; + +import "@openzeppelin/contracts/interfaces/IERC1271.sol"; +import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; + +contract AccountStub is IERC1271 { + using ECDSA for bytes32; + + address public owner; + + // bytes4(keccak256("isValidSignature(bytes,bytes)") + bytes4 internal constant MAGICVALUE = 0x1626ba7e; + bytes4 internal constant INVALID_SIGNATURE = 0xffffffff; + + constructor(address _owner) { + owner = _owner; + } + + function isValidSignature(bytes32 _messageHash, bytes memory _signature) + public + view + override + returns (bytes4 magicValue) + { + address signer = _messageHash.recover(_signature); + if (signer == owner) { + return MAGICVALUE; + } else { + return INVALID_SIGNATURE; + } + } +} diff --git a/contracts/token/ERC20Bridged.sol b/contracts/token/ERC20Bridged.sol index 574ddd8b..3be2887b 100644 --- a/contracts/token/ERC20Bridged.sol +++ b/contracts/token/ERC20Bridged.sol @@ -5,12 +5,12 @@ pragma solidity 0.8.10; import {IERC20Bridged} from "./interfaces/IERC20Bridged.sol"; -import {ERC20Core} from "./ERC20Core.sol"; +import {ERC20Permit} from "./ERC20Permit.sol"; import {ERC20Metadata} from "./ERC20Metadata.sol"; /// @author psirex /// @notice Extends the ERC20 functionality that allows the bridge to mint/burn tokens -contract ERC20Bridged is IERC20Bridged, ERC20Core, ERC20Metadata { +contract ERC20Bridged is IERC20Bridged, ERC20Permit, ERC20Metadata { /// @inheritdoc IERC20Bridged address public immutable bridge; @@ -23,7 +23,14 @@ contract ERC20Bridged is IERC20Bridged, ERC20Core, ERC20Metadata { string memory symbol_, uint8 decimals_, address bridge_ - ) ERC20Metadata(name_, symbol_, decimals_) { + ) + ERC20Permit(name_) + ERC20Metadata( + name_, + symbol_, + decimals_ + ) + { bridge = bridge_; } diff --git a/contracts/token/ERC20Permit.sol b/contracts/token/ERC20Permit.sol new file mode 100644 index 00000000..1ca5b077 --- /dev/null +++ b/contracts/token/ERC20Permit.sol @@ -0,0 +1,87 @@ +// SPDX-FileCopyrightText: 2022 Lido +// SPDX-License-Identifier: GPL-3.0 + +pragma solidity 0.8.10; + +import "@openzeppelin/contracts/proxy/utils/Initializable.sol"; +import "@openzeppelin/contracts/token/ERC20/extensions/draft-IERC20Permit.sol"; +import "@openzeppelin/contracts/utils/cryptography/draft-EIP712.sol"; +import "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol"; + +import {ERC20Core} from "./ERC20Core.sol"; +import {Nonces} from "./utils/Nonces.sol"; + +/** + * @dev Implementation of the ERC20 Permit extension allowing approvals to be made via signatures, as defined in + * https://eips.ethereum.org/EIPS/eip-2612[EIP-2612]. + * + * Adds the {permit} method, which can be used to change an account's ERC20 allowance (see {IERC20-allowance}) by + * presenting a message signed by the account. By not relying on `{IERC20-approve}`, the token holder account doesn't + * need to send a transaction, and thus is not required to hold Ether at all. + */ +contract ERC20Permit is Initializable, ERC20Core, IERC20Permit, EIP712, Nonces { + // solhint-disable-next-line var-name-mixedcase + bytes32 private constant _PERMIT_TYPEHASH = + keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"); + + /** + * @dev Permit deadline has expired. + */ + error ERC2612ExpiredSignature(uint256 deadline); + + /** + * @dev Initializes the {EIP712} domain separator using the `name` parameter, and setting `version` to `"1"`. + * + * It's a good idea to use the same `name` that is defined as the ERC20 token name. + */ + constructor(string memory name) EIP712(name, "1") {} + + /** + * @dev See {IERC20Permit-permit}. + * + * Method is compatible with EOA signatures and EIP-1271 (Smart Contract Wallet) signatures + * At the time of developing this contract OpenZeppelin issue related to this (https://github.com/OpenZeppelin/openzeppelin-contracts/issues/2845) + * was still open since they require EIP-2612 specification change to support EIP-1271 signatures before supporting it + */ + function permit(address owner, address spender, uint256 value, uint256 deadline, uint8 v, bytes32 r, bytes32 s) + public + virtual + { + if (block.timestamp > deadline) { + revert ERC2612ExpiredSignature(deadline); + } + + bytes32 structHash = keccak256(abi.encode(_PERMIT_TYPEHASH, owner, spender, value, _useNonce(owner), deadline)); + + bytes32 hash = _hashTypedDataV4(structHash); + + require( + SignatureChecker.isValidSignatureNow(owner, hash, abi.encodePacked(r, s, v)), + "ERC20Permit: invalid signature" + ); + + _approve(owner, spender, value); + } + + /** + * @dev See {IERC20Permit-nonces}. + */ + function nonces(address owner) public view virtual override(IERC20Permit, Nonces) returns (uint256) { + return super.nonces(owner); + } + + /** + * @dev See {IERC20Permit-DOMAIN_SEPARATOR}. + */ + // solhint-disable-next-line func-name-mixedcase + function DOMAIN_SEPARATOR() external view virtual returns (bytes32) { + return _domainSeparatorV4(); + } + + /** + * @dev This empty reserved space is put in place to allow future versions to add new + * variables without shifting down storage in the inheritance chain. + * See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps + */ + uint256[50] private __gap; +} diff --git a/contracts/token/utils/Nonces.sol b/contracts/token/utils/Nonces.sol new file mode 100644 index 00000000..73f5632f --- /dev/null +++ b/contracts/token/utils/Nonces.sol @@ -0,0 +1,61 @@ +// SPDX-FileCopyrightText: 2022 Lido +// SPDX-License-Identifier: GPL-3.0 + +pragma solidity 0.8.10; + +import {Initializable} from "@openzeppelin/contracts/proxy/utils/Initializable.sol"; + +/** + * @dev Provides tracking nonces for addresses. Nonces will only increment. + */ +abstract contract Nonces is Initializable { + function __Nonces_init() internal onlyInitializing {} + + function __Nonces_init_unchained() internal onlyInitializing {} + + /** + * @dev The nonce used for an `account` is not the expected current nonce. + */ + error InvalidAccountNonce(address account, uint256 currentNonce); + + mapping(address => uint256) private _nonces; + + /** + * @dev Returns an the next unused nonce for an address. + */ + function nonces(address owner) public view virtual returns (uint256) { + return _nonces[owner]; + } + + /** + * @dev Consumes a nonce. + * + * Returns the current value and increments nonce. + */ + function _useNonce(address owner) internal virtual returns (uint256) { + // For each account, the nonce has an initial value of 0, can only be incremented by one, and cannot be + // decremented or reset. This guarantees that the nonce never overflows. + unchecked { + // It is important to do x++ and not ++x here. + return _nonces[owner]++; + } + } + + /** + * @dev Same as {_useNonce} but checking that `nonce` is the next valid for `owner`. + */ + function _useCheckedNonce(address owner, uint256 nonce) internal virtual returns (uint256) { + uint256 current = _useNonce(owner); + if (nonce != current) { + revert InvalidAccountNonce(owner, current); + } + return current; + } + + /** + * @dev This empty reserved space is put in place to allow future versions to add new + * variables without shifting down storage in the inheritance chain. + * See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps + */ + uint256[49] private __gap; +} diff --git a/funding.json b/funding.json new file mode 100644 index 00000000..0d08cbb6 --- /dev/null +++ b/funding.json @@ -0,0 +1,5 @@ +{ + "opRetro": { + "projectId": "0x7747e26c3538f93bc67d9e13f343464996624e96fe549e3be484e848a1f61732" + } +} \ No newline at end of file diff --git a/test/token/ERC20Bridged.unit.test.ts b/test/token/ERC20Bridged.unit.test.ts index 2ec65bfa..e149970e 100644 --- a/test/token/ERC20Bridged.unit.test.ts +++ b/test/token/ERC20Bridged.unit.test.ts @@ -3,6 +3,7 @@ import hre from "hardhat"; import { ERC20Bridged__factory, OssifiableProxy__factory, + AccountStub__factory, } from "../../typechain"; import { unit } from "../../utils/testing"; import { wei } from "../../utils/wei"; @@ -475,6 +476,299 @@ unit("ERC20Bridged", ctxFactory) }, ]) + .test("permit()", async (ctx) => { + const { erc20Bridged } = ctx; + const { premint } = ctx.constants; + const { recipient, holder } = ctx.accounts; + + // validate balance before permit + assert.equalBN(await erc20Bridged.balanceOf(holder.address), premint); + assert.equalBN(await erc20Bridged.balanceOf(recipient.address), 0); + + // validate allowance before permit + assert.equalBN(await erc20Bridged.allowance(holder.address, recipient.address), 0); + + // amount that will be permitted + const amount = wei`1 ether`; + + // get the nonce of the holder + const nonce = await erc20Bridged.nonces(holder.address); + + // build deadline for the permit + const deadline = Math.floor(Date.now() / 1000) + 3600; + + // build the domain data for permit hash + const domainData = { + name: await erc20Bridged.name(), + version: "1", + chainId: (await hre.ethers.provider.getNetwork()).chainId, + verifyingContract: erc20Bridged.address, + }; + + // build the types and values for permit hash + const types = { + Permit: [ + { name: "owner", type: "address" }, + { name: "spender", type: "address" }, + { name: "value", type: "uint256" }, + { name: "nonce", type: "uint256" }, + { name: "deadline", type: "uint256" }, + ], + }; + const value = { + owner: holder.address, + spender: recipient.address, + value: amount, + nonce, + deadline, + }; + + // sign the permit hash + const signature = await holder._signTypedData(domainData, types, value); + + // split the signature into its components + const sig = hre.ethers.utils.splitSignature(signature); + + // perform the permit and ensure the allowance + await erc20Bridged.permit( + holder.address, + recipient.address, + amount, + deadline, + sig.v, + sig.r, + sig.s + ); + assert.equalBN(await erc20Bridged.allowance(holder.address, recipient.address), wei.toBigNumber(amount)); + + // ensure recipient can transfer the tokens, and that the balance is updated + await erc20Bridged.connect(recipient).transferFrom(holder.address, recipient.address, amount); + assert.equalBN( + await erc20Bridged.balanceOf(recipient.address), + wei.toBigNumber(amount) + ); + assert.equalBN( + await erc20Bridged.balanceOf(holder.address), + wei.toBigNumber(premint).sub(amount) + ); + }) + + .test("permit() :: invalid signature", async (ctx) => { + const { erc20Bridged } = ctx; + const { premint } = ctx.constants; + const { recipient, holder } = ctx.accounts; + + // amount that will be permitted + const amount = wei`1 ether`; + + // get the nonce of the holder + const nonce = await erc20Bridged.nonces(holder.address); + + // build deadline for the permit + const deadline = Math.floor(Date.now() / 1000) + 3600; + + // build the domain data for permit hash + const domainData = { + name: await erc20Bridged.name(), + version: "1", + chainId: (await hre.ethers.provider.getNetwork()).chainId, + verifyingContract: erc20Bridged.address, + }; + + // build the types and values for permit hash + const types = { + Permit: [ + { name: "owner", type: "address" }, + { name: "spender", type: "address" }, + { name: "value", type: "uint256" }, + { name: "nonce", type: "uint256" }, + { name: "deadline", type: "uint256" }, + ], + }; + const value = { + owner: holder.address, + spender: recipient.address, + value: amount, + nonce, + deadline, + }; + + // sign the permit hash + const signature = await holder._signTypedData(domainData, types, value); + + // split the signature into its components + const sig = hre.ethers.utils.splitSignature(signature); + + // make the signature invalid + sig.r = hre.ethers.constants.HashZero; + + // try to perform the permit and ensure it reverts + await assert.revertsWith( + erc20Bridged.permit( + holder.address, + recipient.address, + amount, + deadline, + sig.v, + sig.r, + sig.s + ), + "ERC20Permit: invalid signature" + ); + }) + + .test("permit() :: smart contract", async (ctx) => { + const { erc20Bridged } = ctx; + const { premint } = ctx.constants; + const { recipient, holder, deployer } = ctx.accounts; + const { accountStub } = ctx; + + // check owner of the account smart contract + assert.equal(await accountStub.owner(), deployer.address); + + // amount that will be permitted + const amount = wei`1 ether`; + + // transfer tokens + const tx = await erc20Bridged.connect(holder).transfer(accountStub.address, amount); + + // validate balance before permit + assert.equalBN(await erc20Bridged.balanceOf(accountStub.address), amount); + assert.equalBN(await erc20Bridged.balanceOf(recipient.address), 0); + + // validate allowance before permit + assert.equalBN(await erc20Bridged.allowance(accountStub.address, recipient.address), 0); + + // get the nonce of the accountStub + const nonce = await erc20Bridged.nonces(accountStub.address); + + // build deadline for the permit + const deadline = Math.floor(Date.now() / 1000) + 3600; + + // build the domain data for permit hash + const domainData = { + name: await erc20Bridged.name(), + version: "1", + chainId: (await hre.ethers.provider.getNetwork()).chainId, + verifyingContract: erc20Bridged.address, + }; + + // build the types and values for permit hash + const types = { + Permit: [ + { name: "owner", type: "address" }, + { name: "spender", type: "address" }, + { name: "value", type: "uint256" }, + { name: "nonce", type: "uint256" }, + { name: "deadline", type: "uint256" }, + ], + }; + const value = { + owner: accountStub.address, + spender: recipient.address, + value: amount, + nonce, + deadline, + }; + + // sign the permit hash + const signature = await deployer._signTypedData(domainData, types, value); + + // split the signature into its components + const sig = hre.ethers.utils.splitSignature(signature); + + // perform the permit and ensure the allowance + await erc20Bridged.permit( + accountStub.address, + recipient.address, + amount, + deadline, + sig.v, + sig.r, + sig.s + ); + assert.equalBN(await erc20Bridged.allowance(accountStub.address, recipient.address), wei.toBigNumber(amount)); + + // ensure recipient can transfer the tokens, and that the balance is updated + await erc20Bridged.connect(recipient).transferFrom(accountStub.address, recipient.address, amount); + assert.equalBN( + await erc20Bridged.balanceOf(recipient.address), + wei.toBigNumber(amount) + ); + assert.equalBN( + await erc20Bridged.balanceOf(accountStub.address), + 0 + ); + }) + + .test("permit() :: smart contract with invalid signature", async (ctx) => { + const { erc20Bridged } = ctx; + const { premint } = ctx.constants; + const { recipient, holder, deployer } = ctx.accounts; + const { accountStub } = ctx; + + // check owner of the account smart contract + assert.equal(await accountStub.owner(), deployer.address); + + // amount that will be permitted + const amount = wei`1 ether`; + + // get the nonce of the accountStub + const nonce = await erc20Bridged.nonces(accountStub.address); + + // build deadline for the permit + const deadline = Math.floor(Date.now() / 1000) + 3600; + + // build the domain data for permit hash + const domainData = { + name: await erc20Bridged.name(), + version: "1", + chainId: (await hre.ethers.provider.getNetwork()).chainId, + verifyingContract: erc20Bridged.address, + }; + + // build the types and values for permit hash + const types = { + Permit: [ + { name: "owner", type: "address" }, + { name: "spender", type: "address" }, + { name: "value", type: "uint256" }, + { name: "nonce", type: "uint256" }, + { name: "deadline", type: "uint256" }, + ], + }; + const value = { + owner: accountStub.address, + spender: recipient.address, + value: amount, + nonce, + deadline, + }; + + // sign the permit hash + const signature = await deployer._signTypedData(domainData, types, value); + + // split the signature into its components + const sig = hre.ethers.utils.splitSignature(signature); + + // make the signature invalid + sig.r = hre.ethers.constants.HashZero; + + // try to perform the permit and ensure it reverts + await assert.revertsWith( + erc20Bridged.permit( + accountStub.address, + recipient.address, + amount, + deadline, + sig.v, + sig.r, + sig.s + ), + "ERC20Permit: invalid signature" + ); + }) + .test("bridgeMint() :: not owner", async (ctx) => { const { erc20Bridged } = ctx; const { stranger } = ctx.accounts; @@ -631,9 +925,14 @@ async function ctxFactory() { await erc20BridgedProxied.connect(owner).bridgeMint(holder.address, premint); + const accountStub = await new AccountStub__factory(deployer).deploy( + deployer.address + ); + return { accounts: { deployer, owner, recipient, spender, holder, zero, stranger }, constants: { name, symbol, decimals, premint }, erc20Bridged: erc20BridgedProxied, + accountStub: accountStub, }; }