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

Control by disableUnsafeDebugFlag #1746

Merged
merged 13 commits into from
Dec 27, 2022

Conversation

kjeom
Copy link
Contributor

@kjeom kjeom commented Dec 23, 2022

Proposed changes

  • This is a follow-up PR to debug namespace enhancement #1672
  • This PR introduces the rpc.unsafe-debug.disable flag
  • If you set the rpc.unsafe-debug.disable flag,
  • How to set the flag
    • add ADDITIONAL="--rpc.unsafe-debug.disable" in kxxd.conf file

Types of changes

Please put an x in the boxes related to your change.

  • Bugfix
  • New feature or enhancement
  • Others

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I have read the CONTRIBUTING GUIDELINES doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes ($ make test)
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in downstream modules

Further comments

Why this new flag rpc.unsafe-debug.disable is introduced and why some debug APIs are disabled by that flag?

  • The debug APIs are for private use in nature, however some public ENs provide debug namespace over RPC for user convenience
  • Some debug APIs are vulnerable. Some debug APIs are too heavy/costly. Some debug APIs are meaningless to be provided over RPC.
  • Nevertheless, we don't want to discourage public ENs to provide some useful & relatively safe debug APIs (e.g., debug_traceTransaction)

Full list of APIs affected

  • debug APIs that are disabled when rpc.unsafe-debug.disable flag was set

    • debug_printBlock
    • debug_setHead
    • debug_startWarmUp, debug_startContractWarmUp, debug_stopWarmUp
    • debug_startCollectingTrieStats
    • debug_chaindbProperty, debug_chaindbCompact
    • debug_verbosity, debug_verbosityByName, debug_verbosityByID
    • debug_vmodule, debug_backtraceAt, debug_stacks, debug_freeOSMemory, debug_setGCPercent, debug_memStats, debug_gcStats, debug_startPProf, debug_stopPProf, debug_isPProfRunning, debug_cpuProfile, debug_startCPUProfile, debug_stopCPUProfile
    • debug_goTrace, debug_startGoTrace, debug_stopGoTrace
    • debug_blockProfile, debug_setBlockProfileRate, debug_writeBlockProfile, debug_mutexProfile, debug_setMutexProfileRate, debug_writeMutexProfile, debug_writeMemProfile
    • debug_standardTraceBadBlockToFile, debug_standardTraceBlockToFile
    • debug_preimage
    • debug_storageRangeAt
    • debug_setVMLogTarget
  • debug APIs that are NOT disabled even if rpc.unsafe-debug.disable flag was set

    • VM tracing APIs, however with limited functionality (only pre-defined tracers are allowed)
      • debug_traceBadBlock, debug_traceBlock, debug_traceBlockByHash, debug_traceBlockByNumber, debug_traceBlockByNumberRange, debug_traceBlockFromFile, debug_traceTransaction, debug_traceChain
    • debug_dumpBlock, debug_dumpStateTrie, debug_getBlockRlp, debug_getModifiedAccountsByHash, debug_getModifiedAccountsByNumber, debug_getBadBlocks, debug_getModifiedStorageNodesByNumber
    • debug_metrics

@kjeom
Copy link
Contributor Author

kjeom commented Dec 23, 2022

@2dvorak I think we need to test #1676 (comment) again.

cmd/utils/flags.go Outdated Show resolved Hide resolved
cmd/utils/nodecmd/nodeflags.go Outdated Show resolved Hide resolved
node/node.go Outdated Show resolved Hide resolved
node/node.go Outdated Show resolved Hide resolved
@blukat29
Copy link
Contributor

Please fix cmd/utils/nodecmd/consolecmd_test.go:ipcAPIs as well.
To check the fix, you'll have to manually run test (because nodecmd is ignored in make test-others).

env GOPATH=$(go env GOPATH) GO111MODULE=on go run build/ci.go test -p 1 ./cmd/utils/nodecmd/

JayChoi1736
JayChoi1736 previously approved these changes Dec 26, 2022
@2dvorak
Copy link
Collaborator

2dvorak commented Dec 26, 2022

Although the --rpc.unsafe-debug.disable flag was set and unsafe-debug APIs are blocked, the node console still shows all debug API list. Can we fix that too?

> debug
{
  backtraceAt: function(),
  blockProfile: function(),
  chaindbCompact: function(),

... more list ...

  writeMemProfile: function(),
  writeMutexProfile: function()
}
> debug.backtraceAt("")
Error: the method debug_backtraceAt does not exist/is not available
    at web3.js:3278:20
    at web3.js:6810:15
    at web3.js:5221:36
    at <anonymous>:1:1

node/node.go Outdated Show resolved Hide resolved
node/cn/backend.go Outdated Show resolved Hide resolved
@2dvorak 2dvorak added the need to merge Need to merge for the next time label Dec 27, 2022
aidan-kwon
aidan-kwon previously approved these changes Dec 27, 2022
Copy link
Member

@aidan-kwon aidan-kwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except for minors

node/cn/tracers/api.go Outdated Show resolved Hide resolved
node/cn/tracers/api.go Outdated Show resolved Hide resolved
@kjeom kjeom merged commit ec95ff8 into klaytn:dev Dec 27, 2022
@blukat29 blukat29 removed the need to merge Need to merge for the next time label Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants