Skip to content

Commit

Permalink
Add guard to ERC20Migrator migrate function (OpenZeppelin#1659)
Browse files Browse the repository at this point in the history
* Add guard to ERC20Migrator migrate function

* Add tests for premature migration in ERC20Migrator

These tests apply to the new guard condition, but they don't
fail without it, since the functions revert anyway.
They are added for completeness and to ensure full code coverage.

* Use context block around premature migration tests

We should use context blocks for situational details
and describe for features or functions.
  • Loading branch information
nikeshnazareth authored and nventuro committed Mar 2, 2019
1 parent 7f54542 commit 3772233
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 0 deletions.
1 change: 1 addition & 0 deletions contracts/drafts/ERC20Migrator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ contract ERC20Migrator {
* @param amount amount of tokens to be migrated
*/
function migrate(address account, uint256 amount) public {
require(address(_newToken) != address(0));
_legacyToken.safeTransferFrom(account, address(this), amount);
_newToken.mint(account, amount);
}
Expand Down
34 changes: 34 additions & 0 deletions test/drafts/ERC20Migrator.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,40 @@ contract('ERC20Migrator', function ([_, owner, recipient, anotherAccount]) {
});
});

context('before starting the migration', function () {
it('returns the zero address for the new token', async function () {
(await this.migrator.newToken()).should.be.equal(ZERO_ADDRESS);
});

describe('migrateAll', function () {
const amount = totalSupply;

describe('when the approved balance is equal to the owned balance', function () {
beforeEach('approving the whole balance to the new contract', async function () {
await this.legacyToken.approve(this.migrator.address, amount, { from: owner });
});

it('reverts', async function () {
await shouldFail.reverting(this.migrator.migrateAll(owner));
});
});
});

describe('migrate', function () {
const amount = new BN(50);

describe('when the amount is equal to the approved value', function () {
beforeEach('approving tokens to the new contract', async function () {
await this.legacyToken.approve(this.migrator.address, amount, { from: owner });
});

it('reverts', async function () {
await shouldFail.reverting(this.migrator.migrate(owner, amount));
});
});
});
});

describe('once migration began', function () {
beforeEach('beginning migration', async function () {
await this.newToken.addMinter(this.migrator.address);
Expand Down

0 comments on commit 3772233

Please sign in to comment.