-
Notifications
You must be signed in to change notification settings - Fork 81
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 eth_chain_id entrypoint #932
Conversation
I've checked the codebase and found 3 places where we can call
I have question regarding the code above, since it is doing kakarot-ssj/crates/contracts/src/account_contract.cairo Lines 243 to 259 in eeac8bb
The code above is easy to refactor, my solution would be move the line 259 ( kakarot-ssj/crates/evm/src/backend/validation.cairo Lines 19 to 24 in eeac8bb
Regarding the code above, I have the same question as the first one I made. I'm thinking that we could call Any guidance toward the questions above is appreciated! |
regarding 1. yes you should call eth_chain_id for 1. and 3. the example:
|
@enitrat sure! I'll modify the code accordingly. |
2990c5f
to
100511d
Compare
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.
lgtm, just add a unit test in the same file :)
okayy, wouldn't it be better to have the test in nvm, i just saw how you're doing the other tests, will do the same! |
yes, in the same module, under a |
@bitfalt there is a merge conflict; can you rebase? |
649e02c
to
6ff3620
Compare
Just rebased everything, I think I didn't mess up the rebase this time. I'm fairly new to doing rebase instead of merge, so it is a bit confusing to me haha. Let me know if there is anything else! |
Add
eth_chain_id
entrypoint to theKakarotCore
contract. Add a new constant to compute 2**53. Draft PR since replacing of all references is still needed, also want to ask feedback regarding implementation (correct types) to @enitrat.Pull Request type
Please check the type of change your PR introduces:
What is the current behavior?
Resolves: #923
What is the new behavior?
Add an entrypoint to the
KakarotCore
contract get theeth_chain_id
from the transactionutils/src/constants.cairo
for 2**53eth_chain_id
entrypointDoes this introduce a breaking change?
This change is