Skip to content

Conversation

@chrisli30
Copy link
Member

chrisli30 and others added 3 commits October 30, 2025 18:36
…st version (#421)

* fix: is_simulated return in executionContext of run_node_immediately

* feat: reduce verbose logging and remove emojis from UserOp processing

- Add VerboseLogs flag to control debug output volume
- Gate transaction waiting logs with VerboseLogs check
- Remove emojis from key UserOp and bundler messages
- Keep essential success/error messages for monitoring
- Drop PVG floor for wrapped operations (use bundler estimate)
- Fix paymaster timestamp ABI encoding (remove pointer references)
- Add gas price headroom validation (>= 1 gwei)
- Implement self-funded fallback on paymaster -32602 rejection

* refactor: Create shared logger package with NoOpLogger and EnsureLogger utilities

- Created pkg/logger package with re-exported Logger type from eigensdk-go
- Implemented shared NoOpLogger to prevent nil pointer panics
- Added EnsureLogger utility function to eliminate repetitive nil checks
- Removed local noOpLogger implementations from builder.go and vm.go
- Updated all logger function signatures to use logger.Logger consistently
- Removed pkglogger alias in favor of direct logger package import
- Renamed global 'logger' variable to 'globalLogger' in engine.go to avoid
  naming conflicts with logger package import used in other files
- Updated test file to match new SendUserOp signature

Benefits:
- Cleaner API: Users only import pkg/logger, not sdklogging separately
- No repetitive nil checks: EnsureLogger handles it once per function
- Consistent naming: All files use logger.Logger without aliases
- Better encapsulation: Implementation details hidden from consumers
- Follows Nil Object Pattern (NoOp) - common Go idiom for optional dependencies

* contractWrite: deterministic outputs and auto-skip reimbursement when wallet ETH insufficient\n\n- Real+sim paths unify output semantics:\n  - success + no events => data[method]=true\n  - events decoded => flattened fields\n  - failed => empty object\n- Remove ERC20 topic fallback; require ABI for decoding\n- Paymaster path: auto-skip executeBatchWithValues when wallet balance < estimated reimbursement

* chore: go mod tidy

* chore: reduce log noise - move event parsing debug to DEBUG level

* fix: simplify error message of run node contractWrite

* fix: make sure executionContext correctly propogate in contractWrite runNodeImmediately

* refactor: simplify EventTrigger topics to flat array format

- Changed protobuf from nested Topics structure to flat string array
- Topics now: ['signature', 'from', 'to'] instead of [{ values: [...] }]
- Fixed template variable resolution for flat topics array
- Updated extractAddressFromPaddedHex to handle both padded (66 chars) and unpadded (42 chars) addresses
- Removed hardcoded fallback addresses and amounts from Tenderly simulation
- Migrated validateTopicAddressesMatchQuery to test-only code
- Updated all topic processing logic across tenderly_client, trigger/event, vm, and shared_event_enrichment
- Updated test files: run_node_event_trigger_template_test, tenderly_client_test, event_conditional_test

Benefits:
- Cleaner API: no unnecessary nesting
- Fail-fast validation: no silent fallbacks
- Type-safe: format matches client expectations
- Template variables work correctly in both addresses and topics

* fix: update tenderly_client_test.go formatting for flat topics array

- Aligned Topics field formatting after go fmt
- All test queries now use flat array format: Topics: []string{...}

* fix: allow triggerWorkflow to take in an inputVariables parameter

* Fix nonce conflicts with NonceManager for UserOperations

- Add NonceManager to track pending nonces across UserOp submissions
- Prevents 'invalid UserOperation struct/fields' errors from Voltaire bundler
- Uses max(on-chain nonce, cached pending nonce) strategy
- Auto-increments nonce after successful submission
- Auto-resets cache on conflict detection
- Handles both AA25 nonce errors and Voltaire mempool conflicts

The bundler was rejecting UserOps because we were reusing nonces that were
already pending in the mempool. NonceManager maintains an in-memory cache
per sender to prevent conflicts and enable sequential UserOp submission
before previous ones are mined.

Fixes nonce collision issues when submitting multiple UserOps rapidly.

* Fix event transfer trigger response format inconsistent issue

* Address copilot’s commments

* fix: add generic event Tenderly simulation and fix unit tests

* Documentation fix of   docs/Tenderly-EventTrigger-Simulation.md

* Change example values to publicly aavailable endpoints

* Update release file to correctly promote and publish the highest version

---------

Co-authored-by: Wei Lin <[email protected]>
…d template fix (#422)

* fix: enhance orphaned job cleanup with detailed logging

- Check all task statuses (active, completed, failed, canceled, executing) instead of just active
- Add failure_reason, checked_statuses, and last_error to orphaned job logs
- Differentiate between badger.ErrKeyNotFound and other storage errors
- Improve diagnostics for orphaned job detection

* chore: enhance Tenderly simulation error logging

- Add detailed error logging with error_message, error_slug, full_error, and call_data_preview
- Change simulation_type from quick to full for more accurate state
- Improve error visibility in aggregator logs for debugging

* fix: prioritize function return values over event data in contract write responses

- Return values from ABI outputs now take priority over event data
- Event data is used only if no return value exists
- Fallback to boolean true only if neither return value nor events exist
- Change priority logs to Debug level to avoid production noise
- Ensures exactInputSingle returns amountOut instead of true

* fix: add RestAPINode.Config.options support and summarizer injection; tests for SendGrid/Telegram

* feat: added ai summarizer

* Refactor node output handlers and fix tuple test

- Remove nodeConfig parameter from output handler interface and implementations
- Update CreateEmptyOutput signature to use nodeType string only
- Update all output handler method signatures (SetError, SetSuccess, GetError, GetSuccess, GetData)
- Remove unused nodeConfig fields from protobuf Node message
- Fix TestRunNodeImmediately_ContractWrite_TupleWithTemplates to use derived runner address
- Add ethclient connection in tuple test to avoid nil pointer panic

* Remove node_config field from RunNodeWithInputsReq in avs.proto

* Fix runNodeWithInputs to extract node config from inputVariables

Node configuration is now passed through inputVariables with a special 'config' key.
The backend extracts this to create a properly configured TaskNode via CreateNodeFromType.

This design ensures nodes are self-contained and consistent across all three code paths:
- runNodeWithInputs (testing single nodes)
- simulateTask (testing workflows)
- deployed workflows (production execution)

Supports both 'config' and 'nodeConfig' keys for backward compatibility.

* refactor: Update RunNodeWithInputs RPC to use complete TaskNode structure

- Changed RunNodeWithInputsReq protobuf to accept TaskNode instead of separate nodeType/nodeConfig fields
- Backend now uses ExtractNodeConfiguration() to extract config from TaskNode, sharing code with SimulateTask/CreateTask
- Updated RunNodeImmediatelyRPC to use node.Type and extract config from the TaskNode
- Updated aggregator RPC server logging to use node.Type from TaskNode
- Added comprehensive test cases demonstrating the new API pattern
- Created MIGRATION_RUN_NODE_WITH_INPUTS.md guide with before/after examples

Benefits:
- Type safety: All node config is now properly typed in protobuf
- Consistency: Matches SimulateTask and CreateTask API patterns
- Completeness: All fields (value, gasLimit, etc.) are properly exposed in schema
- Validation: Protobuf schema validation at API layer
- Code reuse: Shares ExtractNodeConfiguration() logic across all APIs

* Fix macro secrets initialization and QuoterV2 ABI issues

- Initialize MacroSecrets automatically in Engine.New() from config
- Remove redundant SetMacroSecrets() calls from tests and Aggregator
- Include MacroSecrets in testutil.GetAggregatorConfig() from loaded config
- Fix QuoterV2 ABI to include all 4 return values (amountOut, sqrtPriceX96After, initializedTicksCrossed, gasEstimate)
- Fix QuoterV2 parameter order: (tokenIn, tokenOut, amountIn, fee, sqrtPriceLimitX96)
- Fix quote result extraction to use correct data structure
- Fix Tenderly simulation test nil pointer by providing ethclient instance

This improves encapsulation and ensures tests work without manual secret setup.

* Fix TestLoadSecretForTask by clearing secrets after engine creation

The Engine.New() now initializes macroSecrets from config, so we need to clear them AFTER creating the engine, not before.

* Fix nil ethclient in TestContractWriteTenderlySimulation

Same fix as before - aa.GetSenderAddress() requires a valid ethclient, not nil.

* Fix ExactClientRequest test to use derived runner address

The test was using a hardcoded runner address that didn't match the owner's derived address, causing validation to fail. Now uses the actual derived address.

* Update TestGetHash expected hash to match current PayMaster contract

The PayMaster contract on-chain was updated, which changed the hash returned by GetHash(). Updated the test to match the current contract behavior.

* Add regression tests for scheduler premature termination fix

- Add TestSchedulerExecutesAllNodesInSequence to verify linear chains execute completely
- Add TestSchedulerExecutesNodeAfterBranch to verify nodes after branch paths execute
- These tests verify the fix to runKahnScheduler termination condition that was causing
  nodes at the end of execution chains (like email_report_success) to be skipped

* Fix Kahn scheduler bug with nodes having mixed regular and branch edges

The scheduler incorrectly marked nodes as 'branch targets' if they received
ANY branch condition edge, even if they also had regular edges. This caused
nodes to remain gated when the branch condition wasn't selected, even though
all their regular predecessors had completed.

The bug manifested in production where the email node had:
- A regular edge from run_swap
- A branch condition edge from branch.1 (else path)

When branch.0 (if path) was selected, run_swap executed but the email node
remained gated by the unselected branch.1 edge and never executed.

Fix: Only mark a node as a pure 'branch target' if it has NO regular
predecessors. Nodes with mixed edges now execute when all regular
predecessors complete, regardless of branch selections.

Tests added:
- TestSchedulerExecutesAllNodesInSequence: linear execution
- TestSchedulerExecutesNodeAfterBranch: branch with continuation
- TestSchedulerNodeWithMixedEdges: regression test for the specific bug

* refactor: Update RunTriggerReq to use TaskTrigger structure for consistency

- Change RunTriggerReq to accept complete TaskTrigger instead of separate trigger_type and trigger_config fields
- Makes RunTrigger consistent with RunNodeWithInputs (uses TaskNode) and SimulateTask (uses TaskTrigger)
- Update RPC handler and engine implementation to extract config from TaskTrigger using ExtractTriggerConfigData
- Update event_trigger_test.go to construct proper TaskTrigger objects
- Improve type safety and maintainability by using structured types instead of raw maps

* fix: add more testing around the ai summary function

* Refactor ContractWrite data priority system with event handling

Implements correct priority system for ContractWrite response data:
1. PRIORITY 1: Function return values from ABI outputs
2. PRIORITY 2: Event data (when found, overrides return values)
3. PRIORITY 3: Boolean true fallback (no return value and no events)

Changes:
- Extracted priority logic into helper functions for clarity:
  - addReturnValuesToDecodedData() - handles PRIORITY 1
  - handleEventDataForMethod() - handles PRIORITY 2 & 3

- Simplified main execution flow by replacing inline logic with function calls

- When events are found in transaction receipt, they now override return values
  (provides more useful data than simple booleans like approve() returning true)

- When no events found but return value exists, keeps the return value

- When neither events nor return value exist, uses boolean true as fallback

Testing:
- Added comprehensive test suite: vm_runner_contract_write_data_priority_test.go
- 4 test cases covering all priority levels:
  * Priority2_EventData_OverridesReturnValue ✅
  * Prio  * Prio  * Prio  * Prio  * Prio  * Prio  * Prio  * Prio  * Prio  urnNoEvents ✅
  * Priority1_ComplexReturnValue_KeepsDespiteEvents ✅

All tests pass, confirming correct behavior for:
- ERC20 approve() with Approval event → returns event fields
- ERC20 decimals() without events → returns uint8 value
- Functions with no- Functions with no- Functions with no- Functions withith complex returns → keeps return value despite events i- Functions with no- Functions with no- Functions with no- Functions withith val- Functions with no- Functions with no- Functions with no- Functions withith ean returns,
providing more context about what happened in the transaction.

* chore: remove emoji from Execution Step Log

* fix: resolve template variables before validating contract address in ContractRead node

* Update core/config/config.go

Co-authored-by: Copilot <[email protected]>

* Update core/taskengine/summarizer_ai.go

Co-authored-by: Copilot <[email protected]>

* Improve core/taskengine/summarizer_ai.go

* refactor: consolidate task status storage key logic into shared package

- Create storage/schema package with centralized TaskStatusToStorageKey logic
- Remove duplicate code from core/apqueue/cleanup.go
- Update core/taskengine to use shared storage/schema package
- Improve comment clarity for branch target gating in vm.go scheduler

Fixes code duplication identified by Copilot review

* fix: address Copilot code review suggestions

- Use exact match for 'undefined' check instead of substring match to avoid false positives
- Compile regex pattern once at package level for better performance
- Remove unused methodName and contractAddress parameters from extractTransactionValue

* Export subject and body in restApi’s response data

* Integration tests use salt=2 → smart wallet

* fix: add styled HTML email support for AI summary notifications

* test: replace GetTestConfig() with direct helper calls in tests

Replace all uses of testutil.GetTestConfig() that directly access fields
with calls to dedicated helper functions (GetTestTenderlyAccount,
GetTestTenderlyProject, GetTestTenderlyAccessKey, GetAggregatorConfig).

This modernizes test config access and ensures tests use the proper
helper functions that panic with clear error messages if config is missing.

Files updated:
- core/taskengine/tenderly_client_test.go
- core/taskengine/vm_runner_contract_write_data_priority_test.go
- core/taskengine/vm_runner_contract_write_simulation_test.go

* Change the github actions rpc endpoint value to CHAIN_ENDPOINT

* fix summarizer tests: Added nil check for error before accessing err.Error()

* Claned up deprecated docs

* Relax the AI output verification from summarizer tests

---------

Co-authored-by: Wei Lin <[email protected]>
Co-authored-by: Will Zimmerman <[email protected]>
Co-authored-by: Copilot <[email protected]>
@chrisli30 chrisli30 closed this Oct 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants