Skip to content
This repository has been archived by the owner on Jun 17, 2021. It is now read-only.

add generateAddress2 for CREATE2 #146

Merged
merged 3 commits into from
Oct 5, 2018
Merged

add generateAddress2 for CREATE2 #146

merged 3 commits into from
Oct 5, 2018

Conversation

jwasinger
Copy link
Contributor

@jwasinger jwasinger commented Aug 4, 2018

Precursor to EIP 1014 implementation on EthereumJS VM . Need to add unit tests.

There is currently an open PR #1375 on the EIPs repository which represents the more accurate state of the EIP.

@coveralls
Copy link

coveralls commented Aug 4, 2018

Coverage Status

Coverage increased (+0.02%) to 99.24% when pulling 90d20a0 on create2 into 5c3b651 on master.

@jwasinger jwasinger force-pushed the create2 branch 2 times, most recently from 83d335b to 0070686 Compare August 4, 2018 06:11
index.js Outdated
inputCode = exports.toBuffer(inputCode)

let address = exports.keccak256(Buffer.concat([
Buffer.from('0xff', 'hex'),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't from be part of the hash?

as mentioned in ethereum/EIPs#1014 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup.. good call.

@holgerd77
Copy link
Member

This needs to be updated and the tests fixed.

@holgerd77
Copy link
Member

Have updated this according to the latest discussions. Existing tests should now run through, this still needs specific test cases for the new function though. Have asked on the EIP update PR for some examples.

@holgerd77
Copy link
Member

Update: examples on the EIP update PR page (should be merged soon) are now ready, can be added as test cases here.

rmeissner
rmeissner previously approved these changes Sep 29, 2018
*/
exports.generateAddress2 = function (from, salt, initCode) {
from = exports.toBuffer(from)
salt = exports.toBuffer(salt)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the length of the buffers be checked (as they should be 20 and 32, else the hashes would differ, right?)

@holgerd77
Copy link
Member

Yes, that makes sense

rmeissner
rmeissner previously approved these changes Oct 2, 2018
@rmeissner
Copy link

@jwasinger @holgerd77 Could this be merged? ethereumjs/ethereumjs-monorepo#329 depends on it :)

@holgerd77
Copy link
Member

Will take a look later the day. Would like to add the latest changes to the example files with the gas cost additions, just to be complete here.

@holgerd77
Copy link
Member

@rmeissner Ok, updated the example tests file and will merge once CI is through. Won't make it to do a release today, will have to think a bit more thoroughly on how to organize that and what to put inside. Hopefully tomorrow.

@holgerd77
Copy link
Member

Current idea would be to merge #143 (EIP-155 support), merge this one and then do a v5.3.0 release.

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good now.

@holgerd77 holgerd77 merged commit d85a78f into master Oct 5, 2018
@holgerd77 holgerd77 deleted the create2 branch October 5, 2018 10:07
@holgerd77
Copy link
Member

Update on a release: forgot about the signature changes to comply with Geth and Parity signature scheme introduced in #131 being breaking and not yet released.

To be more on the safe side I will do a v6.0.0 release together with a PR I will prepare to remove the deprecated renamed constants and methods from v5.2.0(e.g. SHA3_NULL_S -> KECCAK256_NULL_S).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants