-
Notifications
You must be signed in to change notification settings - Fork 200
Conversation
contract('ZEPToken', ([ _, owner, another, jurisdiction]) => { | ||
const attributeID = 0; | ||
contract('ZEPToken', ([ _, tokenOwner, another, jurisdictionOwner, validatorOwner, zeppelin ]) => { | ||
const receiveTokensAttributeID = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd use a non-zero ID for testing, because EVM
|
||
function assertTokensCannotBeTransferred() { | ||
it('cannot transfer', async function () { | ||
assert.equal(await this.zepToken.canTransfer(recipient, amount, { from: sender }), false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: why not should
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason, the token approvals fail when using should
}) | ||
} | ||
|
||
describe('when the no one was not allowed to receive tokens', function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"the no one" sounds like a creature out of H P Lovecraft :-P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bad copy paste 😂
Looks good! Left some very minor comments. Something that came to my mind though: if the tokens do reach an unintended owner, we have no way to actually freeze them. We can only control recipients in txs, but not senders. Should we consider adding a check in TPLToken that the whitelist attribute is required for both sender and recipient, and not just recipient? |
@facuspagnuolo note there are conflicts on |
dd4d1d5
to
e914e24
Compare
e914e24
to
16e5594
Compare
Fixes #135