fix: address Copilot review feedback on event trigger simulation

- Populate raw value unconditionally from parsed event data, even when
  token metadata lookup fails. The new schema contract is that value
  is always the raw uint256 base-units string; only valueFormatted
  depends on knowing decimals. Fixes a regression where tokens not in
  the whitelist would silently keep the default value of "0".
- Disambiguate "unknown decimals" from "0 decimals" in the synthetic
  Transfer amount path by switching the API to *uint32. Some ERC20
  tokens legitimately have 0 decimals; treating 0 as a sentinel for
  "unknown, assume 18" would scale them incorrectly. nil now means
  unknown (fall back to 18), 0 is honored as a real value (use 1 unit
  since fractional amounts are not representable).
- Extract loop ExecutionContext propagation into a testable helper and
  add unit tests covering propagation from first iteration with
  context, skipping nil/empty iterations, leaving the parent untouched
  when no iteration provides context, and graceful handling of nil
  parent / empty iteration slice.

Author: will-dz

core/taskengine/engine.go

@@ -2928,7 +2928,7 @@ func (n *Engine) SimulateTask(user *model.User, trigger *avsproto.TaskTrigger, n
 		Index:        task.ExecutionCount,                    // Use current execution count for simulation (0-based)
 		ExecutionFee: buildExecutionFee(n.config.FeeRates),
 		Cogs:         buildCOGSFromSteps(vm.ExecutionLogs),
-		ValueFee:     buildValueFee(vm.ExecutionLogs, n.config.FeeRates),
+		ValueFee:     buildValueFee(task.Nodes, n.config.FeeRates),
 	}
 
 	// Log execution status based on result type

core/taskengine/executor.go

@@ -629,7 +629,7 @@ func (x *TaskExecutor) RunTask(task *model.Task, queueData *QueueExecutionData)
 	execution.Steps = vm.ExecutionLogs                        // Contains all steps including failed ones
 	execution.ExecutionFee = buildExecutionFee(x.engine.config.FeeRates)
 	execution.Cogs = buildCOGSFromSteps(vm.ExecutionLogs)
-	execution.ValueFee = buildValueFee(vm.ExecutionLogs, x.engine.config.FeeRates)
+	execution.ValueFee = buildValueFee(task.Nodes, x.engine.config.FeeRates)
 
 	// Value fee recording placeholder.
 	// V1: value fees are not recorded because actual transaction value (the base for

core/taskengine/fee_estimator.go

@@ -562,22 +562,24 @@ func buildExecutionFee(feeRatesConfig *config.FeeRatesConfig) *avsproto.Fee {
 }
 
 // buildValueFee classifies the workflow and returns the value fee.
