Merge pull request #48 from federicobond/inheritance-bounty

Replace composition for inheritance in Bounty

README.md

@@ -33,11 +33,18 @@ contract MetaCoin is Rejector {
 
 ## Add your own bounty contract
 
-So far you inherit Zeppelin contracts into your own contract through inheritance.
-A bounty contract, however, is a special contract that is deployed on its own.
-Each researcher creates a separate copy of your contract, and can claims bounty by breaking invariants logic on the copy of your contract without hacking your original contract.
+To create a bounty for your contract, inherit from the base Bounty contract and provide an implementation for `deployContract()` returning the new contract address.
 
-To use the bounty contract, please follow the below instruction.
+```
+import "./zeppelin/Bounty.sol";
+import "./YourContract.sol";
+
+contract YourBounty is Bounty {
+  function deployContract() internal returns(address) {
+    return new YourContract()
+  }
+}
+```
 
 ### Implement invariant logic into your smart contract
 
@@ -45,29 +52,20 @@ At contracts/YourContract.sol
 
 ```
 contract YourContract {
-  function checkInvariant() returns(bool){
+  function checkInvariant() returns(bool) {
     // Implement your logic to make sure that none of the state is broken.
   }
 }
-
-contract YourContractFactory {
-  function deployContract() returns (address) {
-    // This contract allows researchers to create a copy of your contract
-    return new YourContract();
-  }
-}
 ```
 
-### Add the bounty contracts as well as your contracts into migrations
+### Deploy your bounty contract as usual
 
 At `migrations/2_deploy_contracts.js`
 
 ```
 module.exports = function(deployer) {
   deployer.deploy(YourContract);
-  deployer.deploy(YourContractFactory).then(function() {
-    return deployer.deploy(Bounty, YourContractFactory.address);
-  });
+  deployer.deploy(YourBounty);
 };
 ```
 

contracts/Bounty.sol

@@ -8,10 +8,6 @@ import './Killable.sol';
  * the contract you bet reward against.
  */
 
-contract Factory {
-  function deployContract() returns (address);
-}
-
 contract Target {
   function checkInvariant() returns(bool);
 }
@@ -19,7 +15,6 @@ contract Target {
 contract Bounty is PullPayment, Killable {
   Target target;
   bool public claimed;
-  address public factoryAddress;
   mapping(address => address) public researchers;
 
   event TargetCreated(address createdAddress);
@@ -28,22 +23,15 @@ contract Bounty is PullPayment, Killable {
     if (claimed) throw;
   }
 
-  modifier withAddress(address _address) {
-    if(_address == 0) throw;
-    _;
-  }
-
-  function Bounty(address _factoryAddress) withAddress(_factoryAddress){
-    factoryAddress = _factoryAddress;
-  }
-
   function createTarget() returns(Target) {
-    target = Target(Factory(factoryAddress).deployContract());
+    target = Target(deployContract());
     researchers[target] = msg.sender;
     TargetCreated(target);
     return target;
   }
 
+  function deployContract() internal returns(address);
+
   function checkInvariant() returns(bool){
     return target.checkInvariant();
   }

contracts/test-helpers/InsecureTargetBounty.sol

@@ -1,13 +1,15 @@
 pragma solidity ^0.4.4;
 
+import "../Bounty.sol";
+
 contract InsecureTargetMock {
   function checkInvariant() returns(bool){
     return false;
   }
 }
 
-contract InsecureTargetFactory {
-  function deployContract() returns (address) {
+contract InsecureTargetBounty is Bounty {
+  function deployContract() internal returns (address) {
     return new InsecureTargetMock();
   }
 }

contracts/test-helpers/SecureTargetBounty.sol

@@ -1,13 +1,15 @@
 pragma solidity ^0.4.4;
 
+import "../Bounty.sol";
+
 contract SecureTargetMock {
   function checkInvariant() returns(bool){
     return true;
   }
 }
 
-contract SecureTargetFactory {
-  function deployContract() returns (address) {
+contract SecureTargetBounty is Bounty {
+  function deployContract() internal returns (address) {
     return new SecureTargetMock();
   }
 }

migrations/2_deploy_contracts.js

@@ -5,9 +5,7 @@ module.exports = function(deployer) {
   deployer.deploy(Ownable);
   deployer.deploy(LimitFunds);
   if(deployer.network == 'test'){
-    deployer.deploy(SecureTargetMock);
-    deployer.deploy(SecureTargetFactory);
-    deployer.deploy(InsecureTargetMock);
-    deployer.deploy(InsecureTargetFactory);
+    deployer.deploy(SecureTargetBounty);
+    deployer.deploy(InsecureTargetBounty);
   };
 };

test/Bounty.js

@@ -7,25 +7,12 @@ var sendReward = function(sender, receiver, value){
 }
 
 contract('Bounty', function(accounts) {
-  it("creates bounty contract with factory address", function(done){
-    var target = SecureTargetMock.deployed();
-
-    Bounty.new(target.address).
-      then(function(bounty){
-        return bounty.factoryAddress.call()
-      }).
-      then(function(address){
-        assert.equal(address, target.address)
-      }).
-      then(done);
-  })
 
   it("sets reward", function(done){
-    var target = SecureTargetMock.deployed();
     var owner = accounts[0];
     var reward = web3.toWei(1, "ether");
 
-    Bounty.new(target.address).
+    SecureTargetBounty.new().
       then(function(bounty){
         sendReward(owner, bounty.address, reward);
         assert.equal(reward, web3.eth.getBalance(bounty.address).toNumber())
@@ -33,24 +20,12 @@ contract('Bounty', function(accounts) {
       then(done);
   })
 
-  it("cannot create bounty without address", function(done){
-    var target = SecureTargetMock.deployed();
-    Bounty.new().
-      then(function(bounty){
-        throw {name : "NoThrowError", message : "should not come here"};
-      }).
-      catch(function(error){
-        assert.notEqual(error.name, "NoThrowError");
-      }).
-      then(done);
-  })
-
   it("empties itself when killed", function(done){
-    var target = SecureTargetMock.deployed();
     var owner = accounts[0];
     var reward = web3.toWei(1, "ether");
     var bounty;
-    Bounty.new(target.address).
+
+    SecureTargetBounty.new().
       then(function(_bounty){
         bounty = _bounty;
         sendReward(owner, bounty.address, reward);
@@ -65,9 +40,9 @@ contract('Bounty', function(accounts) {
 
   describe("Against secure contract", function(){
     it("checkInvariant returns true", function(done){
-      var targetFactory = SecureTargetFactory.deployed();
       var bounty;
-      Bounty.new(targetFactory.address).
+
+      SecureTargetBounty.new().
         then(function(_bounty) {
           bounty = _bounty;
           return bounty.createTarget();
@@ -82,12 +57,11 @@ contract('Bounty', function(accounts) {
     })
 
     it("cannot claim reward", function(done){
-      var targetFactory = SecureTargetFactory.deployed();
       var owner = accounts[0];
       var researcher = accounts[1];
       var reward = web3.toWei(1, "ether");
 
-      Bounty.new(targetFactory.address).
+      SecureTargetBounty.new().
         then(function(bounty) {
           var event = bounty.TargetCreated({});
           event.watch(function(err, result) {
@@ -118,9 +92,9 @@ contract('Bounty', function(accounts) {
 
   describe("Against broken contract", function(){
     it("checkInvariant returns false", function(done){
-      var targetFactory = InsecureTargetFactory.deployed();
       var bounty;
-      Bounty.new(targetFactory.address).
+
+      InsecureTargetBounty.new().
         then(function(_bounty) {
           bounty = _bounty;
           return bounty.createTarget();
@@ -135,12 +109,11 @@ contract('Bounty', function(accounts) {
     })
 
     it("claims reward", function(done){
-      var targetFactory = InsecureTargetFactory.deployed();
       var owner = accounts[0];
       var researcher = accounts[1];
       var reward = web3.toWei(1, "ether");
 
-      Bounty.new(targetFactory.address).
+      InsecureTargetBounty.new().
         then(function(bounty) {
           var event = bounty.TargetCreated({});
           event.watch(function(err, result) {