Skip to content
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

Escrows #1014

Merged
merged 15 commits into from
Jul 3, 2018
Merged

Escrows #1014

Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
2d2075a
Added basic Escrow
nventuro Jun 8, 2018
b879f4c
PullPayment now uses an Escrow, removing all trust from the contract
nventuro Jun 15, 2018
48b64ce
Abstracted the Escrow tests to a behaviour
nventuro Jun 15, 2018
3600240
Added ConditionalEscrow
nventuro Jun 15, 2018
fe63f2e
Added RefundableEscrow.
nventuro Jun 15, 2018
3a33ffb
RefundableCrowdsale now uses a RefundEscrow, removed RefundVault.
nventuro Jun 15, 2018
d389fd3
Merge branch 'master' into feat/escrow
nventuro Jun 18, 2018
e01edb3
Renaming after code review.
nventuro Jun 18, 2018
5dfb9ce
Added log test helper.
nventuro Jun 18, 2018
6166248
Now allowing empty deposits and withdrawals.
nventuro Jun 18, 2018
cd84c37
Style fixes.
nventuro Jun 18, 2018
dabb3d7
Minor review comments.
nventuro Jun 18, 2018
4422858
Add Deposited and Withdrawn events, removed Refunded
nventuro Jun 19, 2018
be8d65f
The base Escrow is now Ownable, users of it (owners) must provide met…
nventuro Jul 2, 2018
597aba7
Merge branch 'master' into feat/escrow
frangio Jul 3, 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
11 changes: 4 additions & 7 deletions test/payment/RefundEscrow.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import EVMRevert from '../helpers/EVMRevert';
import { inLogs } from '../helpers/expectEvent';
Copy link
Contributor

Choose a reason for hiding this comment

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

The preferred usage for better readability is:

import expectEvent from '../helpers/expectEvent';

expectEvent.inLogs(receipt.logs);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought that made more sense, but preferred to copy the existing code style :/

Copy link
Contributor

@frangio frangio Jun 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

There's both styles, unfortunately.

https://github.com/OpenZeppelin/openzeppelin-solidity/blob/f18c3bc438b366f9cb3a8613f5be160c2cbced5e/test/ownership/Whitelist.test.js#L49-L53


const BigNumber = web3.BigNumber;

Expand Down Expand Up @@ -40,8 +41,7 @@ contract('RefundEscrow', function ([owner, beneficiary, refundee1, refundee2]) {

const receipt = await this.escrow.close({ from: owner });

receipt.logs.length.should.equal(1);
receipt.logs[0].event.should.equal('Closed');
await inLogs(receipt.logs, 'Closed');
});

context('closed state', function () {
Expand Down Expand Up @@ -73,8 +73,7 @@ contract('RefundEscrow', function ([owner, beneficiary, refundee1, refundee2]) {

const receipt = await this.escrow.enableRefunds({ from: owner });

receipt.logs.length.should.equal(1);
receipt.logs[0].event.should.equal('RefundsEnabled');
inLogs(receipt.logs, 'RefundsEnabled');
});

context('refund state', function () {
Expand All @@ -96,9 +95,7 @@ contract('RefundEscrow', function ([owner, beneficiary, refundee1, refundee2]) {

refundeeFinalBalance.sub(refundeeInitialBalance).should.be.bignumber.equal(amount);

receipt.logs.length.should.equal(1);
receipt.logs[0].event.should.equal('Refunded');
receipt.logs[0].args.refundee.should.equal(refundee);
inLogs(receipt.logs, 'Refunded', { refundee });
receipt.logs[0].args.weiAmount.should.be.bignumber.equal(amount);
Copy link
Contributor

Choose a reason for hiding this comment

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

Notice that the helper returns the event, so you can do

const event = expectEvent.inLogs(receipt.logs, 'Refunded', { refundee });
event.args.weiAmount.should.be.bignumber.equal(amount);

In this way you also stop relying on the event being at index 0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Neat, I missed that bit, ty!

}
});
Expand Down