-
Notifications
You must be signed in to change notification settings - Fork 274
Updated isPrecomile according to Byzantium precompile-used address space #115
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -411,11 +411,17 @@ describe('isPrecompiled', function () { | |
assert.equal(ethUtils.isPrecompiled('0000000000000000000000000000000000000002'), true) | ||
assert.equal(ethUtils.isPrecompiled('0000000000000000000000000000000000000003'), true) | ||
assert.equal(ethUtils.isPrecompiled('0000000000000000000000000000000000000004'), true) | ||
assert.equal(ethUtils.isPrecompiled('0000000000000000000000000000000000000005'), true) | ||
assert.equal(ethUtils.isPrecompiled('0000000000000000000000000000000000000006'), true) | ||
assert.equal(ethUtils.isPrecompiled('0000000000000000000000000000000000000007'), true) | ||
assert.equal(ethUtils.isPrecompiled('0000000000000000000000000000000000000008'), true) | ||
assert.equal(ethUtils.isPrecompiled(Buffer.from('0000000000000000000000000000000000000001', 'hex')), true) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for (let i = 1; i < 9; ++i) {
assert.true(ethUtils.isPrecompiled(new BN(i).toString(10, 40)))
assert.true(ethUtils.isPrecompiled(new BN(i).toBuffer('be', 20)))
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is definitely more elegant, thanks for the suggestion. Unfortunately it kills the browser tests somehow due to Buffer usage, just tested it, and I didn't want to go deeper in debugging here, think this would be not worth it in this case. Is it ok to leave it as is? Also, can you approve, If you are ok with both explanations? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, this is ok let it as is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool, thanks. |
||
}) | ||
it('should return false', function () { | ||
assert.equal(ethUtils.isPrecompiled('0000000000000000000000000000000000000000'), false) | ||
assert.equal(ethUtils.isPrecompiled('0000000000000000000000000000000000000005'), false) | ||
assert.equal(ethUtils.isPrecompiled('0000000000000000000000000000000000000009'), false) | ||
assert.equal(ethUtils.isPrecompiled('1000000000000000000000000000000000000000'), false) | ||
assert.equal(ethUtils.isPrecompiled(Buffer.from('0000000000000000000000000000000000000000', 'hex')), 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.
Add
isValidAddress
?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.
Hmm, normally your are right. I wouldn't want this here, since the
IsPrecompiled
method is removed anyhow (citing @axic) on next major release. AddingisValidAddress
would change the behavior here and would cause extra/unnecessary problems for people already have this in use.