use proxied simpleswaps (#120)

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;

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;

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 {    
+  }
+
+}

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",

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() {

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')
+      })
+    })
   })
 })

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==