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 Humanode Mainnet #86

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add Humanode Mainnet #86

wants to merge 3 commits into from

Conversation

MOZGIII
Copy link

@MOZGIII MOZGIII commented Nov 23, 2024

There seems to be an issue with the scripts (or our chain?) - can you help, mds1?

image

Copy link

vercel bot commented Nov 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
evm-diff ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 4, 2024 3:36am

@mds1
Copy link
Owner

mds1 commented Dec 4, 2024

I just pushed some fixes in #95, though I don't think they will help based on the RPC URL you're using. Can you try again? It looks like your RPC is similar to some of the other domainsToSkip URLs in that it doesn't allow us to simulate eth_call with no to address

@MOZGIII
Copy link
Author

MOZGIII commented Dec 4, 2024

Ok, elaborate on this please! This RPC runs on a reference implementation (i.e. the "official" one) of the Humanode Network node, so if there are any issues with the RPC implementation we should fix them.
We have an EVM compatibility layer on the chain, and if we are missing some functionality it would be nice to know what exactly is missing.

I think I understand what you mean. How would you suggest we implement it though? The calls must have some value set at to, so I suppose it would be default-initialized with zero bytes. Further thinking brings me to a question: is it actually required per spec to allow not setting to? Need to double-check, but if not - it should probably be fixed at the evm-diff side, i.e. to make the system in general be able to show at least something for a given chain, and this "allows eth_call without to" to be showcased like an optional feature.

That said - I know our implementation doesn't support tracing just yet. We are planning to add it in the future. Could this be the reason why evm-diff fails?

@MOZGIII
Copy link
Author

MOZGIII commented Dec 4, 2024

Fixed it at 972a727 (#86)

@MOZGIII
Copy link
Author

MOZGIII commented Dec 4, 2024

Is preview broken? Doesn't work properly for me, but it looks like everything should work...

@mds1
Copy link
Owner

mds1 commented Dec 4, 2024

Setting the to address to the zero address is different than leaving the to address as null or unspecified. 972a727 results in the eth_call executing a regular transaction to the zero address, whereas a null to address results in a contract creation transaction. EVM Diff needs a contract creation, as this means the data bytes are executed directly as initcode, making it easy to check opcode support.

I am unsure what the official RPC spec specifies (if anything) for allowing an empty to address in eth_call.

That said - I know our implementation doesn't support tracing just yet. We are planning to add it in the future. Could this be the reason why evm-diff fails?

No trace methods are used, so this should be ok

Is preview broken? Doesn't work properly for me, but it looks like everything should work...

I believe it currently doesn't work for PRs that come from a fork repo

@MOZGIII
Copy link
Author

MOZGIII commented Dec 4, 2024

Setting the to address to the zero address is different than leaving the to address as null or unspecified. 972a727 results in the eth_call executing a regular transaction to the zero address, whereas a null to address results in a contract creation transaction. EVM Diff needs a contract creation, as this means the data bytes are executed directly as initcode, making it easy to check opcode support.

Ah, I see, makes sense! Looks like indeed can't simulate contract creation. It is a bit odd to do an eth_call to support this from the naming perspective, but why not. I think we can add it.

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