From b19f52e9990693055373f7c8785cf4a6f98b9103 Mon Sep 17 00:00:00 2001 From: ramtinms Date: Mon, 16 Sep 2024 22:13:04 -0700 Subject: [PATCH 1/3] move map update outside of concurrent path --- fvm/evm/debug/tracer.go | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/fvm/evm/debug/tracer.go b/fvm/evm/debug/tracer.go index 0a047aed3df..52b4a7e5236 100644 --- a/fvm/evm/debug/tracer.go +++ b/fvm/evm/debug/tracer.go @@ -76,16 +76,25 @@ func (t *CallTracer) GetResultByTxHash(txID gethCommon.Hash) json.RawMessage { } func (t *CallTracer) Collect(txID gethCommon.Hash) { + l := t.logger.With(). + Str("tx-id", txID.String()). + Str("block-id", t.blockID.String()). + Logger() + + // collect the trace result + res, found := t.resultsByTxID[txID] + if !found { + l.Error().Msg("trace result not found") + return + } + // remove the result + delete(t.resultsByTxID, txID) + // upload is concurrent and it doesn't produce any errors, as the // client doesn't expect it, we don't want to break execution flow, // in case there are errors we retry, and if we fail after retries // we log them and continue. go func() { - l := t.logger.With(). - Str("tx-id", txID.String()). - Str("block-id", t.blockID.String()). - Logger() - defer func() { if r := recover(); r != nil { err, ok := r.(error) @@ -98,20 +107,12 @@ func (t *CallTracer) Collect(txID gethCommon.Hash) { Msg("failed to collect EVM traces") } }() - - res, found := t.resultsByTxID[txID] - if !found { - l.Error().Msg("trace result not found") - return - } if err := t.uploader.Upload(TraceID(txID, t.blockID), res); err != nil { l.Error().Err(err). Str("traces", string(res)). Msg("failed to upload trace results, no more retries") return } - // remove the result - delete(t.resultsByTxID, txID) l.Debug().Msg("evm traces uploaded successfully") }() From c35bacca47b086b578934ae6d87b441a28f20974 Mon Sep 17 00:00:00 2001 From: ramtinms Date: Tue, 17 Sep 2024 09:46:07 -0700 Subject: [PATCH 2/3] remove unsupported checks --- fvm/evm/emulator/emulator_test.go | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/fvm/evm/emulator/emulator_test.go b/fvm/evm/emulator/emulator_test.go index 4e268a28902..3ac96e9e7df 100644 --- a/fvm/evm/emulator/emulator_test.go +++ b/fvm/evm/emulator/emulator_test.go @@ -1194,7 +1194,6 @@ func TestTransactionTracing(t *testing.T) { blk, uploader, tracer := blockWithTracer(t, emu) var txID gethCommon.Hash - var trace json.RawMessage blockID := flow.Identifier{0x01} uploaded := make(chan struct{}) @@ -1202,7 +1201,7 @@ func TestTransactionTracing(t *testing.T) { uploader.UploadFunc = func(id string, message json.RawMessage) error { uploaded <- struct{}{} require.Equal(t, debug.TraceID(txID, blockID), id) - require.Equal(t, trace, message) + require.NotEmpty(t, message) require.Greater(t, len(message), 0) return nil } @@ -1225,8 +1224,6 @@ func TestTransactionTracing(t *testing.T) { require.NoError(t, res.VMError) txID = res.TxHash tracer.Collect(txID) - trace = tracer.GetResultByTxHash(txID) - require.NotEmpty(t, trace) testAccount.SetNonce(testAccount.Nonce() + 1) require.Eventuallyf(t, func() bool { @@ -1241,7 +1238,6 @@ func TestTransactionTracing(t *testing.T) { blk, uploader, tracer := blockWithTracer(t, emu) var txID gethCommon.Hash - var trace json.RawMessage uploaded := make(chan struct{}) blockID := flow.Identifier{0x01} @@ -1249,7 +1245,7 @@ func TestTransactionTracing(t *testing.T) { uploader.UploadFunc = func(id string, message json.RawMessage) error { uploaded <- struct{}{} require.Equal(t, debug.TraceID(txID, blockID), id) - require.Equal(t, trace, message) + require.NotEmpty(t, message) require.Greater(t, len(message), 0) return nil } @@ -1269,8 +1265,6 @@ func TestTransactionTracing(t *testing.T) { txID = res.TxHash tracer.WithBlockID(blockID) tracer.Collect(txID) - trace = tracer.GetResultByTxHash(txID) - require.NotEmpty(t, trace) testAccount.SetNonce(testAccount.Nonce() + 1) require.Eventuallyf(t, func() bool { @@ -1285,7 +1279,6 @@ func TestTransactionTracing(t *testing.T) { blk, uploader, tracer := blockWithTracer(t, emu) var txID gethCommon.Hash - var trace json.RawMessage blockID := flow.Identifier{0x02} uploaded := make(chan struct{}) @@ -1293,7 +1286,7 @@ func TestTransactionTracing(t *testing.T) { uploader.UploadFunc = func(id string, message json.RawMessage) error { uploaded <- struct{}{} require.Equal(t, debug.TraceID(txID, blockID), id) - require.Equal(t, trace, message) + require.NotEmpty(t, message) require.Greater(t, len(message), 0) return nil } @@ -1313,8 +1306,6 @@ func TestTransactionTracing(t *testing.T) { txID = res.TxHash tracer.WithBlockID(blockID) tracer.Collect(txID) - trace = tracer.GetResultByTxHash(txID) - require.NotEmpty(t, trace) testAccount.SetNonce(testAccount.Nonce() + 1) require.Eventuallyf(t, func() bool { @@ -1362,7 +1353,6 @@ func TestTransactionTracing(t *testing.T) { blk, uploader, tracer := blockWithTracer(t, emu) var txID gethCommon.Hash - var trace json.RawMessage blockID := flow.Identifier{0x02} uploaded := make(chan struct{}) @@ -1370,7 +1360,7 @@ func TestTransactionTracing(t *testing.T) { uploader.UploadFunc = func(id string, message json.RawMessage) error { uploaded <- struct{}{} require.Equal(t, debug.TraceID(txID, blockID), id) - require.Equal(t, trace, message) + require.NotEmpty(t, message) require.Greater(t, len(message), 0) return nil } @@ -1391,8 +1381,6 @@ func TestTransactionTracing(t *testing.T) { txID = res.TxHash tracer.WithBlockID(blockID) tracer.Collect(txID) - trace = tracer.GetResultByTxHash(txID) - require.NotEmpty(t, trace) testAccount.SetNonce(testAccount.Nonce() + 1) require.Eventuallyf(t, func() bool { @@ -1455,7 +1443,6 @@ func TestTransactionTracing(t *testing.T) { blk, uploader, tracer := blockWithTracer(t, emu) var txID gethCommon.Hash - var trace json.RawMessage blockID := flow.Identifier{0x02} uploaded := make(chan struct{}) @@ -1463,7 +1450,7 @@ func TestTransactionTracing(t *testing.T) { uploader.UploadFunc = func(id string, message json.RawMessage) error { uploaded <- struct{}{} require.Equal(t, debug.TraceID(txID, blockID), id) - require.Equal(t, trace, message) + require.NotEmpty(t, message) require.Greater(t, len(message), 0) return nil } @@ -1484,7 +1471,6 @@ func TestTransactionTracing(t *testing.T) { txID = results[0].TxHash tracer.WithBlockID(blockID) tracer.Collect(txID) - trace = tracer.GetResultByTxHash(txID) require.Eventuallyf(t, func() bool { <-uploaded From 067c3acd05d34bd2e96cffac91922c0af5a72133 Mon Sep 17 00:00:00 2001 From: ramtinms Date: Tue, 17 Sep 2024 10:45:17 -0700 Subject: [PATCH 3/3] fix tests --- fvm/evm/emulator/emulator_test.go | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/fvm/evm/emulator/emulator_test.go b/fvm/evm/emulator/emulator_test.go index 3ac96e9e7df..a89ca945c1a 100644 --- a/fvm/evm/emulator/emulator_test.go +++ b/fvm/evm/emulator/emulator_test.go @@ -1103,7 +1103,8 @@ func TestCallingExtraPrecompiles(t *testing.T) { AddressFunc: func() types.Address { return addr }, - RequiredGasFunc: func(input []byte) uint64 { + RequiredGasFunc: func(inp []byte) uint64 { + require.Equal(t, input, inp) return uint64(10) }, RunFunc: func(inp []byte) ([]byte, error) { @@ -1194,6 +1195,7 @@ func TestTransactionTracing(t *testing.T) { blk, uploader, tracer := blockWithTracer(t, emu) var txID gethCommon.Hash + var trace json.RawMessage blockID := flow.Identifier{0x01} uploaded := make(chan struct{}) @@ -1201,7 +1203,7 @@ func TestTransactionTracing(t *testing.T) { uploader.UploadFunc = func(id string, message json.RawMessage) error { uploaded <- struct{}{} require.Equal(t, debug.TraceID(txID, blockID), id) - require.NotEmpty(t, message) + require.Equal(t, trace, message) require.Greater(t, len(message), 0) return nil } @@ -1223,6 +1225,8 @@ func TestTransactionTracing(t *testing.T) { require.NoError(t, res.ValidationError) require.NoError(t, res.VMError) txID = res.TxHash + trace = tracer.GetResultByTxHash(txID) + require.NotEmpty(t, trace) tracer.Collect(txID) testAccount.SetNonce(testAccount.Nonce() + 1) @@ -1238,6 +1242,7 @@ func TestTransactionTracing(t *testing.T) { blk, uploader, tracer := blockWithTracer(t, emu) var txID gethCommon.Hash + var trace json.RawMessage uploaded := make(chan struct{}) blockID := flow.Identifier{0x01} @@ -1245,7 +1250,7 @@ func TestTransactionTracing(t *testing.T) { uploader.UploadFunc = func(id string, message json.RawMessage) error { uploaded <- struct{}{} require.Equal(t, debug.TraceID(txID, blockID), id) - require.NotEmpty(t, message) + require.Equal(t, trace, message) require.Greater(t, len(message), 0) return nil } @@ -1263,6 +1268,8 @@ func TestTransactionTracing(t *testing.T) { ) requireSuccessfulExecution(t, err, res) txID = res.TxHash + trace = tracer.GetResultByTxHash(txID) + require.NotEmpty(t, trace) tracer.WithBlockID(blockID) tracer.Collect(txID) @@ -1279,6 +1286,7 @@ func TestTransactionTracing(t *testing.T) { blk, uploader, tracer := blockWithTracer(t, emu) var txID gethCommon.Hash + var trace json.RawMessage blockID := flow.Identifier{0x02} uploaded := make(chan struct{}) @@ -1286,7 +1294,7 @@ func TestTransactionTracing(t *testing.T) { uploader.UploadFunc = func(id string, message json.RawMessage) error { uploaded <- struct{}{} require.Equal(t, debug.TraceID(txID, blockID), id) - require.NotEmpty(t, message) + require.Equal(t, trace, message) require.Greater(t, len(message), 0) return nil } @@ -1304,6 +1312,8 @@ func TestTransactionTracing(t *testing.T) { res, err := blk.RunTransaction(tx) requireSuccessfulExecution(t, err, res) txID = res.TxHash + trace = tracer.GetResultByTxHash(txID) + require.NotEmpty(t, trace) tracer.WithBlockID(blockID) tracer.Collect(txID) @@ -1353,6 +1363,7 @@ func TestTransactionTracing(t *testing.T) { blk, uploader, tracer := blockWithTracer(t, emu) var txID gethCommon.Hash + var trace json.RawMessage blockID := flow.Identifier{0x02} uploaded := make(chan struct{}) @@ -1360,7 +1371,7 @@ func TestTransactionTracing(t *testing.T) { uploader.UploadFunc = func(id string, message json.RawMessage) error { uploaded <- struct{}{} require.Equal(t, debug.TraceID(txID, blockID), id) - require.NotEmpty(t, message) + require.Equal(t, trace, message) require.Greater(t, len(message), 0) return nil } @@ -1380,6 +1391,8 @@ func TestTransactionTracing(t *testing.T) { requireSuccessfulExecution(t, err, res) txID = res.TxHash tracer.WithBlockID(blockID) + trace = tracer.GetResultByTxHash(txID) + require.NotEmpty(t, trace) tracer.Collect(txID) testAccount.SetNonce(testAccount.Nonce() + 1) @@ -1443,6 +1456,7 @@ func TestTransactionTracing(t *testing.T) { blk, uploader, tracer := blockWithTracer(t, emu) var txID gethCommon.Hash + var trace json.RawMessage blockID := flow.Identifier{0x02} uploaded := make(chan struct{}) @@ -1450,7 +1464,7 @@ func TestTransactionTracing(t *testing.T) { uploader.UploadFunc = func(id string, message json.RawMessage) error { uploaded <- struct{}{} require.Equal(t, debug.TraceID(txID, blockID), id) - require.NotEmpty(t, message) + require.Equal(t, trace, message) require.Greater(t, len(message), 0) return nil } @@ -1469,6 +1483,7 @@ func TestTransactionTracing(t *testing.T) { require.NoError(t, err) require.Len(t, results, 1) txID = results[0].TxHash + trace = tracer.GetResultByTxHash(txID) tracer.WithBlockID(blockID) tracer.Collect(txID)