From a9357cc9d7d3ec65c36d19567f1f5f6829a7ea1e Mon Sep 17 00:00:00 2001 From: libotony Date: Fri, 6 Sep 2024 21:00:39 +0800 Subject: [PATCH] make custom tracer to work with allowed tracers flag (#833) * make custom tracer to work with allowed tracers flag * update the message for unspported tracer * update e2e test version --- .github/workflows/test-e2e.yaml | 4 +- api/api.go | 2 +- api/debug/debug.go | 37 +++++++++----- api/debug/debug_test.go | 88 ++++++++++++++++++--------------- api/doc/thor.yaml | 2 +- cmd/thor/flags.go | 2 +- cmd/thor/utils.go | 19 +++++-- tracers/logger/logger.go | 2 +- tracers/tracers.go | 11 +++++ 9 files changed, 104 insertions(+), 63 deletions(-) diff --git a/.github/workflows/test-e2e.yaml b/.github/workflows/test-e2e.yaml index 44bd68eff..7e08965b6 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/d86b30b6409b27e841eace0f7c5b6e75c0a4e25e - ref: d86b30b6409b27e841eace0f7c5b6e75c0a4e25e + # https://github.com/vechain/thor-e2e-tests/tree/209f6ea9a81a98dc2d5e42bf036d2878c5837036 + ref: 209f6ea9a81a98dc2d5e42bf036d2878c5837036 - name: Download artifact uses: actions/download-artifact@v4 diff --git a/api/api.go b/api/api.go index ea33a0c16..38b412a97 100644 --- a/api/api.go +++ b/api/api.go @@ -50,7 +50,7 @@ func New( enableReqLogger bool, enableMetrics bool, logsLimit uint64, - allowedTracers map[string]interface{}, + allowedTracers []string, soloMode bool, ) (http.HandlerFunc, func()) { origins := strings.Split(strings.TrimSpace(allowedOrigins), ",") diff --git a/api/debug/debug.go b/api/debug/debug.go index 5e6c83b58..81543b109 100644 --- a/api/debug/debug.go +++ b/api/debug/debug.go @@ -43,8 +43,8 @@ type Debug struct { forkConfig thor.ForkConfig callGasLimit uint64 allowCustomTracer bool - allowedTracers map[string]interface{} bft bft.Committer + allowedTracers map[string]struct{} skipPoA bool } @@ -55,16 +55,22 @@ func New( callGaslimit uint64, allowCustomTracer bool, bft bft.Committer, - allowedTracers map[string]interface{}, + allowedTracers []string, soloMode bool) *Debug { + + allowedMap := make(map[string]struct{}) + for _, t := range allowedTracers { + allowedMap[t] = struct{}{} + } + return &Debug{ repo, stater, forkConfig, callGaslimit, allowCustomTracer, - allowedTracers, bft, + allowedMap, soloMode, } } @@ -219,18 +225,27 @@ func (d *Debug) handleTraceCall(w http.ResponseWriter, req *http.Request) error } func (d *Debug) createTracer(name string, config json.RawMessage) (tracers.Tracer, error) { - if strings.TrimSpace(name) == "" { - return nil, fmt.Errorf("tracer name must be defined") + tracerName := strings.TrimSpace(name) + // compatible with old API specs + if tracerName == "" { + tracerName = "structLoggerTracer" // default to struct log tracer + } + + // if it's builtin tracers + if tracers.DefaultDirectory.Lookup(tracerName) { + _, allowAll := d.allowedTracers["all"] + // fail if the requested tracer is not allowed OR "all" not set + if _, allowed := d.allowedTracers[tracerName]; !allowAll && !allowed { + return nil, fmt.Errorf("creating tracer is not allowed: %s", name) + } + return tracers.DefaultDirectory.New(tracerName, config, false) } - _, 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) + if d.allowCustomTracer { + return tracers.DefaultDirectory.New(tracerName, config, true) } - return tracers.DefaultDirectory.New(name, config, d.allowCustomTracer) + return nil, errors.New("tracer is not defined") } func (d *Debug) traceCall(ctx context.Context, tracer tracers.Tracer, header *block.Header, st *state.State, txCtx *xenv.TransactionContext, gas uint64, clause *tx.Clause) (interface{}, error) { diff --git a/api/debug/debug_test.go b/api/debug/debug_test.go index 681433ffb..60a2821a0 100644 --- a/api/debug/debug_test.go +++ b/api/debug/debug_test.go @@ -8,7 +8,6 @@ package debug import ( "bytes" "context" - "crypto/rand" "encoding/json" "fmt" "io" @@ -32,6 +31,7 @@ import ( "github.com/vechain/thor/v2/muxdb" "github.com/vechain/thor/v2/packer" "github.com/vechain/thor/v2/state" + "github.com/vechain/thor/v2/test/datagen" "github.com/vechain/thor/v2/thor" "github.com/vechain/thor/v2/tracers/logger" "github.com/vechain/thor/v2/tx" @@ -52,7 +52,6 @@ func TestDebug(t *testing.T) { // /tracers endpoint for name, tt := range map[string]func(*testing.T){ - "testTraceClauseWithEmptyTracerName": testTraceClauseWithEmptyTracerName, "testTraceClauseWithInvalidTracerName": testTraceClauseWithInvalidTracerName, "testTraceClauseWithEmptyTracerTarget": testTraceClauseWithEmptyTracerTarget, "testTraceClauseWithBadBlockId": testTraceClauseWithBadBlockId, @@ -158,27 +157,19 @@ 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{Name: "logger"}, 400) + res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers", &TraceClauseOption{Name: "structLogger"}, 400) assert.Equal(t, "target: unsupported", strings.TrimSpace(res)) } func testTraceClauseWithBadBlockId(t *testing.T) { traceClauseOption := &TraceClauseOption{ - Name: "logger", + Name: "structLogger", Target: "badBlockId/x/x", } res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers", traceClauseOption, 400) @@ -186,14 +177,14 @@ func testTraceClauseWithBadBlockId(t *testing.T) { } func testTraceClauseWithNonExistingBlockId(t *testing.T) { - _, _, _, err := debug.prepareClauseEnv(context.Background(), randBytes32(), 1, 1) + _, _, _, err := debug.prepareClauseEnv(context.Background(), datagen.RandomHash(), 1, 1) assert.Error(t, err) } func testTraceClauseWithBadTxId(t *testing.T) { traceClauseOption := &TraceClauseOption{ - Name: "logger", + Name: "structLogger", Target: fmt.Sprintf("%s/badTxId/x", blk.Header().ID()), } res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers", traceClauseOption, 400) @@ -203,7 +194,7 @@ func testTraceClauseWithBadTxId(t *testing.T) { func testTraceClauseWithNonExistingTx(t *testing.T) { nonExistingTxId := "0x4500ade0d72115abfc77571aef752df45ba5e87ca81fbd67fbfc46d455b17f91" traceClauseOption := &TraceClauseOption{ - Name: "logger", + Name: "structLogger", Target: fmt.Sprintf("%s/%s/x", blk.Header().ID(), nonExistingTxId), } res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers", traceClauseOption, 403) @@ -213,7 +204,7 @@ func testTraceClauseWithNonExistingTx(t *testing.T) { func testTraceClauseWithBadClauseIndex(t *testing.T) { // Clause index is not a number traceClauseOption := &TraceClauseOption{ - Name: "logger", + Name: "structLogger", Target: fmt.Sprintf("%s/%s/x", blk.Header().ID(), transaction.ID()), } res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers", traceClauseOption, 400) @@ -221,7 +212,7 @@ func testTraceClauseWithBadClauseIndex(t *testing.T) { // Clause index is out of range traceClauseOption = &TraceClauseOption{ - Name: "logger", + Name: "structLogger", Target: fmt.Sprintf("%s/%s/%d", blk.Header().ID(), transaction.ID(), uint64(math.MaxUint64)), } res = httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers", traceClauseOption, 400) @@ -257,7 +248,7 @@ func testTraceClauseWithCustomTracer(t *testing.T) { func testTraceClause(t *testing.T) { traceClauseOption := &TraceClauseOption{ - Name: "logger", + Name: "structLogger", Target: fmt.Sprintf("%s/%s/1", blk.Header().ID(), transaction.ID()), } expectedExecutionResult := &logger.ExecutionResult{ @@ -277,7 +268,7 @@ func testTraceClause(t *testing.T) { func testTraceClauseWithTxIndexOutOfBound(t *testing.T) { traceClauseOption := &TraceClauseOption{ - Name: "logger", + Name: "structLogger", Target: fmt.Sprintf("%s/10/1", blk.Header().ID()), } @@ -288,7 +279,7 @@ func testTraceClauseWithTxIndexOutOfBound(t *testing.T) { func testTraceClauseWithClauseIndexOutOfBound(t *testing.T) { traceClauseOption := &TraceClauseOption{ - Name: "logger", + Name: "structLogger", Target: fmt.Sprintf("%s/%s/10", blk.Header().ID(), transaction.ID()), } @@ -303,7 +294,7 @@ func testHandleTraceCallWithMalformedBodyRequest(t *testing.T) { } func testHandleTraceCallWithEmptyTraceCallOption(t *testing.T) { - traceCallOption := &TraceCallOption{Name: "logger"} + traceCallOption := &TraceCallOption{Name: "structLogger"} expectedExecutionResult := &logger.ExecutionResult{ Gas: 0, Failed: false, @@ -321,15 +312,15 @@ func testHandleTraceCallWithEmptyTraceCallOption(t *testing.T) { } func testTraceCallNextBlock(t *testing.T) { - traceCallOption := &TraceCallOption{Name: "logger"} + traceCallOption := &TraceCallOption{Name: "structLogger"} httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers/call?revision=next", traceCallOption, 200) } func testHandleTraceCall(t *testing.T) { - addr := randAddress() + addr := datagen.RandomAddress() provedWork := math.HexOrDecimal256(*big.NewInt(1000)) traceCallOption := &TraceCallOption{ - Name: "logger", + Name: "structLogger", To: &addr, Value: &math.HexOrDecimal256{}, Data: "0x00", @@ -373,7 +364,7 @@ func testHandleTraceCallWithValidRevisions(t *testing.T) { StructLogs: make([]logger.StructLogRes, 0), } - res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers/call?revision="+revision, &TraceCallOption{Name: "logger"}, 200) + res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers/call?revision="+revision, &TraceCallOption{Name: "structLogger"}, 200) var parsedExecutionRes *logger.ExecutionResult if err := json.Unmarshal([]byte(res), &parsedExecutionRes); err != nil { @@ -413,9 +404,9 @@ func testHandleTraceCallWithMalfomredRevision(t *testing.T) { } func testHandleTraceCallWithInsufficientGas(t *testing.T) { - addr := randAddress() + addr := datagen.RandomAddress() traceCallOption := &TraceCallOption{ - Name: "logger", + Name: "structLogger", To: &addr, Value: &math.HexOrDecimal256{}, Data: "0x00", @@ -433,9 +424,9 @@ func testHandleTraceCallWithInsufficientGas(t *testing.T) { } func testHandleTraceCallWithBadBlockRef(t *testing.T) { - addr := randAddress() + addr := datagen.RandomAddress() traceCallOption := &TraceCallOption{ - Name: "logger", + Name: "structLogger", To: &addr, Value: &math.HexOrDecimal256{}, Data: "0x00", @@ -453,9 +444,9 @@ func testHandleTraceCallWithBadBlockRef(t *testing.T) { } func testHandleTraceCallWithInvalidLengthBlockRef(t *testing.T) { - addr := randAddress() + addr := datagen.RandomAddress() traceCallOption := &TraceCallOption{ - Name: "logger", + Name: "structLogger", To: &addr, Value: &math.HexOrDecimal256{}, Data: "0x00", @@ -487,7 +478,7 @@ func testStorageRangeWithError(t *testing.T) { func testStorageRange(t *testing.T) { opt := StorageRangeOption{ - Address: randAddress(), + Address: datagen.RandomAddress(), KeyStart: "0x00", MaxResult: 100, Target: fmt.Sprintf("%s/%s/0", blk.Header().ID(), transaction.ID()), @@ -596,8 +587,7 @@ func initDebugServer(t *testing.T) { forkConfig := thor.GetForkConfig(b.Header().ID()) router := mux.NewRouter() - allTracersEnabled := map[string]interface{}{"all": new(interface{})} - debug = New(repo, stater, forkConfig, 21000, true, solo.NewBFTEngine(repo), allTracersEnabled, false) + debug = New(repo, stater, forkConfig, 21000, true, solo.NewBFTEngine(repo), []string{"all"}, false) debug.Mount(router, "/debug") ts = httptest.NewServer(router) } @@ -620,12 +610,28 @@ func httpPostAndCheckResponseStatus(t *testing.T, url string, obj interface{}, r return string(r) } -func randAddress() (addr thor.Address) { - rand.Read(addr[:]) - return -} +func TestCreateTracer(t *testing.T) { + debug := &Debug{} + + // all + debug.allowedTracers = map[string]struct{}{"all": {}} + tr, err := debug.createTracer("", nil) + assert.Nil(t, err) + assert.IsType(t, &logger.StructLogger{}, tr) + _, err = debug.createTracer("{result:()=>{}, fault:()=>{}}", nil) + assert.EqualError(t, err, "tracer is not defined") + + tr, err = debug.createTracer("structLogger", nil) + assert.Nil(t, err) + assert.IsType(t, &logger.StructLogger{}, tr) + + // none + debug.allowedTracers = map[string]struct{}{} + _, err = debug.createTracer("structLogger", nil) + assert.EqualError(t, err, "creating tracer is not allowed: structLogger") -func randBytes32() (b thor.Bytes32) { - rand.Read(b[:]) - return + // custom tracer + debug.allowCustomTracer = true + _, err = debug.createTracer("{result:()=>{}, fault:()=>{}}", nil) + assert.Nil(t, err) } diff --git a/api/doc/thor.yaml b/api/doc/thor.yaml index 08977d0fa..59d4e699b 100644 --- a/api/doc/thor.yaml +++ b/api/doc/thor.yaml @@ -2117,7 +2117,7 @@ components: name: type: string enum: - - logger + - structLogger - 4byte - call - noop diff --git a/cmd/thor/flags.go b/cmd/thor/flags.go index 382bac66d..0a512319f 100644 --- a/cmd/thor/flags.go +++ b/cmd/thor/flags.go @@ -169,7 +169,7 @@ var ( allowedTracersFlag = cli.StringFlag{ Name: "api-allowed-tracers", Value: "none", - Usage: "define allowed API tracers", + Usage: "comma separated list of allowed API tracers(none,all,call,prestate etc.)", } // solo mode only flags diff --git a/cmd/thor/utils.go b/cmd/thor/utils.go index fbbecfc90..d755cf732 100644 --- a/cmd/thor/utils.go +++ b/cmd/thor/utils.go @@ -753,15 +753,24 @@ func readIntFromUInt64Flag(val uint64) (int, error) { return i, nil } -func parseTracerList(list string) map[string]interface{} { +func parseTracerList(list string) []string { inputs := strings.Split(list, ",") - tracerMap := map[string]interface{}{} + tracers := make([]string, 0, len(inputs)) + for _, i := range inputs { - if i == "" { + name := strings.TrimSpace(i) + if name == "" { continue } - tracerMap[i] = new(interface{}) + if name == "none" { + return []string{} + } + if name == "all" { + return []string{"all"} + } + + tracers = append(tracers, i) } - return tracerMap + return tracers } diff --git a/tracers/logger/logger.go b/tracers/logger/logger.go index f128cac61..654602bcd 100644 --- a/tracers/logger/logger.go +++ b/tracers/logger/logger.go @@ -35,7 +35,7 @@ import ( ) func init() { - tracers.DefaultDirectory.Register("logger", NewStructLogger, false) + tracers.DefaultDirectory.Register("structLoggerTracer", NewStructLogger, false) } // Storage represents a contract's storage. diff --git a/tracers/tracers.go b/tracers/tracers.go index 4f5c7122a..e41c77f67 100644 --- a/tracers/tracers.go +++ b/tracers/tracers.go @@ -79,6 +79,17 @@ func (d *directory) RegisterJSEval(f jsCtorFn) { d.jsEval = f } +// Lookup returns true if the given tracer is registered. +func (d *directory) Lookup(name string) bool { + if _, ok := d.elems[name]; ok { + return ok + } + + // backward compatible, allow users emit "Tracer" suffix + _, ok := d.elems[name+"Tracer"] + return ok +} + // New returns a new instance of a tracer, by iterating through the // registered lookups. Name is either name of an existing tracer // or an arbitrary JS code.