Re-enable trace_transaction and trace_get - issue 1833 (#1835)

* Fixes trace_transaction to use new OETracer from trace_block

* Fixes trace_get to use OETracer from trace_block

* Trace only up to txIndex as per suggestion in trace_transaction

* Cleaning up a few old comments

cmd/rpcdaemon/commands/trace_filtering.go

@@ -23,7 +23,6 @@ import (
 )
 
 // Transaction implements trace_transaction
-// TODO(tjayrush): I think this should return an []interface{}, so we can return both Parity and Geth traces
 func (api *TraceAPIImpl) Transaction(ctx context.Context, txHash common.Hash) (ParityTraces, error) {
 	tx, err := api.kv.BeginRo(ctx)
 	if err != nil {
@@ -31,37 +30,85 @@ func (api *TraceAPIImpl) Transaction(ctx context.Context, txHash common.Hash) (P
 	}
 	defer tx.Rollback()
 
-	traces, err := api.getTransactionTraces(tx, ctx, txHash)
+	txn, _, blockNumber, txIndex := rawdb.ReadTransaction(ethdb.NewRoTxDb(tx), txHash)
+	if txn == nil {
+		return nil, nil // not error, see https://github.com/ledgerwatch/turbo-geth/issues/1645
+	}
+
+	bn := hexutil.Uint64(blockNumber)
+
+	// Extract transactions from block
+	hash, hashErr := rawdb.ReadCanonicalHash(tx, blockNumber)
+	if hashErr != nil {
+		return nil, hashErr
+	}
+	block, senders, sendersErr := rawdb.ReadBlockWithSenders(ethdb.NewRoTxDb(tx), hash, uint64(bn))
+	if sendersErr != nil {
+		return nil, sendersErr
+	}
+	if block == nil {
+		return nil, nil
+	}
+
+	blockTxs := block.Transactions()
+	if len(blockTxs) != len(senders) {
+		return nil, errors.New("block txs len != senders len")
+	}
+
+	txs := make([]TransactionWithSender, 0, len(senders))
+	for n, tx := range blockTxs {
+		if uint64(n) <= txIndex {
+			txs = append(txs, TransactionWithSender{
+				tx:     tx,
+				sender: senders[n],
+			})
+		}
+	}
+
+	baseBn := bn
+	if baseBn > 0 {
+		baseBn -= 1
+	}
+
+	// Returns an array of trace arrays, one trace array for each transaction
+	traces, err := api.callManyTransactions(ctx, tx, txs, hash, rpc.BlockNumber(baseBn))
 	if err != nil {
 		return nil, err
 	}
-	return traces, err
+
+	out := make([]ParityTrace, 0, len(traces))
+	blockno := uint64(bn)
+	for txno, trace := range traces {
+		txhash := txs[txno].tx.Hash()
+		txpos := uint64(txno)
+		// We're only looking for a specific transaction
+		if txpos == txIndex {
+			for _, pt := range trace.Trace {
+				pt.BlockHash = &hash
+				pt.BlockNumber = &blockno
+				pt.TransactionHash = &txhash
+				pt.TransactionPosition = &txpos
+				out = append(out, *pt)
+			}
+		}
+	}
+
+	return out, err
 }
 
 // Get implements trace_get
-// TODO(tjayrush): This command should take an rpc.BlockNumber .This would allow blockNumbers and 'latest',
-// TODO(tjayrush): 'pending', etc. Parity only accepts block hash.
-// TODO(tjayrush): Also, for some reason, Parity definesthe second parameter as an array of indexes, but
-// TODO(tjayrush): only accepts a single one
-// TODO(tjayrush): I think this should return an interface{}, so we can return both Parity and Geth traces
 func (api *TraceAPIImpl) Get(ctx context.Context, txHash common.Hash, indicies []hexutil.Uint64) (*ParityTrace, error) {
-	tx, err := api.kv.BeginRo(ctx)
-	if err != nil {
-		return nil, err
-	}
-	defer tx.Rollback()
-
-	// TODO(tjayrush): Parity fails if it gets more than a single index. Returns nothing in this case.
+	// Parity fails if it gets more than a single index. It returns nothing in this case. Must we?
 	if len(indicies) > 1 {
 		return nil, nil
 	}
 
-	traces, err := api.getTransactionTraces(tx, ctx, txHash)
+	traces, err := api.Transaction(ctx, txHash)
 	if err != nil {
 		return nil, err
 	}
 
-	// TODO(tjayrush): For some reason, the 'get' index is one-based
+	// 'trace_get' index starts at one (oddly)
 	firstIndex := int(indicies[0]) + 1
 	for i, trace := range traces {
 		if i == firstIndex {
@@ -428,54 +475,6 @@ func isAddressInFilter(addr *common.Address, filter []*common.Address) bool {
 	return i != len(filter)
 }
 
-// getTransactionTraces - returns the traces for a single transaction. Used by trace_get and trace_transaction.
-// TODO(tjayrush):
-// Implementation Notes:
-// -- For convienience, we return both Parity and Geth traces for now. In the future we will either separate
-//    these functions or eliminate Geth traces
-// -- The function convertToParityTraces takes a hierarchical Geth trace and returns a flattened Parity trace
-func (api *TraceAPIImpl) getTransactionTraces(tx ethdb.Tx, ctx context.Context, txHash common.Hash) (ParityTraces, error) {
-	getter := adapter.NewBlockGetter(tx)
-	chainConfig, err := api.chainConfig(tx)
-	if err != nil {
-		return nil, err
-	}
-	traceType := "callTracer" // nolint: goconst
-
-	txn, blockHash, blockNumber, txIndex := rawdb.ReadTransaction(ethdb.NewRoTxDb(tx), txHash)
-
-	getHeader := func(hash common.Hash, number uint64) *types.Header {
-		return rawdb.ReadHeader(tx, hash, number)
-	}
-	msg, blockCtx, txCtx, ibs, _, err := transactions.ComputeTxEnv(ctx, getter, chainConfig, getHeader, ethash.NewFaker(), tx, blockHash, txIndex)
-	if err != nil {
-		return nil, err
-	}
-
-	// Time spent 176 out of 205
-	trace, err := transactions.TraceTx(ctx, msg, blockCtx, txCtx, ibs, &tracers.TraceConfig{Tracer: &traceType}, chainConfig)
-	if err != nil {
-		return nil, err
-	}
-
-	traceJSON, ok := trace.(json.RawMessage)
-	if !ok {
-		return nil, fmt.Errorf("unknown type in trace_filter")
-	}
-
-	var gethTrace GethTrace
-	jsonStr, _ := traceJSON.MarshalJSON()
-	// Time spent 26 out of 205
-	json.Unmarshal(jsonStr, &gethTrace) // nolint:errcheck
-
-	traces := ParityTraces{}
-	// Time spent 3 out of 205
-	converted := api.convertToParityTrace(gethTrace, blockHash, blockNumber, txn, txIndex, []int{})
-	traces = append(traces, converted...)
-
-	return traces, nil
-}
-
 // TraceFilterRequest represents the arguments for trace_filter
 type TraceFilterRequest struct {
 	FromBlock   *hexutil.Uint64   `json:"fromBlock"`