From 3263c78ed612f9cd3c6234b4bd619ecf8ace1787 Mon Sep 17 00:00:00 2001 From: Chris Li Date: Thu, 16 Apr 2026 12:28:06 -0700 Subject: [PATCH 1/5] fix: silence expected user-workflow failures from Sentry error alerts (#526) Co-authored-by: Will Zimmerman --- aggregator/rpc_server.go | 6 +- core/taskengine/engine.go | 5 +- core/taskengine/executor.go | 5 +- core/taskengine/tenderly_client.go | 9 ++- core/taskengine/vm_runner_contract_write.go | 39 +++++----- core/taskengine/vm_runner_eth_transfer.go | 4 +- pkg/erc4337/preset/bundler_error.go | 43 +++++++++++ pkg/erc4337/preset/bundler_error_test.go | 82 +++++++++++++++++++++ 8 files changed, 168 insertions(+), 25 deletions(-) create mode 100644 pkg/erc4337/preset/bundler_error.go create mode 100644 pkg/erc4337/preset/bundler_error_test.go diff --git a/aggregator/rpc_server.go b/aggregator/rpc_server.go index 4a37698b..ce803c51 100644 --- a/aggregator/rpc_server.go +++ b/aggregator/rpc_server.go @@ -455,7 +455,11 @@ func (r *RpcServer) WithdrawFunds(ctx context.Context, payload *avsproto.Withdra ) if err != nil { - r.config.Logger.Error("failed to send withdrawal UserOp", + // See preset.LogBundlerError: Warn on on-chain revert (user's withdrawal + // reverted — e.g. ERC20 transfer to blacklisted recipient, insufficient + // token balance after race), Error on infra/AA (bundler down, AA21, etc.). + preset.LogBundlerError(r.config.Logger, err, + "failed to send withdrawal UserOp", "error", err, "user", user.Address.String(), "recipient", payload.RecipientAddress, diff --git a/core/taskengine/engine.go b/core/taskengine/engine.go index aad95f2b..80dfb405 100644 --- a/core/taskengine/engine.go +++ b/core/taskengine/engine.go @@ -3080,7 +3080,10 @@ func (n *Engine) SimulateTask(user *model.User, trigger *avsproto.TaskTrigger, n cleanErrorMsg = stackTraceRegex.ReplaceAllString(cleanErrorMsg, "") cleanErrorMsg = strings.TrimSpace(cleanErrorMsg) - n.logger.Error("workflow simulation completed with failures", + // User-workflow simulation failure: per-step errors are captured in the + // persisted execution steps. Log summary at Warn so it stays out of Sentry + // error alerts. + n.logger.Warn("workflow simulation completed with failures", "error", cleanErrorMsg, "task_id", task.Id, "simulation_id", simulationID, diff --git a/core/taskengine/executor.go b/core/taskengine/executor.go index a0ac0f18..bacb88d6 100644 --- a/core/taskengine/executor.go +++ b/core/taskengine/executor.go @@ -653,7 +653,10 @@ func (x *TaskExecutor) RunTask(task *model.Task, queueData *QueueExecutionData) case ExecutionSuccess: x.logger.Info("task execution completed successfully", "task_id", task.Id, "execution_id", queueData.ExecutionID, "total_steps", len(vm.ExecutionLogs)) case ExecutionFailed: - x.logger.Error("task execution completed with failures", + // User-workflow failure: per-step errors are already logged at their sites + // and the ExecutionStatus_EXECUTION_STATUS_FAILED is persisted below. Log + // the summary at Warn so it stays out of Sentry error alerts. + x.logger.Warn("task execution completed with failures", "error", executionError, "task_id", task.Id, "execution_id", queueData.ExecutionID, diff --git a/core/taskengine/tenderly_client.go b/core/taskengine/tenderly_client.go index 5777fa58..b7e08752 100644 --- a/core/taskengine/tenderly_client.go +++ b/core/taskengine/tenderly_client.go @@ -1049,7 +1049,10 @@ func (tc *TenderlyClient) SimulateContractWrite(ctx context.Context, contractAdd if status, ok := sim["status"].(bool); ok && !status { result.Success = false if em, ok := sim["error_message"].(string); ok && em != "" { - tc.logger.Error("❌ Tenderly simulation failed: simulation.status=false", + // Simulation catching a future revert is the feature working + // as intended — user-workflow failure, not infra. Log at Warn + // so it stays out of Sentry error alerts. + tc.logger.Warn("tenderly simulation failed: simulation.status=false", "contract", contractAddress, "method", methodName, "error_message", em, @@ -1081,7 +1084,9 @@ func (tc *TenderlyClient) SimulateContractWrite(ctx context.Context, contractAdd // Look for error in nested calls (like ERC20 transferFrom failures) if errMsg, ok := callMap["error"].(string); ok && errMsg != "" { errorMsg = errMsg - tc.logger.Error("❌ Tenderly simulation failed: transaction reverted", + // User-workflow revert caught by simulation — log at + // Warn to keep out of Sentry error alerts. + tc.logger.Warn("tenderly simulation failed: transaction reverted", "contract", contractAddress, "method", methodName, "error_from_call_trace", errMsg, diff --git a/core/taskengine/vm_runner_contract_write.go b/core/taskengine/vm_runner_contract_write.go index 01201406..20425b43 100644 --- a/core/taskengine/vm_runner_contract_write.go +++ b/core/taskengine/vm_runner_contract_write.go @@ -814,7 +814,12 @@ func (r *ContractWriteProcessor) executeRealUserOpTransaction(ctx context.Contex } } - r.vm.logger.Error("🚫 BUNDLER FAILED - UserOp transaction failed, workflow execution FAILED", + // preset.LogBundlerError picks Error vs Warn based on the error: on-chain + // reverts (expected user-workflow outcomes) log at Warn so they don't page + // Sentry; real infra/AA failures (bundler down, AA21/AA23/AA25, paymaster + // revert) stay at Error. + preset.LogBundlerError(r.vm.logger, err, + "bundler: UserOp transaction failed, workflow execution FAILED", "bundler_error", err, "bundler_url", r.smartWalletConfig.BundlerURL, "method", methodName, @@ -1209,43 +1214,35 @@ func (r *ContractWriteProcessor) convertTenderlyResultToFlexibleFormat(result *C receipt, _ := structpb.NewValue(receiptMap) - // Extract return value from Tenderly response + // Extract return value from Tenderly response. + // ReturnData is nil when the provider did not return output data (e.g. simulation + // reverted — tenderly_client.go clears ReturnData in that case). That path leaves + // Value as nil, which is the expected behavior. var returnValue *structpb.Value if result.ReturnData != nil { - r.vm.logger.Info("🔍 CRITICAL DEBUG - ReturnData found", - "method", result.MethodName, - "returnData_name", result.ReturnData.Name, - "returnData_type", result.ReturnData.Type, - "returnData_value", result.ReturnData.Value) - // Parse the JSON value from ReturnData and convert to protobuf var parsedValue interface{} if err := json.Unmarshal([]byte(result.ReturnData.Value), &parsedValue); err == nil { // Successfully parsed JSON, convert to protobuf if valueProto, err := structpb.NewValue(parsedValue); err == nil { returnValue = valueProto - r.vm.logger.Info("✅ CRITICAL DEBUG - Successfully created returnValue protobuf", - "method", result.MethodName, - "parsedValue", parsedValue) } else { - r.vm.logger.Error("❌ CRITICAL DEBUG - Failed to create protobuf from parsedValue", + r.vm.logger.Debug("failed to create protobuf from parsed ReturnData", "method", result.MethodName, "error", err) } } else { - r.vm.logger.Error("❌ CRITICAL DEBUG - Failed to unmarshal JSON from ReturnData.Value", + // Non-JSON return types (bytes32, address, etc.) are expected; fall through + // to raw-string handling below. + r.vm.logger.Debug("ReturnData is not JSON, falling back to raw string", "method", result.MethodName, - "error", err, - "raw_value", result.ReturnData.Value) + "error", err) // Fallback: treat as raw string if JSON parsing fails if valueProto, err := structpb.NewValue(result.ReturnData.Value); err == nil { returnValue = valueProto } } - } else { - r.vm.logger.Error("❌ CRITICAL DEBUG - ReturnData is nil", - "method", result.MethodName) } // No fallback default value. If provider does not return output data, Value remains nil @@ -1624,7 +1621,11 @@ func (r *ContractWriteProcessor) Execute(stepID string, node *avsproto.ContractW } } } else { - r.vm.logger.Error("🚨 DEPLOYED WORKFLOW: Method execution failed", + // User-workflow failure: method returned success=false. The concrete + // cause is already logged by the upstream site (Tenderly simulation at + // tenderly_client.go, or bundler/AA at line ~817). Re-logging at Warn + // here keeps operator-visible context without paging Sentry. + r.vm.logger.Warn("deployed workflow: method execution failed", "method_name", result.MethodName, "error_message", result.Error, "error_length", len(result.Error), diff --git a/core/taskengine/vm_runner_eth_transfer.go b/core/taskengine/vm_runner_eth_transfer.go index 60917d0d..dcdf1192 100644 --- a/core/taskengine/vm_runner_eth_transfer.go +++ b/core/taskengine/vm_runner_eth_transfer.go @@ -344,7 +344,9 @@ func (p *ETHTransferProcessor) executeRealETHTransfer(stepID, destination, amoun ) if err != nil { - p.vm.logger.Error("🚫 BUNDLER FAILED - ETH transfer UserOp transaction failed", + // See preset.LogBundlerError: Warn on on-chain revert, Error on infra/AA. + preset.LogBundlerError(p.vm.logger, err, + "bundler: ETH transfer UserOp transaction failed", "bundler_error", err, "bundler_url", p.smartWalletConfig.BundlerURL, "destination", destination, diff --git a/pkg/erc4337/preset/bundler_error.go b/pkg/erc4337/preset/bundler_error.go new file mode 100644 index 00000000..4e7fa812 --- /dev/null +++ b/pkg/erc4337/preset/bundler_error.go @@ -0,0 +1,43 @@ +package preset + +import ( + "strings" + + "github.com/AvaProtocol/EigenLayer-AVS/pkg/logger" +) + +// userOpRevertMarker identifies errors returned by SendUserOp when the UserOp +// was included on-chain but the target contract call reverted. The marker +// string is emitted from waitForUserOpConfirmation via fmt.Errorf. +const userOpRevertMarker = "success=false in UserOperationEvent" + +// IsUserOpRevert reports whether err represents an on-chain revert of the user +// target contract (UserOp was mined but UserOperationEvent.success == false), +// as distinct from infra/AA failures such as bundler unreachable, AA21 prefund, +// AA23 reverted, AA25 invalid nonce, or paymaster revert. +// +// On-chain reverts are expected user-workflow outcomes and should not escalate +// to Sentry error alerts. Infra/AA failures should. +func IsUserOpRevert(err error) bool { + if err == nil { + return false + } + return strings.Contains(err.Error(), userOpRevertMarker) +} + +// LogBundlerError logs a bundler/UserOp failure at the severity appropriate +// for its cause: Warn for on-chain reverts (see IsUserOpRevert) so they do not +// page Sentry, Error for real infra/AA failures that operators must see. +// +// Callers pass the error both for classification (the first argument) and, +// conventionally, as a tag value so the logged record includes the full error. +func LogBundlerError(lgr logger.Logger, err error, msg string, tags ...any) { + if lgr == nil { + return + } + if IsUserOpRevert(err) { + lgr.Warn(msg, tags...) + return + } + lgr.Error(msg, tags...) +} diff --git a/pkg/erc4337/preset/bundler_error_test.go b/pkg/erc4337/preset/bundler_error_test.go new file mode 100644 index 00000000..a2bc3907 --- /dev/null +++ b/pkg/erc4337/preset/bundler_error_test.go @@ -0,0 +1,82 @@ +package preset + +import ( + "errors" + "fmt" + "sync" + "testing" + + sdklogging "github.com/Layr-Labs/eigensdk-go/logging" +) + +func TestIsUserOpRevert(t *testing.T) { + cases := []struct { + name string + err error + want bool + }{ + {"nil", nil, false}, + {"unrelated", errors.New("dial tcp: connection refused"), false}, + {"AA21 prefund", errors.New("AA21 didn't pay prefund"), false}, + {"AA25 nonce", errors.New("AA25 invalid account nonce"), false}, + {"direct marker", errors.New("UserOp execution failed (success=false in UserOperationEvent) - tx: 0xabc"), true}, + {"wrapped marker", fmt.Errorf("UserOp execution failed: %w", errors.New("UserOp execution failed (success=false in UserOperationEvent) - tx: 0xabc")), true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := IsUserOpRevert(tc.err); got != tc.want { + t.Errorf("IsUserOpRevert(%v) = %v, want %v", tc.err, got, tc.want) + } + }) + } +} + +// bundlerErrorSpyLogger captures which severity method was invoked for LogBundlerError. +type bundlerErrorSpyLogger struct { + mu sync.Mutex + calls []string // method names in order +} + +func (s *bundlerErrorSpyLogger) record(method string) { + s.mu.Lock() + defer s.mu.Unlock() + s.calls = append(s.calls, method) +} + +func (s *bundlerErrorSpyLogger) Debug(string, ...any) {} +func (s *bundlerErrorSpyLogger) Debugf(string, ...any) {} +func (s *bundlerErrorSpyLogger) Info(string, ...any) {} +func (s *bundlerErrorSpyLogger) Infof(string, ...any) {} +func (s *bundlerErrorSpyLogger) Warn(string, ...any) { s.record("Warn") } +func (s *bundlerErrorSpyLogger) Warnf(string, ...any) { s.record("Warn") } +func (s *bundlerErrorSpyLogger) Error(string, ...any) { s.record("Error") } +func (s *bundlerErrorSpyLogger) Errorf(string, ...any) { s.record("Error") } +func (s *bundlerErrorSpyLogger) Fatal(string, ...any) {} +func (s *bundlerErrorSpyLogger) Fatalf(string, ...any) {} +func (s *bundlerErrorSpyLogger) With(...any) sdklogging.Logger { return s } + +func TestLogBundlerError(t *testing.T) { + cases := []struct { + name string + err error + want string + }{ + {"on-chain revert → Warn", errors.New("UserOp execution failed (success=false in UserOperationEvent) - tx: 0xabc"), "Warn"}, + {"AA21 infra → Error", errors.New("AA21 didn't pay prefund"), "Error"}, + {"bundler down → Error", errors.New("dial tcp: connection refused"), "Error"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + spy := &bundlerErrorSpyLogger{} + LogBundlerError(spy, tc.err, "bundler failed", "err", tc.err) + if len(spy.calls) != 1 || spy.calls[0] != tc.want { + t.Errorf("expected single %s call, got %v", tc.want, spy.calls) + } + }) + } +} + +func TestLogBundlerError_NilLogger(t *testing.T) { + // Must not panic. + LogBundlerError(nil, errors.New("anything"), "msg") +} From 2f71d77e3bd922140b7e151e4556209170c3b5e3 Mon Sep 17 00:00:00 2001 From: Chris Li Date: Fri, 17 Apr 2026 11:08:39 -0700 Subject: [PATCH 2/5] fix: ERC20 approve state propagation for Uniswap workflow simulation (#527) Co-authored-by: Will Zimmerman --- .../simulate_uniswap_workflow_test.go | 449 ++++++++++++++++++ core/taskengine/simulation_state.go | 18 + core/taskengine/vm_runner_contract_write.go | 42 +- ...02-uniswap-simulation-state-propagation.md | 68 +++ 4 files changed, 574 insertions(+), 3 deletions(-) create mode 100644 core/taskengine/simulate_uniswap_workflow_test.go create mode 100644 docs/changes/0002-uniswap-simulation-state-propagation.md diff --git a/core/taskengine/simulate_uniswap_workflow_test.go b/core/taskengine/simulate_uniswap_workflow_test.go new file mode 100644 index 00000000..d9953411 --- /dev/null +++ b/core/taskengine/simulate_uniswap_workflow_test.go @@ -0,0 +1,449 @@ +package taskengine + +import ( + "context" + "encoding/hex" + "fmt" + "math/big" + "strings" + "testing" + + "github.com/AvaProtocol/EigenLayer-AVS/core/chainio/aa" + "github.com/AvaProtocol/EigenLayer-AVS/core/config" + "github.com/AvaProtocol/EigenLayer-AVS/core/testutil" + "github.com/AvaProtocol/EigenLayer-AVS/model" + avsproto "github.com/AvaProtocol/EigenLayer-AVS/protobuf" + "github.com/AvaProtocol/EigenLayer-AVS/storage" + "github.com/ethereum/go-ethereum" + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/ethclient" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/types/known/structpb" +) + +// Sepolia Uniswap V3 addresses (from studio/app/lib/uniswap/v3/data/sepolia.json) +const ( + SEPOLIA_QUOTER_V2 = "0xEd1f6473345F45b75F8179591dd5bA1888cf2FB3" +) + +// TestSimulateTask_StopLossWorkflow_Sepolia replicates the complete stop-loss-on-uniswap +// Studio template as a SimulateTask integration test. This exercises the full node chain: +// +// trigger → get_balance → calculate_swap_amount → has_enough_balance (branch) +// └─ if true → approve_token → get_swap_quote → calculate_slippage → execute_swap +// +// The critical assertion is that approve_token's allowance state propagates through +// to execute_swap via SimulationStateMap. Secondary assertions verify that balance, +// code, branch, and contractRead nodes all work in the full workflow context. +// +// Uses salt:0 wallet with real Sepolia USDC balance but zero on-chain allowance +// to SwapRouter02, making the approve→swap propagation conclusive. +func TestSimulateTask_StopLossWorkflow_Sepolia(t *testing.T) { + if testing.Short() { + t.Skip("Skipping Sepolia integration test in short mode") + } + + ownerAddr, ok := testutil.MustGetTestOwnerAddress() + if !ok { + t.Skip("OWNER_EOA not set; skipping") + } + ownerAddress := *ownerAddr + + cfg, err := config.NewConfig(testutil.GetConfigPath(testutil.DefaultConfigPath)) + require.NoError(t, err, "load aggregator config") + + aa.SetFactoryAddress(cfg.SmartWallet.FactoryAddress) + + client, err := ethclient.Dial(cfg.SmartWallet.EthRpcUrl) + require.NoError(t, err, "connect to Sepolia RPC") + defer client.Close() + + // salt:0 — zero on-chain allowance to SwapRouter02 + smartWalletAddr, err := aa.GetSenderAddress(client, ownerAddress, big.NewInt(0)) + require.NoError(t, err, "derive smart wallet address") + t.Logf("Owner EOA: %s", ownerAddress.Hex()) + t.Logf("Smart wallet: %s (salt:0)", smartWalletAddr.Hex()) + + usdcBalance := fetchERC20Balance(t, client, SEPOLIA_USDC, smartWalletAddr.Hex()) + t.Logf("USDC balance: %s (6 decimals)", usdcBalance.String()) + require.True(t, usdcBalance.Cmp(big.NewInt(2_000_000)) >= 0, + "wallet must hold at least 2 USDC; fund %s", smartWalletAddr.Hex()) + + db := testutil.TestMustDB() + t.Cleanup(func() { storage.Destroy(db.(*storage.BadgerStorage)) }) + + engine := New(db, cfg, nil, testutil.GetLogger()) + t.Cleanup(func() { engine.Stop() }) + + user := &model.User{ + Address: ownerAddress, + SmartAccountAddress: smartWalletAddr, + } + require.NoError(t, + StoreWallet(db, ownerAddress, &model.SmartWallet{ + Owner: &ownerAddress, + Address: smartWalletAddr, + Factory: &cfg.SmartWallet.FactoryAddress, + Salt: big.NewInt(0), + }), + "register smart wallet", + ) + + runner := smartWalletAddr.Hex() + swapAmount := "2000000" // 2 USDC (fixed, not MAX — simpler for test) + approveAmount := "4000000" // 4 USDC (> swap to cover Uniswap fee buffer) + + // ======================================================================== + // Nodes — mirrors studio/templates/stop-loss-on-uniswap.json node graph + // ======================================================================== + + // Node 1: get_balance — fetch wallet ETH + USDC balances + getBalance := &avsproto.TaskNode{ + Id: "get_balance", + Name: "get_balance", + Type: avsproto.NodeType_NODE_TYPE_BALANCE, + TaskType: &avsproto.TaskNode_Balance{ + Balance: &avsproto.BalanceNode{ + Config: &avsproto.BalanceNode_Config{ + Address: runner, + Chain: "sepolia", + TokenAddresses: []string{ + "0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee", // ETH + SEPOLIA_USDC, + }, + }, + }, + }, + } + + // Node 2: calculate_swap_amount — JS code that reads balance + settings + // Simplified from template: uses fixed amount instead of MAX logic + calculateSwapAmount := &avsproto.TaskNode{ + Id: "calculate_swap_amount", + Name: "calculate_swap_amount", + Type: avsproto.NodeType_NODE_TYPE_CUSTOM_CODE, + TaskType: &avsproto.TaskNode_CustomCode{ + CustomCode: &avsproto.CustomCodeNode{ + Config: &avsproto.CustomCodeNode_Config{ + Lang: avsproto.Lang_LANG_JAVASCRIPT, + Source: fmt.Sprintf(` +const inputBalance = get_balance.data.find( + (token) => token.tokenAddress && + token.tokenAddress.toLowerCase() === "%s".toLowerCase() +); +const hasEnoughBalance = inputBalance && Number(inputBalance.balanceFormatted) > 0; +const swapAmount = "%s"; +return { inputBalance, hasEnoughBalance, swapAmount }; +`, SEPOLIA_USDC, swapAmount), + }, + }, + }, + } + + // Node 3: has_enough_balance — branch on balance check + hasEnoughBalance := &avsproto.TaskNode{ + Id: "has_enough_balance", + Name: "has_enough_balance", + Type: avsproto.NodeType_NODE_TYPE_BRANCH, + TaskType: &avsproto.TaskNode_Branch{ + Branch: &avsproto.BranchNode{ + Config: &avsproto.BranchNode_Config{ + Conditions: []*avsproto.BranchNode_Condition{ + {Id: "0", Type: "if", Expression: "{{calculate_swap_amount.data.hasEnoughBalance}}"}, + {Id: "1", Type: "else", Expression: ""}, + }, + }, + }, + }, + } + + // Node 4: approve_token — ERC20 approve(SwapRouter02, amount) + approveToken := &avsproto.TaskNode{ + Id: "approve_token", + Name: "approve_token", + Type: avsproto.NodeType_NODE_TYPE_CONTRACT_WRITE, + TaskType: &avsproto.TaskNode_ContractWrite{ + ContractWrite: &avsproto.ContractWriteNode{ + Config: &avsproto.ContractWriteNode_Config{ + ContractAddress: SEPOLIA_USDC, + ContractAbi: []*structpb.Value{sv(t, map[string]interface{}{ + "type": "function", "name": "approve", "stateMutability": "nonpayable", + "inputs": []interface{}{ + map[string]interface{}{"name": "spender", "type": "address"}, + map[string]interface{}{"name": "amount", "type": "uint256"}, + }, + "outputs": []interface{}{map[string]interface{}{"name": "", "type": "bool"}}, + })}, + MethodCalls: []*avsproto.ContractWriteNode_MethodCall{{ + MethodName: "approve", + MethodParams: []string{SEPOLIA_SWAPROUTER, approveAmount}, + }}, + }, + }, + }, + } + + // Node 5: get_swap_quote — QuoterV2.quoteExactInputSingle (contractRead) + getSwapQuote := &avsproto.TaskNode{ + Id: "get_swap_quote", + Name: "get_swap_quote", + Type: avsproto.NodeType_NODE_TYPE_CONTRACT_READ, + TaskType: &avsproto.TaskNode_ContractRead{ + ContractRead: &avsproto.ContractReadNode{ + Config: &avsproto.ContractReadNode_Config{ + ContractAddress: SEPOLIA_QUOTER_V2, + ContractAbi: []*structpb.Value{sv(t, map[string]interface{}{ + "type": "function", "name": "quoteExactInputSingle", "stateMutability": "nonpayable", + "inputs": []interface{}{ + map[string]interface{}{ + "name": "params", "type": "tuple", + "components": []interface{}{ + map[string]interface{}{"name": "tokenIn", "type": "address"}, + map[string]interface{}{"name": "tokenOut", "type": "address"}, + map[string]interface{}{"name": "amountIn", "type": "uint256"}, + map[string]interface{}{"name": "fee", "type": "uint24"}, + map[string]interface{}{"name": "sqrtPriceLimitX96", "type": "uint160"}, + }, + }, + }, + "outputs": []interface{}{ + map[string]interface{}{"name": "amountOut", "type": "uint256"}, + map[string]interface{}{"name": "sqrtPriceX96After", "type": "uint160"}, + map[string]interface{}{"name": "initializedTicksCrossed", "type": "uint32"}, + map[string]interface{}{"name": "gasEstimate", "type": "uint256"}, + }, + })}, + MethodCalls: []*avsproto.ContractReadNode_MethodCall{{ + MethodName: "quoteExactInputSingle", + MethodParams: []string{fmt.Sprintf( + `["%s", "%s", "{{calculate_swap_amount.data.swapAmount}}", 3000, 0]`, + SEPOLIA_USDC, SEPOLIA_WETH, + )}, + }}, + }, + }, + }, + } + + // Node 6: calculate_slippage — JS code applying slippage to quote output + calculateSlippage := &avsproto.TaskNode{ + Id: "calculate_slippage", + Name: "calculate_slippage", + Type: avsproto.NodeType_NODE_TYPE_CUSTOM_CODE, + TaskType: &avsproto.TaskNode_CustomCode{ + CustomCode: &avsproto.CustomCodeNode{ + Config: &avsproto.CustomCodeNode_Config{ + Lang: avsproto.Lang_LANG_JAVASCRIPT, + Source: ` +const amountOut = BigInt(get_swap_quote.data.quoteExactInputSingle.amountOut); +const slippagePercent = 3n; +const amountOutMinimum = (amountOut * (100n - slippagePercent)) / 100n; +return { amountOutMinimum: amountOutMinimum.toString() }; +`, + }, + }, + }, + } + + // Node 7: execute_swap — SwapRouter02.exactInputSingle (contractWrite) + executeSwap := &avsproto.TaskNode{ + Id: "execute_swap", + Name: "execute_swap", + Type: avsproto.NodeType_NODE_TYPE_CONTRACT_WRITE, + TaskType: &avsproto.TaskNode_ContractWrite{ + ContractWrite: &avsproto.ContractWriteNode{ + Config: &avsproto.ContractWriteNode_Config{ + ContractAddress: SEPOLIA_SWAPROUTER, + ContractAbi: []*structpb.Value{sv(t, map[string]interface{}{ + "type": "function", "name": "exactInputSingle", "stateMutability": "payable", + "inputs": []interface{}{ + map[string]interface{}{ + "name": "params", "type": "tuple", + "components": []interface{}{ + map[string]interface{}{"name": "tokenIn", "type": "address"}, + map[string]interface{}{"name": "tokenOut", "type": "address"}, + map[string]interface{}{"name": "fee", "type": "uint24"}, + map[string]interface{}{"name": "recipient", "type": "address"}, + map[string]interface{}{"name": "amountIn", "type": "uint256"}, + map[string]interface{}{"name": "amountOutMinimum", "type": "uint256"}, + map[string]interface{}{"name": "sqrtPriceLimitX96", "type": "uint160"}, + }, + }, + }, + "outputs": []interface{}{map[string]interface{}{"name": "amountOut", "type": "uint256"}}, + })}, + MethodCalls: []*avsproto.ContractWriteNode_MethodCall{{ + MethodName: "exactInputSingle", + MethodParams: []string{fmt.Sprintf( + `["%s", "%s", 3000, "%s", "{{calculate_swap_amount.data.swapAmount}}", "{{calculate_slippage.data.amountOutMinimum}}", 0]`, + SEPOLIA_USDC, SEPOLIA_WETH, runner, + )}, + }}, + }, + }, + }, + } + + nodes := []*avsproto.TaskNode{ + getBalance, calculateSwapAmount, hasEnoughBalance, + approveToken, getSwapQuote, calculateSlippage, executeSwap, + } + + // ======================================================================== + // Edges — mirrors the template's node graph + // ======================================================================== + edges := []*avsproto.TaskEdge{ + {Id: "e1", Source: "manual_trigger", Target: "get_balance"}, + {Id: "e2", Source: "get_balance", Target: "calculate_swap_amount"}, + {Id: "e3", Source: "calculate_swap_amount", Target: "has_enough_balance"}, + // Branch: condition "0" (if true) → approve path + {Id: "e4", Source: "has_enough_balance.0", Target: "approve_token"}, + {Id: "e5", Source: "approve_token", Target: "get_swap_quote"}, + {Id: "e6", Source: "get_swap_quote", Target: "calculate_slippage"}, + {Id: "e7", Source: "calculate_slippage", Target: "execute_swap"}, + } + + trigger := &avsproto.TaskTrigger{ + Id: "manual_trigger", + Name: "manual_trigger", + Type: avsproto.TriggerType_TRIGGER_TYPE_MANUAL, + TriggerType: &avsproto.TaskTrigger_Manual{ + Manual: &avsproto.ManualTrigger{ + Config: &avsproto.ManualTrigger_Config{ + Lang: avsproto.Lang_LANG_JSON, + Data: sv(t, map[string]interface{}{"test": "full_workflow"}), + }, + }, + }, + } + + inputVariables := map[string]interface{}{ + "settings": map[string]interface{}{ + "name": "Stop-Loss on Uniswap V3", + "runner": runner, + "chain_id": int64(11155111), + "chain": "sepolia", + }, + } + + // ======================================================================== + // Execute + // ======================================================================== + t.Log("--- Running SimulateTask (full stop-loss workflow) ---") + execution, err := engine.SimulateTask(user, trigger, nodes, edges, inputVariables) + require.NoError(t, err, "SimulateTask should not return a transport error") + require.NotNil(t, execution, "execution must not be nil") + + // ======================================================================== + // Diagnostic dump + // ======================================================================== + stepsByID := map[string]*avsproto.Execution_Step{} + for _, s := range execution.Steps { + stepsByID[s.Id] = s + } + + t.Logf("Execution status: %s", execution.Status) + t.Logf("Execution error : %q", execution.Error) + for i, s := range execution.Steps { + t.Logf("Step %d: id=%-25s name=%-25s success=%v", i, s.Id, s.Name, s.Success) + if s.Error != "" { + t.Logf(" error: %s", s.Error) + } + if s.Log != "" { + for _, ln := range strings.Split(strings.TrimSpace(s.Log), "\n") { + t.Logf(" log: %s", ln) + } + } + } + + // ======================================================================== + // Per-step assertions + // ======================================================================== + + // Trigger + triggerStep := stepsByID["manual_trigger"] + require.NotNil(t, triggerStep, "trigger step must exist") + assert.True(t, triggerStep.Success, "trigger must succeed") + + // get_balance — fetches real on-chain balances via RPC + balanceStep := stepsByID["get_balance"] + require.NotNil(t, balanceStep, "get_balance step must exist") + assert.True(t, balanceStep.Success, "get_balance must succeed; error=%q", balanceStep.Error) + + // calculate_swap_amount — JS code reads balance output + calcStep := stepsByID["calculate_swap_amount"] + require.NotNil(t, calcStep, "calculate_swap_amount step must exist") + assert.True(t, calcStep.Success, "calculate_swap_amount must succeed; error=%q", calcStep.Error) + + // has_enough_balance — branch should take the "if" (true) path + branchStep := stepsByID["has_enough_balance"] + require.NotNil(t, branchStep, "branch step must exist") + assert.True(t, branchStep.Success, "branch must succeed; error=%q", branchStep.Error) + + // approve_token — ERC20 approve simulation + approveStep := stepsByID["approve_token"] + require.NotNil(t, approveStep, "approve_token step must exist (branch took true path)") + assert.True(t, approveStep.Success, "approve must succeed; error=%q", approveStep.Error) + + // get_swap_quote — QuoterV2 contractRead + quoteStep := stepsByID["get_swap_quote"] + require.NotNil(t, quoteStep, "get_swap_quote step must exist") + assert.True(t, quoteStep.Success, "quote must succeed; error=%q", quoteStep.Error) + + // calculate_slippage — JS code reads quote output + slippageStep := stepsByID["calculate_slippage"] + require.NotNil(t, slippageStep, "calculate_slippage step must exist") + assert.True(t, slippageStep.Success, "slippage calc must succeed; error=%q", slippageStep.Error) + + // execute_swap — the final assertion: swap must succeed + swapStep := stepsByID["execute_swap"] + require.NotNil(t, swapStep, "execute_swap step must exist") + if !swapStep.Success { + t.Logf("SWAP FAILURE DIAGNOSIS:") + t.Logf(" error: %s", swapStep.Error) + switch { + case strings.Contains(strings.ToLower(swapStep.Error), "allowance"): + t.Log(" => Allowance propagation from approve_token failed") + case strings.Contains(strings.ToLower(swapStep.Error), "balance"): + t.Log(" => Insufficient token balance") + case strings.Contains(strings.ToLower(swapStep.Error), "slippage") || + strings.Contains(strings.ToLower(swapStep.Error), "too little received"): + t.Log(" => Slippage tolerance exceeded — pool liquidity or price moved") + default: + t.Log(" => Unrecognized failure — check step log above") + } + } + assert.True(t, swapStep.Success, + "swap must succeed — full workflow propagation test. error=%q", swapStep.Error) + + // Whole-execution status + assert.Equal(t, avsproto.ExecutionStatus_EXECUTION_STATUS_SUCCESS, execution.Status, + "workflow terminal status should be SUCCESS") +} + +// sv is a shorthand for mustStructValue to keep node definitions compact. +func sv(t *testing.T, v interface{}) *structpb.Value { + t.Helper() + val, err := structpb.NewValue(v) + require.NoError(t, err, "structpb.NewValue") + return val +} + +// fetchERC20Balance calls balanceOf(holder) on the token contract. +func fetchERC20Balance(t *testing.T, client *ethclient.Client, token, holder string) *big.Int { + t.Helper() + holderBytes := common.HexToAddress(holder).Bytes() + callData := append([]byte{0x70, 0xa0, 0x82, 0x31}, common.LeftPadBytes(holderBytes, 32)...) + to := common.HexToAddress(token) + + result, err := client.CallContract(context.Background(), ethereum.CallMsg{ + To: &to, + Data: callData, + }, nil) + if err != nil { + t.Logf("fetchERC20Balance: CallContract failed: %v (raw callData=0x%s)", err, hex.EncodeToString(callData)) + return big.NewInt(0) + } + return new(big.Int).SetBytes(result) +} diff --git a/core/taskengine/simulation_state.go b/core/taskengine/simulation_state.go index dd19d8cf..7ff643bc 100644 --- a/core/taskengine/simulation_state.go +++ b/core/taskengine/simulation_state.go @@ -153,11 +153,29 @@ func erc20BalanceSlot(holder common.Address, mappingSlot int64) common.Hash { return crypto.Keccak256Hash(append(key, slot...)) } +// erc20AllowanceSlot computes the keccak256 storage slot for allowance[owner][spender] +// given the mapping's base slot index in the contract's storage layout. +// Formula: keccak256(abi.encode(spender, keccak256(abi.encode(owner, allowanceSlot)))) +func erc20AllowanceSlot(owner, spender common.Address, mappingSlot int64) common.Hash { + // Inner hash: keccak256(abi.encode(owner, allowanceSlot)) + ownerPadded := common.LeftPadBytes(owner.Bytes(), 32) + slotPadded := common.LeftPadBytes(big.NewInt(mappingSlot).Bytes(), 32) + innerHash := crypto.Keccak256Hash(append(ownerPadded, slotPadded...)) + + // Outer hash: keccak256(abi.encode(spender, innerHash)) + spenderPadded := common.LeftPadBytes(spender.Bytes(), 32) + return crypto.Keccak256Hash(append(spenderPadded, innerHash.Bytes()...)) +} + // Common ERC20 balance mapping slot indices across different implementations. // 0: standard OpenZeppelin ERC20, 1-3: various token implementations, // 9: USDC (FiatTokenV2 proxy), 51: Compound cToken-style contracts. var commonBalanceSlots = []int64{0, 1, 2, 3, 9, 51} +// Common ERC20 allowance mapping slot indices across different implementations. +// 1: OpenZeppelin ERC20 v4/v5, 2-3: legacy implementations, 10: USDC FiatTokenV2. +var commonAllowanceSlots = []int64{1, 2, 3, 10} + // ProbeERC20BalanceSlot discovers which storage slot a token contract uses for // its _balances mapping by comparing eth_getStorageAt results against balanceOf. // Returns the slot index, or an error if no match is found. diff --git a/core/taskengine/vm_runner_contract_write.go b/core/taskengine/vm_runner_contract_write.go index 20425b43..101f1342 100644 --- a/core/taskengine/vm_runner_contract_write.go +++ b/core/taskengine/vm_runner_contract_write.go @@ -72,6 +72,13 @@ func (r *ContractWriteProcessor) resolveSimulationMode(node *avsproto.ContractWr return vmDefault } +// isMethodWithParams returns true when methodName matches target (case-insensitive) +// and resolvedParams has at least minParams entries. Use this to gate post-simulation +// state injection for specific ERC-20 methods (e.g. "approve" needs spender + amount). +func isMethodWithParams(methodName, target string, resolvedParams []string, minParams int) bool { + return strings.EqualFold(methodName, target) && len(resolvedParams) >= minParams +} + func (r *ContractWriteProcessor) getInputData(node *avsproto.ContractWriteNode) (string, string, []*avsproto.ContractWriteNode_MethodCall, error) { var contractAddress, callData string var methodCalls []*avsproto.ContractWriteNode_MethodCall @@ -481,15 +488,44 @@ func (r *ContractWriteProcessor) executeMethodCall( } } - // Merge state diffs from this simulation into the accumulated state map - // so subsequent simulation steps see a consistent view of on-chain state. - if r.vm.simulationState != nil && simulationResult != nil && simulationResult.Success && len(simulationResult.RawStateDiff) > 0 { + // Propagate simulation state so subsequent nodes see this step's effects. + simSuccess := r.vm.simulationState != nil && simulationResult != nil && simulationResult.Success + + // Merge raw state diffs returned by Tenderly (if any). + if simSuccess && len(simulationResult.RawStateDiff) > 0 { r.vm.simulationState.MergeRawStateDiff(simulationResult.RawStateDiff) r.vm.logger.Debug("Merged simulation state diff into accumulator", "method", methodName, "diffEntries", len(simulationResult.RawStateDiff)) } + // After a successful approve() simulation, explicitly inject the allowance + // storage slot so downstream nodes (e.g. swap) see it. Tenderly's HTTP + // simulate API returns raw_state_diff: null for approve calls, so the + // MergeRawStateDiff above is a no-op. We compute and set the + // allowance[owner][spender] slot directly. + if simSuccess && isMethodWithParams(methodName, "approve", resolvedMethodParams, 2) { + spender := common.HexToAddress(resolvedMethodParams[0]) + // Base 0 auto-detects decimal, hex (0x...), and octal. + amount, ok := new(big.Int).SetString(resolvedMethodParams[1], 0) + if !ok || amount.Sign() < 0 { + r.vm.logger.Debug("Skipping allowance override: invalid or negative amount", + "raw", resolvedMethodParams[1]) + } else { + valueHex := fmt.Sprintf("0x%064x", amount) + for _, candidateSlot := range commonAllowanceSlots { + slotHash := erc20AllowanceSlot(senderAddress, spender, candidateSlot) + r.vm.simulationState.SetStorageSlot(contractAddress.Hex(), slotHash.Hex(), valueHex) + } + r.vm.logger.Debug("Injected allowance override after approve simulation", + "token", contractAddress.Hex(), + "owner", senderAddress.Hex(), + "spender", spender.Hex(), + "amount", amount.String(), + "slots_set", len(commonAllowanceSlots)) + } + } + // Convert Tenderly simulation result to legacy protobuf format mr := r.convertTenderlyResultToFlexibleFormat(simulationResult, parsedABI, callData) // Try to stamp real latest block number/hash from our configured RPC diff --git a/docs/changes/0002-uniswap-simulation-state-propagation.md b/docs/changes/0002-uniswap-simulation-state-propagation.md new file mode 100644 index 00000000..c43bce79 --- /dev/null +++ b/docs/changes/0002-uniswap-simulation-state-propagation.md @@ -0,0 +1,68 @@ +# Fix: ERC20 Approve State Propagation in Workflow Simulation + +- **Date**: 2026-04-16 +- **Status**: Implemented +- **Branch**: `test/uniswap-simulate-propagation` +- **Related**: #413, #517 + +## Context + +When simulating a Uniswap stop-loss workflow (source of truth: `studio/templates/stop-loss-on-uniswap.json`), the `approve()` node succeeded but its allowance state did not propagate to the downstream `exactInputSingle()` swap node. The swap consistently failed with `ERC20: transfer amount exceeds allowance`. + +PR #517 introduced `SimulationStateMap` with `MergeRawStateDiff` to carry Tenderly's `raw_state_diff` between sequential simulation steps. The assumption was that Tenderly returns storage changes in that field, which are then fed as `state_objects` overrides into the next simulation. + +## Root cause + +Tenderly's HTTP `/simulate` endpoint (with `simulation_type: "full"`) returns `raw_state_diff: null` for `approve()` calls. The field key exists in the response at `transaction.transaction_info.raw_state_diff`, but the value is always `null`. This makes `MergeRawStateDiff` a silent no-op for the approve step, so the downstream swap never sees the new allowance. + +### Diagnosis path + +1. Wrote an integration test (`TestSimulateTask_StopLossWorkflow_Sepolia`) using a salt:0 wallet with real Sepolia USDC balance but **zero** on-chain allowance to SwapRouter02. This made the test conclusive: swap can only succeed if the approve's allowance propagates. +2. Confirmed propagation failure: approve succeeded, swap failed with `ERC20: transfer amount exceeds allowance`. +3. Added a raw Tenderly HTTP diagnostic test confirming `raw_state_diff` is `null` (not absent, not empty array — literally `null`). +4. Verified the extraction path (`transaction.transaction_info.raw_state_diff`) is correct by inspecting all response keys — the code was looking in the right place, but there was no data to extract. + +## Fix + +After a successful `approve()` simulation, explicitly compute and inject the `allowance[owner][spender]` storage slot into `SimulationStateMap`. This bypasses the missing `raw_state_diff` by directly setting the slot that `transferFrom` will check during the swap. + +### Changes + +| File | Change | +|---|---| +| `core/taskengine/simulation_state.go` | Added `erc20AllowanceSlot(owner, spender, mappingSlot)` — computes `keccak256(spender, keccak256(owner, slot))` for nested mapping lookup. Added `commonAllowanceSlots` (indices 1, 2, 3, 10) covering OpenZeppelin, legacy, and USDC implementations. | +| `core/taskengine/vm_runner_contract_write.go` | Added `isMethodWithParams(methodName, target, params, minParams)` predicate. After approve sim succeeds, injects the allowance value across all common slots. Extracted `simSuccess` variable to deduplicate the repeated `simulationState != nil && simulationResult != nil && simulationResult.Success` guard. | +| `core/taskengine/simulate_uniswap_workflow_test.go` | Integration test: approve→swap via `SimulateTask` on Sepolia with zero on-chain allowance. Derived from the Studio stop-loss-on-uniswap template. Includes diagnostic output on failure and balance pre-flight check. | + +### How it works + +``` +approve(SwapRouter02, 4 USDC) simulation succeeds + ↓ +isMethodWithParams(methodName, "approve", params, 2) → true + ↓ +For each slot in commonAllowanceSlots [1, 2, 3, 10]: + slotHash = keccak256(spender || keccak256(owner || slot)) + simulationState.SetStorageSlot(USDC, slotHash, 0x...amount) + ↓ +exactInputSingle() simulation runs with state_objects + containing allowance[wallet][router] = 4 USDC + ↓ +transferFrom succeeds → swap succeeds +``` + +## What this does NOT fix + +- **Tenderly `raw_state_diff` being null**: the Tenderly API behavior is unchanged. If they fix it in the future, `MergeRawStateDiff` will start working and the explicit injection becomes a harmless redundancy. +- **Arbitrary state propagation between nodes**: only `approve()` gets explicit injection. Other methods still rely on `raw_state_diff` (which works for non-approve calls where Tenderly does return diffs). +- **User-facing `erc20_overrides` API** (#413): users still cannot seed arbitrary ERC20 state in `RunNodeImmediately`. That requires a new proto field and is tracked separately. + +## Sepolia test addresses + +| Name | Address | Source | +|---|---|---| +| USDC | `0x1c7D4B196Cb0C7B01d743Fbc6116a902379C7238` | `studio/app/lib/erc20/sepolia.json` | +| WETH | `0xfFf9976782d46CC05630D1f6eBAb18b2324d6B14` | same | +| SwapRouter02 | `0x3bFA4769FB09eefC5a80d6E87c3B9C650f7Ae48E` | `studio/app/lib/uniswap/v3/data/sepolia.json` | +| QuoterV2 | `0xEd1f6473345F45b75F8179591dd5bA1888cf2FB3` | same | +| Test wallet | salt:0 derived from `OWNER_EOA` | zero on-chain USDC allowance to SwapRouter02 | From 88e91393fd8e7957670820aedecf9d266cc396e7 Mon Sep 17 00:00:00 2001 From: Will Zimmerman Date: Wed, 22 Apr 2026 15:12:37 -0700 Subject: [PATCH 3/5] fix: skip Sepolia test on under-funded wallet, harden approve override parsing Address Copilot review feedback on PR #528: - Convert hard require.True to t.Skipf when the salt:0 wallet holds less than 2 USDC, matching the existing OWNER_EOA-not-set skip pattern. Avoids spurious CI failures when funding lapses. - In the post-approve allowance override, reject invalid spender addresses up front via common.IsHexAddress and parse the amount as base-10 by default (honoring 0x/0X prefix as hex) instead of big.Int.SetString base 0, which silently treats leading-zero strings as octal. --- .../simulate_uniswap_workflow_test.go | 5 ++- core/taskengine/vm_runner_contract_write.go | 44 ++++++++++++------- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/core/taskengine/simulate_uniswap_workflow_test.go b/core/taskengine/simulate_uniswap_workflow_test.go index d9953411..09a3b864 100644 --- a/core/taskengine/simulate_uniswap_workflow_test.go +++ b/core/taskengine/simulate_uniswap_workflow_test.go @@ -67,8 +67,9 @@ func TestSimulateTask_StopLossWorkflow_Sepolia(t *testing.T) { usdcBalance := fetchERC20Balance(t, client, SEPOLIA_USDC, smartWalletAddr.Hex()) t.Logf("USDC balance: %s (6 decimals)", usdcBalance.String()) - require.True(t, usdcBalance.Cmp(big.NewInt(2_000_000)) >= 0, - "wallet must hold at least 2 USDC; fund %s", smartWalletAddr.Hex()) + if usdcBalance.Cmp(big.NewInt(2_000_000)) < 0 { + t.Skipf("smart wallet %s must hold at least 2 USDC to run this test", smartWalletAddr.Hex()) + } db := testutil.TestMustDB() t.Cleanup(func() { storage.Destroy(db.(*storage.BadgerStorage)) }) diff --git a/core/taskengine/vm_runner_contract_write.go b/core/taskengine/vm_runner_contract_write.go index 101f1342..29ed5a67 100644 --- a/core/taskengine/vm_runner_contract_write.go +++ b/core/taskengine/vm_runner_contract_write.go @@ -505,24 +505,36 @@ func (r *ContractWriteProcessor) executeMethodCall( // MergeRawStateDiff above is a no-op. We compute and set the // allowance[owner][spender] slot directly. if simSuccess && isMethodWithParams(methodName, "approve", resolvedMethodParams, 2) { - spender := common.HexToAddress(resolvedMethodParams[0]) - // Base 0 auto-detects decimal, hex (0x...), and octal. - amount, ok := new(big.Int).SetString(resolvedMethodParams[1], 0) - if !ok || amount.Sign() < 0 { - r.vm.logger.Debug("Skipping allowance override: invalid or negative amount", - "raw", resolvedMethodParams[1]) + rawSpender := strings.TrimSpace(resolvedMethodParams[0]) + rawAmount := strings.TrimSpace(resolvedMethodParams[1]) + if !common.IsHexAddress(rawSpender) { + r.vm.logger.Debug("Skipping allowance override: invalid spender address", + "raw", resolvedMethodParams[0]) } else { - valueHex := fmt.Sprintf("0x%064x", amount) - for _, candidateSlot := range commonAllowanceSlots { - slotHash := erc20AllowanceSlot(senderAddress, spender, candidateSlot) - r.vm.simulationState.SetStorageSlot(contractAddress.Hex(), slotHash.Hex(), valueHex) + spender := common.HexToAddress(rawSpender) + // Default to base 10; honor 0x/0X prefix as hex. Avoid base 0 because + // it treats leading-zero strings (e.g. "010") as octal. + base := 10 + if strings.HasPrefix(rawAmount, "0x") || strings.HasPrefix(rawAmount, "0X") { + base = 16 + } + amount, ok := new(big.Int).SetString(rawAmount, base) + if !ok || amount.Sign() < 0 { + r.vm.logger.Debug("Skipping allowance override: invalid or negative amount", + "raw", resolvedMethodParams[1]) + } else { + valueHex := fmt.Sprintf("0x%064x", amount) + for _, candidateSlot := range commonAllowanceSlots { + slotHash := erc20AllowanceSlot(senderAddress, spender, candidateSlot) + r.vm.simulationState.SetStorageSlot(contractAddress.Hex(), slotHash.Hex(), valueHex) + } + r.vm.logger.Debug("Injected allowance override after approve simulation", + "token", contractAddress.Hex(), + "owner", senderAddress.Hex(), + "spender", spender.Hex(), + "amount", amount.String(), + "slots_set", len(commonAllowanceSlots)) } - r.vm.logger.Debug("Injected allowance override after approve simulation", - "token", contractAddress.Hex(), - "owner", senderAddress.Hex(), - "spender", spender.Hex(), - "amount", amount.String(), - "slots_set", len(commonAllowanceSlots)) } } From 4358d02fc0da02cdce1931bcd9a6751fa334f851 Mon Sep 17 00:00:00 2001 From: Will Zimmerman Date: Wed, 22 Apr 2026 15:16:01 -0700 Subject: [PATCH 4/5] test: hard-fail Sepolia stop-loss test when prereqs missing Revert the t.Skipf change from the previous commit and additionally convert the existing OWNER_EOA t.Skip to a require.True. Integration tests that cannot run because env vars or wallet funding are missing should fail loudly so the gap gets fixed, not be silently skipped. The testing.Short() guard at the top stays - that is a deliberate fast-mode opt-out, not a missing-prereq skip. --- core/taskengine/simulate_uniswap_workflow_test.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/core/taskengine/simulate_uniswap_workflow_test.go b/core/taskengine/simulate_uniswap_workflow_test.go index 09a3b864..c798fb61 100644 --- a/core/taskengine/simulate_uniswap_workflow_test.go +++ b/core/taskengine/simulate_uniswap_workflow_test.go @@ -45,9 +45,7 @@ func TestSimulateTask_StopLossWorkflow_Sepolia(t *testing.T) { } ownerAddr, ok := testutil.MustGetTestOwnerAddress() - if !ok { - t.Skip("OWNER_EOA not set; skipping") - } + require.True(t, ok, "OWNER_EOA must be set to run this Sepolia integration test") ownerAddress := *ownerAddr cfg, err := config.NewConfig(testutil.GetConfigPath(testutil.DefaultConfigPath)) @@ -67,9 +65,8 @@ func TestSimulateTask_StopLossWorkflow_Sepolia(t *testing.T) { usdcBalance := fetchERC20Balance(t, client, SEPOLIA_USDC, smartWalletAddr.Hex()) t.Logf("USDC balance: %s (6 decimals)", usdcBalance.String()) - if usdcBalance.Cmp(big.NewInt(2_000_000)) < 0 { - t.Skipf("smart wallet %s must hold at least 2 USDC to run this test", smartWalletAddr.Hex()) - } + require.True(t, usdcBalance.Cmp(big.NewInt(2_000_000)) >= 0, + "wallet must hold at least 2 USDC; fund %s", smartWalletAddr.Hex()) db := testutil.TestMustDB() t.Cleanup(func() { storage.Destroy(db.(*storage.BadgerStorage)) }) From 347bc2fd64839d49c9b77b5f8a2cf7d22a7cbb30 Mon Sep 17 00:00:00 2001 From: Will Zimmerman Date: Wed, 22 Apr 2026 15:26:17 -0700 Subject: [PATCH 5/5] refactor: extract big.Int parsing into pkg/bigint to remove octal traps Add pkg/bigint.Parse(s) that decodes a numeric string as base-10 by default and as base-16 when prefixed with 0x/0X, with whitespace trimmed and explicit errors on empty or unparseable input. Unlike big.Int.SetString with base 0, it does not silently treat leading- zero strings (e.g. "010") as octal. Refactor three callsites onto the helper: - pkg/erc4337/userop/parse.go: UserOp BigInt field decoder previously used SetString(..., 0), exposing the octal trap on user-supplied numeric fields. - core/taskengine/utils.go: ABI uint/int parameter conversion already did the same hex-or-decimal split inline; collapse to the helper while preserving the existing error format that utils_calldata_test depends on. - core/taskengine/vm_runner_contract_write.go: post-approve allowance override added in #527 used the same inline split; collapse it. --- core/taskengine/utils.go | 23 +-------- core/taskengine/vm_runner_contract_write.go | 12 ++--- pkg/bigint/parse.go | 37 ++++++++++++++ pkg/bigint/parse_test.go | 54 +++++++++++++++++++++ pkg/erc4337/userop/parse.go | 9 ++-- 5 files changed, 100 insertions(+), 35 deletions(-) create mode 100644 pkg/bigint/parse.go create mode 100644 pkg/bigint/parse_test.go diff --git a/core/taskengine/utils.go b/core/taskengine/utils.go index 822c543e..a08c3a1a 100644 --- a/core/taskengine/utils.go +++ b/core/taskengine/utils.go @@ -13,6 +13,7 @@ import ( "github.com/AvaProtocol/EigenLayer-AVS/core/taskengine/macros" "github.com/AvaProtocol/EigenLayer-AVS/core/taskengine/modules" + "github.com/AvaProtocol/EigenLayer-AVS/pkg/bigint" "github.com/AvaProtocol/EigenLayer-AVS/pkg/erc20" avsproto "github.com/AvaProtocol/EigenLayer-AVS/protobuf" "github.com/dop251/goja" @@ -705,27 +706,7 @@ func parseABIParameter(param string, abiType abi.Type) (interface{}, error) { return common.HexToAddress(param), nil case abi.UintTy, abi.IntTy: - // Handle big integers - // Validate that the parameter is a valid number - paramTrimmed := strings.TrimSpace(param) - if paramTrimmed == "" { - return nil, fmt.Errorf("expected numeric value, got ''") - } - - value := new(big.Int) - var ok bool - if strings.HasPrefix(paramTrimmed, "0x") { - _, ok = value.SetString(paramTrimmed[2:], 16) - } else { - // Check if it's a valid decimal number (allows digits, optional negative sign) - _, ok = value.SetString(paramTrimmed, 10) - } - - if !ok { - return nil, fmt.Errorf("expected numeric value, got '%s'", paramTrimmed) - } - - return value, nil + return bigint.Parse(param) case abi.BoolTy: switch strings.ToLower(param) { diff --git a/core/taskengine/vm_runner_contract_write.go b/core/taskengine/vm_runner_contract_write.go index 29ed5a67..720a9863 100644 --- a/core/taskengine/vm_runner_contract_write.go +++ b/core/taskengine/vm_runner_contract_write.go @@ -20,6 +20,7 @@ import ( "github.com/AvaProtocol/EigenLayer-AVS/core/chainio/aa" "github.com/AvaProtocol/EigenLayer-AVS/core/config" + "github.com/AvaProtocol/EigenLayer-AVS/pkg/bigint" "github.com/AvaProtocol/EigenLayer-AVS/pkg/byte4" "github.com/AvaProtocol/EigenLayer-AVS/pkg/eip1559" "github.com/AvaProtocol/EigenLayer-AVS/pkg/erc4337/bundler" @@ -506,20 +507,13 @@ func (r *ContractWriteProcessor) executeMethodCall( // allowance[owner][spender] slot directly. if simSuccess && isMethodWithParams(methodName, "approve", resolvedMethodParams, 2) { rawSpender := strings.TrimSpace(resolvedMethodParams[0]) - rawAmount := strings.TrimSpace(resolvedMethodParams[1]) if !common.IsHexAddress(rawSpender) { r.vm.logger.Debug("Skipping allowance override: invalid spender address", "raw", resolvedMethodParams[0]) } else { spender := common.HexToAddress(rawSpender) - // Default to base 10; honor 0x/0X prefix as hex. Avoid base 0 because - // it treats leading-zero strings (e.g. "010") as octal. - base := 10 - if strings.HasPrefix(rawAmount, "0x") || strings.HasPrefix(rawAmount, "0X") { - base = 16 - } - amount, ok := new(big.Int).SetString(rawAmount, base) - if !ok || amount.Sign() < 0 { + amount, parseErr := bigint.Parse(resolvedMethodParams[1]) + if parseErr != nil || amount.Sign() < 0 { r.vm.logger.Debug("Skipping allowance override: invalid or negative amount", "raw", resolvedMethodParams[1]) } else { diff --git a/pkg/bigint/parse.go b/pkg/bigint/parse.go new file mode 100644 index 00000000..1c918541 --- /dev/null +++ b/pkg/bigint/parse.go @@ -0,0 +1,37 @@ +// Package bigint provides shared big.Int parsing helpers. +package bigint + +import ( + "fmt" + "math/big" + "strings" +) + +// Parse decodes s as a base-10 integer, or as base-16 if s has a "0x" or "0X" +// prefix. Whitespace around s is trimmed. Returns an error on empty or +// unparseable input. +// +// Unlike (*big.Int).SetString with base 0, Parse does not treat leading-zero +// strings as octal: "010" parses as 10, not 8. Use this for any user-supplied +// numeric string where octal interpretation would be surprising or wrong +// (e.g. ERC20 amounts, RPC payloads, contract method params). +func Parse(s string) (*big.Int, error) { + trimmed := strings.TrimSpace(s) + if trimmed == "" { + return nil, fmt.Errorf("expected numeric value, got ''") + } + base := 10 + digits := trimmed + if strings.HasPrefix(digits, "0x") || strings.HasPrefix(digits, "0X") { + base = 16 + digits = digits[2:] + if digits == "" { + return nil, fmt.Errorf("expected numeric value, got '%s'", trimmed) + } + } + n, ok := new(big.Int).SetString(digits, base) + if !ok { + return nil, fmt.Errorf("expected numeric value, got '%s'", trimmed) + } + return n, nil +} diff --git a/pkg/bigint/parse_test.go b/pkg/bigint/parse_test.go new file mode 100644 index 00000000..678450d5 --- /dev/null +++ b/pkg/bigint/parse_test.go @@ -0,0 +1,54 @@ +package bigint + +import ( + "math/big" + "testing" +) + +func TestParse(t *testing.T) { + cases := []struct { + name string + in string + want string // big.Int decimal string, ignored when wantErr is set + wantErr string + }{ + {name: "zero", in: "0", want: "0"}, + {name: "decimal", in: "4000000", want: "4000000"}, + {name: "leading zero is decimal not octal", in: "010", want: "10"}, + {name: "negative decimal", in: "-5", want: "-5"}, + {name: "lowercase hex", in: "0x3D0900", want: "4000000"}, + {name: "uppercase hex prefix", in: "0X3d0900", want: "4000000"}, + {name: "hex with leading zero digits", in: "0x00ff", want: "255"}, + {name: "trims whitespace", in: " 42 ", want: "42"}, + {name: "huge value preserved", in: "100000000000000000000", want: "100000000000000000000"}, + + {name: "empty", in: "", wantErr: "expected numeric value, got ''"}, + {name: "whitespace only", in: " ", wantErr: "expected numeric value, got ''"}, + {name: "0x with no digits", in: "0x", wantErr: "expected numeric value, got '0x'"}, + {name: "non-numeric", in: "MAX", wantErr: "expected numeric value, got 'MAX'"}, + {name: "invalid hex digits", in: "0xZZZ", wantErr: "expected numeric value, got '0xZZZ'"}, + {name: "decimal with letters", in: "12abc", wantErr: "expected numeric value, got '12abc'"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := Parse(tc.in) + if tc.wantErr != "" { + if err == nil { + t.Fatalf("Parse(%q) = %v, want error %q", tc.in, got, tc.wantErr) + } + if err.Error() != tc.wantErr { + t.Fatalf("Parse(%q) error = %q, want %q", tc.in, err.Error(), tc.wantErr) + } + return + } + if err != nil { + t.Fatalf("Parse(%q) unexpected error: %v", tc.in, err) + } + want, _ := new(big.Int).SetString(tc.want, 10) + if got.Cmp(want) != 0 { + t.Fatalf("Parse(%q) = %s, want %s", tc.in, got.String(), tc.want) + } + }) + } +} diff --git a/pkg/erc4337/userop/parse.go b/pkg/erc4337/userop/parse.go index 381c5ad4..803b698c 100644 --- a/pkg/erc4337/userop/parse.go +++ b/pkg/erc4337/userop/parse.go @@ -9,6 +9,7 @@ import ( "reflect" "sync" + "github.com/AvaProtocol/EigenLayer-AVS/pkg/bigint" "github.com/ethereum/go-ethereum/common" validator "github.com/go-playground/validator/v10" "github.com/mitchellh/mapstructure" @@ -40,15 +41,13 @@ func decodeOpTypes( // String to big.Int conversion if f == reflect.String && t == reflect.Struct { - n := new(big.Int) - var ok bool dataStr, ok := data.(string) if !ok { return nil, errors.New("expected string for bigInt conversion") } - n, ok = n.SetString(dataStr, 0) - if !ok { - return nil, errors.New("bigInt conversion failed") + n, err := bigint.Parse(dataStr) + if err != nil { + return nil, fmt.Errorf("bigInt conversion failed: %w", err) } return n, nil }