Skip to content
This repository was archived by the owner on Aug 19, 2024. It is now read-only.

Expose unsafe debug APIs via IPC#2075

Merged
sjnam merged 10 commits intoklaytn:devfrom
sjnam:unsafe-debug
Jan 5, 2024
Merged

Expose unsafe debug APIs via IPC#2075
sjnam merged 10 commits intoklaytn:devfrom
sjnam:unsafe-debug

Conversation

@sjnam
Copy link

@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?

@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

@sjnam sjnam merged commit f8ffef5 into klaytn:dev Jan 5, 2024
@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 subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants