From f1aa5ae733617d0366500b3210895415c7068f20 Mon Sep 17 00:00:00 2001 From: Pedro Gomes Date: Thu, 22 Aug 2024 17:42:49 +0100 Subject: [PATCH] Add api-allowed-tracers flag (#823) * Add allowed-tracers flag * fix test * pr comments * tidy up err message * Adding none as the default tracer * invalid tracer name * update swagger * update e2e tests --- .github/workflows/test-e2e.yaml | 4 ++-- api/api.go | 3 ++- api/debug/debug.go | 18 ++++++++++++---- api/debug/debug_test.go | 38 ++++++++++++++++++++++++++++----- api/doc/thor.yaml | 2 +- cmd/thor/flags.go | 6 ++++++ cmd/thor/main.go | 6 ++++++ cmd/thor/utils.go | 13 +++++++++++ docs/usage.md | 1 + tracers/logger/logger.go | 6 +++++- tracers/logger/logger_test.go | 8 ++++--- tracers/tracers.go | 2 +- 12 files changed, 89 insertions(+), 18 deletions(-) diff --git a/.github/workflows/test-e2e.yaml b/.github/workflows/test-e2e.yaml index 76b885396..44bd68eff 100644 --- a/.github/workflows/test-e2e.yaml +++ b/.github/workflows/test-e2e.yaml @@ -44,8 +44,8 @@ jobs: uses: actions/checkout@v4 with: repository: vechain/thor-e2e-tests - # https://github.com/vechain/thor-e2e-tests/tree/00bd3f1b949b05da94e82686e0089a11a136c34c - ref: 87aeb1747995e8b235a796303b0fc08ab16262c6 + # https://github.com/vechain/thor-e2e-tests/tree/d86b30b6409b27e841eace0f7c5b6e75c0a4e25e + ref: d86b30b6409b27e841eace0f7c5b6e75c0a4e25e - name: Download artifact uses: actions/download-artifact@v4 diff --git a/api/api.go b/api/api.go index 0d929d294..0ac399295 100644 --- a/api/api.go +++ b/api/api.go @@ -50,6 +50,7 @@ func New( enableReqLogger bool, enableMetrics bool, logsLimit uint64, + allowedTracers map[string]interface{}, ) (http.HandlerFunc, func()) { origins := strings.Split(strings.TrimSpace(allowedOrigins), ",") for i, o := range origins { @@ -82,7 +83,7 @@ func New( Mount(router, "/blocks") transactions.New(repo, txPool). Mount(router, "/transactions") - debug.New(repo, stater, forkConfig, callGasLimit, allowCustomTracer, bft). + debug.New(repo, stater, forkConfig, callGasLimit, allowCustomTracer, bft, allowedTracers). Mount(router, "/debug") node.New(nw). Mount(router, "/node") diff --git a/api/debug/debug.go b/api/debug/debug.go index 8e30db2fa..7eecd9b8c 100644 --- a/api/debug/debug.go +++ b/api/debug/debug.go @@ -8,6 +8,7 @@ package debug import ( "context" "encoding/json" + "fmt" "math" "math/big" "net/http" @@ -29,7 +30,6 @@ import ( "github.com/vechain/thor/v2/state" "github.com/vechain/thor/v2/thor" "github.com/vechain/thor/v2/tracers" - "github.com/vechain/thor/v2/tracers/logger" "github.com/vechain/thor/v2/trie" "github.com/vechain/thor/v2/tx" "github.com/vechain/thor/v2/vm" @@ -47,9 +47,10 @@ type Debug struct { callGasLimit uint64 allowCustomTracer bool bft bft.Finalizer + allowedTracers map[string]interface{} } -func New(repo *chain.Repository, stater *state.Stater, forkConfig thor.ForkConfig, callGaslimit uint64, allowCustomTracer bool, bft bft.Finalizer) *Debug { +func New(repo *chain.Repository, stater *state.Stater, forkConfig thor.ForkConfig, callGaslimit uint64, allowCustomTracer bool, bft bft.Finalizer, allowedTracers map[string]interface{}) *Debug { return &Debug{ repo, stater, @@ -57,6 +58,7 @@ func New(repo *chain.Repository, stater *state.Stater, forkConfig thor.ForkConfi callGaslimit, allowCustomTracer, bft, + allowedTracers, } } @@ -211,9 +213,17 @@ func (d *Debug) handleTraceCall(w http.ResponseWriter, req *http.Request) error } func (d *Debug) createTracer(name string, config json.RawMessage) (tracers.Tracer, error) { - if name == "" { - return logger.NewStructLogger(config) + if strings.TrimSpace(name) == "" { + return nil, fmt.Errorf("tracer name must be defined") } + _, noTracers := d.allowedTracers["none"] + _, allTracers := d.allowedTracers["all"] + + // fail if the requested tracer is not allowed OR if the "all" tracers code isn't active + if _, ok := d.allowedTracers[name]; noTracers || (!ok && !allTracers) { + return nil, fmt.Errorf("creating tracer is not allowed: %s", name) + } + return tracers.DefaultDirectory.New(name, config, d.allowCustomTracer) } diff --git a/api/debug/debug_test.go b/api/debug/debug_test.go index 7075e728a..467fc95fb 100644 --- a/api/debug/debug_test.go +++ b/api/debug/debug_test.go @@ -52,6 +52,8 @@ func TestDebug(t *testing.T) { // /tracers endpoint for name, tt := range map[string]func(*testing.T){ + "testTraceClauseWithEmptyTracerName": testTraceClauseWithEmptyTracerName, + "testTraceClauseWithInvalidTracerName": testTraceClauseWithInvalidTracerName, "testTraceClauseWithEmptyTracerTarget": testTraceClauseWithEmptyTracerTarget, "testTraceClauseWithBadBlockId": testTraceClauseWithBadBlockId, "testTraceClauseWithNonExistingBlockId": testTraceClauseWithNonExistingBlockId, @@ -156,13 +158,27 @@ func TestStorageRangeMaxResult(t *testing.T) { assert.Equal(t, 10, len(storageRangeRes.Storage)) } +func testTraceClauseWithEmptyTracerName(t *testing.T) { + res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers", &TraceClauseOption{}, 403) + assert.Equal(t, "tracer name must be defined", strings.TrimSpace(res)) + + res = httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers", &TraceClauseOption{Name: " "}, 403) + assert.Equal(t, "tracer name must be defined", strings.TrimSpace(res)) +} + +func testTraceClauseWithInvalidTracerName(t *testing.T) { + res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers", &TraceClauseOption{Name: "non-existent"}, 403) + assert.Contains(t, res, "unable to create custom tracer") +} + func testTraceClauseWithEmptyTracerTarget(t *testing.T) { - res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers", &TraceClauseOption{}, 400) + res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers", &TraceClauseOption{Name: "logger"}, 400) assert.Equal(t, "target: unsupported", strings.TrimSpace(res)) } func testTraceClauseWithBadBlockId(t *testing.T) { traceClauseOption := &TraceClauseOption{ + Name: "logger", Target: "badBlockId/x/x", } res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers", traceClauseOption, 400) @@ -177,6 +193,7 @@ func testTraceClauseWithNonExistingBlockId(t *testing.T) { func testTraceClauseWithBadTxId(t *testing.T) { traceClauseOption := &TraceClauseOption{ + Name: "logger", Target: fmt.Sprintf("%s/badTxId/x", blk.Header().ID()), } res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers", traceClauseOption, 400) @@ -186,6 +203,7 @@ func testTraceClauseWithBadTxId(t *testing.T) { func testTraceClauseWithNonExistingTx(t *testing.T) { nonExistingTxId := "0x4500ade0d72115abfc77571aef752df45ba5e87ca81fbd67fbfc46d455b17f91" traceClauseOption := &TraceClauseOption{ + Name: "logger", Target: fmt.Sprintf("%s/%s/x", blk.Header().ID(), nonExistingTxId), } res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers", traceClauseOption, 403) @@ -195,6 +213,7 @@ func testTraceClauseWithNonExistingTx(t *testing.T) { func testTraceClauseWithBadClauseIndex(t *testing.T) { // Clause index is not a number traceClauseOption := &TraceClauseOption{ + Name: "logger", Target: fmt.Sprintf("%s/%s/x", blk.Header().ID(), transaction.ID()), } res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers", traceClauseOption, 400) @@ -202,6 +221,7 @@ func testTraceClauseWithBadClauseIndex(t *testing.T) { // Clause index is out of range traceClauseOption = &TraceClauseOption{ + Name: "logger", Target: fmt.Sprintf("%s/%s/%d", blk.Header().ID(), transaction.ID(), uint64(math.MaxUint64)), } res = httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers", traceClauseOption, 400) @@ -237,6 +257,7 @@ func testTraceClauseWithCustomTracer(t *testing.T) { func testTraceClause(t *testing.T) { traceClauseOption := &TraceClauseOption{ + Name: "logger", Target: fmt.Sprintf("%s/%s/1", blk.Header().ID(), transaction.ID()), } expectedExecutionResult := &logger.ExecutionResult{ @@ -256,6 +277,7 @@ func testTraceClause(t *testing.T) { func testTraceClauseWithTxIndexOutOfBound(t *testing.T) { traceClauseOption := &TraceClauseOption{ + Name: "logger", Target: fmt.Sprintf("%s/10/1", blk.Header().ID()), } @@ -266,6 +288,7 @@ func testTraceClauseWithTxIndexOutOfBound(t *testing.T) { func testTraceClauseWithClauseIndexOutOfBound(t *testing.T) { traceClauseOption := &TraceClauseOption{ + Name: "logger", Target: fmt.Sprintf("%s/%s/10", blk.Header().ID(), transaction.ID()), } @@ -280,7 +303,7 @@ func testHandleTraceCallWithMalformedBodyRequest(t *testing.T) { } func testHandleTraceCallWithEmptyTraceCallOption(t *testing.T) { - traceCallOption := &TraceCallOption{} + traceCallOption := &TraceCallOption{Name: "logger"} expectedExecutionResult := &logger.ExecutionResult{ Gas: 0, Failed: false, @@ -298,7 +321,7 @@ func testHandleTraceCallWithEmptyTraceCallOption(t *testing.T) { } func testTraceCallNextBlock(t *testing.T) { - traceCallOption := &TraceCallOption{} + traceCallOption := &TraceCallOption{Name: "logger"} httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers/call?revision=next", traceCallOption, 200) } @@ -306,6 +329,7 @@ func testHandleTraceCall(t *testing.T) { addr := randAddress() provedWork := math.HexOrDecimal256(*big.NewInt(1000)) traceCallOption := &TraceCallOption{ + Name: "logger", To: &addr, Value: &math.HexOrDecimal256{}, Data: "0x00", @@ -349,7 +373,7 @@ func testHandleTraceCallWithValidRevisions(t *testing.T) { StructLogs: make([]logger.StructLogRes, 0), } - res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers/call?revision="+revision, &TraceCallOption{}, 200) + res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers/call?revision="+revision, &TraceCallOption{Name: "logger"}, 200) var parsedExecutionRes *logger.ExecutionResult if err := json.Unmarshal([]byte(res), &parsedExecutionRes); err != nil { @@ -391,6 +415,7 @@ func testHandleTraceCallWithMalfomredRevision(t *testing.T) { func testHandleTraceCallWithInsufficientGas(t *testing.T) { addr := randAddress() traceCallOption := &TraceCallOption{ + Name: "logger", To: &addr, Value: &math.HexOrDecimal256{}, Data: "0x00", @@ -410,6 +435,7 @@ func testHandleTraceCallWithInsufficientGas(t *testing.T) { func testHandleTraceCallWithBadBlockRef(t *testing.T) { addr := randAddress() traceCallOption := &TraceCallOption{ + Name: "logger", To: &addr, Value: &math.HexOrDecimal256{}, Data: "0x00", @@ -429,6 +455,7 @@ func testHandleTraceCallWithBadBlockRef(t *testing.T) { func testHandleTraceCallWithInvalidLengthBlockRef(t *testing.T) { addr := randAddress() traceCallOption := &TraceCallOption{ + Name: "logger", To: &addr, Value: &math.HexOrDecimal256{}, Data: "0x00", @@ -569,7 +596,8 @@ func initDebugServer(t *testing.T) { forkConfig := thor.GetForkConfig(b.Header().ID()) router := mux.NewRouter() - debug = New(repo, stater, forkConfig, 21000, true, solo.NewBFTEngine(repo)) + allTracersEnabled := map[string]interface{}{"all": new(interface{})} + debug = New(repo, stater, forkConfig, 21000, true, solo.NewBFTEngine(repo), allTracersEnabled) debug.Mount(router, "/debug") ts = httptest.NewServer(router) } diff --git a/api/doc/thor.yaml b/api/doc/thor.yaml index 2c6d03d47..2769ca18b 100644 --- a/api/doc/thor.yaml +++ b/api/doc/thor.yaml @@ -2117,7 +2117,7 @@ components: name: type: string enum: - - "" + - logger - 4byte - call - noop diff --git a/cmd/thor/flags.go b/cmd/thor/flags.go index b93167e21..382bac66d 100644 --- a/cmd/thor/flags.go +++ b/cmd/thor/flags.go @@ -166,6 +166,12 @@ var ( Usage: "set tx limit per account in pool", } + allowedTracersFlag = cli.StringFlag{ + Name: "api-allowed-tracers", + Value: "none", + Usage: "define allowed API tracers", + } + // solo mode only flags onDemandFlag = cli.BoolFlag{ Name: "on-demand", diff --git a/cmd/thor/main.go b/cmd/thor/main.go index bd357beb6..a66dbc563 100644 --- a/cmd/thor/main.go +++ b/cmd/thor/main.go @@ -11,6 +11,7 @@ import ( "io" "os" "path/filepath" + "strings" "time" "github.com/ethereum/go-ethereum/accounts/keystore" @@ -35,6 +36,7 @@ import ( // Force-load the tracer engines to trigger registration _ "github.com/vechain/thor/v2/tracers/js" + _ "github.com/vechain/thor/v2/tracers/logger" _ "github.com/vechain/thor/v2/tracers/native" ) @@ -96,6 +98,7 @@ func main() { adminAddrFlag, enableAdminFlag, txPoolLimitPerAccountFlag, + allowedTracersFlag, }, Action: defaultAction, Commands: []cli.Command{ @@ -130,6 +133,7 @@ func main() { metricsAddrFlag, adminAddrFlag, enableAdminFlag, + allowedTracersFlag, }, Action: soloAction, }, @@ -261,6 +265,7 @@ func defaultAction(ctx *cli.Context) error { ctx.Bool(enableAPILogsFlag.Name), ctx.Bool(enableMetricsFlag.Name), ctx.Uint64(apiLogsLimitFlag.Name), + parseTracerList(strings.TrimSpace(ctx.String(allowedTracersFlag.Name))), ) defer func() { log.Info("closing API..."); apiCloser() }() @@ -410,6 +415,7 @@ func soloAction(ctx *cli.Context) error { ctx.Bool(enableAPILogsFlag.Name), ctx.Bool(enableMetricsFlag.Name), ctx.Uint64(apiLogsLimitFlag.Name), + parseTracerList(strings.TrimSpace(ctx.String(allowedTracersFlag.Name))), ) defer func() { log.Info("closing API..."); apiCloser() }() diff --git a/cmd/thor/utils.go b/cmd/thor/utils.go index cb810364d..88704dd1d 100644 --- a/cmd/thor/utils.go +++ b/cmd/thor/utils.go @@ -798,3 +798,16 @@ func readIntFromUInt64Flag(val uint64) (int, error) { return i, nil } + +func parseTracerList(list string) map[string]interface{} { + inputs := strings.Split(list, ",") + tracerMap := map[string]interface{}{} + for _, i := range inputs { + if i == "" { + continue + } + tracerMap[i] = new(interface{}) + } + + return tracerMap +} diff --git a/docs/usage.md b/docs/usage.md index e1ece95bb..7358e3f0e 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -168,6 +168,7 @@ bin/thor -h | `--api-call-gas-limit` | Limit contract call gas (default: 50000000) | | `--api-backtrace-limit` | Limit the distance between 'position' and best block for subscriptions APIs (default: 1000) | | `--api-allow-custom-tracer` | Allow custom JS tracer to be used for the tracer API | +| `--api-allowed-tracers` | Comma-separated list of allowed tracers (default: "none") | | `--enable-api-logs` | Enables API requests logging | | `--api-logs-limit` | Limit the number of logs returned by /logs API (default: 1000) | | `--verbosity` | Log verbosity (0-9) (default: 3) | diff --git a/tracers/logger/logger.go b/tracers/logger/logger.go index 936dbf192..ff3735a6e 100644 --- a/tracers/logger/logger.go +++ b/tracers/logger/logger.go @@ -34,6 +34,10 @@ import ( "github.com/vechain/thor/v2/vm" ) +func init() { + tracers.DefaultDirectory.Register("logger", NewStructLogger, false) +} + // Storage represents a contract's storage. type Storage map[common.Hash]common.Hash @@ -119,7 +123,7 @@ type StructLogger struct { } // NewStructLogger returns a new logger -func NewStructLogger(cfg json.RawMessage) (*StructLogger, error) { +func NewStructLogger(cfg json.RawMessage) (tracers.Tracer, error) { var config Config if cfg != nil { if err := json.Unmarshal(cfg, &config); err != nil { diff --git a/tracers/logger/logger_test.go b/tracers/logger/logger_test.go index 0cc8dc0ec..ebf4b8772 100644 --- a/tracers/logger/logger_test.go +++ b/tracers/logger/logger_test.go @@ -58,11 +58,12 @@ func (*dummyStatedb) SetState(_ common.Address, _ common.Hash, _ common.Hash) {} func TestStoreCapture(t *testing.T) { var ( - logger, _ = NewStructLogger(nil) - env = vm.NewEVM(vm.Context{}, &dummyStatedb{}, &vm.ChainConfig{ChainConfig: *params.TestChainConfig}, vm.Config{Tracer: logger}) + unCastedLogger, _ = NewStructLogger(nil) + env = vm.NewEVM(vm.Context{}, &dummyStatedb{}, &vm.ChainConfig{ChainConfig: *params.TestChainConfig}, vm.Config{Tracer: unCastedLogger.(*StructLogger)}) contract = vm.NewContract(&dummyContractRef{}, &dummyContractRef{}, new(big.Int), 100000) ) + logger := unCastedLogger.(*StructLogger) contract.Code = []byte{byte(vm.PUSH1), 0x1, byte(vm.PUSH1), 0x0, byte(vm.SSTORE)} var index common.Hash logger.CaptureStart(env, common.Address{}, contract.Address(), false, nil, 0, nil) @@ -123,7 +124,8 @@ func TestFormatLogs(t *testing.T) { } func TestCaptureStart(t *testing.T) { - logger, _ := NewStructLogger(nil) + unCastedLogger, _ := NewStructLogger(nil) + logger := unCastedLogger.(*StructLogger) env := &vm.EVM{} logger.CaptureStart(env, common.Address{}, common.Address{}, false, nil, 0, nil) diff --git a/tracers/tracers.go b/tracers/tracers.go index 0872efe20..4f5c7122a 100644 --- a/tracers/tracers.go +++ b/tracers/tracers.go @@ -95,7 +95,7 @@ func (d *directory) New(name string, cfg json.RawMessage, allowCustom bool) (Tra // Assume JS code tracer, err := d.jsEval(name, cfg) if err != nil { - return nil, errors.Wrap(err, "create custom tracer") + return nil, errors.Wrap(err, "unable to create custom tracer") } return tracer, nil } else {