-
Notifications
You must be signed in to change notification settings - Fork 134
proxy: add evm based functionality #3454
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
Conversation
|
||
let callResult = (await vp.evm.call(header, tx, optimisticStateFetch)).valueOr: | ||
raise newException(ValueError, error) | ||
|
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.
You may want to add something like this to better handle the revert scenario:
https://github.com/status-im/nimbus-eth1/pull/3391/files#diff-eba712bc02a7203c0d7e0f5a52d9a85763dd9c523bcf1a24a8b8068ba7fb8068
if callResult.error.len() > 0:
raise (ref ApplicationError)(
code: 3,
msg: callResult.error,
data: Opt.some(JsonString("\"" & callResult.output.to0xHex() & "\"")),
)
This was added into the Nimbus Portal eth_call
RPC a while back.
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.
Noted. #3101 added error handling to the meta issue
@chirag-parmar The performance of these RPC endpoints that use the Async EVM without caching will be rather slow so I would prioritize getting that caching change in. Have you tested/tried running |
I ran a couple of manual tests but mostly for in memory blocks. But I agree, caching is high priority. Currently working on that |
related: #3100
adds evm bases RPC calls -
eth_call
,eth_estimateGas
andeth_createAccessList
meta issue: #3101