-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 parity-specific listStorageKeys RPC. #1145
Conversation
Thanks!
Yes, we allow three formats:
We have a middleware that looks for address inputs, which we have defined here: web3.py/web3/_utils/rpc_abi.py Lines 35 to 39 in 0cbb1e4
See Also, note that another PR should be opened as a backport to the |
I registered my function in the middleware you pointed me to. It seems to work now :) |
@@ -57,6 +57,8 @@ | |||
'personal_unlockAccount': ['address', None, None], | |||
'personal_sign': [None, 'address', None], | |||
'trace_call': TRACE_PARAMS_ABIS, | |||
# parity | |||
'parity_listStorageKeys': ['address', None, None, None], |
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 seems fine for now, but in general, we probably want a different solution. It seems weird to have this included in this default middleware, for a non-default module. (Comment mostly intended for maintainers)
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.
I've been thinking about
- global middlewares (our current solution)
- module middlewares (what we'd need to keep this change out of the global namespace and only on the
web3.parity
module) - method normalizers (part of what we talked about in the
asyncio
work of moving thepythonic
middleware functionality into the methods themselves). - contract middleware (middlewares that are only used for web3 interactions initiated from a
Contract
).
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.
Ah yes, that's familiar. I guess in that framework this is a method normalizer, really, since it depends on argument type and order.
Contract middleware is one I don't remember talking about. Do you have an example? Are you thinking of attaching a middleware to a specific contract instance, or across all, or what?
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.
attaching a middleware to a specific contract instance
This, though thinking through it there isn't a lot that this gives us that couldn't be accomplished by pushing users towards using separate w3
instances in cases where they want different behavior across different contract instances (think in-flight signing, or gas estimations).
I love the ascii bunny, too XD |
What was wrong?
There was no support for the parity_listStorageKeys RPC:
https://wiki.parity.io/JSONRPC-parity-module#parity_liststoragekeys
How was it fixed?
I added a function to implement it in the parity module.
How was it tested?
I tested locally, and also ran the parity integration tests.
I added a test, but because the current integration tests doesn't seem to use "--fat-db", parity returns None. I make sure it does.
However, the test is run in two variants, and one fails:
tests/integration/parity/test_parity_http.py::TestParityTraceModuleTest::test_list_storage_keys_no_support[<lambda>]
Where this one passes:
tests/integration/parity/test_parity_http.py::TestParityTraceModuleTest::test_list_storage_keys_no_support[address_conversion_func1]
The failure is because the address seems to be passed as raw bytes, and cannot be serialized to json. That's supposed to be allowed? How to handle that? I don't see code to handle it in the other RPCs.
Cute Animal Picture
(that's an ASCII Bunny viewed from behind)