diff --git a/contracts/ERC677BridgeToken.sol b/contracts/ERC677BridgeToken.sol index a8d063e4e..44234b0e4 100644 --- a/contracts/ERC677BridgeToken.sol +++ b/contracts/ERC677BridgeToken.sol @@ -46,7 +46,7 @@ contract ERC677BridgeToken is IBurnableMintableERC677Token, DetailedERC20, Burna } function getTokenInterfacesVersion() external pure returns (uint64 major, uint64 minor, uint64 patch) { - return (2, 4, 0); + return (2, 5, 0); } function superTransfer(address _to, uint256 _value) internal returns (bool) { diff --git a/contracts/PermittableToken.sol b/contracts/PermittableToken.sol index 805e244ec..2ca2be870 100644 --- a/contracts/PermittableToken.sol +++ b/contracts/PermittableToken.sol @@ -7,8 +7,10 @@ contract PermittableToken is ERC677BridgeToken { // EIP712 niceties bytes32 public DOMAIN_SEPARATOR; - // bytes32 public constant PERMIT_TYPEHASH = keccak256("Permit(address holder,address spender,uint256 nonce,uint256 expiry,bool allowed)"); - bytes32 public constant PERMIT_TYPEHASH = 0xea2aa0a1be11a07ed86d755c93467f4f82362b452371d1ba94d1715123511acb; + // bytes32 public constant PERMIT_TYPEHASH_LEGACY = keccak256("Permit(address holder,address spender,uint256 nonce,uint256 expiry,bool allowed)"); + bytes32 public constant PERMIT_TYPEHASH_LEGACY = 0xea2aa0a1be11a07ed86d755c93467f4f82362b452371d1ba94d1715123511acb; + // bytes32 public constant PERMIT_TYPEHASH = keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"); + bytes32 public constant PERMIT_TYPEHASH = 0x6e71edae12b1b97f4d1f60370fef10105fa2faae0126114a169c64845d6126c9; mapping(address => uint256) public nonces; mapping(address => mapping(address => uint256)) public expirations; @@ -56,7 +58,7 @@ contract PermittableToken is ERC677BridgeToken { } else { // If allowance is unlimited by `permit`, `approve`, or `increaseAllowance` // function, don't adjust it. But the expiration date must be empty or in the future - require(expirations[_sender][msg.sender] == 0 || expirations[_sender][msg.sender] >= _now()); + require(expirations[_sender][msg.sender] == 0 || expirations[_sender][msg.sender] >= now); } } else { // If `_sender` is `msg.sender`, @@ -71,20 +73,16 @@ contract PermittableToken is ERC677BridgeToken { /// @param _to The address which will spend the funds. /// @param _value The amount of tokens to be spent. function approve(address _to, uint256 _value) public returns (bool result) { - result = super.approve(_to, _value); - if (_value == uint256(-1)) { - delete expirations[msg.sender][_to]; - } + _approveAndResetExpirations(msg.sender, _to, _value); + return true; } /// @dev Atomically increases the allowance granted to spender by the caller. /// @param _to The address which will spend the funds. /// @param _addedValue The amount of tokens to increase the allowance by. function increaseAllowance(address _to, uint256 _addedValue) public returns (bool result) { - result = super.increaseAllowance(_to, _addedValue); - if (allowed[msg.sender][_to] == uint256(-1)) { - delete expirations[msg.sender][_to]; - } + _approveAndResetExpirations(msg.sender, _to, allowed[msg.sender][_to].add(_addedValue)); + return true; } /// @dev An alias for `transfer` function. @@ -134,33 +132,107 @@ contract PermittableToken is ERC677BridgeToken { bytes32 _r, bytes32 _s ) external { - require(_holder != address(0)); - require(_spender != address(0)); - require(_expiry == 0 || _now() <= _expiry); - - require(_v == 27 || _v == 28); - require(uint256(_s) <= 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0); - - bytes32 digest = keccak256( - abi.encodePacked( - "\x19\x01", - DOMAIN_SEPARATOR, - keccak256(abi.encode(PERMIT_TYPEHASH, _holder, _spender, _nonce, _expiry, _allowed)) - ) - ); + require(_expiry == 0 || now <= _expiry); - require(_holder == ecrecover(digest, _v, _r, _s)); + bytes32 digest = _digest(abi.encode(PERMIT_TYPEHASH_LEGACY, _holder, _spender, _nonce, _expiry, _allowed)); + + require(_holder == _recover(digest, _v, _r, _s)); require(_nonce == nonces[_holder]++); uint256 amount = _allowed ? uint256(-1) : 0; - allowed[_holder][_spender] = amount; expirations[_holder][_spender] = _allowed ? _expiry : 0; - emit Approval(_holder, _spender, amount); + _approve(_holder, _spender, amount); } - function _now() internal view returns (uint256) { - return now; + /** @dev Allows to spend holder's unlimited amount by the specified spender according to EIP2612. + * The function can be called by anyone, but requires having allowance parameters + * signed by the holder according to EIP712. + * @param _holder The holder's address. + * @param _spender The spender's address. + * @param _value Allowance value to set as a result of the call. + * @param _deadline The deadline timestamp to call the permit function. Must be a timestamp in the future. + * Note that timestamps are not precise, malicious miner/validator can manipulate them to some extend. + * Assume that there can be a 900 seconds time delta between the desired timestamp and the actual expiration. + * @param _v A final byte of signature (ECDSA component). + * @param _r The first 32 bytes of signature (ECDSA component). + * @param _s The second 32 bytes of signature (ECDSA component). + */ + function permit( + address _holder, + address _spender, + uint256 _value, + uint256 _deadline, + uint8 _v, + bytes32 _r, + bytes32 _s + ) external { + require(now <= _deadline); + + uint256 nonce = nonces[_holder]++; + bytes32 digest = _digest(abi.encode(PERMIT_TYPEHASH, _holder, _spender, _value, nonce, _deadline)); + + require(_holder == _recover(digest, _v, _r, _s)); + + _approveAndResetExpirations(_holder, _spender, _value); + } + + /** + * @dev Sets a new allowance value for the given owner and spender addresses. + * Resets expiration timestamp in case of unlimited approval. + * @param _owner address tokens holder. + * @param _spender address of tokens spender. + * @param _amount amount of approved tokens. + */ + function _approveAndResetExpirations(address _owner, address _spender, uint256 _amount) internal { + _approve(_owner, _spender, _amount); + + // it is not necessary to reset _expirations in other cases, since it is only used together with infinite allowance + if (_amount == uint256(-1)) { + delete expirations[_owner][_spender]; + } + } + + /** + * @dev Internal function for issuing an allowance. + * @param _owner address of the tokens owner. + * @param _spender address of the approved tokens spender. + * @param _amount amount of the approved tokens. + */ + function _approve(address _owner, address _spender, uint256 _amount) internal { + require(_owner != address(0), "ERC20: approve from the zero address"); + require(_spender != address(0), "ERC20: approve to the zero address"); + + allowed[_owner][_spender] = _amount; + emit Approval(_owner, _spender, _amount); + } + + /** + * @dev Calculates the message digest for encoded EIP712 typed struct. + * @param _typedStruct encoded payload. + */ + function _digest(bytes memory _typedStruct) internal view returns (bytes32) { + return keccak256(abi.encodePacked("\x19\x01", DOMAIN_SEPARATOR, keccak256(_typedStruct))); + } + + /** + * @dev Derives the signer address for the given message digest and ECDSA signature params. + * @param _digest signed message digest. + * @param _v a final byte of signature (ECDSA component). + * @param _r the first 32 bytes of the signature (ECDSA component). + * @param _s the second 32 bytes of the signature (ECDSA component). + */ + function _recover(bytes32 _digest, uint8 _v, bytes32 _r, bytes32 _s) internal pure returns (address) { + require(_v == 27 || _v == 28, "ECDSA: invalid signature 'v' value"); + require( + uint256(_s) <= 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0, + "ECDSA: invalid signature 's' value" + ); + + address signer = ecrecover(_digest, _v, _r, _s); + require(signer != address(0), "ECDSA: invalid signature"); + + return signer; } } diff --git a/contracts/mocks/ERC677BridgeTokenRewardableMock.sol b/contracts/mocks/ERC677BridgeTokenRewardableMock.sol index d05742ede..e24f2cf86 100644 --- a/contracts/mocks/ERC677BridgeTokenRewardableMock.sol +++ b/contracts/mocks/ERC677BridgeTokenRewardableMock.sol @@ -19,13 +19,4 @@ contract ERC677BridgeTokenRewardableMock is ERC677BridgeTokenRewardable { function setStakingContractMock(address _stakingContract) public { stakingContract = _stakingContract; } - - function setNow(uint256 _timestamp) public { - _blockTimestamp = _timestamp; - } - - function _now() internal view returns (uint256) { - return _blockTimestamp != 0 ? _blockTimestamp : now; - } - } diff --git a/contracts/mocks/PermittableTokenMock.sol b/contracts/mocks/PermittableTokenMock.sol deleted file mode 100644 index dfe0d90e0..000000000 --- a/contracts/mocks/PermittableTokenMock.sol +++ /dev/null @@ -1,23 +0,0 @@ -pragma solidity 0.4.24; - -import "../PermittableToken.sol"; - -contract PermittableTokenMock is PermittableToken { - uint256 private _blockTimestamp; - - constructor(string _name, string _symbol, uint8 _decimals, uint256 _chainId) - public - PermittableToken(_name, _symbol, _decimals, _chainId) - { - // solhint-disable-previous-line no-empty-blocks - } - - function setNow(uint256 _timestamp) public { - _blockTimestamp = _timestamp; - } - - function _now() internal view returns (uint256) { - return _blockTimestamp != 0 ? _blockTimestamp : now; - } - -} diff --git a/package-lock.json b/package-lock.json index 6b9c45c94..e4dff6bf7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13522,32 +13522,6 @@ "resolved": "https://registry.npmjs.org/ethereum-protocol/-/ethereum-protocol-1.0.1.tgz", "integrity": "sha512-3KLX1mHuEsBW0dKG+c6EOJS1NBNqdCICvZW9sInmZTt5aY0oxmHVggYRE0lJu1tcnMD1K+AKHdLi6U43Awm1Vg==" }, - "ethereumjs-abi": { - "version": "0.6.8", - "resolved": "git+https://github.com/ethereumjs/ethereumjs-abi.git#1cfbb13862f90f0b391d8a699544d5fe4dfb8c7b", - "dev": true, - "requires": { - "bn.js": "^4.11.8", - "ethereumjs-util": "^6.0.0" - }, - "dependencies": { - "ethereumjs-util": { - "version": "6.1.0", - "resolved": "https://registry.npmjs.org/ethereumjs-util/-/ethereumjs-util-6.1.0.tgz", - "integrity": "sha512-URESKMFbDeJxnAxPppnk2fN6Y3BIatn9fwn76Lm8bQlt+s52TpG8dN9M66MLPuRAiAOIqL3dfwqWJf0sd0fL0Q==", - "dev": true, - "requires": { - "bn.js": "^4.11.0", - "create-hash": "^1.1.2", - "ethjs-util": "0.1.6", - "keccak": "^1.0.2", - "rlp": "^2.0.0", - "safe-buffer": "^5.1.1", - "secp256k1": "^3.0.1" - } - } - } - }, "ethereumjs-account": { "version": "2.0.5", "resolved": "https://registry.npmjs.org/ethereumjs-account/-/ethereumjs-account-2.0.5.tgz", diff --git a/package.json b/package.json index d0769d013..a24d45ab1 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,6 @@ "eslint-plugin-node": "^10.0.0", "eslint-plugin-prettier": "^3.0.1", "eth-gas-reporter": "^0.2.11", - "ethereumjs-abi": "0.6.8", "ethereumjs-util": "5.2.0", "nodemon": "^1.17.3", "prettier": "^1.18.2", diff --git a/test/helpers/eip712.sign.permit.js b/test/helpers/eip712.sign.permit.js deleted file mode 100644 index 6d46b49bc..000000000 --- a/test/helpers/eip712.sign.permit.js +++ /dev/null @@ -1,119 +0,0 @@ -const abi = require('ethereumjs-abi') -const ethUtil = require('ethereumjs-util') - -const domainSchema = [ - { name: 'name', type: 'string' }, - { name: 'version', type: 'string' }, - { name: 'chainId', type: 'uint256' }, - { name: 'verifyingContract', type: 'address' } -] - -const permitSchema = [ - { name: 'holder', type: 'address' }, - { name: 'spender', type: 'address' }, - { name: 'nonce', type: 'uint256' }, - { name: 'expiry', type: 'uint256' }, - { name: 'allowed', type: 'bool' } -] - -const types = { - EIP712Domain: domainSchema, - Permit: permitSchema -} - -function dependencies(primaryType, found = []) { - if (found.includes(primaryType)) { - return found - } - if (types[primaryType] === undefined) { - return found - } - found.push(primaryType) - for (const field of types[primaryType]) { - for (const dep of dependencies(field.type, found)) { - if (!found.includes(dep)) { - found.push(dep) - } - } - } - return found -} - -function encodeData(primaryType, data) { - const encTypes = [] - const encValues = [] - - // Add typehash - encTypes.push('bytes32') - encValues.push(typeHash(primaryType)) - - // Add field contents - for (const field of types[primaryType]) { - let value = data[field.name] - if (field.type === 'string' || field.type === 'bytes') { - encTypes.push('bytes32') - value = ethUtil.keccak256(value) - encValues.push(value) - } else if (types[field.type] !== undefined) { - encTypes.push('bytes32') - value = ethUtil.keccak256(encodeData(field.type, value)) - encValues.push(value) - } else if (field.type.lastIndexOf(']') === field.type.length - 1) { - throw Error('TODO: Arrays currently unimplemented in encodeData') - } else { - encTypes.push(field.type) - encValues.push(value) - } - } - - return abi.rawEncode(encTypes, encValues) -} - -function encodeType(primaryType) { - // Get dependencies primary first, then alphabetical - let deps = dependencies(primaryType) - deps = deps.filter(t => t !== primaryType) - deps = [primaryType].concat(deps.sort()) - - // Format as a string with fields - let result = '' - for (const type of deps) { - result += `${type}(${types[type].map(({ name, type }) => `${type} ${name}`).join(',')})` - } - return result -} - -function signHash(typedData) { - return ethUtil.keccak256( - Buffer.concat([ - Buffer.from('1901', 'hex'), - structHash('EIP712Domain', typedData.domain), - structHash(typedData.primaryType, typedData.message) - ]) - ) -} - -function structHash(primaryType, data) { - return ethUtil.keccak256(encodeData(primaryType, data)) -} - -function typeHash(primaryType) { - return ethUtil.keccak256(encodeType(primaryType)) -} - -function sign(domain, message, privateKey) { - const typedData = { - types, - primaryType: 'Permit', - domain, - message - } - const sig = ethUtil.ecsign(signHash(typedData), ethUtil.toBuffer(privateKey)) - return { - v: sig.v, - r: ethUtil.bufferToHex(sig.r), - s: ethUtil.bufferToHex(sig.s) - } -} - -module.exports = sign diff --git a/test/poa20_test.js b/test/poa20_test.js index 88d58deef..8d237971c 100644 --- a/test/poa20_test.js +++ b/test/poa20_test.js @@ -8,14 +8,33 @@ const ForeignBridge = artifacts.require('ForeignBridgeErcToNative.sol') const BridgeValidators = artifacts.require('BridgeValidators.sol') const { expect } = require('chai') -const ethUtil = require('ethereumjs-util') +const { fromRpcSig } = require('ethereumjs-util') const { ERROR_MSG, ERROR_MSG_OPCODE, ZERO_ADDRESS, BN, toBN } = require('./setup') const { ether, expectEventInLogs } = require('./helpers/helpers') -const permitSign = require('./helpers/eip712.sign.permit') + +async function ethSignTypedData(from, data) { + const result = await new Promise((res, rej) => + web3.currentProvider.send( + { jsonrpc: '2.0', method: 'eth_signTypedData', params: [from, data], id: 1 }, + (err, sig) => (err ? rej(err) : res(sig)) + ) + ) + const sig = fromRpcSig(result.result) + return [sig.v, sig.r, sig.s] +} + +async function evmIncreaseTime(delta) { + return new Promise((res, rej) => + web3.currentProvider.send({ jsonrpc: '2.0', method: 'evm_increaseTime', params: [delta], id: 1 }, (err, sig) => + err ? rej(err) : res(sig) + ) + ) +} const minPerTx = ether('0.01') const oneEther = ether('1') +const twoEthers = ether('2') const ZERO = new BN(0) function testERC677BridgeToken(accounts, rewardable, permittable, createToken) { @@ -40,7 +59,7 @@ function testERC677BridgeToken(accounts, rewardable, permittable, createToken) { beforeEach(async () => { const args = ['POA ERC20 Foundation', 'POA20', 18] if (permittable) { - args.push(100) + args.push(1337) } token = await createToken(args) }) @@ -507,340 +526,243 @@ function testERC677BridgeToken(accounts, rewardable, permittable, createToken) { }) if (permittable) { describe('permit', () => { - const privateKey = '0x2bdd21761a483f71054e14f5b827213567971c676928d9a1808cbfa4b7501210' - const infinite = new BN('ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff', 16) - let holder - let spender - let nonce - let expiry - let allowed - - beforeEach(async () => { - holder = '0x040b798028e9abded00Bfc65e7CF01484013db17' - spender = accounts[9] - nonce = await token.nonces.call(holder) - expiry = 0 - allowed = true - - holder.toLowerCase().should.be.equal(ethUtil.bufferToHex(ethUtil.privateToAddress(privateKey)).toLowerCase()) // make sure privateKey is holder's key - - // Mint some extra tokens for the `holder` - await token.mint(holder, '10000', { from: owner }).should.be.fulfilled - ;(await token.balanceOf.call(holder)).should.be.bignumber.equal(new BN('10000')) - web3.eth.accounts.wallet.add(privateKey) - await web3.eth.sendTransaction({ from: owner, to: holder, value: oneEther }) - }) - it('should permit', async () => { - // Holder signs the `permit` params with their privateKey - const signature = permitSign( - { - name: await token.name.call(), - version: await token.version.call(), - chainId: '100', - verifyingContract: token.address - }, - { - holder, - spender, - nonce, - expiry, - allowed - }, - privateKey - ) - - ;(await token.allowance.call(holder, spender)).should.be.bignumber.equal(new BN('0')) - - // An arbitrary address calls the `permit` function - const { logs } = await token.permit( - holder, - spender, + const EIP712Domain = [ + { name: 'name', type: 'string' }, + { name: 'version', type: 'string' }, + { name: 'chainId', type: 'uint256' }, + { name: 'verifyingContract', type: 'address' } + ] + let domain + let permit + let permitLegacy + + const makeLegacyMsg = (nonce, expiry, allowed) => ({ + types: { + EIP712Domain, + Permit: [ + { name: 'holder', type: 'address' }, + { name: 'spender', type: 'address' }, + { name: 'nonce', type: 'uint256' }, + { name: 'expiry', type: 'uint256' }, + { name: 'allowed', type: 'bool' } + ] + }, + primaryType: 'Permit', + domain, + message: { + holder: owner, + spender: user, nonce, expiry, - allowed, - signature.v, - signature.r, - signature.s - ).should.be.fulfilled - - logs[0].event.should.be.equal('Approval') - logs[0].args.owner.should.be.equal(holder) - logs[0].args.spender.should.be.equal(spender) - logs[0].args.value.should.be.bignumber.equal(infinite) - - // Now allowance is infinite - ;(await token.allowance.call(holder, spender)).should.be.bignumber.equal(infinite) - - // The caller of `permit` can't spend holder's funds - await token.transferFrom(holder, accounts[9], '10000').should.be.rejected - ;(await token.balanceOf.call(holder)).should.be.bignumber.equal(new BN('10000')) - - // Spender can transfer all holder's funds - await token.transferFrom(holder, accounts[9], '10000', { from: spender }).should.be.fulfilled - ;(await token.balanceOf.call(holder)).should.be.bignumber.equal(new BN('0')) - ;(await token.balanceOf.call(accounts[9])).should.be.bignumber.equal(new BN('10000')) - ;(await token.nonces.call(holder)).should.be.bignumber.equal(nonce.add(new BN('1'))) - - // The allowance is still infinite after transfer - ;(await token.allowance.call(holder, spender)).should.be.bignumber.equal(infinite) - }) - it('should fail when invalid expiry', async () => { - expiry = 900 - - const signature = permitSign( - { - name: await token.name.call(), - version: await token.version.call(), - chainId: '100', - verifyingContract: token.address - }, - { - holder, - spender, - nonce, - expiry, - allowed - }, - privateKey - ) - - await token.setNow(1000).should.be.fulfilled - await token - .permit(holder, spender, nonce, expiry, allowed, signature.v, signature.r, signature.s) - .should.be.rejectedWith(ERROR_MSG) - - await token.setNow(800).should.be.fulfilled - await token.permit(holder, spender, nonce, expiry, allowed, signature.v, signature.r, signature.s).should.be - .fulfilled - }) - it('should consider expiry', async () => { - expiry = 900 - - const signature = permitSign( - { - name: await token.name.call(), - version: await token.version.call(), - chainId: '100', - verifyingContract: token.address - }, - { - holder, - spender, - nonce, - expiry, - allowed - }, - privateKey - ) - - await token.setNow(800).should.be.fulfilled - await token.permit(holder, spender, nonce, expiry, allowed, signature.v, signature.r, signature.s).should.be - .fulfilled - ;(await token.expirations.call(holder, spender)).should.be.bignumber.equal(new BN(expiry)) - - // Spender can transfer holder's funds - await token.setNow(899).should.be.fulfilled - await token.transferFrom(holder, accounts[9], '6000', { from: spender }).should.be.fulfilled - ;(await token.balanceOf.call(holder)).should.be.bignumber.equal(new BN('4000')) - ;(await token.balanceOf.call(accounts[9])).should.be.bignumber.equal(new BN('6000')) - - // Spender can't transfer the remaining holder's funds because of expiry - await token.setNow(901).should.be.fulfilled - await token.transferFrom(holder, accounts[9], '4000', { from: spender }).should.be.rejectedWith(ERROR_MSG) - }) - it('should reset expiration on subsequent approve', async () => { - expiry = 900 - const signature = permitSign( - { - name: await token.name.call(), - version: await token.version.call(), - chainId: '100', - verifyingContract: token.address - }, - { - holder, - spender, - nonce, - expiry, - allowed - }, - privateKey - ) - await token.setNow(800).should.be.fulfilled - await token.permit(holder, spender, nonce, expiry, allowed, signature.v, signature.r, signature.s).should.be - .fulfilled - ;(await token.expirations.call(holder, spender)).should.be.bignumber.equal(new BN(expiry)) - const data = await token.contract.methods.approve(spender, infinite).encodeABI() - await web3.eth.sendTransaction({ from: holder, to: token.address, data, gas: 100000 }).should.be.fulfilled - ;(await token.expirations.call(holder, spender)).should.be.bignumber.equal(ZERO) + allowed + } }) - it('should reset expiration on subsequent increaseAllowance', async () => { - expiry = 900 - const signature = permitSign( - { - name: await token.name.call(), - version: await token.version.call(), - chainId: '100', - verifyingContract: token.address - }, - { - holder, - spender, - nonce, - expiry, - allowed - }, - privateKey - ) - await token.setNow(800).should.be.fulfilled - await token.permit(holder, spender, nonce, expiry, allowed, signature.v, signature.r, signature.s).should.be - .fulfilled - ;(await token.expirations.call(holder, spender)).should.be.bignumber.equal(new BN(expiry)) - let data = await token.contract.methods.approve(spender, infinite.sub(toBN(1))).encodeABI() - await web3.eth.sendTransaction({ from: holder, to: token.address, data, gas: 100000 }).should.be.fulfilled - ;(await token.expirations.call(holder, spender)).should.be.bignumber.equal(new BN(expiry)) - data = await token.contract.methods.increaseAllowance(spender, 1).encodeABI() - await web3.eth.sendTransaction({ from: holder, to: token.address, data, gas: 100000 }).should.be.fulfilled - ;(await token.expirations.call(holder, spender)).should.be.bignumber.equal(ZERO) + const makeMsg = (nonce, deadline, value) => ({ + types: { + EIP712Domain, + Permit: [ + { name: 'owner', type: 'address' }, + { name: 'spender', type: 'address' }, + { name: 'value', type: 'uint256' }, + { name: 'nonce', type: 'uint256' }, + { name: 'deadline', type: 'uint256' } + ] + }, + primaryType: 'Permit', + domain, + message: { + owner, + spender: user, + value: value || oneEther.toString(), + nonce, + deadline + } }) - it('should disallow unlimited allowance', async () => { - expiry = 900 - await token.setNow(800).should.be.fulfilled - - let signature = permitSign( - { - name: await token.name.call(), - version: await token.version.call(), - chainId: '100', - verifyingContract: token.address - }, - { - holder, - spender, - nonce, - expiry, - allowed - }, - privateKey - ) - - await token.permit(holder, spender, nonce, expiry, allowed, signature.v, signature.r, signature.s).should.be - .fulfilled - ;(await token.allowance.call(holder, spender)).should.be.bignumber.equal(infinite) - ;(await token.expirations.call(holder, spender)).should.be.bignumber.equal(new BN(expiry)) - - // Spender can transfer holder's funds - await token.transferFrom(holder, accounts[9], '6000', { from: spender }).should.be.fulfilled - ;(await token.balanceOf.call(holder)).should.be.bignumber.equal(new BN('4000')) - ;(await token.balanceOf.call(accounts[9])).should.be.bignumber.equal(new BN('6000')) - - nonce = nonce - 0 + 1 - allowed = false - - signature = permitSign( - { - name: await token.name.call(), - version: await token.version.call(), - chainId: '100', - verifyingContract: token.address - }, - { - holder, - spender, - nonce, - expiry, - allowed - }, - privateKey - ) - - await token.permit(holder, spender, nonce, expiry, allowed, signature.v, signature.r, signature.s).should.be - .fulfilled - ;(await token.allowance.call(holder, spender)).should.be.bignumber.equal(new BN('0')) - ;(await token.expirations.call(holder, spender)).should.be.bignumber.equal(new BN('0')) - - // Spender can't transfer the remaining holder's funds because of zero allowance - await token.transferFrom(holder, accounts[9], '4000', { from: spender }).should.be.rejected + + beforeEach(() => { + domain = { + name: 'POA ERC20 Foundation', + version: '1', + chainId: 1337, + verifyingContract: token.address + } + permit = token.methods['permit(address,address,uint256,uint256,uint8,bytes32,bytes32)'] + permitLegacy = token.methods['permit(address,address,uint256,uint256,bool,uint8,bytes32,bytes32)'] }) - it('should fail when invalid signature or parameters', async () => { - let signature = permitSign( - { - name: await token.name.call(), - version: await token.version.call(), - chainId: '100', - verifyingContract: token.address - }, - { - holder, - spender, - nonce, - expiry, - allowed - }, - privateKey - ) - - allowed = !allowed - await token - .permit(holder, spender, nonce, expiry, allowed, signature.v, signature.r, signature.s) - .should.be.rejectedWith(ERROR_MSG) + describe('legacy permit', () => { + const INFINITY = new BN('ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff', 16) - allowed = !allowed + it('should accept signed message', async () => { + const expiry = 10000000000 + const sig1 = await ethSignTypedData(owner, makeLegacyMsg(0, 100, true)) + const sig2 = await ethSignTypedData(owner, makeLegacyMsg(1, expiry, true)) + const sig3 = await ethSignTypedData(owner, makeLegacyMsg(0, expiry, true)) - await token - .permit( - holder, - spender, - nonce, - expiry, - allowed, - signature.v, - signature.s, // here should be `signature.r` in a correct case - signature.r // here should be `signature.s` in a correct case - ) - .should.be.rejectedWith(ERROR_MSG) + await permitLegacy(owner, user, 0, 100, true, ...sig1).should.be.rejected // too small deadline + await permitLegacy(owner, user, 0, expiry, true, ...sig2).should.be.rejected // invalid nonce + await permitLegacy(owner, user, 1, expiry, true, ...sig2).should.be.rejected // not current nonce + await permitLegacy(owner, user, 0, expiry, true, ...sig3).should.be.fulfilled // valid for nonce == 0 + await permitLegacy(owner, user, 0, expiry, true, ...sig3).should.be.rejected // invalid duplicate, invalid nonce + await permitLegacy(owner, user, 1, expiry, true, ...sig3).should.be.rejected // invalid nonce + await permitLegacy(user, user, 1, expiry, true, ...sig2).should.be.rejected // invalid sender + await permitLegacy(owner, owner, 1, expiry, true, ...sig2).should.be.rejected // invalid receiver + await permitLegacy(owner, user, 1, expiry + 1, true, ...sig2).should.be.rejected // invalid expiry + await permitLegacy(owner, user, 1, expiry, false, ...sig2).should.be.rejected // invalid allowed + await permitLegacy(owner, user, 1, expiry, true, ...sig2).should.be.fulfilled // valid for nonce == 1 + await permitLegacy(owner, user, 1, expiry, true, ...sig2).should.be.rejected // invalid duplicate, invalid nonce - signature = permitSign( - { - name: await token.name.call(), - version: await token.version.call(), - chainId: '100', - verifyingContract: token.address - }, - { - holder, - spender, - nonce: nonce - 0 + 1, - expiry, - allowed - }, - privateKey - ) + expect(await token.allowance(owner, user)).to.be.bignumber.equal(INFINITY) + expect(await token.nonces(owner)).to.be.bignumber.equal('2') + expect(await token.expirations(owner, user)).to.be.bignumber.equal(toBN(expiry)) + }) - await token - .permit(holder, spender, nonce - 0 + 1, expiry, allowed, signature.v, signature.r, signature.s) - .should.be.rejectedWith(ERROR_MSG) + it('should cancel expirations on infinite approval from approve()', async () => { + const expiry = 10000000000 + const sig = await ethSignTypedData(owner, makeLegacyMsg(0, expiry, true)) + await permitLegacy(owner, user, 0, expiry, true, ...sig).should.be.fulfilled + + expect(await token.allowance(owner, user)).to.be.bignumber.equal(INFINITY) + expect(await token.expirations(owner, user)).to.be.bignumber.equal(toBN(expiry)) + + await token.approve(user, 1).should.be.fulfilled + + expect(await token.allowance(owner, user)).to.be.bignumber.equal('1') + expect(await token.expirations(owner, user)).to.be.bignumber.equal(toBN(expiry)) + + await token.approve(user, INFINITY).should.be.fulfilled + + expect(await token.allowance(owner, user)).to.be.bignumber.equal(INFINITY) + expect(await token.expirations(owner, user)).to.be.bignumber.equal(ZERO) + }) + + it('should cancel expirations on infinite approval from increaseAllowance()', async () => { + const expiry = 10000000000 + const sig = await ethSignTypedData(owner, makeLegacyMsg(0, expiry, true)) + await permitLegacy(owner, user, 0, expiry, true, ...sig).should.be.fulfilled + + expect(await token.allowance(owner, user)).to.be.bignumber.equal(INFINITY) + expect(await token.expirations(owner, user)).to.be.bignumber.equal(toBN(expiry)) + + await token.approve(user, 1).should.be.fulfilled + + expect(await token.allowance(owner, user)).to.be.bignumber.equal('1') + expect(await token.expirations(owner, user)).to.be.bignumber.equal(toBN(expiry)) + + await token.increaseAllowance(user, 1).should.be.fulfilled + + expect(await token.allowance(owner, user)).to.be.bignumber.equal('2') + expect(await token.expirations(owner, user)).to.be.bignumber.equal(toBN(expiry)) + + await token.increaseAllowance(user, INFINITY.subn(2)).should.be.fulfilled + + expect(await token.allowance(owner, user)).to.be.bignumber.equal(INFINITY) + expect(await token.expirations(owner, user)).to.be.bignumber.equal(ZERO) + }) + + it('should cancel expirations on infinite approval from permit()', async () => { + const expiry = 10000000000 + const sig1 = await ethSignTypedData(owner, makeLegacyMsg(0, expiry, true)) + const sig2 = await ethSignTypedData(owner, makeMsg(1, expiry)) + const sig3 = await ethSignTypedData(owner, makeMsg(2, expiry, INFINITY.toString())) + await permitLegacy(owner, user, 0, expiry, true, ...sig1).should.be.fulfilled + + expect(await token.allowance(owner, user)).to.be.bignumber.equal(INFINITY) + expect(await token.expirations(owner, user)).to.be.bignumber.equal(toBN(expiry)) + + await permit(owner, user, oneEther, expiry, ...sig2).should.be.fulfilled + + expect(await token.allowance(owner, user)).to.be.bignumber.equal(oneEther) + expect(await token.expirations(owner, user)).to.be.bignumber.equal(toBN(expiry)) + + await permit(owner, user, INFINITY, expiry, ...sig3).should.be.fulfilled + + expect(await token.allowance(owner, user)).to.be.bignumber.equal(INFINITY) + expect(await token.expirations(owner, user)).to.be.bignumber.equal(ZERO) + }) + + it('should cancel approval when allowed is false', async () => { + const expiry = 10000000000 + const sig1 = await ethSignTypedData(owner, makeLegacyMsg(0, expiry, true)) + const sig2 = await ethSignTypedData(owner, makeLegacyMsg(1, expiry, false)) + await permitLegacy(owner, user, 0, expiry, true, ...sig1).should.be.fulfilled + + expect(await token.allowance(owner, user)).to.be.bignumber.equal(INFINITY) + expect(await token.expirations(owner, user)).to.be.bignumber.equal(toBN(expiry)) + + await permitLegacy(owner, user, 1, expiry, false, ...sig2).should.be.fulfilled + + expect(await token.allowance(owner, user)).to.be.bignumber.equal(ZERO) + expect(await token.expirations(owner, user)).to.be.bignumber.equal(ZERO) + }) + + it('should accept infinite approval without deadline', async () => { + const sig1 = await ethSignTypedData(owner, makeLegacyMsg(0, 0, true)) + const sig2 = await ethSignTypedData(owner, makeLegacyMsg(1, 0, false)) + await permitLegacy(owner, user, 0, 0, true, ...sig1).should.be.fulfilled + + expect(await token.allowance(owner, user)).to.be.bignumber.equal(INFINITY) + expect(await token.expirations(owner, user)).to.be.bignumber.equal(ZERO) + + await permitLegacy(owner, user, 1, 0, false, ...sig2).should.be.fulfilled + + expect(await token.allowance(owner, user)).to.be.bignumber.equal(ZERO) + expect(await token.expirations(owner, user)).to.be.bignumber.equal(ZERO) + }) + + it('should allow to use allowance without deadline', async () => { + await token.mint(owner, ether('10')).should.be.fulfilled + const sig = await ethSignTypedData(owner, makeLegacyMsg(0, 0, true)) + await permitLegacy(owner, user, 0, 0, true, ...sig).should.be.fulfilled + + await token.transferFrom(owner, user, oneEther, { from: user }).should.be.fulfilled + expect(await token.allowance(owner, user)).to.be.bignumber.equal(INFINITY) + expect(await token.balanceOf(user)).to.be.bignumber.equal(oneEther) + expect(await token.balanceOf(owner)).to.be.bignumber.equal(ether('9')) + }) + + it('should not allow to use approval after deadline', async () => { + await token.mint(owner, ether('10')).should.be.fulfilled + const expiry = 10000000000 + const sig = await ethSignTypedData(owner, makeLegacyMsg(0, expiry, true)) + await permitLegacy(owner, user, 0, expiry, true, ...sig).should.be.fulfilled + + await token.transferFrom(owner, user, oneEther, { from: user }).should.be.fulfilled + + expect(await token.allowance(owner, user)).to.be.bignumber.equal(INFINITY) + expect(await token.balanceOf(user)).to.be.bignumber.equal(oneEther) + expect(await token.balanceOf(owner)).to.be.bignumber.equal(ether('9')) + + await evmIncreaseTime(expiry) + + await token.transferFrom(owner, user, oneEther, { from: user }).should.be.rejected + expect(await token.allowance(owner, user)).to.be.bignumber.equal(INFINITY) + expect(await token.balanceOf(user)).to.be.bignumber.equal(oneEther) + expect(await token.balanceOf(owner)).to.be.bignumber.equal(ether('9')) + }) + }) - signature = permitSign( - { - name: await token.name.call(), - version: await token.version.call(), - chainId: '100', - verifyingContract: token.address - }, - { - holder, - spender, - nonce, - expiry, - allowed - }, - privateKey - ) - - await token.permit(holder, spender, nonce, expiry, allowed, signature.v, signature.r, signature.s).should.be - .fulfilled + describe('ERC2612', () => { + it('should accept signed message', async () => { + const deadline = 100000000000 + const sig1 = await ethSignTypedData(owner, makeMsg(0, 100)) + const sig2 = await ethSignTypedData(owner, makeMsg(1, deadline)) + const sig3 = await ethSignTypedData(owner, makeMsg(0, deadline)) + + await permit(owner, user, oneEther, 100, ...sig1).should.be.rejected // too small deadline + await permit(owner, user, oneEther, deadline, ...sig2).should.be.rejected // invalid nonce + await permit(owner, user, oneEther, deadline, ...sig3).should.be.fulfilled // valid for nonce == 0 + await permit(owner, user, oneEther, deadline, ...sig3).should.be.rejected // invalid duplicate, invalid nonce + await permit(user, user, oneEther, deadline, ...sig2).should.be.rejected // invalid sender + await permit(owner, owner, oneEther, deadline, ...sig2).should.be.rejected // invalid receiver + await permit(owner, user, twoEthers, deadline, ...sig2).should.be.rejected // invalud value + await permit(owner, user, oneEther, deadline + 1, ...sig2).should.be.rejected // invalid deadline + await permit(owner, user, oneEther, deadline, ...sig2).should.be.fulfilled // valid for nonce == 1 + await permit(owner, user, oneEther, deadline, ...sig2).should.be.rejected // invalid duplicate, invalid nonce + expect(await token.allowance(owner, user)).to.be.bignumber.equal(oneEther) + expect(await token.nonces(owner)).to.be.bignumber.equal('2') + expect(await token.expirations(owner, user)).to.be.bignumber.equal(ZERO) + }) }) }) } diff --git a/test/test.sh b/test/test.sh index 94d35f73a..a5c2c999c 100755 --- a/test/test.sh +++ b/test/test.sh @@ -39,4 +39,4 @@ else PROVIDER=http://ganache:8545 docker-compose -f test/docker-compose.yml up compound || true fi node_modules/.bin/truffle test --network ganache "$@" -fi \ No newline at end of file +fi