-// Uses the task's nodes to determine if on-chain execution is present.
-func buildValueFee(steps []*avsproto.Execution_Step, feeRatesConfig *config.FeeRatesConfig) *avsproto.ValueFee {
+// Classification is based on the workflow definition (nodes), not on runtime
+// gas presence — otherwise simulation runs (Tenderly, no real gas) would be
+// classified differently from production runs of the same workflow.
+func buildValueFee(nodes []*avsproto.TaskNode, feeRatesConfig *config.FeeRatesConfig) *avsproto.ValueFee {
 	rates := convertFeeRatesConfig(feeRatesConfig)
 
 	hasOnChainSteps := false
-	for _, step := range steps {
-		stepType := step.Type
-		if stepType == avsproto.NodeType_NODE_TYPE_CONTRACT_WRITE.String() ||
-			stepType == avsproto.NodeType_NODE_TYPE_ETH_TRANSFER.String() {
+	for _, node := range nodes {
+		switch {
+		case node.GetEthTransfer() != nil, node.GetContractWrite() != nil:
 			hasOnChainSteps = true
-			break
+		case node.GetLoop() != nil:
+			loop := node.GetLoop()
+			if loop.GetEthTransfer() != nil || loop.GetContractWrite() != nil {
+				hasOnChainSteps = true
+			}
 		}
-		// A loop step that recorded gas costs ran on-chain work via its inner runner
-		if stepType == avsproto.NodeType_NODE_TYPE_LOOP.String() &&
-			step.TotalGasCost != "" && step.TotalGasCost != "0" {
-			hasOnChainSteps = true
+		if hasOnChainSteps {
 			break
 		}
 	}

core/taskengine/fee_estimator_test.go

@@ -110,6 +110,46 @@ func TestClassifyWorkflowValue_OnChainDetection(t *testing.T) {
 	assert.Equal(t, avsproto.ExecutionTier_EXECUTION_TIER_UNSPECIFIED, resp.Tier, "Empty node should be UNSPECIFIED")
 }
 
+// TestBuildValueFee_ClassifiesByDefinitionNotGas asserts that the post-execution
+// value-fee classifier looks at the workflow definition (task.Nodes), not at
+// runtime gas presence in the execution logs. Without this, simulation runs
+// (Tenderly, no real gas) get classified as "no on-chain execution nodes" while
+// production runs of the same workflow get Tier 1, causing fee divergence.
+func TestBuildValueFee_ClassifiesByDefinitionNotGas(t *testing.T) {
+	// Revenue-splitter shape: customCode (split) -> loop(contractWrite).
+	// In simulation the loop step has no TotalGasCost, but it must still be
+	// classified as on-chain because the workflow definition says so.
+	splitterNodes := []*avsproto.TaskNode{
+		{TaskType: &avsproto.TaskNode_CustomCode{CustomCode: &avsproto.CustomCodeNode{}}},
+		{TaskType: &avsproto.TaskNode_Loop{Loop: &avsproto.LoopNode{
+			Runner: &avsproto.LoopNode_ContractWrite{ContractWrite: &avsproto.ContractWriteNode{}},
+		}}},
+	}
+
+	vf := buildValueFee(splitterNodes, nil)
+	require.NotNil(t, vf)
+	assert.Equal(t, avsproto.ExecutionTier_EXECUTION_TIER_1, vf.Tier,
+		"loop(contractWrite) workflow must be Tier 1 even when simulation reports zero gas")
+	assert.Equal(t, "0.03", vf.Fee.Amount)
+	assert.Equal(t, "PERCENTAGE", vf.Fee.Unit)
+
+	// Pure off-chain workflow stays UNSPECIFIED / 0%.
+	offChainNodes := []*avsproto.TaskNode{
+		{TaskType: &avsproto.TaskNode_CustomCode{CustomCode: &avsproto.CustomCodeNode{}}},
+		{TaskType: &avsproto.TaskNode_RestApi{RestApi: &avsproto.RestAPINode{}}},
+	}
+	vfOff := buildValueFee(offChainNodes, nil)
+	assert.Equal(t, avsproto.ExecutionTier_EXECUTION_TIER_UNSPECIFIED, vfOff.Tier)
+	assert.Equal(t, "0", vfOff.Fee.Amount)
+
+	// Direct contractWrite (no loop) classifies as on-chain.
+	directNodes := []*avsproto.TaskNode{
+		{TaskType: &avsproto.TaskNode_ContractWrite{ContractWrite: &avsproto.ContractWriteNode{}}},
+	}
+	vfDirect := buildValueFee(directNodes, nil)
+	assert.Equal(t, avsproto.ExecutionTier_EXECUTION_TIER_1, vfDirect.Tier)
+}
+
 func TestDefaultFeeRates(t *testing.T) {
 	rates := getDefaultFeeRates()
 

core/taskengine/loop_helpers.go

@@ -14,6 +14,29 @@ import (
 // These functions support template variable substitution, nested node creation, and
 // input variable resolution for loop iterations in VM.executeLoopWithQueue.
 
+// propagateLoopExecutionContext copies the ExecutionContext from the first iteration
+// step that has one set onto the parent loop step. This is needed because
+// CreateNodeExecutionStep can't infer the simulation mode for loops — the simulation
+// flag lives on the inner runner (contractWrite, ethTransfer), not the loop itself.
+// Without this, a loop wrapping Tenderly-simulated contract writes would mislabel
+// itself as is_simulated=false, provider=chain_rpc.
+//
+// We pick the first non-nil iteration context (rather than e.g. validating all
+// match) because all iterations of a loop run through the same runner and therefore
+// share the same simulation mode by construction. If iterationSteps is empty or all
+// entries lack ExecutionContext, the parent step is left untouched.
+func propagateLoopExecutionContext(parent *avsproto.Execution_Step, iterationSteps []*avsproto.Execution_Step) {
+	if parent == nil {
+		return
+	}
+	for _, iterStep := range iterationSteps {
+		if iterStep != nil && iterStep.ExecutionContext != nil {
+			parent.ExecutionContext = iterStep.ExecutionContext
+			return
+		}
+	}
+}
+
 // substituteTemplateVariables replaces template variables like {{value}} and {{index}} with actual values.
 // This is a pure function with no VM dependency.
 //

core/taskengine/loop_helpers_test.go

@@ -0,0 +1,93 @@
+package taskengine
+
+import (
+	"testing"
+
+	avsproto "github.com/AvaProtocol/EigenLayer-AVS/protobuf"
+	"google.golang.org/protobuf/types/known/structpb"
+)
+
+// TestPropagateLoopExecutionContext verifies that a loop step inherits the
+// ExecutionContext from its first iteration step that has one. This is what
+// makes a loop wrapping Tenderly-simulated contract writes correctly report
+// `is_simulated: true, provider: tenderly` instead of the misleading
+// `chain_rpc` default that CreateNodeExecutionStep would otherwise leave behind.
+func TestPropagateLoopExecutionContext(t *testing.T) {
+	mkCtx := func(simulated bool, provider string) *structpb.Value {
+		v, err := structpb.NewValue(map[string]interface{}{
+			"is_simulated": simulated,
+			"provider":     provider,
+			"chain_id":     11155111,
+		})
+		if err != nil {
+			t.Fatalf("structpb.NewValue: %v", err)
+		}
+		return v
+	}
+
+	t.Run("propagates from first iteration with context", func(t *testing.T) {
+		parent := &avsproto.Execution_Step{Id: "loop1"}
+		iter0 := &avsproto.Execution_Step{Id: "iter0", ExecutionContext: mkCtx(true, "tenderly")}
+		iter1 := &avsproto.Execution_Step{Id: "iter1", ExecutionContext: mkCtx(true, "tenderly")}
+
+		propagateLoopExecutionContext(parent, []*avsproto.Execution_Step{iter0, iter1})
+
+		if parent.ExecutionContext == nil {
+			t.Fatal("expected parent ExecutionContext to be set")
+		}
+		ctxMap := parent.ExecutionContext.GetStructValue().AsMap()
+		if got := ctxMap["is_simulated"]; got != true {
+			t.Errorf("is_simulated = %v, want true", got)
+		}
+		if got := ctxMap["provider"]; got != "tenderly" {
+			t.Errorf("provider = %v, want tenderly", got)
+		}
+	})
+
+	t.Run("skips nil iterations and picks first non-nil context", func(t *testing.T) {
+		parent := &avsproto.Execution_Step{Id: "loop1"}
+		iter0 := &avsproto.Execution_Step{Id: "iter0"} // no ExecutionContext
+		iter1 := (*avsproto.Execution_Step)(nil)
+		iter2 := &avsproto.Execution_Step{Id: "iter2", ExecutionContext: mkCtx(false, "bundler")}
+
+		propagateLoopExecutionContext(parent, []*avsproto.Execution_Step{iter0, iter1, iter2})
+
+		if parent.ExecutionContext == nil {
+			t.Fatal("expected parent ExecutionContext to be set from iter2")
+		}
+		ctxMap := parent.ExecutionContext.GetStructValue().AsMap()
+		if got := ctxMap["provider"]; got != "bundler" {
+			t.Errorf("provider = %v, want bundler", got)
+		}
+		if got := ctxMap["is_simulated"]; got != false {
+			t.Errorf("is_simulated = %v, want false", got)
+		}
+	})
+
+	t.Run("leaves parent untouched when no iteration has context", func(t *testing.T) {
+		baseline := mkCtx(false, "chain_rpc")
+		parent := &avsproto.Execution_Step{Id: "loop1", ExecutionContext: baseline}
+		iter0 := &avsproto.Execution_Step{Id: "iter0"}
+
+		propagateLoopExecutionContext(parent, []*avsproto.Execution_Step{iter0})
+
+		if parent.ExecutionContext != baseline {
+			t.Error("expected parent ExecutionContext to remain unchanged when no iteration provides one")
+		}
+	})
+
+	t.Run("handles empty iterations slice", func(t *testing.T) {
+		parent := &avsproto.Execution_Step{Id: "loop1"}
+		propagateLoopExecutionContext(parent, nil)
+		if parent.ExecutionContext != nil {
+			t.Error("expected parent ExecutionContext to remain nil for empty iterations")
+		}
+	})
+
+	t.Run("handles nil parent gracefully", func(t *testing.T) {
+		// Should not panic.
+		propagateLoopExecutionContext(nil, []*avsproto.Execution_Step{
+			{ExecutionContext: mkCtx(true, "tenderly")},
+		})
+	})
+}

core/taskengine/run_node_immediately.go

@@ -1599,10 +1599,12 @@ func (n *Engine) runEventTriggerWithTenderlySimulation(ctx context.Context, quer
 	// the default synthetic value dwarfs realistic wallet balances and downstream
 	// Tenderly transfer simulations revert with "ERC20: transfer amount exceeds balance".
 	// Cached after first lookup, so subsequent simulations are free.
-	var simulatedTokenDecimals uint32
+	// nil = unknown (fall back to 18); a real 0 is preserved (legitimate ERC20 case).
+	var simulatedTokenDecimals *uint32
 	if n.tokenEnrichmentService != nil && len(query.GetAddresses()) > 0 {
 		if md, mdErr := n.tokenEnrichmentService.GetTokenMetadata(query.GetAddresses()[0]); mdErr == nil && md != nil {
-			simulatedTokenDecimals = md.Decimals
+			d := md.Decimals
+			simulatedTokenDecimals = &d
 		}
 	}
 

core/taskengine/shared_event_enrichment.go

@@ -264,34 +264,36 @@ func enrichTransferEventShared(eventLog *types.Log, parsedData map[string]interf
 		}
 	}
 
-	// Populate token metadata if available
+	// Always populate the raw value from the parsed event, regardless of whether
+	// token metadata is available. The new schema contract is that `value` is the
+	// raw uint256 base-units string — clients depend on this for BigInt math even
+	// when metadata lookup fails (token not in whitelist, RPC error, etc.).
+	// `valueFormatted` is only computed when we know the decimals.
+	var rawValueStr string
+	if transferEventData != nil {
+		if rawValue, ok := transferEventData["value"]; ok {
+			switch v := rawValue.(type) {
+			case *big.Int:
+				rawValueStr = v.String()
+			case string:
+				rawValueStr = v
+			default:
+				rawValueStr = fmt.Sprintf("%v", v)
+			}
+		}
+	}
+	if rawValueStr != "" {
+		transferResponse.Value = rawValueStr
+	}
+
+	// Populate token metadata and the formatted value if available
 	if tokenMetadata != nil {
 		transferResponse.TokenName = tokenMetadata.Name
 		transferResponse.TokenSymbol = tokenMetadata.Symbol
 		transferResponse.TokenDecimals = tokenMetadata.Decimals
 
-		// Populate both raw and formatted value:
-		//   - value          → raw uint256 base-units string (for on-chain math, e.g. BigInt())
-		//   - valueFormatted → human-readable decimal-adjusted string (for display)
-		// This matches Moralis / Etherscan / subgraph conventions and avoids the
-		// foot-gun where downstream BigInt() consumers receive "1.5" and crash.
-		if transferEventData != nil {
-			if rawValue, ok := transferEventData["value"]; ok {
-				var valueStr string
-
-				// Handle different value types (big.Int, string, etc.)
-				switch v := rawValue.(type) {
-				case *big.Int:
-					valueStr = v.String()
-				case string:
-					valueStr = v
-				default:
-					valueStr = fmt.Sprintf("%v", v)
-				}
-
-				transferResponse.Value = valueStr
-				transferResponse.ValueFormatted = tokenService.FormatTokenValue(valueStr, tokenMetadata.Decimals)
-			}
+		if rawValueStr != "" {
+			transferResponse.ValueFormatted = tokenService.FormatTokenValue(rawValueStr, tokenMetadata.Decimals)
 		}
 
 		if logger != nil {

core/taskengine/tenderly_client.go

@@ -94,13 +94,16 @@ func NewTenderlyClient(cfg *config.Config, logger sdklogging.Logger) *TenderlyCl
 // that know the token's actual decimals should use SimulateEventTriggerWithDecimals
 // so the synthetic value scales correctly (e.g., USDC with 6 decimals).
 func (tc *TenderlyClient) SimulateEventTrigger(ctx context.Context, query *avsproto.EventTrigger_Query, chainID int64) (*types.Log, error) {
-	return tc.SimulateEventTriggerWithDecimals(ctx, query, chainID, 0)
+	return tc.SimulateEventTriggerWithDecimals(ctx, query, chainID, nil)
 }
 
 // SimulateEventTriggerWithDecimals is like SimulateEventTrigger but lets callers
 // pass the token's decimals so simulated Transfer amounts represent ~1.5 tokens
-// regardless of the token's decimal precision. Pass 0 to use the 18-decimal default.
-func (tc *TenderlyClient) SimulateEventTriggerWithDecimals(ctx context.Context, query *avsproto.EventTrigger_Query, chainID int64, tokenDecimals uint32) (*types.Log, error) {
+// regardless of the token's decimal precision. Pass nil if the token's decimals
+// are unknown — the simulator will fall back to an 18-decimal assumption. Note
+// that 0 is a legitimate decimals value (some ERC20s have it), so a pointer is
+// used to disambiguate "unknown" from "zero".
+func (tc *TenderlyClient) SimulateEventTriggerWithDecimals(ctx context.Context, query *avsproto.EventTrigger_Query, chainID int64, tokenDecimals *uint32) (*types.Log, error) {
 	if err := ctx.Err(); err != nil {
 		return nil, fmt.Errorf("context error before simulation: %w", err)
 	}
@@ -286,9 +289,10 @@ func (tc *TenderlyClient) createMockAnswerUpdatedLog(contractAddress string, pri
 }
 
 // simulateTransferEvent simulates an actual ERC20 transfer transaction using Tenderly simulation API.
-// tokenDecimals scales the synthetic default amount (1.5 tokens) to the token's decimal precision;
-// pass 0 to default to 18 decimals.
-func (tc *TenderlyClient) simulateTransferEvent(ctx context.Context, contractAddress string, query *avsproto.EventTrigger_Query, chainID int64, tokenDecimals uint32) (*types.Log, error) {
+// tokenDecimals scales the synthetic default amount (1.5 tokens) to the token's decimal precision.
+// Pass nil for unknown decimals — the simulator falls back to 18. Pointer is used so that 0 (a
+// legitimate ERC20 decimals value) is not conflated with "unknown".
+func (tc *TenderlyClient) simulateTransferEvent(ctx context.Context, contractAddress string, query *avsproto.EventTrigger_Query, chainID int64, tokenDecimals *uint32) (*types.Log, error) {
 	tc.logger.Info("🔮 Simulating ERC20 Transfer transaction via Tenderly API",
 		"contract", contractAddress,
 		"chain_id", chainID)
@@ -334,19 +338,31 @@ func (tc *TenderlyClient) simulateTransferEvent(ctx context.Context, contractAdd
 		toAddress = common.HexToAddress("0x0000000000000000000000000000000000000002")
 	}
 
-	// Default synthetic amount: 1.5 tokens, scaled to the token's actual decimals.
+	// Default synthetic amount: ~1.5 tokens, scaled to the token's actual decimals.
 	// Without scaling, an 18-decimal default (1.5e18) becomes 1.5 trillion units for
 	// a 6-decimal token like USDC, which then dwarfs any realistic wallet balance and
 	// causes downstream Tenderly transfer simulations to revert with "exceeds balance".
-	decimalsForScale := tokenDecimals
-	if decimalsForScale == 0 {
-		decimalsForScale = 18
-	}
-	// transferAmount = 15 * 10^(decimalsForScale-1)  → represents 1.5 tokens
-	transferAmount := new(big.Int).Mul(
-		big.NewInt(15),
-		new(big.Int).Exp(big.NewInt(10), big.NewInt(int64(decimalsForScale)-1), nil),
-	)
+	//
+	// Decimal handling:
+	//   - tokenDecimals == nil   → unknown, fall back to 18-decimal assumption
+	//   - tokenDecimals == 0     → real 0-decimal token (e.g. some governance tokens);
+	//                              fractional amounts aren't representable, use 1 unit
+	//   - tokenDecimals > 0      → represent 1.5 tokens as 15 * 10^(decimals-1)
+	var transferAmount *big.Int
+	switch {
+	case tokenDecimals == nil:
+		transferAmount = new(big.Int).Mul(
+			big.NewInt(15),
+			new(big.Int).Exp(big.NewInt(10), big.NewInt(17), nil), // 1.5 * 10^18
+		)
+	case *tokenDecimals == 0:
+		transferAmount = big.NewInt(1)
+	default:
+		transferAmount = new(big.Int).Mul(
+			big.NewInt(15),
+			new(big.Int).Exp(big.NewInt(10), big.NewInt(int64(*tokenDecimals)-1), nil),
+		)
+	}
 
 	for _, condition := range query.GetConditions() {
 		if condition.GetFieldName() == "value" || condition.GetFieldName() == "amount" {