diff --git a/CHANGELOG.md b/CHANGELOG.md index f902f6c0c08..71b6c1349f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ * `Initializable`: Make initializer check stricter during construction. ([#2531](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2531)) * `ERC721`: remove enumerability of tokens from the base implementation. This feature is now provided separately through the `ERC721Enumerable` extension. ([#2511](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2511)) * `AccessControl`: removed enumerability by default for a more lightweight contract. It is now opt-in through `AccessControlEnumerable`. ([#2512](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2512)) + * Meta Transactions: add `ERC2771Context` and a `MinimalForwarder` for meta-transactions. ([#2508](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2508)) ## 3.4.0 (2021-02-02) diff --git a/contracts/cryptography/ECDSA.sol b/contracts/cryptography/ECDSA.sol index e70815df1fd..76dab5d2113 100644 --- a/contracts/cryptography/ECDSA.sol +++ b/contracts/cryptography/ECDSA.sol @@ -72,9 +72,9 @@ library ECDSA { /** * @dev Returns an Ethereum Signed Message, created from a `hash`. This - * replicates the behavior of the - * https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_sign[`eth_sign`] - * JSON-RPC method. + * produces hash corresponding to the one signed with the + * https://eth.wiki/json-rpc/API#eth_sign[`eth_sign`] + * JSON-RPC method as part of EIP-191. * * See {recover}. */ @@ -83,4 +83,17 @@ library ECDSA { // enforced by the type signature above return keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", hash)); } + + /** + * @dev Returns an Ethereum Signed Typed Data, created from a + * `domainSeparator` and a `structHash`. This produces hash corresponding + * to the one signed with the + * https://eips.ethereum.org/EIPS/eip-712[`eth_signTypedData`] + * JSON-RPC method as part of EIP-712. + * + * See {recover}. + */ + function toTypedDataHash(bytes32 domainSeparator, bytes32 structHash) internal pure returns (bytes32) { + return keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash)); + } } diff --git a/contracts/drafts/EIP712.sol b/contracts/drafts/EIP712.sol index 68ee9de4a22..c3be3ea7630 100644 --- a/contracts/drafts/EIP712.sol +++ b/contracts/drafts/EIP712.sol @@ -2,6 +2,8 @@ pragma solidity ^0.8.0; +import "../cryptography/ECDSA.sol"; + /** * @dev https://eips.ethereum.org/EIPS/eip-712[EIP 712] is a standard for hashing and signing of typed structured data. * @@ -95,6 +97,6 @@ abstract contract EIP712 { * ``` */ function _hashTypedDataV4(bytes32 structHash) internal view virtual returns (bytes32) { - return keccak256(abi.encodePacked("\x19\x01", _domainSeparatorV4(), structHash)); + return ECDSA.toTypedDataHash(_domainSeparatorV4(), structHash); } } diff --git a/contracts/metatx/ERC2771Context.sol b/contracts/metatx/ERC2771Context.sol new file mode 100644 index 00000000000..5ab22f5d9e7 --- /dev/null +++ b/contracts/metatx/ERC2771Context.sol @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "../utils/Context.sol"; + +/* + * @dev Context variant with ERC2771 support. + */ +abstract contract ERC2771Context is Context { + address immutable _trustedForwarder; + + constructor(address trustedForwarder) { + _trustedForwarder = trustedForwarder; + } + + function isTrustedForwarder(address forwarder) public view virtual returns(bool) { + return forwarder == _trustedForwarder; + } + + function _msgSender() internal view virtual override returns (address sender) { + if (isTrustedForwarder(msg.sender)) { + // The assembly code is more direct than the Solidity version using `abi.decode`. + assembly { sender := shr(96, calldataload(sub(calldatasize(), 20))) } + } else { + return super._msgSender(); + } + } + + function _msgData() internal view virtual override returns (bytes calldata) { + if (isTrustedForwarder(msg.sender)) { + return msg.data[:msg.data.length-20]; + } else { + return super._msgData(); + } + } +} diff --git a/contracts/metatx/MinimalForwarder.sol b/contracts/metatx/MinimalForwarder.sol new file mode 100644 index 00000000000..6c028ec049a --- /dev/null +++ b/contracts/metatx/MinimalForwarder.sol @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "../cryptography/ECDSA.sol"; +import "../drafts/EIP712.sol"; + +/* + * @dev Simple minimal forwarder to be used together with an ERC2771 compatible contract. See {ERC2771Context}. + */ +contract MinimalForwarder is EIP712 { + using ECDSA for bytes32; + + struct ForwardRequest { + address from; + address to; + uint256 value; + uint256 gas; + uint256 nonce; + bytes data; + } + + bytes32 private constant TYPEHASH = keccak256("ForwardRequest(address from,address to,uint256 value,uint256 gas,uint256 nonce,bytes data)"); + + mapping(address => uint256) private _nonces; + + constructor() EIP712("MinimalForwarder", "0.0.1") {} + + function getNonce(address from) public view returns (uint256) { + return _nonces[from]; + } + + function verify(ForwardRequest calldata req, bytes calldata signature) public view returns (bool) { + address signer = _hashTypedDataV4(keccak256(abi.encode( + TYPEHASH, + req.from, + req.to, + req.value, + req.gas, + req.nonce, + keccak256(req.data) + ))).recover(signature); + return _nonces[req.from] == req.nonce && signer == req.from; + } + + function execute(ForwardRequest calldata req, bytes calldata signature) public payable returns (bool, bytes memory) { + require(verify(req, signature), "MinimalForwarder: signature does not match request"); + _nonces[req.from] = req.nonce + 1; + + // solhint-disable-next-line avoid-low-level-calls + (bool success, bytes memory returndata) = req.to.call{gas: req.gas, value: req.value}(abi.encodePacked(req.data, req.from)); + // Validate that the relayer has sent enough gas for the call. + // See https://ronan.eth.link/blog/ethereum-gas-dangers/ + assert(gasleft() > req.gas / 63); + + return (success, returndata); + } +} diff --git a/contracts/metatx/README.adoc b/contracts/metatx/README.adoc new file mode 100644 index 00000000000..51a97913fa4 --- /dev/null +++ b/contracts/metatx/README.adoc @@ -0,0 +1,12 @@ += Meta Transactions + +[.readme-notice] +NOTE: This document is better viewed at https://docs.openzeppelin.com/contracts/api/math + +== Core + +{{ERC2771Context}} + +== Utils + +{{MinimalForwarder}} diff --git a/contracts/mocks/ERC2771ContextMock.sol b/contracts/mocks/ERC2771ContextMock.sol new file mode 100644 index 00000000000..14c5b104c4c --- /dev/null +++ b/contracts/mocks/ERC2771ContextMock.sol @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "./ContextMock.sol"; +import "../metatx/ERC2771Context.sol"; + +// By inheriting from ERC2771Context, Context's internal functions are overridden automatically +contract ERC2771ContextMock is ContextMock, ERC2771Context { + constructor(address trustedForwarder) ERC2771Context(trustedForwarder) {} + + function _msgSender() internal override(Context, ERC2771Context) view virtual returns (address) { + return ERC2771Context._msgSender(); + } + + function _msgData() internal override(Context, ERC2771Context) view virtual returns (bytes calldata) { + return ERC2771Context._msgData(); + } +} diff --git a/test/metatx/ERC2771Context.test.js b/test/metatx/ERC2771Context.test.js new file mode 100644 index 00000000000..50a94c25cc4 --- /dev/null +++ b/test/metatx/ERC2771Context.test.js @@ -0,0 +1,113 @@ +const ethSigUtil = require('eth-sig-util'); +const Wallet = require('ethereumjs-wallet').default; +const { EIP712Domain } = require('../helpers/eip712'); + +const { expectEvent } = require('@openzeppelin/test-helpers'); +const { expect } = require('chai'); + +const ERC2771ContextMock = artifacts.require('ERC2771ContextMock'); +const MinimalForwarder = artifacts.require('MinimalForwarder'); +const ContextMockCaller = artifacts.require('ContextMockCaller'); + +const { shouldBehaveLikeRegularContext } = require('../utils/Context.behavior'); + +const name = 'MinimalForwarder'; +const version = '0.0.1'; + +contract('ERC2771Context', function (accounts) { + beforeEach(async function () { + this.forwarder = await MinimalForwarder.new(); + this.recipient = await ERC2771ContextMock.new(this.forwarder.address); + + this.domain = { + name, + version, + chainId: await web3.eth.getChainId(), + verifyingContract: this.forwarder.address, + }; + this.types = { + EIP712Domain, + ForwardRequest: [ + { name: 'from', type: 'address' }, + { name: 'to', type: 'address' }, + { name: 'value', type: 'uint256' }, + { name: 'gas', type: 'uint256' }, + { name: 'nonce', type: 'uint256' }, + { name: 'data', type: 'bytes' }, + ], + }; + }); + + it('recognize trusted forwarder', async function () { + expect(await this.recipient.isTrustedForwarder(this.forwarder.address)); + }); + + context('when called directly', function () { + beforeEach(async function () { + this.context = this.recipient; // The Context behavior expects the contract in this.context + this.caller = await ContextMockCaller.new(); + }); + + shouldBehaveLikeRegularContext(...accounts); + }); + + context('when receiving a relayed call', function () { + beforeEach(async function () { + this.wallet = Wallet.generate(); + this.sender = web3.utils.toChecksumAddress(this.wallet.getAddressString()); + this.data = { + types: this.types, + domain: this.domain, + primaryType: 'ForwardRequest', + }; + }); + + describe('msgSender', function () { + it('returns the relayed transaction original sender', async function () { + const data = this.recipient.contract.methods.msgSender().encodeABI(); + + const req = { + from: this.sender, + to: this.recipient.address, + value: '0', + gas: '100000', + nonce: (await this.forwarder.getNonce(this.sender)).toString(), + data, + }; + + const sign = ethSigUtil.signTypedMessage(this.wallet.getPrivateKey(), { data: { ...this.data, message: req } }); + + // rejected by lint :/ + // expect(await this.forwarder.verify(req, sign)).to.be.true; + + const { tx } = await this.forwarder.execute(req, sign); + await expectEvent.inTransaction(tx, ERC2771ContextMock, 'Sender', { sender: this.sender }); + }); + }); + + describe('msgData', function () { + it('returns the relayed transaction original data', async function () { + const integerValue = '42'; + const stringValue = 'OpenZeppelin'; + const data = this.recipient.contract.methods.msgData(integerValue, stringValue).encodeABI(); + + const req = { + from: this.sender, + to: this.recipient.address, + value: '0', + gas: '100000', + nonce: (await this.forwarder.getNonce(this.sender)).toString(), + data, + }; + + const sign = ethSigUtil.signTypedMessage(this.wallet.getPrivateKey(), { data: { ...this.data, message: req } }); + + // rejected by lint :/ + // expect(await this.forwarder.verify(req, sign)).to.be.true; + + const { tx } = await this.forwarder.execute(req, sign); + await expectEvent.inTransaction(tx, ERC2771ContextMock, 'Data', { data, integerValue, stringValue }); + }); + }); + }); +}); diff --git a/test/metatx/MinimalForwarder.test.js b/test/metatx/MinimalForwarder.test.js new file mode 100644 index 00000000000..6151f97ecf7 --- /dev/null +++ b/test/metatx/MinimalForwarder.test.js @@ -0,0 +1,166 @@ +const ethSigUtil = require('eth-sig-util'); +const Wallet = require('ethereumjs-wallet').default; +const { EIP712Domain } = require('../helpers/eip712'); + +const { expectRevert, constants } = require('@openzeppelin/test-helpers'); +const { expect } = require('chai'); + +const MinimalForwarder = artifacts.require('MinimalForwarder'); + +const name = 'MinimalForwarder'; +const version = '0.0.1'; + +contract('MinimalForwarder', function (accounts) { + beforeEach(async function () { + this.forwarder = await MinimalForwarder.new(); + this.domain = { + name, + version, + chainId: await web3.eth.getChainId(), + verifyingContract: this.forwarder.address, + }; + this.types = { + EIP712Domain, + ForwardRequest: [ + { name: 'from', type: 'address' }, + { name: 'to', type: 'address' }, + { name: 'value', type: 'uint256' }, + { name: 'gas', type: 'uint256' }, + { name: 'nonce', type: 'uint256' }, + { name: 'data', type: 'bytes' }, + ], + }; + }); + + context('with message', function () { + beforeEach(async function () { + this.wallet = Wallet.generate(); + this.sender = web3.utils.toChecksumAddress(this.wallet.getAddressString()); + this.req = { + from: this.sender, + to: constants.ZERO_ADDRESS, + value: '0', + gas: '100000', + nonce: Number(await this.forwarder.getNonce(this.sender)), + data: '0x', + }; + this.sign = ethSigUtil.signTypedMessage( + this.wallet.getPrivateKey(), + { + data: { + types: this.types, + domain: this.domain, + primaryType: 'ForwardRequest', + message: this.req, + }, + }, + ); + }); + + context('verify', function () { + context('valid signature', function () { + beforeEach(async function () { + expect(await this.forwarder.getNonce(this.req.from)) + .to.be.bignumber.equal(web3.utils.toBN(this.req.nonce)); + }); + + it('success', async function () { + expect(await this.forwarder.verify(this.req, this.sign)).to.be.equal(true); + }); + + afterEach(async function () { + expect(await this.forwarder.getNonce(this.req.from)) + .to.be.bignumber.equal(web3.utils.toBN(this.req.nonce)); + }); + }); + + context('invalid signature', function () { + it('tampered from', async function () { + expect(await this.forwarder.verify({ ...this.req, from: accounts[0] }, this.sign)) + .to.be.equal(false); + }); + it('tampered to', async function () { + expect(await this.forwarder.verify({ ...this.req, to: accounts[0] }, this.sign)) + .to.be.equal(false); + }); + it('tampered value', async function () { + expect(await this.forwarder.verify({ ...this.req, value: web3.utils.toWei('1') }, this.sign)) + .to.be.equal(false); + }); + it('tampered nonce', async function () { + expect(await this.forwarder.verify({ ...this.req, nonce: this.req.nonce + 1 }, this.sign)) + .to.be.equal(false); + }); + it('tampered data', async function () { + expect(await this.forwarder.verify({ ...this.req, data: '0x1742' }, this.sign)) + .to.be.equal(false); + }); + it('tampered signature', async function () { + const tamperedsign = web3.utils.hexToBytes(this.sign); + tamperedsign[42] ^= 0xff; + expect(await this.forwarder.verify(this.req, web3.utils.bytesToHex(tamperedsign))) + .to.be.equal(false); + }); + }); + }); + + context('execute', function () { + context('valid signature', function () { + beforeEach(async function () { + expect(await this.forwarder.getNonce(this.req.from)) + .to.be.bignumber.equal(web3.utils.toBN(this.req.nonce)); + }); + + it('success', async function () { + await this.forwarder.execute(this.req, this.sign); // expect to not revert + }); + + afterEach(async function () { + expect(await this.forwarder.getNonce(this.req.from)) + .to.be.bignumber.equal(web3.utils.toBN(this.req.nonce + 1)); + }); + }); + + context('invalid signature', function () { + it('tampered from', async function () { + await expectRevert( + this.forwarder.execute({ ...this.req, from: accounts[0] }, this.sign), + 'MinimalForwarder: signature does not match request', + ); + }); + it('tampered to', async function () { + await expectRevert( + this.forwarder.execute({ ...this.req, to: accounts[0] }, this.sign), + 'MinimalForwarder: signature does not match request', + ); + }); + it('tampered value', async function () { + await expectRevert( + this.forwarder.execute({ ...this.req, value: web3.utils.toWei('1') }, this.sign), + 'MinimalForwarder: signature does not match request', + ); + }); + it('tampered nonce', async function () { + await expectRevert( + this.forwarder.execute({ ...this.req, nonce: this.req.nonce + 1 }, this.sign), + 'MinimalForwarder: signature does not match request', + ); + }); + it('tampered data', async function () { + await expectRevert( + this.forwarder.execute({ ...this.req, data: '0x1742' }, this.sign), + 'MinimalForwarder: signature does not match request', + ); + }); + it('tampered signature', async function () { + const tamperedsign = web3.utils.hexToBytes(this.sign); + tamperedsign[42] ^= 0xff; + await expectRevert( + this.forwarder.execute(this.req, web3.utils.bytesToHex(tamperedsign)), + 'MinimalForwarder: signature does not match request', + ); + }); + }); + }); + }); +});