-
Notifications
You must be signed in to change notification settings - Fork 75
Open
Description
Problem
The current naming and usage of "trigger output" in TriggerTaskReq is confusing and misleading. The field is actually used for simulating trigger conditions during debugging/testing, not for defining actual trigger output data.
Current Confusion
- Name: "trigger output" suggests it's the output/result of a trigger
- Reality: It's simulation data to test "would this trigger fire under these conditions?"
- Manual Trigger Issue: Manual triggers always fire when called - they don't need simulation data
Examples of Proper Usage
// Other triggers need simulation data:
blockTrigger.simulate({ blockNumber: 12345 }) // "What if block 12345 was mined?"
eventTrigger.simulate({ eventData: {...} }) // "What if this event occurred?"
cronTrigger.simulate({ timestamp: 1234567890 }) // "What if it's this time?"
// Manual trigger doesn't need simulation - it just triggers:
manualTrigger.trigger() // "Trigger now!" (no conditions to simulate)Changes Made
- ✅ Removed manual trigger output - Manual triggers don't need simulation data
- ✅ Simplified SDK logic - No longer sends unnecessary trigger output for manual triggers
- ✅ Maintained functionality - Manual triggers still work correctly (confirmed in aggregator logs)
Recommended Improvements
- Rename field:
trigger_output→trigger_simulation_dataordebug_conditions - Update documentation: Clarify that this field is for debugging/testing trigger conditions
- Protocol cleanup: Consider making this field optional for triggers that don't need simulation
Impact
- ✅ Manual triggers work without unnecessary data
- ✅ Cleaner client code (removed ~20 lines of unnecessary logic)
- ✅ Better conceptual clarity about trigger simulation vs execution
- ✅ Maintained backward compatibility
This change improves the architectural clarity while maintaining full functionality.
Metadata
Metadata
Assignees
Labels
No labels