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

Add get_create2_address() #80

Merged
merged 2 commits into from
Nov 11, 2024
Merged

Add get_create2_address() #80

merged 2 commits into from
Nov 11, 2024

Conversation

fjarri
Copy link
Collaborator

@fjarri fjarri commented Nov 10, 2024

Supersedes #79

@artemki2077, compared to your original proposal I changed salt to be an integer argument - do you think it is sensible? The examples I found online seem to be all using uint256 as its type, but if in your usage you would have to call .to_bytes() every time, we may as well have it as bytes.

Copy link

codecov bot commented Nov 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
pons/__init__.py 100.00% <100.00%> (ø)
pons/_utils.py 100.00% <100.00%> (ø)

@artemki2077
Copy link

Supersedes #79

@artemki2077, compared to your original proposal I changed salt to be an integer argument - do you think it is sensible? The examples I found online seem to be all using uint256 as its type, but if in your usage you would have to call .to_bytes() every time, we may as well have it as bytes.

well, actually I would use bytes. Since I was initially experimenting with create 2 and there was a problem that for some reason the address was calculated incorrectly. As I suspect, this was due to the fact that in solidity uint256 takes 32 bytes, and in python int is translated without a fixed byte length value, which resulted in a different hash, so I used the written function, as well as the salt parameter in the function in the contract translated into bytes, since both there and there the bytes are the same as in python, and there were no problems. Mb I'm wrong

@artemki2077
Copy link

Supersedes #79
@artemki2077, compared to your original proposal I changed salt to be an integer argument - do you think it is sensible? The examples I found online seem to be all using uint256 as its type, but if in your usage you would have to call .to_bytes() every time, we may as well have it as bytes.

well, actually I would use bytes. Since I was initially experimenting with create 2 and there was a problem that for some reason the address was calculated incorrectly. As I suspect, this was due to the fact that in solidity uint256 takes 32 bytes, and in python int is translated without a fixed byte length value, which resulted in a different hash, so I used the written function, as well as the salt parameter in the function in the contract translated into bytes, since both there and there the bytes are the same as in python, and there were no problems. Mb I'm wrong

I'm sorry, I just noticed that the function is converting int to 32 bytes, so there are no problems. Cool

@fjarri
Copy link
Collaborator Author

fjarri commented Nov 11, 2024

I actually thought about it and returned the original bytes salt. Thought about making it strongly typed (like Address), but perhaps that's too much. Also I need to export TypedData from ethereum-rpc for that.

Added the get_create_address() as well.

@fjarri fjarri merged commit 4a33023 into master Nov 11, 2024
8 checks passed
@fjarri
Copy link
Collaborator Author

fjarri commented Nov 11, 2024

Do you want me to make a release, or did you want to add something else?

@fjarri fjarri deleted the create2 branch November 11, 2024 06:19
@artemki2077
Copy link

Do you want me to make a release, or did you want to add something else?

no, I wanted to add get_create_address, but as I looked, you have already implemented it

@fjarri
Copy link
Collaborator Author

fjarri commented Nov 12, 2024

Published v0.8.1 with this PR.

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.

2 participants