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

FR: Separate Dangerous Debugs From Useful Debugs #21963

Open
MysticRyuujin opened this issue Dec 4, 2020 · 5 comments
Open

FR: Separate Dangerous Debugs From Useful Debugs #21963

MysticRyuujin opened this issue Dec 4, 2020 · 5 comments

Comments

@MysticRyuujin
Copy link
Contributor

Rationale

The debug_ namespace of the JSON RPC server has some really useful functions available to end user, however, it also has some very dangerous functions in the same namespace. It's impossible to turn on the useful features without also turning on the dangerous ones.

Things like debug_traceTransaction should probably not be in the same namespace as debug_setHead

Alternatively, a useful feature that Nethermind and Tubo-Geth have is a JSON RPC filter, where we can selectively enable or disable specific functions instead of exposing all function under a given namespace.

Implementation

Do you have ideas regarding the implementation of this feature?
I just see the 2 paths listed above as options, move dangerous functions to their own namespace, or let us create a filter, which might be useful for all of the available namespaces.

Are you willing to implement this feature?
I would if I had any idea what I was doing and was a competent Go programmer, but I am not 😄

@ligi
Copy link
Member

ligi commented Dec 6, 2020

Moving might be problematic as this is a breaking change. Also something like this should be coordinated between clients (so Ideally start a thread on eth-magicians and/or an EIP)
So I think the only short-term solution is a filter. Just wondering if it should be part of geth or for the sake of reusability/modularity a proxy in front of geth.

@MysticRyuujin
Copy link
Contributor Author

MysticRyuujin commented Dec 6, 2020

I would agree with the EIP thing if the debug namespace was part of any EIPs, I'm not sure they are. Parity/OE only has debug_getBadBlocks because Martin asked them to implement it.

I'm usually the first one to complain about RPC stuff not being standardized lol

My objective/goal here is to be able to safely expose these RPC endpoints on a node, directly (without a proxy), without the risk of accidentally exposing something dangerous that would affect the database of an Archive Node.

Even turning them on with a proxy in front of it (which we do have now) is still worrying.

I'm perfectly happy with a filter.

@fjl
Copy link
Contributor

fjl commented Dec 9, 2020

I'm also not happy about some of those functions, especially the ones that write files. We should move those to a separate namespace.

I think the method filter is not the best solution. It works, but the APIs should really be structured properly.

@holiman
Copy link
Contributor

holiman commented Feb 4, 2021

@MysticRyuujin in the case of debug_traceTransaction, isn't that one of the methods that you would like to serve to your user, as part of the non-dangerous set? Because if so, then it doesn't really solve anything if we tag it as 'dangerous'.

I think the root problem for that is that we need to limit the amount of data that we shove back over RPC. Normally, a crash in rpc-handing is 'localized' to the goroutine, and doesn't cause a geth-global crash. But OOM is one of the exceptions.

@MysticRyuujin
Copy link
Contributor Author

My linking to this issue is not in response to that call being dangerous, just linking issues because people who care about these RPC methods probably care about this as well.

I don't see these as independent issues, I view it as a systemic design issue with the debug namespace as a whole.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants