New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace Buffer.from(bn.toArray()) with bn.toArrayLike(Buffer) #180
Conversation
@holgerd77 @jwasinger please review this too |
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.
Everything look good to me.
@@ -49,7 +49,7 @@ function VM (opts = {}) { | |||
|
|||
if (this.opts.activatePrecompiles) { | |||
for (var i = 1; i <= 4; i++) { | |||
this.trie.put(Buffer.from(new BN(i).toArray('be', 20)), new Account().serialize()) | |||
this.trie.put(new BN(i).toArrayLike(Buffer, 'be', 20), new Account().serialize()) |
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.
Ok, same resulting buffer objects.
.imul(new BN(b)) | ||
.mod(utils.TWO_POW256) | ||
.toArray('be', 32) | ||
) | ||
.toArrayLike(Buffer, 'be', 32) |
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.
Both ok.
} else { | ||
a = utils.fromSigned(a) | ||
r = utils.toUnsigned(a.div(b)) | ||
return utils.toUnsigned(a.div(b)) |
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.
Both ok.
} else { | ||
a = new BN(a).imul(new BN(b)) | ||
r = a.mod(c).toArray('be', 32) | ||
return a.mod(c).toArrayLike(Buffer, 'be', 32) |
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.
All 3 ok.
} else { | ||
result = Buffer.from([1]) | ||
return Buffer.from([1]) |
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.
Ok.
.inotn(256) | ||
.toArray('be', 32)) | ||
.toArrayLike(Buffer, 'be', 32) |
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.
All 4 ok.
@@ -456,7 +433,7 @@ module.exports = { | |||
return utils.intToBuffer(runState.memoryWordCount * 32) | |||
}, | |||
GAS: function (runState) { | |||
return Buffer.from(runState.gasLeft.toArray('be', 32)) | |||
return runState.gasLeft.toArrayLike(Buffer, 'be', 32) |
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.
Ok.
@@ -733,7 +710,7 @@ module.exports = { | |||
runState.selfdestruct[contractAddress.toString('hex')] = selfdestructToAddress | |||
runState.stopped = true | |||
|
|||
var newBalance = Buffer.from(new BN(contract.balance).add(new BN(toAccount.balance)).toArray()) | |||
var newBalance = new BN(contract.balance).add(new BN(toAccount.balance)).toArrayLike(Buffer) |
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.
Ok, tested one example for equivalency.
@@ -310,7 +310,7 @@ proto.generateGenesis = function (initState, cb) { | |||
var addresses = Object.keys(initState) | |||
async.eachSeries(addresses, function (address, done) { | |||
var account = new Account() | |||
account.balance = Buffer.from((new BN(initState[address])).toArray()) | |||
account.balance = new BN(initState[address]).toArrayLike(Buffer) |
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.
All 3 ok.
just for reference, this is using indutny/bn.js:
|
The point of this is to avoid double serialisation. Currently bn.js is serialised to a Javascript Array which is then ported into a Javascript Buffer. After this change it is directly serialised into a Buffer.