Skip to content
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

feat: add computeCreateAddress helper function in Script #109

Merged
merged 2 commits into from
Jul 10, 2022

Conversation

devanoneth
Copy link
Contributor

No description provided.

@transmissions11
Copy link
Collaborator

ik forge-std doesn't place a large emphasis on comments, but i think given the amount of magic constants in this code its worth using a commented impl like LibRLP https://github.com/Rari-Capital/solmate/blob/v7/src/utils/LibRLP.sol

src/Script.sol Outdated Show resolved Hide resolved
src/Script.sol Outdated
@@ -11,4 +11,50 @@ abstract contract Script {
address(bytes20(uint160(uint256(keccak256('hevm cheat code')))));

Vm public constant vm = Vm(VM_ADDRESS);

// Calcuate the corresponding contract creation address for a given address and nonce
function addressFrom(address origin, uint256 nonce) internal pure returns (address creation) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thoughts on renaming to something like computeCreateAddress, that way adding another computeCreate2Address fits nicely?

@devanoneth
Copy link
Contributor Author

ik forge-std doesn't place a large emphasis on comments, but i think given the amount of magic constants in this code its worth using a commented impl like LibRLP https://github.com/Rari-Capital/solmate/blob/v7/src/utils/LibRLP.sol

Nice point, I've updated it to use an impl based on that one, thanks!

@devanoneth devanoneth changed the title feat: add addressFrom helper function in Script feat: add computeCreateAddress helper function in Script Jul 10, 2022
Copy link
Member

@brockelmore brockelmore left a comment

Choose a reason for hiding this comment

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

lgtm, barring name change

@devanoneth
Copy link
Contributor Author

@brockelmore would you prefer addressFrom or something else?

@brockelmore
Copy link
Member

computeCreateAddress re @mds1's suggestion

@devanoneth
Copy link
Contributor Author

@brockelmore agreed, already changed to that in my last commit ser

@brockelmore
Copy link
Member

lmfao sorry, will merge

@brockelmore brockelmore merged commit 914702a into foundry-rs:master Jul 10, 2022
@devanoneth devanoneth deleted the feat/addressFrom branch July 10, 2022 20:26
@brockelmore
Copy link
Member

just realized this broke solidity 0.6.x compiler version cuz CI was tightly configured to only run for certain contributors :(. @devanonon can you try to fix, sorry

@devanoneth
Copy link
Contributor Author

@brockelmore Just realized that too and am on it, no worries :)

brockelmore added a commit that referenced this pull request Jul 10, 2022
@devanoneth devanoneth restored the feat/addressFrom branch July 10, 2022 21:04
devanoneth added a commit to devanoneth/forge-std that referenced this pull request Jul 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants