-
Notifications
You must be signed in to change notification settings - Fork 39
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
Remove JsonRpcError #82
Conversation
src/types/simulation.rs
Outdated
@@ -55,7 +54,6 @@ pub struct StakeInfo { | |||
pub enum SimulateValidationError<M: Middleware> { | |||
UserOperationRejected { message: String }, | |||
OpcodeValidation { entity: String, opcode: String }, | |||
UserOperationExecution { message: String }, |
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.
@Vid201 I don't think this is needed now. But I need your confirmation on it.
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.
It is needed for this test: https://github.com/eth-infinitism/bundler-spec-tests/blob/master/tests/rpc/test_eth_estimateUserOperationGas.py#L23.
I suggest to setup local environment for running tests, so that these changes won't break the bundler spec tests (https://github.com/eth-infinitism/bundler-spec-tests).
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.
Just start local geth, run bundler (run-bundler-debug
), and deploy entry point. Then you can run tests from bundler-spec-tests repo: pdm run pytest -rA -W ignore::DeprecationWarning --url http://127.0.0.1:3000/ --entry-point 0x0576a174D229E3cFA37253523E645A78A0C91B57 --ethereum-node http://127.0.0.1:8545/
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.
Makes sense. I went through the official spec and didn't find it. It turns out to be in bundler-spec-tests.
src/types/sanity_check.rs
Outdated
@@ -48,9 +47,6 @@ pub enum BadUserOperationError<M: Middleware> { | |||
SenderVerification { | |||
sender: Address, | |||
}, | |||
UserOperationExecution { | |||
message: String, | |||
}, |
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.
@Vid201 I don't think this is needed now. But I need your confirmation on it.
395e898
to
ad7a5e9
Compare
Yeah, you're right. I just run the executor and the tests are failing. |
) | ||
})?; | ||
res.data = serde_json::to_string(&SimulationError::from( | ||
SimulateValidationError::<M>::UnknownError { |
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.
This should probably be handled differently, the UserOperationExecution is never returned.
Check #84 |
After upgrading ethers-rs in #79.
The JsonRpcError is not reliadble source of revert error because the api changed in ethers-rs.
This pr is intended to remove JsonRpcError struct.