From 8b629d3bd708b40f7612af190ab7599eda09d5e2 Mon Sep 17 00:00:00 2001 From: perissology Date: Thu, 16 Nov 2017 15:15:32 -0800 Subject: [PATCH 1/2] add plugin whitelist fixes #33 --- contracts/LiquidPledgingBase.sol | 54 +++++++- contracts/test/TestSimpleProjectPlugin.sol | 52 ++++++++ .../test/TestSimpleProjectPluginFactory.sol | 19 +++ package.json | 9 +- test/AdminPlugins.js | 118 ++++++++++++++++++ 5 files changed, 246 insertions(+), 6 deletions(-) create mode 100644 contracts/test/TestSimpleProjectPlugin.sol create mode 100644 contracts/test/TestSimpleProjectPluginFactory.sol create mode 100644 test/AdminPlugins.js diff --git a/contracts/LiquidPledgingBase.sol b/contracts/LiquidPledgingBase.sol index 8c79ece..b3630aa 100644 --- a/contracts/LiquidPledgingBase.sol +++ b/contracts/LiquidPledgingBase.sol @@ -1,6 +1,7 @@ pragma solidity ^0.4.11; import "./ILiquidPledgingPlugin.sol"; +import "../node_modules/giveth-common-contracts/contracts/Owned.sol"; /// @dev This is declares a few functions from `Vault` so that the /// `LiquidPledgingBase` contract can interface with the `Vault` contract @@ -9,7 +10,7 @@ contract Vault { function () payable; } -contract LiquidPledgingBase { +contract LiquidPledgingBase is Owned { // Limits inserted to prevent large loops that could prevent canceling uint constant MAX_DELEGATES = 20; uint constant MAX_SUBPROJECT_LEVEL = 20; @@ -47,6 +48,9 @@ contract LiquidPledgingBase { // this mapping allows you to search for a specific pledge's index number by the hash of that pledge mapping (bytes32 => uint64) hPledge2idx;//TODO Fix typo + mapping (bytes32 => bool) pluginWhitelist; + + bool public usePluginWhitelist = true; ///// @@ -73,12 +77,13 @@ contract LiquidPledgingBase { /////// -// Adminss functions +// Admin functions ////// /// @notice Creates a giver. function addGiver(string name, string url, uint64 commitTime, ILiquidPledgingPlugin plugin ) returns (uint64 idGiver) { + require(isValidPlugin(plugin)); idGiver = uint64(admins.length); @@ -119,6 +124,7 @@ contract LiquidPledgingBase { /// @notice Creates a new Delegate function addDelegate(string name, string url, uint64 commitTime, ILiquidPledgingPlugin plugin) returns (uint64 idDelegate) { //TODO return index number + require(isValidPlugin(plugin)); idDelegate = uint64(admins.length); @@ -158,6 +164,8 @@ contract LiquidPledgingBase { /// @notice Creates a new Project function addProject(string name, string url, address projectAdmin, uint64 parentProject, uint64 commitTime, ILiquidPledgingPlugin plugin) returns (uint64 idProject) { + require(isValidPlugin(plugin)); + if (parentProject != 0) { PledgeAdmin storage pa = findAdmin(parentProject); require(pa.adminType == PledgeAdminType.Project); @@ -373,4 +381,46 @@ contract LiquidPledgingBase { function checkAdminOwner(PledgeAdmin m) internal constant { require((msg.sender == m.addr) || (msg.sender == address(m.plugin))); } + +//////// +// Plugin Whitelist Methods +/////// + + function addValidPlugin(bytes32 contractHash) external onlyOwner { + pluginWhitelist[contractHash] = true; + } + + function removeValidPlugin(bytes32 contractHash) external onlyOwner { + pluginWhitelist[contractHash] = false; + } + + function useWhitelist(bool useWhitelist) external onlyOwner { + usePluginWhitelist = useWhitelist; + } + + function isValidPlugin(address addr) public returns(bool) { + if (!usePluginWhitelist || addr == 0x0) return true; + + bytes32 contractHash = getCodeHash(addr); + + return pluginWhitelist[contractHash]; + } + + function getCodeHash(address addr) public returns(bytes32) { + bytes memory o_code; + assembly { + // retrieve the size of the code, this needs assembly + let size := extcodesize(addr) + // allocate output byte array - this could also be done without assembly + // by using o_code = new bytes(size) + o_code := mload(0x40) + // new "memory end" including padding + mstore(0x40, add(o_code, and(add(add(size, 0x20), 0x1f), not(0x1f)))) + // store length in memory + mstore(o_code, size) + // actually retrieve the code, this needs assembly + extcodecopy(addr, add(o_code, 0x20), 0, size) + } + return keccak256(o_code); + } } diff --git a/contracts/test/TestSimpleProjectPlugin.sol b/contracts/test/TestSimpleProjectPlugin.sol new file mode 100644 index 0000000..2ab4c67 --- /dev/null +++ b/contracts/test/TestSimpleProjectPlugin.sol @@ -0,0 +1,52 @@ +pragma solidity ^0.4.11; + +import "../LiquidPledging.sol"; + +// simple liquidPledging plugin contract for testing whitelist +contract TestSimpleProjectPlugin { + + uint64 public idProject; + bool initPending; + + event BeforeTransfer(uint64 pledgeAdmin, uint64 pledgeFrom, uint64 pledgeTo, uint64 context, uint amount); + event AfterTransfer(uint64 pledgeAdmin, uint64 pledgeFrom, uint64 pledgeTo, uint64 context, uint amount); + + function TestSimpleProjectPlugin() { + require(msg.sender != tx.origin); // Avoids being created directly by mistake. + initPending = true; + } + + function init( + LiquidPledging liquidPledging, + string name, + string url, + uint64 parentProject + ) { + require(initPending); + idProject = liquidPledging.addProject(name, url, address(this), parentProject, 0, ILiquidPledgingPlugin(this)); + initPending = false; + } + + function beforeTransfer( + uint64 pledgeAdmin, + uint64 pledgeFrom, + uint64 pledgeTo, + uint64 context, + uint amount + ) external returns (uint maxAllowed) { + require(!initPending); + BeforeTransfer(pledgeAdmin, pledgeFrom, pledgeTo, context, amount); + } + + function afterTransfer( + uint64 pledgeAdmin, + uint64 pledgeFrom, + uint64 pledgeTo, + uint64 context, + uint amount + ) external { + require(!initPending); + AfterTransfer(pledgeAdmin, pledgeFrom, pledgeTo, context, amount); + } + +} diff --git a/contracts/test/TestSimpleProjectPluginFactory.sol b/contracts/test/TestSimpleProjectPluginFactory.sol new file mode 100644 index 0000000..390b5d9 --- /dev/null +++ b/contracts/test/TestSimpleProjectPluginFactory.sol @@ -0,0 +1,19 @@ +pragma solidity ^0.4.11; + +import "./TestSimpleProjectPlugin.sol"; +import "../LiquidPledging.sol"; + +// simple factory for deploying TestSimpleProjectPlugin.sol contract +contract TestSimpleProjectPluginFactory { + + function deploy( + LiquidPledging liquidPledging, + string name, + string url, + uint64 parentProject + ) { + TestSimpleProjectPlugin p = new TestSimpleProjectPlugin(); + p.init(liquidPledging, name, url, parentProject); + } + +} diff --git a/package.json b/package.json index 7b8e50a..9dffb08 100644 --- a/package.json +++ b/package.json @@ -7,8 +7,8 @@ "test": "test" }, "scripts": { - "build": "solcpiler", - "test": "solcpiler; mocha --harmony" + "build": "solcpiler -i './contracts/**/*.sol'", + "test": "npm run build; mocha --harmony" }, "repository": { "type": "git", @@ -38,13 +38,14 @@ "lerna": "^2.2.0", "random-bytes": "^1.0.0", "mocha": "^3.5.0", - "solcpiler": "0.0.4", + "solcpiler": "0.0.7", "web3": "1.0.0-beta.24" }, "homepage": "https://github.com/Giveth/liquidpledging#readme", "dependencies": { "async": "^2.4.0", "chai": "^4.1.0", - "eth-contract-class": "0.0.6" + "eth-contract-class": "0.0.6", + "giveth-common-contracts": "^0.4.0" } } diff --git a/test/AdminPlugins.js b/test/AdminPlugins.js new file mode 100644 index 0000000..214da6d --- /dev/null +++ b/test/AdminPlugins.js @@ -0,0 +1,118 @@ +/* eslint-env mocha */ +/* eslint-disable no-await-in-loop */ +const TestRPC = require('ethereumjs-testrpc'); +const Web3 = require('web3'); +const chai = require('chai'); +const liquidpledging = require('../index.js'); +const assertFail = require('./helpers/assertFail'); + +const LiquidPledging = liquidpledging.LiquidPledgingMock; +const LiquidPledgingState = liquidpledging.LiquidPledgingState; +const simpleProjectPluginFactoryAbi = require('../build/TestSimpleProjectPluginFactory.sol').TestSimpleProjectPluginFactoryAbi; +const simpleProjectPluginFactoryByteCode = require('../build/TestSimpleProjectPluginFactory.sol').TestSimpleProjectPluginFactoryByteCode; +const simpleProjectPluginRuntimeByteCode = require('../build/TestSimpleProjectPluginFactory.sol').TestSimpleProjectPluginRuntimeByteCode; +const Vault = liquidpledging.Vault; +const assert = chai.assert; + +const printState = async (liquidPledgingState) => { + const st = await liquidPledgingState.getState(); + console.log(JSON.stringify(st, null, 2)); +}; + +describe('LiquidPledging plugins test', function () { + this.timeout(0); + + let testrpc; + let web3; + let accounts; + let liquidPledging; + let liquidPledgingState; + let vault; + let giver1; + let adminProject1; + let adminDelegate1; + + before(async () => { + testrpc = TestRPC.server({ + ws: true, + gasLimit: 6500000, + total_accounts: 10, + }); + + testrpc.listen(8546, '127.0.0.1'); + + web3 = new Web3('ws://localhost:8546'); + accounts = await web3.eth.getAccounts(); + giver1 = accounts[ 1 ]; + adminProject1 = accounts[ 2 ]; + adminDelegate1 = accounts[ 3 ]; + }); + + after((done) => { + testrpc.close(); + done(); + }); + + it('Should deploy LiquidPledging contract', async function() { + vault = await Vault.new(web3); + liquidPledging = await LiquidPledging.new(web3, vault.$address, { gas: 6500000 }); + await vault.setLiquidPledging(liquidPledging.$address); + liquidPledgingState = new LiquidPledgingState(liquidPledging); + }); + + it('Should create create giver with no plugin', async function() { + await liquidPledging.addGiver('Giver1', '', 0, '0x0', { from: adminProject1 }); + + const nAdmins = await liquidPledging.numberOfPledgeAdmins(); + assert.equal(nAdmins, 1); + }); + + it('Should fail to create giver with invalid plugin', async function() { + await assertFail(async () => { + await liquidPledging.addGiver('Giver2', '', 0, vault.$address, { from: giver1 }); + }); + }); + + it('Should fail to create delegate with invalid plugin', async function() { + await assertFail(async () => { + await liquidPledging.addDelegate('delegate1', '', 0, liquidPledging.$address, { from: adminDelegate1}); + }); + }); + + it('Should fail to create project with invalid plugin', async function() { + await assertFail(async () => { + await liquidPledging.addProject('Project1', '', giver1, 0, 0, vault.$address, { from: adminProject1}); + }); + }); + + it('Should deploy TestSimpleProjectPlugin and add project', async function() { + // add plugin as valid plugin + const codeHash = web3.utils.soliditySha3(simpleProjectPluginRuntimeByteCode); + console.log(codeHash); + await liquidPledging.addValidPlugin(codeHash); + + // deploy new plugin + const factoryContract = await new web3.eth.Contract(simpleProjectPluginFactoryAbi) + .deploy({ + data: simpleProjectPluginFactoryByteCode, + arguments: [] + }).send({ from: adminProject1, gas: 5000000 }); + + await factoryContract.methods + .deploy(liquidPledging.$address, "SimplePlugin1", "", 0) + .send({ from: adminProject1, gas: 4000000 }); + + const nAdmins = await liquidPledging.numberOfPledgeAdmins(); + assert.equal(nAdmins, 2); + }); + + it('Should allow all plugins', async function() { + await liquidPledging.useWhitelist(false, { gas: 100000 }); + + await liquidPledging.addGiver('Giver2', '', 0, vault.$address, { from: giver1, gas: 200000 }); + + const nAdmins = await liquidPledging.numberOfPledgeAdmins(); + assert.equal(nAdmins, 3); + }); +}); + From e2083f4f12254a360baa3e15d27118e78ab8a5d4 Mon Sep 17 00:00:00 2001 From: perissology Date: Thu, 16 Nov 2017 15:30:29 -0800 Subject: [PATCH 2/2] fix tests after web3 update --- test/AdminPlugins.js | 1 - test/NormalOperation.js | 50 ++++++++++++++++++++--------------------- 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/test/AdminPlugins.js b/test/AdminPlugins.js index 214da6d..20ff4f3 100644 --- a/test/AdminPlugins.js +++ b/test/AdminPlugins.js @@ -88,7 +88,6 @@ describe('LiquidPledging plugins test', function () { it('Should deploy TestSimpleProjectPlugin and add project', async function() { // add plugin as valid plugin const codeHash = web3.utils.soliditySha3(simpleProjectPluginRuntimeByteCode); - console.log(codeHash); await liquidPledging.addValidPlugin(codeHash); // deploy new plugin diff --git a/test/NormalOperation.js b/test/NormalOperation.js index b5f29dc..90ab7d6 100644 --- a/test/NormalOperation.js +++ b/test/NormalOperation.js @@ -80,7 +80,7 @@ describe('LiquidPledging test', function () { assert.equal(res[4], 86400); }); it('Should make a donation', async () => { - await liquidPledging.donate(1, 1, { from: giver1, value: utils.toWei(1) }); + await liquidPledging.donate(1, 1, { from: giver1, value: utils.toWei('1') }); const nPledges = await liquidPledging.numberOfPledges(); assert.equal(nPledges, 1); await liquidPledging.getPledge(1); @@ -97,13 +97,13 @@ describe('LiquidPledging test', function () { assert.equal(res[4], 0); }); it('Giver should delegate on the delegate', async () => { - await liquidPledging.transfer(1, 1, utils.toWei(0.5), 2, { from: giver1 }); + await liquidPledging.transfer(1, 1, utils.toWei('0.5'), 2, { from: giver1 }); const nPledges = await liquidPledging.numberOfPledges(); assert.equal(nPledges, 2); const res1 = await liquidPledging.getPledge(1); - assert.equal(res1[0], utils.toWei(0.5)); + assert.equal(res1[0], utils.toWei('0.5')); const res2 = await liquidPledging.getPledge(2); - assert.equal(res2[0], utils.toWei(0.5)); + assert.equal(res2[0], utils.toWei('0.5')); assert.equal(res2[1], 1); // One delegate const d = await liquidPledging.getPledgeDelegate(2, 1); @@ -140,11 +140,11 @@ describe('LiquidPledging test', function () { }); it('Delegate should assign to project1', async () => { const n = Math.floor(new Date().getTime() / 1000); - await liquidPledging.transfer(2, 2, utils.toWei(0.2), 3, { from: delegate1 }); + await liquidPledging.transfer(2, 2, utils.toWei('0.2'), 3, { from: delegate1 }); const nPledges = await liquidPledging.numberOfPledges(); assert.equal(nPledges, 3); const res3 = await liquidPledging.getPledge(3); - assert.equal(res3[0], utils.toWei(0.2)); + assert.equal(res3[0], utils.toWei('0.2')); assert.equal(res3[1], 1); // Owner assert.equal(res3[2], 1); // Delegates assert.equal(res3[3], 3); // Proposed Project @@ -153,11 +153,11 @@ describe('LiquidPledging test', function () { assert.equal(res3[6], 0); // Not Paid }); it('Giver should change his mind and assign half of it to project2', async () => { - await liquidPledging.transfer(1, 3, utils.toWei(0.1), 4, { from: giver1 }); + await liquidPledging.transfer(1, 3, utils.toWei('0.1'), 4, { from: giver1 }); const nPledges = await liquidPledging.numberOfPledges(); assert.equal(nPledges, 4); const res3 = await liquidPledging.getPledge(3); - assert.equal(res3[0], utils.toWei(0.1)); + assert.equal(res3[0], utils.toWei('0.1')); const res4 = await liquidPledging.getPledge(4); assert.equal(res4[1], 4); // Owner assert.equal(res4[2], 0); // Delegates @@ -169,11 +169,11 @@ describe('LiquidPledging test', function () { it('After the time, the project1 should be able to spend part of it', async () => { const n = Math.floor(new Date().getTime() / 1000); await liquidPledging.setMockedTime(n + 86401); - await liquidPledging.withdraw(3, utils.toWei(0.05), { from: adminProject1 }); + await liquidPledging.withdraw(3, utils.toWei('0.05'), { from: adminProject1 }); const nPledges = await liquidPledging.numberOfPledges(); assert.equal(nPledges, 6); const res5 = await liquidPledging.getPledge(5); - assert.equal(res5[0], utils.toWei(0.05)); + assert.equal(res5[0], utils.toWei('0.05')); assert.equal(res5[1], 3); // Owner assert.equal(res5[2], 0); // Delegates assert.equal(res5[3], 0); // Proposed Project @@ -181,7 +181,7 @@ describe('LiquidPledging test', function () { assert.equal(res5[5], 2); // Old Node assert.equal(res5[6], 0); // Not Paid const res6 = await liquidPledging.getPledge(6); - assert.equal(res6[0], utils.toWei(0.05)); + assert.equal(res6[0], utils.toWei('0.05')); assert.equal(res6[1], 3); // Owner assert.equal(res6[2], 0); // Delegates assert.equal(res6[3], 0); // Proposed Project @@ -202,7 +202,7 @@ describe('LiquidPledging test', function () { const nPledges = await liquidPledging.numberOfPledges(); assert.equal(nPledges, 7); const res7 = await liquidPledging.getPledge(7); - assert.equal(res7[0], utils.toWei(0.05)); + assert.equal(res7[0], utils.toWei('0.05')); assert.equal(res7[1], 3); // Owner assert.equal(res7[2], 0); // Delegates assert.equal(res7[3], 0); // Proposed Project @@ -219,11 +219,11 @@ describe('LiquidPledging test', function () { const st = await liquidPledgingState.getState(liquidPledging); assert.equal(utils.fromWei(st.pledges[5].amount), 0.05); await assertFail(async () => { - await liquidPledging.withdraw(5, utils.toWei(0.01), { from: adminProject1 }); + await liquidPledging.withdraw(5, utils.toWei('0.01'), { from: adminProject1 }); }); }); it('Delegate should send part of this ETH to project2', async () => { - await liquidPledging.transfer(2, 5, utils.toWei(0.03), 4, { + await liquidPledging.transfer(2, 5, utils.toWei('0.03'), 4, { $extraGas: 100000, from: delegate1, }); @@ -236,7 +236,7 @@ describe('LiquidPledging test', function () { assert.equal(st.pledges[8].intendedProject, 4); }); it('Giver should be able to send the remaining to project2', async () => { - await liquidPledging.transfer(1, 5, utils.toWei(0.02), 4, { from: giver1, $extraGas: 100000 }); + await liquidPledging.transfer(1, 5, utils.toWei('0.02'), 4, { from: giver1, $extraGas: 100000 }); const st = await liquidPledgingState.getState(liquidPledging); assert.equal(st.pledges.length, 9); assert.equal(utils.fromWei(st.pledges[5].amount), 0); @@ -249,14 +249,14 @@ describe('LiquidPledging test', function () { assert.equal(nAdmins, 6); }); it('Project 2 delegate in delegate2', async () => { - await liquidPledging.transfer(4, 4, utils.toWei(0.02), 6, { from: adminProject2 }); + await liquidPledging.transfer(4, 4, utils.toWei('0.02'), 6, { from: adminProject2 }); const st = await liquidPledgingState.getState(liquidPledging); assert.equal(st.pledges.length, 10); assert.equal(utils.fromWei(st.pledges[9].amount), 0.02); assert.equal(utils.fromWei(st.pledges[4].amount), 0.1); }); it('delegate2 assigns to projec2a', async () => { - await liquidPledging.transfer(6, 9, utils.toWei(0.01), 5, { from: delegate2 }); + await liquidPledging.transfer(6, 9, utils.toWei('0.01'), 5, { from: delegate2 }); const st = await liquidPledgingState.getState(liquidPledging); assert.equal(st.pledges.length, 11); assert.equal(utils.fromWei(st.pledges[9].amount), 0.01); @@ -265,7 +265,7 @@ describe('LiquidPledging test', function () { it('project2a authorize to spend a litle', async () => { const n = Math.floor(new Date().getTime() / 1000); await liquidPledging.setMockedTime(n + (86401 * 3)); - await liquidPledging.withdraw(10, utils.toWei(0.005), { from: adminProject2a }); + await liquidPledging.withdraw(10, utils.toWei('0.005'), { from: adminProject2a }); const st = await liquidPledgingState.getState(liquidPledging); assert.equal(st.pledges.length, 13); assert.equal(utils.fromWei(st.pledges[10].amount), 0); @@ -277,7 +277,7 @@ describe('LiquidPledging test', function () { }); it('Should not be able to withdraw it', async () => { await assertFail(async () => { - await liquidPledging.withdraw(12, utils.toWei(0.005), { from: giver1 }); + await liquidPledging.withdraw(12, utils.toWei('0.005'), { from: giver1 }); }); }); it('Should be able to cancel payment', async () => { @@ -291,11 +291,11 @@ describe('LiquidPledging test', function () { }); it('original owner should recover the remaining funds', async () => { const pledges = [ - { amount: utils.toWei(0.5), id: 1 }, - { amount: utils.toWei(0.31), id: 2 }, - { amount: utils.toWei(0.1), id: 4 }, - { amount: utils.toWei(0.03), id: 8 }, - { amount: utils.toWei(0.01), id: 9 }, + { amount: utils.toWei('0.5'), id: 1 }, + { amount: utils.toWei('0.31'), id: 2 }, + { amount: utils.toWei('0.1'), id: 4 }, + { amount: utils.toWei('0.03'), id: 8 }, + { amount: utils.toWei('0.01'), id: 9 }, ]; // .substring is to remove the 0x prefix on the toHex result @@ -316,7 +316,7 @@ describe('LiquidPledging test', function () { it('Should make a donation and create giver', async () => { const oldNPledges = await liquidPledging.numberOfPledges(); const oldNAdmins = await liquidPledging.numberOfPledgeAdmins(); - await liquidPledging.donate(0, 1, { from: giver2, value: utils.toWei(1) }); + await liquidPledging.donate(0, 1, { from: giver2, value: utils.toWei('1') }); const nPledges = await liquidPledging.numberOfPledges(); assert.equal(utils.toDecimal(nPledges), utils.toDecimal(oldNPledges) + 1); const nAdmins = await liquidPledging.numberOfPledgeAdmins();