Skip to content

Conversation

@weilicious
Copy link
Collaborator

@weilicious weilicious commented Oct 23, 2025

This pull request introduces several improvements focused on error handling, logging, input variable propagation, and output serialization for the aggregator and task engine modules. The most significant changes include graceful handling of database lock errors, propagation of trigger input variables into task execution, enhancements to output handlers for deep conversion of protobuf values, and improved logger usage. These updates collectively improve reliability, debuggability, and interoperability of the system.

Error Handling and Logging Improvements

  • Graceful handling of database lock errors in aggregator.go: Instead of panicking, the system now logs an info message and returns a user-friendly error when the database is locked by another process, preventing stack traces and improving diagnostics. [1] [2]
  • Logger usage in taskengine/engine.go is standardized by replacing the global logger variable with globalLogger, ensuring consistent logging throughout the module. [1] [2] [3]
  • Logger is now passed to smart wallet operations for improved debug/verbose logging in rpc_server.go. [1] [2]

Input Variable Propagation

  • Trigger input variables from protobuf requests are now extracted and propagated into the VM execution context, allowing dynamic per-trigger customization of task execution. This is reflected in both the QueueExecutionData struct and the RunTask method. [1] [2] [3]

Output Handler Serialization Enhancements

  • Output handlers for REST API, Custom Code, Balance, and Contract Read/Write nodes now recursively convert protobuf values to plain Go types before serialization, ensuring clean and interoperable output data. [1] [2] [3] [4] [5] [6] [7]
  • The execution context is now preserved in contract write node outputs for improved traceability.

Configuration and Testing Updates

  • The controller address is now derived from the controller private key and stored in the smart wallet config, simplifying signature verification and test setup. [1] [2] [3]
  • Added a new test for loop nodes with contract write runners to validate per-iteration data population and simulation output.

These changes collectively enhance the robustness, maintainability, and clarity of the aggregator and task engine subsystems.

- 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
…er 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
… 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
…rs in runNodeWithInputs

- Added convertProtobufValueToPlain helper to recursively convert nested *structpb.Value objects to plain Go types
- Fixed RestAPIOutputHandler to properly convert metadata with full REST response (status, headers, data, etc.)
- Fixed FilterOutputHandler, GraphQLOutputHandler, LoopOutputHandler, BalanceOutputHandler, and ContractReadOutputHandler with deep conversion
- Fixed CustomCodeOutputHandler to exclude executionContext from data output
- Fixed ContractWriteOutputHandler to properly convert receipts and events with deep conversion
- Resolved 'proto: invalid type: *structpb.Value' errors in all node output handlers
- All RestAPI (9/9), FilterNode (8/8), and CustomCode (23/23) tests now passing
- ContractWrite tests improved (24/26 passing, 2 unrelated failures)
- 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
- Aligned Topics field formatting after go fmt
- All test queries now use flat array format: Topics: []string{...}
- 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 race condition where parallel loop iterations were sharing and
overwriting the 'value' variable in the shared VM state, causing all
iterations to see the last iteration's data.

Changes:
- Skip shared VM state injection for CustomCode nodes in parallel loops
- Use closure pattern to properly capture loop variables per iteration
- Deep copy iteration items to ensure complete data isolation
- CustomCode nodes now execute with fully isolated JavaScript VMs

The bug manifested as all parallel loop iterations returning the same
(last) item's data instead of their respective iteration data. This
was caused by goroutines temporarily modifying v.vars simultaneously,
creating a race condition where the 'value' variable would be
overwritten before JavaScript code could read it.

Fixes parallel loop execution for CustomCode runners.
@chrisli30 chrisli30 merged commit a797311 into staging Oct 24, 2025
17 checks passed
@chrisli30 chrisli30 deleted the wei-fix_userops branch October 24, 2025 06:07
chrisli30 pushed a commit that referenced this pull request Oct 24, 2025
* Clean and update documents in the ./docs folder

* fix:load AA controller address globally at startup

* Simplify scripts/aa-wallet-toolkit.go

* 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

* Updated docker-compose with the latest aggregator and operator examples

* 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

* tests: update manual_trigger_loop_test for new output semantics and isSimulated placement

* 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

* fix: Add deep protobuf Value conversion to prevent serialization errors in runNodeWithInputs

- Added convertProtobufValueToPlain helper to recursively convert nested *structpb.Value objects to plain Go types
- Fixed RestAPIOutputHandler to properly convert metadata with full REST response (status, headers, data, etc.)
- Fixed FilterOutputHandler, GraphQLOutputHandler, LoopOutputHandler, BalanceOutputHandler, and ContractReadOutputHandler with deep conversion
- Fixed CustomCodeOutputHandler to exclude executionContext from data output
- Fixed ContractWriteOutputHandler to properly convert receipts and events with deep conversion
- Resolved 'proto: invalid type: *structpb.Value' errors in all node output handlers
- All RestAPI (9/9), FilterNode (8/8), and CustomCode (23/23) tests now passing
- ContractWrite tests improved (24/26 passing, 2 unrelated failures)

* 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: resolve race condition in parallel loop execution

Fix race condition where parallel loop iterations were sharing and
overwriting the 'value' variable in the shared VM state, causing all
iterations to see the last iteration's data.

Changes:
- Skip shared VM state injection for CustomCode nodes in parallel loops
- Use closure pattern to properly capture loop variables per iteration
- Deep copy iteration items to ensure complete data isolation
- CustomCode nodes now execute with fully isolated JavaScript VMs

The bug manifested as all parallel loop iterations returning the same
(last) item's data instead of their respective iteration data. This
was caused by goroutines temporarily modifying v.vars simultaneously,
creating a race condition where the 'value' variable would be
overwritten before JavaScript code could read it.

Fixes parallel loop execution for CustomCode runners.

* fix: add generic event Tenderly simulation and fix unit tests
@chrisli30 chrisli30 mentioned this pull request 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