From be09536fe348201877e151583b2976162cf797ce Mon Sep 17 00:00:00 2001 From: Makoto Inoue Date: Sun, 8 Oct 2017 20:44:43 +0100 Subject: [PATCH 1/6] Remove code to return deposit --- contracts/Conference.sol | 64 ++++++++++------------------------------ test/conference.js | 6 ++-- 2 files changed, 18 insertions(+), 52 deletions(-) diff --git a/contracts/Conference.sol b/contracts/Conference.sol index 7bc6999..0733af4 100644 --- a/contracts/Conference.sol +++ b/contracts/Conference.sol @@ -37,50 +37,8 @@ contract Conference is Destructible { event ClearEvent(address addr, uint256 leftOver); /* Modifiers */ - modifier sentDepositOrReturn { - if (msg.value == deposit) { - _; - }else{ - assert(msg.sender.send(msg.value)); - } - } - modifier onlyActive { - if (ended == false) { - _; - } - } - - modifier onlyActiveOrReturn { - if (ended == false) { - _; - }else{ - assert(msg.sender.send(msg.value)); - } - } - - modifier withinLimitOrReturn { - if (registered < limitOfParticipants ) { - _; - }else{ - assert(msg.sender.send(msg.value)); - } - } - - modifier isEnded { - if (ended){ - _; - } - } - - modifier onlyAfter(uint _time) { - if (now > _time){ - _; - } - } - - modifier ifConfirmed(bytes32 _code){ - require(confirmationRepository.claim(_code, msg.sender)); + require(ended == false); _; } @@ -137,16 +95,22 @@ contract Conference is Destructible { RegisterEvent(msg.sender, _participant, ''); } - function registerInternal(string _participant) internal sentDepositOrReturn withinLimitOrReturn onlyActiveOrReturn { + function registerInternal(string _participant) internal { + require(msg.value == deposit); + require(registered < limitOfParticipants); + require(!ended); require(!isRegistered(msg.sender)); + registered++; participantsIndex[registered] = msg.sender; participants[msg.sender] = Participant(_participant, msg.sender, false, false); } - function attendWithConfirmation(bytes32 _confirmation) public ifConfirmed(_confirmation){ + function attendWithConfirmation(bytes32 _confirmation) public { require(isRegistered(msg.sender)); require(!isAttended(msg.sender)); + require(confirmationRepository.claim(_confirmation, msg.sender)); + participants[msg.sender].attended = true; attended++; AttendEvent(msg.sender); @@ -192,14 +156,14 @@ contract Conference is Destructible { /* Admin only functions */ - function payback() onlyOwner{ + function payback() public onlyOwner{ payoutAmount = payout(); ended = true; endedAt = now; PaybackEvent(payoutAmount); } - function cancel() onlyOwner onlyActive{ + function cancel() public onlyOwner onlyActive{ payoutAmount = deposit; cancelled = true; ended = true; @@ -208,10 +172,12 @@ contract Conference is Destructible { } /* return the remaining of balance if there are any unclaimed after cooling period */ - function clear() public onlyOwner isEnded onlyAfter(endedAt + coolingPeriod) { + function clear() public onlyOwner{ + require(now > endedAt + coolingPeriod); + require(ended); var leftOver = totalBalance(); + require(owner.send(leftOver)); ClearEvent(owner, leftOver); - assert(owner.send(leftOver)); } function setLimitOfParticipants(uint _limitOfParticipants) public onlyOwner{ diff --git a/test/conference.js b/test/conference.js index d8ddbe8..5e23844 100644 --- a/test/conference.js +++ b/test/conference.js @@ -209,7 +209,7 @@ contract('Conference', function(accounts) { it('cannot register any more', async function(){ await conference.payback({from:owner}); currentRegistered = await conference.registered.call(); - await conference.register('some handler', {from:notRegistered, value:deposit}); + await conference.register('some handler', {from:notRegistered, value:deposit}).catch(function(){}); assert.strictEqual((await conference.registered.call()).toNumber(), currentRegistered.toNumber()); assert.equal(await conference.ended.call(), true); }) @@ -218,7 +218,7 @@ contract('Conference', function(accounts) { it('cannot attend any more', async function(){ await conference.payback({from:owner}); currentAttended = await conference.attended.call(); - await conference.attend([notAttended], {from:owner}); + await conference.attend([notAttended], {from:owner}).catch(function(){}); assert.strictEqual((await conference.attended.call()).toNumber(), currentAttended.toNumber()); assert.equal(await conference.ended.call(), true); }) @@ -267,7 +267,7 @@ contract('Conference', function(accounts) { it('cannot register any more', async function(){ await conference.cancel(); currentRegistered = await conference.registered.call(); - await conference.register('some handler', {from:notRegistered, value:deposit}); + await conference.register('some handler', {from:notRegistered, value:deposit}).catch(function(){}); assert.strictEqual((await conference.registered.call()).toNumber(), currentRegistered.toNumber()); assert.equal(await conference.ended.call(), true); }) From 8fa8995f9acdefce03a23ad98d75e50197bffd13 Mon Sep 17 00:00:00 2001 From: Makoto Inoue Date: Sun, 8 Oct 2017 20:44:52 +0100 Subject: [PATCH 2/6] Add more test --- test/conference.js | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/test/conference.js b/test/conference.js index 5e23844..ccb31aa 100644 --- a/test/conference.js +++ b/test/conference.js @@ -99,20 +99,30 @@ contract('Conference', function(accounts) { assert.strictEqual(web3.eth.getBalance(conference.address).toNumber(), beforeContractBalance.toNumber()); assert.equal(await conference.isRegistered.call(owner), false); }) + + it('cannot register twice with same address', async function(){ + await conference.register.sendTransaction(twitterHandle, {from:owner, value:deposit}); + await conference.register.sendTransaction(twitterHandle, {from:owner, value:deposit}).catch(function(){}); + assert.strictEqual(web3.eth.getBalance(conference.address).toNumber(), deposit); + assert.equal(await conference.registered.call(), 1); + assert.equal(await conference.isRegistered.call(owner), true); + }) }) describe('on attend', function(){ + var non_registered = accounts[4]; + beforeEach(async function(){ await conference.register(twitterHandle, {value:deposit, from:non_owner}); }) - it('isAttended is true if owner calls attend function', async function(){ + it('can be called by owner', async function(){ await conference.attend([non_owner], {from:owner}); assert.equal(await conference.isAttended.call(non_owner), true); assert.equal((await conference.attended.call()).toNumber(), 1); }) - it('isAttended is false if non owner calls attend function', async function(){ + it('cannot be called by non owner', async function(){ await conference.attend([non_owner], {from:non_owner}).catch(function(){}); assert.equal(await conference.isAttended.call(non_owner), false); assert.equal(await conference.attended.call(), 0); @@ -123,6 +133,20 @@ contract('Conference', function(accounts) { it('isAttended is false if attended function for the account is not called', async function(){ assert.equal(await conference.isAttended.call(owner), false); }) + + it('cannot be attended if the list includes non registered address', async function(){ + await conference.attend([non_owner, non_registered], {from:owner}).catch(function(){}); + assert.equal(await conference.isAttended.call(non_owner), false); + assert.equal(await conference.isAttended.call(non_registered), false); + assert.equal((await conference.attended.call()).toNumber(), 0); + }) + + it('cannot be attended twice', async function(){ + await conference.attend([non_owner], {from:owner}); + await conference.attend([non_owner], {from:owner}).catch(function(){}); + assert.equal(await conference.isAttended.call(non_owner), true); + assert.equal((await conference.attended.call()).toNumber(), 1); + }) }) describe('self attend with code', function(){ @@ -166,6 +190,20 @@ contract('Conference', function(accounts) { assert.equal(await conference.attended.call(), 1); assert.equal(await confirmation.report.call(confirmation_code), non_owner); }) + + it('cannot be attended twice', async function(){ + await conference.register(twitterHandle, {from:non_owner, value:deposit}); + await conference.attend([non_owner], {from:owner}); + await conference.attendWithConfirmation([non_owner], {from:non_owner}).catch(function(){}); + assert.equal(await conference.isAttended.call(non_owner), true); + assert.equal((await conference.attended.call()).toNumber(), 1); + }) + + it('cannot be attended if not registered', async function(){ + await conference.attendWithConfirmation([non_owner], {from:non_owner}).catch(function(){}); + assert.equal(await conference.isAttended.call(non_owner), false); + assert.equal((await conference.attended.call()).toNumber(), 0); + }) }) describe('on payback', function(){ From 8e6a50e1c394b46e69d7ea84a4a9f1db0ba03c63 Mon Sep 17 00:00:00 2001 From: Makoto Inoue Date: Sun, 8 Oct 2017 20:53:49 +0100 Subject: [PATCH 3/6] Add either onlyActive or onlyEnded to all functions --- contracts/Conference.sol | 21 +++++++++++++-------- log/stress_0002.log | 10 +++++----- log/stress_0020.log | 12 ++++++------ log/stress_0100.log | 12 ++++++------ 4 files changed, 30 insertions(+), 25 deletions(-) diff --git a/contracts/Conference.sol b/contracts/Conference.sol index 0733af4..d77ca43 100644 --- a/contracts/Conference.sol +++ b/contracts/Conference.sol @@ -38,7 +38,12 @@ contract Conference is Destructible { /* Modifiers */ modifier onlyActive { - require(ended == false); + require(!ended); + _; + } + + modifier onlyEnded { + require(ended); _; } @@ -85,12 +90,12 @@ contract Conference is Destructible { } } - function registerWithEncryption(string _participant, string _encrypted) public payable{ + function registerWithEncryption(string _participant, string _encrypted) public payable onlyActive{ registerInternal(_participant); RegisterEvent(msg.sender, _participant, _encrypted); } - function register(string _participant) public payable{ + function register(string _participant) public payable onlyActive{ registerInternal(_participant); RegisterEvent(msg.sender, _participant, ''); } @@ -106,7 +111,7 @@ contract Conference is Destructible { participants[msg.sender] = Participant(_participant, msg.sender, false, false); } - function attendWithConfirmation(bytes32 _confirmation) public { + function attendWithConfirmation(bytes32 _confirmation) public onlyActive{ require(isRegistered(msg.sender)); require(!isAttended(msg.sender)); require(confirmationRepository.claim(_confirmation, msg.sender)); @@ -116,7 +121,7 @@ contract Conference is Destructible { AttendEvent(msg.sender); } - function withdraw() public{ + function withdraw() public onlyEnded{ require(payoutAmount > 0); Participant participant = participants[msg.sender]; require(participant.addr == msg.sender); @@ -156,7 +161,7 @@ contract Conference is Destructible { /* Admin only functions */ - function payback() public onlyOwner{ + function payback() public onlyOwner onlyActive{ payoutAmount = payout(); ended = true; endedAt = now; @@ -172,7 +177,7 @@ contract Conference is Destructible { } /* return the remaining of balance if there are any unclaimed after cooling period */ - function clear() public onlyOwner{ + function clear() public onlyOwner onlyEnded{ require(now > endedAt + coolingPeriod); require(ended); var leftOver = totalBalance(); @@ -180,7 +185,7 @@ contract Conference is Destructible { ClearEvent(owner, leftOver); } - function setLimitOfParticipants(uint _limitOfParticipants) public onlyOwner{ + function setLimitOfParticipants(uint _limitOfParticipants) public onlyOwner onlyActive{ limitOfParticipants = _limitOfParticipants; } diff --git a/log/stress_0002.log b/log/stress_0002.log index ce91208..eb8f588 100644 --- a/log/stress_0002.log +++ b/log/stress_0002.log @@ -1,7 +1,7 @@ type gasUsed gasPrice 1ETH*USD gasUsed*gasPrice(Ether) gasUsed*gasPrice(USD) -create 1692580 2 303 0.00338516 1.02570348 +create 1686592 2 303 0.003373184 1.022074752 addConfirmation 89601 2 303 0.000179202 0.054298206 -register 119815 2 303 0.00023963 0.07260789000000001 -batchAttend 67251 2 303 0.000134502 0.040754106 -payback 84325 2 303 0.00016865 0.05110095 -withdraw 37660 2 303 0.00007532 0.022821960000000002 +register 119996 2 303 0.000239992 0.07271757599999999 +batchAttend 67248 2 303 0.000134496 0.040752288000000005 +payback 84552 2 303 0.000169104 0.051238512 +withdraw 37890 2 303 0.00007578 0.02296134 diff --git a/log/stress_0020.log b/log/stress_0020.log index a12b8e1..a71f785 100644 --- a/log/stress_0020.log +++ b/log/stress_0020.log @@ -1,7 +1,7 @@ type gasUsed gasPrice 1ETH*USD gasUsed*gasPrice(Ether) gasUsed*gasPrice(USD) -create 1692580 2 303 0.00338516 1.02570348 -addConfirmation 694106 2 303 0.001388212 0.420628236 -register 119815 2 303 0.00023963 0.07260789000000001 -batchAttend 331086 2 303 0.000662172 0.200638116 -payback 84325 2 303 0.00016865 0.05110095 -withdraw 37660 2 303 0.00007532 0.022821960000000002 +create 1686592 2 303 0.003373184 1.022074752 +addConfirmation 694042 2 303 0.001388084 0.420589452 +register 119996 2 303 0.000239992 0.07271757599999999 +batchAttend 331147 2 303 0.000662294 0.200675082 +payback 84552 2 303 0.000169104 0.051238512 +withdraw 37890 2 303 0.00007578 0.02296134 diff --git a/log/stress_0100.log b/log/stress_0100.log index d3f9e1b..afa8b56 100644 --- a/log/stress_0100.log +++ b/log/stress_0100.log @@ -1,7 +1,7 @@ type gasUsed gasPrice 1ETH*USD gasUsed*gasPrice(Ether) gasUsed*gasPrice(USD) -create 1692580 2 303 0.00338516 1.02570348 -addConfirmation 3380715 2 303 0.00676143 2.04871329 -register 119815 2 303 0.00023963 0.07260789000000001 -batchAttend 1503410 2 303 0.00300682 0.91106646 -payback 84325 2 303 0.00016865 0.05110095 -withdraw 37660 2 303 0.00007532 0.022821960000000002 +create 1686592 2 303 0.003373184 1.022074752 +addConfirmation 3381099 2 303 0.006762198 2.048945994 +register 119996 2 303 0.000239992 0.07271757599999999 +batchAttend 1503663 2 303 0.003007326 0.911219778 +payback 84552 2 303 0.000169104 0.051238512 +withdraw 37890 2 303 0.00007578 0.02296134 From e29760969d497873384d141014c1ae470ec3e73c Mon Sep 17 00:00:00 2001 From: Makoto Inoue Date: Sun, 8 Oct 2017 21:02:00 +0100 Subject: [PATCH 4/6] Remove duplicate check (already called at public functions) --- contracts/Conference.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/Conference.sol b/contracts/Conference.sol index d77ca43..3e6fbee 100644 --- a/contracts/Conference.sol +++ b/contracts/Conference.sol @@ -103,7 +103,6 @@ contract Conference is Destructible { function registerInternal(string _participant) internal { require(msg.value == deposit); require(registered < limitOfParticipants); - require(!ended); require(!isRegistered(msg.sender)); registered++; From 2e4dbb26f3cb036520efee6e4694043b1d9cab68 Mon Sep 17 00:00:00 2001 From: Makoto Inoue Date: Sun, 8 Oct 2017 21:02:12 +0100 Subject: [PATCH 5/6] Add AppConfig --- app_config.js | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 app_config.js diff --git a/app_config.js b/app_config.js new file mode 100644 index 0000000..532cfce --- /dev/null +++ b/app_config.js @@ -0,0 +1,29 @@ +// blockparty specific configs +module.exports = { + development: { + contract_addresses:{ + 'Conference': null, + 'ConfirmationRepository': null + }, + name: 'PRIVATE NET', + etherscan_url: null + }, + test: { + }, + ropsten: { + contract_addresses:{ + 'Conference': null, + 'ConfirmationRepository': null + }, + name: 'ROPSTEN NET', + etherscan_url: 'https://ropsten.etherscan.io' + }, + mainnet: { + contract_addresses:{ + 'Conference': null, + 'ConfirmationRepository': '0xe0e0229484b1088e0a751ddffd05b2e6b833e3a2' + }, + name: 'MAINNET', + etherscan_url: 'https://etherscan.io' + } +}; From fe5457c7f4bfcda127e0b2652d1e83e554b40cee Mon Sep 17 00:00:00 2001 From: Makoto Inoue Date: Sun, 8 Oct 2017 21:05:02 +0100 Subject: [PATCH 6/6] Add set_contract --- scripts/util/set_contract.js | 37 ++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 scripts/util/set_contract.js diff --git a/scripts/util/set_contract.js b/scripts/util/set_contract.js new file mode 100644 index 0000000..b21d6cc --- /dev/null +++ b/scripts/util/set_contract.js @@ -0,0 +1,37 @@ +let arg = require('yargs').argv; +var prompt = require('prompt'); +if (!arg.network) { + throw("Need arg.network") +} + +function getPrompt(){ + return new Promise(function(resolve,reject){ + prompt.get(['confirm'], function (err, result) { + if (result.confirm == 'yes') { + resolve(result); + }else{ + reject("Need confirmation") + } + }); + }); +} + +module.exports = async function(artifacts, contractName){ + const app_config = require('../../app_config.js')[arg.network]; + const fileName = `${contractName}.sol`; + const Artifact = artifacts.require(fileName); + if (app_config.contract_addresses[contractName]) { + console.log(`Overriding ${contractName} with ${app_config.contract_addresses[contractName]}`) + contract = await Artifact.at(app_config.contract_addresses[contractName]); + }else{ + contract = await Artifact.deployed(); + console.log(`Using default address speciied at artifact ${contract.address}`) + } + if (contractName == 'Conference') { + prompt.start(); + var name = await contract.name.call() + console.log(`Are you sure you are intereacting with ${name}? (type "yes" to confirm)`) + await getPrompt(); + } + return contract; +};