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

Expose unsafe debug APIs via IPC #2075

Merged
merged 10 commits into from
Jan 5, 2024
Merged

Expose unsafe debug APIs via IPC #2075

merged 10 commits into from
Jan 5, 2024

Conversation

sjnam
Copy link
Contributor

@sjnam sjnam commented Dec 19, 2023

Proposed changes

Unsafe debug APIs are

  • available if attached via IPC (i.e. ken attach klay.ipc) regardless of --rpc.unsafe-debug.disable flag nor --rpcapi
  • available if attached via HTTP or WS AND --rpc.unsafe-debug.disable == false AND --rpcapi contains debug
  • not available otherwise

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

Related issues

  • Please leave the issue numbers or links related to this PR here.

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

@sjnam sjnam self-assigned this Dec 19, 2023
@sjnam sjnam marked this pull request as draft December 19, 2023 03:59
@sjnam sjnam changed the title Unsafe debug API Expose unsafe debug APIs via IPC Dec 19, 2023
@sjnam sjnam marked this pull request as ready for review December 19, 2023 22:45
hyunsooda
hyunsooda previously approved these changes Dec 20, 2023
@hyunsooda
Copy link
Contributor

Question for the last two commits, Has the semantics changed to disallow unsafe APIs via RPC even when the rpc.unsafe-debug.disable option is provided?

@sjnam sjnam marked this pull request as draft December 21, 2023 04:53
hyunsooda
hyunsooda previously approved these changes Dec 26, 2023
@sjnam sjnam marked this pull request as ready for review December 26, 2023 00:42
Copy link
Collaborator

@2dvorak 2dvorak left a comment

Choose a reason for hiding this comment

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

@sjnam Seems like debug.traceTransaction works in the opposite way.

root@EN-0:/# ken attach /klaytn/klay.ipc
> debug.traceTransaction("0x4dd0c11d2430acdd73153a94ab900a025010d113a68097c042511f3768352775", {tracer: '{data: [], step: function(log) {}, result: function() { return this.data; }, fault: function(){}}'})
Error: Only predefined tracers are supported
        at web3.js:6810:9(39)
        at send (web3.js:5221:62(29))
        at <eval>:1:23(6)

root@EN-0:/# ken attach http://localhost:8551
> debug.traceTransaction("0x4dd0c11d2430acdd73153a94ab900a025010d113a68097c042511f3768352775", {tracer: '{data: [], step: function(log) {}, result: function() { return this.data; }, fault: function(){}}'})
[]

Could you have a look?

node/cn/tracers/api.go Outdated Show resolved Hide resolved
@2dvorak
Copy link
Collaborator

2dvorak commented Jan 2, 2024

Tested like below, seems like working as intended.

#!/bin/bash

set -e

IPC_URI=/klaytn/klay.ipc
RPC_URI=http://localhost:8551

SAFE_APIS=(
"getBlockRlp(0)"
"dumpBlock(0)"
"traceBlockByNumber(1)"
)

UNSAFE_APIS=(
"printBlock(0)"
"writeBlockProfile(\"asdf\")"
"startWarmUp()"
"traceBlockByNumberRange(1,2)"
)

function fail() {
    local endpoint=$1
    local api=$2
    echo "Test failed for debug.$api to $endpoint"
    exit 1
}

# SAFE APIs should be OK with RPC
echo -n "Testing safe APIs via RPC..."
for (( i=0; i<${#SAFE_APIS[@]}; i++ )); do
    ken attach --exec debug.${SAFE_APIS[i]} $RPC_URI | egrep -q "Error|Fatal" && fail $RPC_URI ${SAFE_APIS[i]}
done
echo "OK"

# UNSAFE APIs should fail with RPC
echo -n "Testing unsafe APIs via RPC..."
for (( i=0; i<${#UNSAFE_APIS[@]}; i++ )); do
    ken attach --exec debug.${UNSAFE_APIS[i]} $RPC_URI | grep -q "not available" || fail $RPC_URI ${UNSAFE_APIS[i]}
done
echo "OK"

# SAFE APIs should work fine with IPC
echo -n "Testing safe APIs via IPC..."
for (( i=0; i<${#SAFE_APIS[@]}; i++ )); do
    ken attach --exec debug.${SAFE_APIS[i]} $IPC_URI | egrep -q "Error|Fatal" && fail $IPC_URI ${SAFE_APIS[i]}
done
echo "OK"

# UNSAFE APIs should be OK WITH IPC
echo -n "Testing unsafe APIs via IPC..."
for (( i=0; i<${#UNSAFE_APIS[@]}; i++ )); do
    ken attach --exec debug.${UNSAFE_APIS[i]} $IPC_URI | egrep -q "Error|Fatal" && fail $IPC_URI ${UNSAFE_APIS[i]}
done
echo "OK"

# Special case for debug.traceCall
echo -n "Testing debug.traceCall with custom tracer..."
ken attach --exec "debug.traceCall({from: '0xB2da01761B494F5F257fD3bA626fBAbFaE104313', to: '0xB2da01761B494F5F257fD3bA626fBAbFaE104313', input: '0x6057361d0000000000000000000000000000000000000000000000000000000000000003'}, 'latest', {tracer: '{data: [], step: function(log) {}, result: function() { return this.data; }, fault: function(){}}'})" $RPC_URI | grep -q "Only predefined tracers" || fail $RPC_URI "debug.traceTransaction(custom tracer)"
ken attach --exec "debug.traceCall({from: '0xB2da01761B494F5F257fD3bA626fBAbFaE104313', to: '0xB2da01761B494F5F257fD3bA626fBAbFaE104313', input: '0x6057361d0000000000000000000000000000000000000000000000000000000000000003'}, 'latest', {tracer: '{data: [], step: function(log) {}, result: function() { return this.data; }, fault: function(){}}'})" $IPC_URI | egrep -q "Error|Fatal" && fail $IPC_URI "traceCall(custom tracer)"
echo "OK"

echo "All tests OK"

Copy link
Contributor

@blukat29 blukat29 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 adding some comments

node/cn/backend.go Show resolved Hide resolved
networks/rpc/endpoints.go Outdated Show resolved Hide resolved
cmd/kbn/node.go Outdated Show resolved Hide resolved
node/cn/tracers/api.go Outdated Show resolved Hide resolved
node/node.go Outdated Show resolved Hide resolved
node/cn/tracers/api.go Outdated Show resolved Hide resolved
@sjnam sjnam merged commit f8ffef5 into klaytn:dev Jan 5, 2024
11 checks passed
@sjnam sjnam deleted the unsafe-debug branch January 5, 2024 01:54
@blukat29 blukat29 mentioned this pull request Jan 22, 2024
13 tasks
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

4 participants