Updated isPrecomile according to Byzantium precompile-used address space #115
Conversation
|
@Silur Hi, thanks for the quick ES5 PR review, can you also do a short review of this (nothing exciting)? Would like to have this included before we do a release. |
| @@ -510,13 +510,13 @@ exports.generateAddress = function (from, nonce) { | |||
| } | |||
|
|
|||
| /** | |||
| * Returns true if the supplied address belongs to a precompiled account | |||
| * Returns true if the supplied address belongs to a precompiled account (Byzantium) | |||
| * @param {Buffer|String} address | |||
| * @return {Boolean} | |||
| */ | |||
| exports.isPrecompiled = function (address) { | |||
| const a = exports.unpad(address) | |||
There was a problem hiding this comment.
Hmm, normally your are right. I wouldn't want this here, since the IsPrecompiled method is removed anyhow (citing @axic) on next major release. Adding isValidAddress would change the behavior here and would cause extra/unnecessary problems for people already have this in use.
| 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.
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.
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.
Yea, this is ok let it as is.
Addresses #110