Skip to content

fix: bounty tests, superceeds #799 #1112

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nventuro merged 2 commits into from
Jul 26, 2018
Merged

fix: bounty tests, superceeds #799 #1112

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
157d2a7
fix: bounty tests, superceeds #799
shrugs Jul 25, 2018
0e843b2
fix: updates from PR notes
shrugs Jul 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
158 changes: 67 additions & 91 deletions test/Bounty.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,113 +1,89 @@
const { ethGetBalance, ethSendTransaction } = require('./helpers/web3');
const expectEvent = require('./helpers/expectEvent');
const { assertRevert } = require('./helpers/assertRevert');

var SecureTargetBounty = artifacts.require('SecureTargetBounty');
var InsecureTargetBounty = artifacts.require('InsecureTargetBounty');
const SecureTargetBounty = artifacts.require('SecureTargetBounty');
const InsecureTargetBounty = artifacts.require('InsecureTargetBounty');

require('chai')
.use(require('chai-bignumber')(web3.BigNumber))
.should();

const sendReward = async (from, to, value) => ethSendTransaction({
from, to, value,
from,
to,
value,
});

function awaitEvent (event, handler) {
return new Promise((resolve, reject) => {
function wrappedHandler (...args) {
Promise.resolve(handler(...args)).then(resolve).catch(reject);
}
const reward = web3.toWei(1, 'ether');

event.watch(wrappedHandler);
});
}
contract('Bounty', function ([_, owner, researcher]) {
describe('Against secure contract', function () {
beforeEach(async function () {
this.bounty = await SecureTargetBounty.new({ from: owner });
});

contract('Bounty', function (accounts) {
it('sets reward', async function () {
let owner = accounts[0];
let reward = web3.toWei(1, 'ether');
let bounty = await SecureTargetBounty.new();
await sendReward(owner, bounty.address, reward);
it('sets reward', async function () {
Copy link
Contributor

@nventuro nventuro Jul 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd remove this it altogether, it's already being tested for in all other test cases (and if we were to add it, we should have it on both describes).

Copy link
Contributor Author

@shrugs shrugs Jul 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's ok to be explicit here. I've refactored the tests in the next commit to make this cleaner

await sendReward(owner, this.bounty.address, reward);

const balance = await ethGetBalance(bounty.address);
assert.equal(reward, balance.toNumber());
});
const balance = await ethGetBalance(this.bounty.address);
balance.should.be.bignumber.eq(reward);
});

it('empties itself when destroyed', async function () {
let owner = accounts[0];
let reward = web3.toWei(1, 'ether');
let bounty = await SecureTargetBounty.new();
await sendReward(owner, bounty.address, reward);
it('empties itself when destroyed', async function () {
Copy link
Contributor

@nventuro nventuro Jul 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we should be testing this: this is not Bounty's code, but Destructible's.

await sendReward(owner, this.bounty.address, reward);

const balance = await ethGetBalance(bounty.address);
assert.equal(reward, balance.toNumber());
const balance = await ethGetBalance(this.bounty.address);
balance.should.be.bignumber.eq(reward);

await bounty.destroy();
const updatedBalance = await ethGetBalance(bounty.address);
assert.equal(0, updatedBalance.toNumber());
});
await this.bounty.destroy({ from: owner });

const updatedBalance = await ethGetBalance(this.bounty.address);
updatedBalance.should.be.bignumber.eq(0);
});

describe('Against secure contract', function () {
it('cannot claim reward', async function () {
let owner = accounts[0];
let researcher = accounts[1];
let reward = web3.toWei(1, 'ether');
let bounty = await SecureTargetBounty.new();
let event = bounty.TargetCreated({});

let watcher = async function (err, result) {
event.stopWatching();
if (err) { throw err; }

var targetAddress = result.args.createdAddress;
await sendReward(owner, bounty.address, reward);

const balance = await ethGetBalance(bounty.address);
assert.equal(reward, balance.toNumber());

try {
await bounty.claim(targetAddress, { from: researcher });
assert.isTrue(false); // should never reach here
} catch (error) {
let reClaimedBounty = await bounty.claimed.call();
assert.isFalse(reClaimedBounty);
}
try {
await bounty.withdrawPayments({ from: researcher });
Copy link
Contributor Author

@shrugs shrugs Jul 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no idea what this was testing; the transaction actually goes through but obviously withdraws 0 wei. this is covered in pullpayment test, so I removed it here

assert.isTrue(false); // should never reach here
} catch (err) {
const updatedBalance = await ethGetBalance(bounty.address);
assert.equal(reward, updatedBalance.toNumber());
}
};
await bounty.createTarget({ from: researcher });
await awaitEvent(event, watcher);
const result = await this.bounty.createTarget({ from: researcher });
const event = await expectEvent.inLogs(
result.logs,
'TargetCreated'
);
Copy link
Contributor

@nventuro nventuro Jul 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, but why the newlines? The same line on the other it doesn't have them.


const targetAddress = event.args.createdAddress;
await sendReward(owner, this.bounty.address, reward);

const balance = await ethGetBalance(this.bounty.address);
balance.should.be.bignumber.eq(reward);

await assertRevert(
this.bounty.claim(targetAddress, { from: researcher }),
);
});
});

describe('Against broken contract', function () {
beforeEach(async function () {
this.bounty = await InsecureTargetBounty.new();
});

it('claims reward', async function () {
let owner = accounts[0];
let researcher = accounts[1];
let reward = web3.toWei(1, 'ether');
let bounty = await InsecureTargetBounty.new();
let event = bounty.TargetCreated({});

let watcher = async function (err, result) {
event.stopWatching();
if (err) { throw err; }
let targetAddress = result.args.createdAddress;
await sendReward(owner, bounty.address, reward);

const balance = await ethGetBalance(bounty.address);
assert.equal(reward, balance.toNumber());

await bounty.claim(targetAddress, { from: researcher });
let claim = await bounty.claimed.call();

assert.isTrue(claim);

await bounty.withdrawPayments({ from: researcher });
const updatedBalance = await ethGetBalance(bounty.address);
assert.equal(0, updatedBalance.toNumber());
};
await bounty.createTarget({ from: researcher });
await awaitEvent(event, watcher);
const result = await this.bounty.createTarget({ from: researcher });
const event = await expectEvent.inLogs(result.logs, 'TargetCreated');

const targetAddress = event.args.createdAddress;
await sendReward(owner, this.bounty.address, reward);

const balance = await ethGetBalance(this.bounty.address);
balance.should.be.bignumber.eq(reward);

await this.bounty.claim(targetAddress, { from: researcher });
const claim = await this.bounty.claimed.call();
Copy link
Contributor

@nventuro nventuro Jul 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you write .claimed()?


claim.should.eq(true);

await this.bounty.withdrawPayments({ from: researcher });
const updatedBalance = await ethGetBalance(this.bounty.address);
updatedBalance.should.be.bignumber.eq(0);
Copy link
Contributor

@nventuro nventuro Jul 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also test:

a) that the researcher got the reward (curr - prev balances = reward)
b) that the bounty is no longer payable after claiming

});
});
});