Fixed potential out of bounds error when dealing with the keccak preimages

core/tracing/hooks.go

@@ -216,12 +216,15 @@ type Hooks struct {
 	// Block hash read
 	OnBlockHashRead BlockHashReadHook
 
-	// Firehose backward compatibility
+	// Firehose requirements.
-	// This hook exist because some current Firehose supported chains requires it
+	//
-	// but this field is going to be deprecated and newer chains will not produced
+	// Search a368bc8a3737 within the repository to find all the details
-	// those events anymore. The hook is registered conditionally based on the
-	// tracer configuration.
 	OnNewAccount func(address common.Address)
+
+	// Firehose requirements.
+	//
+	// Search 11471b22bb0b within the repository to find all the details
+	OnKeccakPreimage func(hash common.Hash, preimage []byte)
 }
 
 // BalanceChangeReason is used to indicate the reason for a balance change, useful

core/vm/instructions.go

@@ -246,6 +246,14 @@ func opKeccak256(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) (
 	if evm.Config.EnablePreimageRecording {
 		evm.StateDB.AddPreimage(interpreter.hasherBuf, data)
 	}
+
+	// Firehose requirements.
+	//
+	// Search 11471b22bb0b within the repository to find all the details
+	if evm.Config.Tracer != nil && evm.Config.Tracer.OnKeccakPreimage != nil {
+		evm.Config.Tracer.OnKeccakPreimage(interpreter.hasherBuf, data)
+	}
+
 	size.SetBytes(interpreter.hasherBuf[:])
 	return nil, nil
 }

eth/tracers/firehose.go

@@ -98,10 +98,35 @@ func NewTracingHooksFromFirehose(tracer *Firehose) *tracing.Hooks {
 		OnSystemCallStart: tracer.OnSystemCallStart,
 		OnSystemCallEnd:   tracer.OnSystemCallEnd,
 
+		// For a reason yet to be discovered, some transactions panics when trying to
+		// compute the keccak hash from a preimage when it comes the time to retrieve
+		// the memory location by inspecting the opcode's EVM stack arguments. The panic
+		// is an index out of bound error.
+		//
+		// To avoid the problem altogether, we return to our old Firehose tracer hook directly
+		// when the instructions is called within the EVM. That has the added benefit of
+		// avoiding re-computing the keccak results of the preimage again since at this location,
+		// we have both the result and the preimage.
+		//
+		// The cons of this is that we need to keep some Geth internal changes to have the hook
+		// called. But this is minimal as we do have to maintain a fork and the changes are actually
+		// minimal.
+		//
+		// Comment 11471b22bb0b (search '11471b22bb0b' within the repository to see all related code locations)
+		OnKeccakPreimage: tracer.OnKeccakPreimage,
+
+		// Firehose backward compatibility
+		//
+		// This hook exist because some current Firehose supported chains requires it
+		// but this field is going to be deprecated and newer chains will not produced
+		// those events anymore.
+		//
 		// This should actually be conditional but it's not possible to do it in the hooks
 		// directly because the chain ID will be known only after the `OnBlockchainInit` call.
 		// So we register it unconditionally and the actual `OnNewAccount` hook will decide
 		// what it needs to do.
+		//
+		// Comment a368bc8a3737 (search 'a368bc8a3737' within the repository to see all related code locations)
 		OnNewAccount: tracer.OnNewAccount,
 	}
 }
@@ -1099,8 +1124,9 @@ func (f *Firehose) OnOpcode(pc uint64, op byte, gas, cost uint64, scope tracing.
 		}
 
 		switch opCode {
-		case vm.KECCAK256:
+		// Bogus, search 11471b22bb0b within the repository to find all the details
-			f.onOpcodeKeccak256(activeCall, scope.StackData(), Memory(scope.MemoryData()))
+		// case vm.KECCAK256:
+		// 	f.onOpcodeKeccak256(activeCall, scope.StackData(), Memory(scope.MemoryData()))
 
 		case vm.SELFDESTRUCT:
 			f.ensureInCall()
@@ -1109,6 +1135,33 @@ func (f *Firehose) OnOpcode(pc uint64, op byte, gas, cost uint64, scope tracing.
 	}
 }
 
+func (f *Firehose) OnKeccakPreimage(hash common.Hash, preImage []byte) {
+	f.ensureInBlockAndInTrxAndInCall()
+
+	activeCall := f.callStack.Peek()
+	if activeCall.KeccakPreimages == nil {
+		activeCall.KeccakPreimages = make(map[string]string)
+	}
+
+	encodedData := hex.EncodeToString(preImage)
+	if *f.applyBackwardCompatibility {
+		// Known Firehose issue: It appears the old Firehose instrumentation have a bug
+		// where when the keccak256 preimage is empty, it is written as "." which is
+		// completely wrong.
+		//
+		// To keep the same behavior, we will write the preimage as a "." when the encoded
+		// data is an empty string.
+		if encodedData == "" {
+			encodedData = "."
+		}
+	}
+
+	activeCall.KeccakPreimages[hex.EncodeToString(hash[:])] = encodedData
+}
+
+// Unused due to bogus behavior, search 11471b22bb0b within the repository to find all the details
+var _ any = new(Firehose).onOpcodeKeccak256
+
 // onOpcodeKeccak256 is called during the SHA3 (a.k.a KECCAK256) opcode it's known
 // in Firehose tracer as Keccak preimages. The preimage is the input data that
 // was used to produce the given keccak hash.