diff --git a/contracts/ERC20SimpleSwap.sol b/contracts/ERC20SimpleSwap.sol index e3272d7..34aca99 100644 --- a/contracts/ERC20SimpleSwap.sol +++ b/contracts/ERC20SimpleSwap.sol @@ -1,8 +1,11 @@ +// SPDX-License-Identifier: BSD-3-Clause pragma solidity =0.6.12; import "@openzeppelin/contracts/math/SafeMath.sol"; import "@openzeppelin/contracts/math/Math.sol"; import "@openzeppelin/contracts/cryptography/ECDSA.sol"; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import "@openzeppelin/contracts/presets/ERC20PresetMinterPauser.sol"; + /** @title Chequebook contract without waivers @@ -106,12 +109,15 @@ contract ERC20SimpleSwap { bool public bounced; /** - @notice sets the issuer, defaultHardDepositTimeout and receives an initial deposit + @notice sets the issuer, token and the defaultHardDepositTimeout. can only be called once. @param _issuer the issuer of cheques from this chequebook (needed as an argument for "Setting up a chequebook as a payment"). _issuer must be an Externally Owned Account, or it must support calling the function cashCheque + @param _token the token this chequebook uses @param _defaultHardDepositTimeout duration in seconds which by default will be used to reduce hardDeposit allocations */ - constructor(address _issuer, address _token, uint _defaultHardDepositTimeout) public { + function init(address _issuer, address _token, uint _defaultHardDepositTimeout) public { + require(_issuer != address(0), "invalid issuer"); + require(issuer == address(0), "already initialized"); issuer = _issuer; token = ERC20(_token); defaultHardDepositTimeout = _defaultHardDepositTimeout; diff --git a/contracts/Migrations.sol b/contracts/Migrations.sol deleted file mode 100644 index 4b687de..0000000 --- a/contracts/Migrations.sol +++ /dev/null @@ -1,24 +0,0 @@ -pragma solidity =0.6.12; -import "@openzeppelin/contracts/presets/ERC20PresetMinterPauser.sol"; - -contract Migrations { - address public owner; - uint public last_completed_migration; - - modifier restricted() { - if (msg.sender == owner) _; - } - - constructor() public { - owner = msg.sender; - } - - function setCompleted(uint completed) public restricted { - last_completed_migration = completed; - } - - function upgrade(address new_address) public restricted { - Migrations upgraded = Migrations(new_address); - upgraded.setCompleted(last_completed_migration); - } -} diff --git a/contracts/SimpleSwapFactory.sol b/contracts/SimpleSwapFactory.sol index 8ae6f09..5db51a0 100644 --- a/contracts/SimpleSwapFactory.sol +++ b/contracts/SimpleSwapFactory.sol @@ -1,5 +1,7 @@ +// SPDX-License-Identifier: BSD-3-Clause pragma solidity =0.6.12; import "./ERC20SimpleSwap.sol"; +import "@openzeppelin/contracts/proxy/Clones.sol"; /** @title Factory contract for SimpleSwap @@ -16,18 +18,26 @@ contract SimpleSwapFactory { /* address of the ERC20-token, to be used by the to-be-deployed chequebooks */ address public ERC20Address; + /* address of the code contract from which all chequebooks are cloned */ + address public master; constructor(address _ERC20Address) public { ERC20Address = _ERC20Address; + ERC20SimpleSwap _master = new ERC20SimpleSwap(); + // set the issuer of the master contract to prevent misuse + _master.init(address(1), address(0), 0); + master = address(_master); } /** - @notice deployes a new SimpleSwap contract + @notice creates a clone of the master SimpleSwap contract @param issuer the issuer of cheques for the new chequebook @param defaultHardDepositTimeoutDuration duration in seconds which by default will be used to reduce hardDeposit allocations + @param salt salt to include in create2 to enable the same address to deploy multiple chequebooks */ - function deploySimpleSwap(address issuer, uint defaultHardDepositTimeoutDuration) - public returns (address) { - address contractAddress = address(new ERC20SimpleSwap(issuer, ERC20Address, defaultHardDepositTimeoutDuration)); + function deploySimpleSwap(address issuer, uint defaultHardDepositTimeoutDuration, bytes32 salt) + public returns (address) { + address contractAddress = Clones.cloneDeterministic(master, keccak256(abi.encode(msg.sender, salt))); + ERC20SimpleSwap(contractAddress).init(issuer, ERC20Address, defaultHardDepositTimeoutDuration); deployedContracts[contractAddress] = true; emit SimpleSwapDeployed(contractAddress); return contractAddress; diff --git a/contracts/TestToken.sol b/contracts/TestToken.sol new file mode 100644 index 0000000..89066e8 --- /dev/null +++ b/contracts/TestToken.sol @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity ^0.6.12; + +import "@openzeppelin/contracts/presets/ERC20PresetMinterPauser.sol"; + +contract TestToken is ERC20PresetMinterPauser { + + constructor() ERC20PresetMinterPauser("Test", "TST") public { + } + +} diff --git a/package.json b/package.json index 3af7d05..a11624c 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,7 @@ "url": "https://github.com/ethersphere/swap-swear-and-swindle.git" }, "dependencies": { - "@openzeppelin/contracts": "^3.1.0" + "@openzeppelin/contracts": "^3.4.1" }, "devDependencies": { "@nomiclabs/hardhat-truffle5": "^2.0.0", diff --git a/test/ERC20SimpleSwap.should.js b/test/ERC20SimpleSwap.should.js index 895daae..6645ca4 100644 --- a/test/ERC20SimpleSwap.should.js +++ b/test/ERC20SimpleSwap.should.js @@ -7,24 +7,36 @@ const { } = require("@openzeppelin/test-helpers"); const ERC20SimpleSwap = artifacts.require('ERC20SimpleSwap') -const ERC20PresetMinterPauser = artifacts.require("ERC20PresetMinterPauser") +const SimpleSwapFactory = artifacts.require('SimpleSwapFactory') +const TestToken = artifacts.require("TestToken") const { signCheque, signCashOut, signCustomDecreaseTimeout } = require("./swutils"); const { expect } = require('chai'); function shouldDeploy(issuer, defaultHardDepositTimeout, from, value) { + + const salt = "0x000000000000000000000000000000000000000000000000000000000000abcd" + beforeEach(async function() { - this.ERC20PresetMinterPauser = await ERC20PresetMinterPauser.new("TestToken", "TEST", {from: issuer}) - await this.ERC20PresetMinterPauser.mint(issuer, 1000000000, {from: issuer}); - this.ERC20SimpleSwap = await ERC20SimpleSwap.new(issuer, this.ERC20PresetMinterPauser.address, defaultHardDepositTimeout, {from: from}) + this.TestToken = await TestToken.new({from: issuer}) + await this.TestToken.mint(issuer, 1000000000, {from: issuer}); + this.simpleSwapFactory = await SimpleSwapFactory.new(this.TestToken.address) + let { logs } = await this.simpleSwapFactory.deploySimpleSwap(issuer, defaultHardDepositTimeout, salt) + this.ERC20SimpleSwapAddress = logs[0].args.contractAddress + this.ERC20SimpleSwap = await ERC20SimpleSwap.at(this.ERC20SimpleSwapAddress) if(value != 0) { - await this.ERC20PresetMinterPauser.transfer(this.ERC20SimpleSwap.address, value, {from: issuer}); + await this.TestToken.transfer(this.ERC20SimpleSwap.address, value, {from: issuer}); } this.postconditions = { issuer: await this.ERC20SimpleSwap.issuer(), defaultHardDepositTimeout: await this.ERC20SimpleSwap.defaultHardDepositTimeout() } }) + + it('should not allow a second init', async function() { + await expectRevert(this.ERC20SimpleSwap.init(issuer, this.TestToken.address, 0), "already initialized") + }) + it('should set the issuer', function() { expect(this.postconditions.issuer).to.be.equal(issuer) }) @@ -194,8 +206,8 @@ function cashChequeInternal(beneficiary, recipient, cumulativePayout, callerPayo function shouldCashChequeBeneficiary(recipient, cumulativePayout, signee, from) { beforeEach(async function() { this.preconditions = { - callerBalance: await this.ERC20PresetMinterPauser.balanceOf(from), - recipientBalance: await this.ERC20PresetMinterPauser.balanceOf(recipient), + callerBalance: await this.TestToken.balanceOf(from), + recipientBalance: await this.TestToken.balanceOf(recipient), totalHardDeposit: await this.ERC20SimpleSwap.totalHardDeposit(), hardDepositFor: await this.ERC20SimpleSwap.hardDeposits(from), liquidBalance: await this.ERC20SimpleSwap.liquidBalance(), @@ -212,8 +224,8 @@ function shouldCashChequeBeneficiary(recipient, cumulativePayout, signee, from) this.receipt = receipt this.postconditions = { - callerBalance: await this.ERC20PresetMinterPauser.balanceOf(from), - recipientBalance: await this.ERC20PresetMinterPauser.balanceOf(recipient), + callerBalance: await this.TestToken.balanceOf(from), + recipientBalance: await this.TestToken.balanceOf(recipient), totalHardDeposit: await this.ERC20SimpleSwap.totalHardDeposit(), hardDepositFor: await this.ERC20SimpleSwap.hardDeposits(from), liquidBalance: await this.ERC20SimpleSwap.liquidBalance(), @@ -245,8 +257,8 @@ function shouldCashCheque(beneficiary, recipient, cumulativePayout, callerPayout const beneficiarySig = await signCashOut(this.ERC20SimpleSwap, from, cumulativePayout, recipient, callerPayout, beneficiarySignee) const issuerSig = await signCheque(this.ERC20SimpleSwap, beneficiary, cumulativePayout, issuerSignee) this.preconditions = { - callerBalance: await this.ERC20PresetMinterPauser.balanceOf(from), - recipientBalance: await this.ERC20PresetMinterPauser.balanceOf(recipient), + callerBalance: await this.TestToken.balanceOf(from), + recipientBalance: await this.TestToken.balanceOf(recipient), totalHardDeposit: await this.ERC20SimpleSwap.totalHardDeposit(), hardDepositFor: await this.ERC20SimpleSwap.hardDeposits(beneficiary), liquidBalance: await this.ERC20SimpleSwap.liquidBalance(), @@ -260,8 +272,8 @@ function shouldCashCheque(beneficiary, recipient, cumulativePayout, callerPayout this.receipt = receipt this.postconditions = { - callerBalance: await this.ERC20PresetMinterPauser.balanceOf(from), - recipientBalance: await this.ERC20PresetMinterPauser.balanceOf(recipient), + callerBalance: await this.TestToken.balanceOf(from), + recipientBalance: await this.TestToken.balanceOf(recipient), totalHardDeposit: await this.ERC20SimpleSwap.totalHardDeposit(), hardDepositFor: await this.ERC20SimpleSwap.hardDeposits(beneficiary), liquidBalance: await this.ERC20SimpleSwap.liquidBalance(), @@ -492,14 +504,14 @@ function shouldNotSetCustomHardDepositTimeout(toSubmit, toSign, signee, from, va function shouldWithdraw(amount, from) { beforeEach(async function() { this.preconditions = { - callerBalance: await this.ERC20PresetMinterPauser.balanceOf(from), + callerBalance: await this.TestToken.balanceOf(from), liquidBalance: await this.ERC20SimpleSwap.liquidBalance() } await this.ERC20SimpleSwap.withdraw(amount, {from: from}) this.postconditions = { - callerBalance: await this.ERC20PresetMinterPauser.balanceOf(from), + callerBalance: await this.TestToken.balanceOf(from), liquidBalance: await this.ERC20SimpleSwap.liquidBalance() } }) @@ -529,7 +541,7 @@ function shouldDeposit(amount, from) { totalHardDeposit: await this.ERC20SimpleSwap.totalHardDeposit(), liquidBalance: await this.ERC20SimpleSwap.liquidBalance() } - const { logs } = await this.ERC20PresetMinterPauser.transfer(this.ERC20SimpleSwap.address, amount, {from: from}) + const { logs } = await this.TestToken.transfer(this.ERC20SimpleSwap.address, amount, {from: from}) this.logs = logs }) it('should update the liquidBalance of the checkbook', async function() { diff --git a/test/SimpleSwapFactory.test.js b/test/SimpleSwapFactory.test.js index fd22e68..712acab 100644 --- a/test/SimpleSwapFactory.test.js +++ b/test/SimpleSwapFactory.test.js @@ -2,31 +2,34 @@ const { BN, balance, constants, - expectEvent + expectEvent, + expectRevert } = require("@openzeppelin/test-helpers"); const { expect } = require('chai'); const SimpleSwapFactory = artifacts.require('./SimpleSwapFactory') const ERC20SimpleSwap = artifacts.require('./ERC20SimpleSwap') -const ERC20PresetMinterPauser = artifacts.require("ERC20PresetMinterPauser") +const TestToken = artifacts.require("./TestToken") -contract('SimpleSwapFactory', function([issuer]) { +contract('SimpleSwapFactory', function([issuer, other]) { + + const salt = "0x000000000000000000000000000000000000000000000000000000000000abcd" function shouldDeployERC20SimpleSwap(issuer, DEFAULT_HARDDEPOSIT_DECREASE_TIMEOUT, value) { beforeEach(async function() { - this.ERC20PresetMinterPauser = await ERC20PresetMinterPauser.new("TestToken", "TEST", {from: issuer}) - this.simpleSwapFactory = await SimpleSwapFactory.new(this.ERC20PresetMinterPauser.address) - let { logs } = await this.simpleSwapFactory.deploySimpleSwap(issuer, DEFAULT_HARDDEPOSIT_DECREASE_TIMEOUT) + this.TestToken = await TestToken.new({from: issuer}) + this.simpleSwapFactory = await SimpleSwapFactory.new(this.TestToken.address) + let { logs } = await this.simpleSwapFactory.deploySimpleSwap(issuer, DEFAULT_HARDDEPOSIT_DECREASE_TIMEOUT, salt) this.ERC20SimpleSwapAddress = logs[0].args.contractAddress this.ERC20SimpleSwap = await ERC20SimpleSwap.at(this.ERC20SimpleSwapAddress) if(value != 0) { - await this.ERC20PresetMinterPauser.mint(issuer, value) // mint tokens - await this.ERC20PresetMinterPauser.transfer(this.ERC20SimpleSwap.address, value, {from: issuer}); // deposit those tokens in chequebook + await this.TestToken.mint(issuer, value) // mint tokens + await this.TestToken.transfer(this.ERC20SimpleSwap.address, value, {from: issuer}); // deposit those tokens in chequebook } }) - it('should deploy the correct bytecode', async function() { - expect(await web3.eth.getCode(this.ERC20SimpleSwapAddress)).to.be.equal(ERC20SimpleSwap.deployedBytecode) + it('should allow other addresses to deploy with same salt', async function() { + await this.simpleSwapFactory.deploySimpleSwap(issuer, DEFAULT_HARDDEPOSIT_DECREASE_TIMEOUT, salt, { from: other }) }) it('should deploy with the right issuer', async function() { @@ -48,7 +51,7 @@ contract('SimpleSwapFactory', function([issuer]) { }) it('should have set the ERC20 address correctly', async function() { - expect(await this.ERC20SimpleSwap.token()).to.be.equal(this.ERC20PresetMinterPauser.address) + expect(await this.ERC20SimpleSwap.token()).to.be.equal(this.TestToken.address) }) } @@ -60,5 +63,13 @@ contract('SimpleSwapFactory', function([issuer]) { describe("when we deposit while deploying SimpleSwap", function() { shouldDeployERC20SimpleSwap(issuer, new BN(86400), new BN(10)) }) + + describe("when we deposit while issuer 0", function() { + it('should fail', async function() { + this.TestToken = await TestToken.new({from: issuer}) + this.simpleSwapFactory = await SimpleSwapFactory.new(this.TestToken.address) + await expectRevert(this.simpleSwapFactory.deploySimpleSwap(constants.ZERO_ADDRESS, 0, salt), 'invalid issuer') + }) + }) }) }) \ No newline at end of file diff --git a/yarn.lock b/yarn.lock index 5c38afa..16fab77 100644 --- a/yarn.lock +++ b/yarn.lock @@ -295,7 +295,7 @@ find-up "^4.1.0" fs-extra "^8.1.0" -"@openzeppelin/contracts@^3.1.0": +"@openzeppelin/contracts@^3.4.1": version "3.4.1" resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-3.4.1.tgz#03c891fec7f93be0ae44ed74e57a122a38732ce7" integrity sha512-cUriqMauq1ylzP2TxePNdPqkwI7Le3Annh4K9rrpvKfSBB/bdW+Iu1ihBaTIABTAAJ85LmKL5SSPPL9ry8d1gQ==