8000 Control by disableUnsafeDebugFlag by kjeom · Pull Request #1746 · klaytn/klaytn · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Aug 19, 2024. It is now read-only.

Control by disableUnsafeDebugFlag #1746

Merged
merged 13 commits into from
Dec 27, 2022
Merged

Control by disableUnsafeDebugFlag #1746

merged 13 commits into from
Dec 27, 2022

Conversation

kjeom
Copy link
@kjeom kjeom commented Dec 23, 2022 •
edited by 2dvorak
Loading

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_standardTraceBlock 8000 ToFile
    • 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 kjeom self-assigned this Dec 23, 2022
@kjeom kjeom added this to the v1.10 (Kore) milestone Dec 23, 2022
@kjeom
Copy link
Author
kjeom commented Dec 23, 2022

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

aidan-kwon
aidan-kwon reviewed Dec 25, 2022
View reviewed changes
@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
View reviewed changes
2dvorak
2dvorak reviewed Dec 26, 2022
View reviewed changes
@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

aidan-kwon
aidan-kwon reviewed Dec 27, 2022
View reviewed changes
aidan-kwon
aidan-kwon reviewed Dec 27, 2022
View reviewed changes
@aidan-kwon aidan-kwon mentioned this pull request Dec 27, 2022
Closed
7 tasks
@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
View reviewed changes
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

aidan-kwon
aidan-kwon reviewed Dec 27, 2022
View reviewed changes
aidan-kwon
aidan-kwon reviewed Dec 27, 2022
View reviewed changes
@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 subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@2dvorak 2dvorak 2dvorak approved these changes

@aidan-kwon aidan-kwon aidan-kwon approved these changes

@KimKyungup KimKyungup Awaiting requested review from KimKyungup

@ehnuje ehnuje Awaiting requested review from ehnuje

@henry-will henry-will Awaiting requested review from henry-will

@hyunsooda hyunsooda Awaiting requested review from hyunsooda hyunsooda is a code owner

@jimni1222 jimni1222 Awaiting requested review from jimni1222

@nohkwak nohkwak Awaiting requested review from nohkwak

@kjhman21 kjhman21 Awaiting requested review from kjhman21

@jeongkyun-oh jeongkyun-oh Awaiting requested review from jeongkyun-oh

@JayChoi1736 JayChoi1736 Awaiting requested review from JayChoi1736 JayChoi1736 is a code owner

Assignees

@kjeom kjeom

Labels
None yet
Projects
None yet
Milestone
v1.10.0 (Kore)
Development

Successfully merging this pull request may close these issues.
5 participants
@kjeom @blukat29 @2dvorak @aidan-kwon @JayChoi1736
0