NoBey/openzeppelin-contracts
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Added message string for require() (#1704)

Browse Source 
* Error handling in ERC20 and ERC721

* Added message string for require.

* Fixed solhint errors.

* Updated PR as per issue #1709

* changes as per #1709 and openzeppelin forum.

* Changes in require statement

* Changes in require statement

* build pipeline fix

* Changes as per @nventuro's comment.

* Update revert reason strings.

* Fianal update of revert reason strings.

* WIP: Updating reason strings in test cases

* WIP: Added changes to ERC20 and ERC721

* Fixes linting errors in *.tes.js files

* Achieved 100% code coverage

* Updated the test cases with shouldFail.reverting.withMessage()

* Fix package-lock.

* address review comments

* fix linter issues

* fix remaining revert reasons
This commit is contained in:
Balaji Pachai
2019-04-24 19:47:08 +05:30
committed by Nicolás Venturo
parent 4a0a67b04c
commit 3682c6575c
93 changed files with 769 additions and 423 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
test/payment/PaymentSplitter.test.js
View File

@ -7,27 +7,37 @@ contract('PaymentSplitter', function ([_, owner, payee1, payee2, payee3, nonpaye
const amount = ether('1');
it('rejects an empty set of payees', async function () {
await shouldFail.reverting(PaymentSplitter.new([], []));
await shouldFail.reverting.withMessage(PaymentSplitter.new([], []), 'PaymentSplitter: no payees');
});
it('rejects more payees than shares', async function () {
await shouldFail.reverting(PaymentSplitter.new([payee1, payee2, payee3], [20, 30]));
await shouldFail.reverting.withMessage(PaymentSplitter.new([payee1, payee2, payee3], [20, 30]),
'PaymentSplitter: payees and shares length mismatch'
);
});
it('rejects more shares than payees', async function () {
await shouldFail.reverting(PaymentSplitter.new([payee1, payee2], [20, 30, 40]));
await shouldFail.reverting.withMessage(PaymentSplitter.new([payee1, payee2], [20, 30, 40]),
'PaymentSplitter: payees and shares length mismatch'
);
});
it('rejects null payees', async function () {
await shouldFail.reverting(PaymentSplitter.new([payee1, ZERO_ADDRESS], [20, 30]));
await shouldFail.reverting.withMessage(PaymentSplitter.new([payee1, ZERO_ADDRESS], [20, 30]),
'PaymentSplitter: account is the zero address'
);
});
it('rejects zero-valued shares', async function () {
await shouldFail.reverting(PaymentSplitter.new([payee1, payee2], [20, 0]));
await shouldFail.reverting.withMessage(PaymentSplitter.new([payee1, payee2], [20, 0]),
'PaymentSplitter: shares are 0'
);
});
it('rejects repeated payees', async function () {
await shouldFail.reverting(PaymentSplitter.new([payee1, payee1], [20, 30]));
await shouldFail.reverting.withMessage(PaymentSplitter.new([payee1, payee1], [20, 30]),
'PaymentSplitter: account already has shares'
);
});
context('once deployed', function () {
@ -64,12 +74,16 @@ contract('PaymentSplitter', function ([_, owner, payee1, payee2, payee3, nonpaye
});
it('should throw if no funds to claim', async function () {
await shouldFail.reverting(this.contract.release(payee1));
await shouldFail.reverting.withMessage(this.contract.release(payee1),
'PaymentSplitter: account is not due payment'
);
});
it('should throw if non-payee want to claim', async function () {
await send.ether(payer1, this.contract.address, amount);
await shouldFail.reverting(this.contract.release(nonpayee1));
await shouldFail.reverting.withMessage(this.contract.release(nonpayee1),
'PaymentSplitter: account has no shares'
);
});
it('should distribute funds to payees', async function () {

4
test/payment/escrow/ConditionalEscrow.test.js
View File

@ -26,7 +26,9 @@ contract('ConditionalEscrow', function ([_, owner, payee, ...otherAccounts]) {
it('reverts on withdrawals', async function () {
await this.escrow.deposit(payee, { from: owner, value: amount });
await shouldFail.reverting(this.escrow.withdraw(payee, { from: owner }));
await shouldFail.reverting.withMessage(this.escrow.withdraw(payee, { from: owner }),
'ConditionalEscrow: payee is not allowed to withdraw'
);
});
});
});

8
test/payment/escrow/Escrow.behavior.js
View File

@ -18,7 +18,9 @@ function shouldBehaveLikeEscrow (primary, [payee1, payee2]) {
});
it('only the primary account can deposit', async function () {
await shouldFail.reverting(this.escrow.deposit(payee1, { from: payee2 }));
await shouldFail.reverting.withMessage(this.escrow.deposit(payee1, { from: payee2 }),
'Secondary: caller is not the primary account'
);
});
it('emits a deposited event', async function () {
@ -68,7 +70,9 @@ function shouldBehaveLikeEscrow (primary, [payee1, payee2]) {
});
it('only the primary account can withdraw', async function () {
await shouldFail.reverting(this.escrow.withdraw(payee1, { from: payee1 }));
await shouldFail.reverting.withMessage(this.escrow.withdraw(payee1, { from: payee1 }),
'Secondary: caller is not the primary account'
);
});
it('emits a withdrawn event', async function () {

52
test/payment/escrow/RefundEscrow.test.js
View File

@ -8,8 +8,8 @@ contract('RefundEscrow', function ([_, primary, beneficiary, refundee1, refundee
const refundees = [refundee1, refundee2];
it('requires a non-null beneficiary', async function () {
await shouldFail.reverting(
RefundEscrow.new(ZERO_ADDRESS, { from: primary })
await shouldFail.reverting.withMessage(
RefundEscrow.new(ZERO_ADDRESS, { from: primary }), 'RefundEscrow: beneficiary is the zero address'
);
});
@ -32,17 +32,23 @@ contract('RefundEscrow', function ([_, primary, beneficiary, refundee1, refundee
it('does not refund refundees', async function () {
await this.escrow.deposit(refundee1, { from: primary, value: amount });
await shouldFail.reverting(this.escrow.withdraw(refundee1));
await shouldFail.reverting.withMessage(this.escrow.withdraw(refundee1),
'ConditionalEscrow: payee is not allowed to withdraw'
);
});
it('does not allow beneficiary withdrawal', async function () {
await this.escrow.deposit(refundee1, { from: primary, value: amount });
await shouldFail.reverting(this.escrow.beneficiaryWithdraw());
await shouldFail.reverting.withMessage(this.escrow.beneficiaryWithdraw(),
'RefundEscrow: beneficiary can only withdraw while closed'
);
});
});
it('only the primary account can enter closed state', async function () {
await shouldFail.reverting(this.escrow.close({ from: beneficiary }));
await shouldFail.reverting.withMessage(this.escrow.close({ from: beneficiary }),
'Secondary: caller is not the primary account'
);
const { logs } = await this.escrow.close({ from: primary });
expectEvent.inLogs(logs, 'RefundsClosed');
@ -56,11 +62,15 @@ contract('RefundEscrow', function ([_, primary, beneficiary, refundee1, refundee
});
it('rejects deposits', async function () {
await shouldFail.reverting(this.escrow.deposit(refundee1, { from: primary, value: amount }));
await shouldFail.reverting.withMessage(this.escrow.deposit(refundee1, { from: primary, value: amount }),
'RefundEscrow: can only deposit while active'
);
});
it('does not refund refundees', async function () {
await shouldFail.reverting(this.escrow.withdraw(refundee1));
await shouldFail.reverting.withMessage(this.escrow.withdraw(refundee1),
'ConditionalEscrow: payee is not allowed to withdraw'
);
});
it('allows beneficiary withdrawal', async function () {
@ -70,16 +80,22 @@ contract('RefundEscrow', function ([_, primary, beneficiary, refundee1, refundee
});
it('prevents entering the refund state', async function () {
await shouldFail.reverting(this.escrow.enableRefunds({ from: primary }));
await shouldFail.reverting.withMessage(this.escrow.enableRefunds({ from: primary }),
'RefundEscrow: can only enable refunds while active'
);
});
it('prevents re-entering the closed state', async function () {
await shouldFail.reverting(this.escrow.close({ from: primary }));
await shouldFail.reverting.withMessage(this.escrow.close({ from: primary }),
'RefundEscrow: can only close while active'
);
});
});
it('only the primary account can enter refund state', async function () {
await shouldFail.reverting(this.escrow.enableRefunds({ from: beneficiary }));
await shouldFail.reverting.withMessage(this.escrow.enableRefunds({ from: beneficiary }),
'Secondary: caller is not the primary account'
);
const { logs } = await this.escrow.enableRefunds({ from: primary });
expectEvent.inLogs(logs, 'RefundsEnabled');
@ -93,7 +109,9 @@ contract('RefundEscrow', function ([_, primary, beneficiary, refundee1, refundee
});
it('rejects deposits', async function () {
await shouldFail.reverting(this.escrow.deposit(refundee1, { from: primary, value: amount }));
await shouldFail.reverting.withMessage(this.escrow.deposit(refundee1, { from: primary, value: amount }),
'RefundEscrow: can only deposit while active'
);
});
it('refunds refundees', async function () {
@ -105,15 +123,21 @@ contract('RefundEscrow', function ([_, primary, beneficiary, refundee1, refundee
});
it('does not allow beneficiary withdrawal', async function () {
await shouldFail.reverting(this.escrow.beneficiaryWithdraw());
await shouldFail.reverting.withMessage(this.escrow.beneficiaryWithdraw(),
'RefundEscrow: beneficiary can only withdraw while closed'
);
});
it('prevents entering the closed state', async function () {
await shouldFail.reverting(this.escrow.close({ from: primary }));
await shouldFail.reverting.withMessage(this.escrow.close({ from: primary }),
'RefundEscrow: can only close while active'
);
});
it('prevents re-entering the refund state', async function () {
await shouldFail.reverting(this.escrow.enableRefunds({ from: primary }));
await shouldFail.reverting.withMessage(this.escrow.enableRefunds({ from: primary }),
'RefundEscrow: can only enable refunds while active'
);
});
});
});