feat: support libevm debug handler RPCs#124
feat: support libevm debug handler RPCs#124JonathanOppenheimer wants to merge 12 commits intomainfrom
Conversation
JonathanOppenheimer
requested review from
ARR4N and
StephenButtolph
as code owners
Co-authored-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com> Signed-off-by: Jonathan Oppenheimer <147infiniti@gmail.com>
sae/rpc_test.go
Outdated
| // Reference: https://geth.ethereum.org/docs/interacting-with-geth/rpc/ns-debug | ||
| // Every method is listed below: | ||
| // | ||
| // - debug_blockProfile | ||
| // - debug_cpuProfile | ||
| // - debug_freeOSMemory | ||
| // - debug_gcStats | ||
| // - debug_goTrace | ||
| // - debug_memStats | ||
| // - debug_mutexProfile | ||
| // - debug_setBlockProfileRate | ||
| // - debug_setGCPercent | ||
| // - debug_setMutexProfileFraction | ||
| // - debug_stacks | ||
| // - debug_startCPUProfile | ||
| // - debug_startGoTrace | ||
| // - debug_stopCPUProfile | ||
| // - debug_stopGoTrace | ||
| // - debug_verbosity | ||
| // - debug_vmodule | ||
| // - debug_writeBlockProfile | ||
| // - debug_writeMemProfile | ||
| // - debug_writeMutexProfile |
There was a problem hiding this comment.
Why does someone reading the test code need to know this?
There was a problem hiding this comment.
A reader does not -- I personally wanted all fully "supported" RPCs to 'Command-F-able' in rpc_test.go. I take it this is not worth it to you?
There was a problem hiding this comment.
Comments should generally explain why* something is being done. This one explains what some tangential piece of code does, which is redundant and can also become out of date. We also already have the searchable list at the point of registration.
FWIW the list at the point of registration is only necessary because the upstream rpc package is "magical" and doesn't require explicit registration of methods (like, for example, gRPC and proto). IMO it's an anti-pattern, and the fact that we need to document it like that is evidence of that.
*Not relevant here, but useful knowledge: the problem with "what" and "how" comments is that they're a crutch for the code being difficult to understand. They generally suggest that a refactoring is required, whereas "why" comments can't be communicated with code alone.
There was a problem hiding this comment.
Yup totally fair -- I was more referring to enumerating which RPCs are tested (as it's no longer explicitly clear now that we are only testing two of the RPCs, rather than all of them like in the first iteration), for visibility (following every RPC needs to be explicitly tested). As you said, the list is necessary in rpc.go because it's "magical" -- I was thinking the same here for what's tested. I'll remove!
Co-authored-by: Stephen Buttolph <stephen@avalabs.org> Signed-off-by: Jonathan Oppenheimer <147infiniti@gmail.com>
4 participants
Add support for
{"debug", debug.Handler},and add simple tests to verify that the RPC doesn't explode when called.Stephen probably won't read this.