feat(automation): connect live oracle prices to automation price triggers#205
Conversation
…ocation-utilities feat: Add Soroban Contract Invocation Utilities [Issue Galaxy-KJ#82]
📝 WalkthroughWalkthroughThis PR integrates oracle-backed price feeds into AutomationService, enabling price-based automations (PRICE_ABOVE/PRICE_BELOW conditions) to evaluate against live asset prices. The implementation adds optional oracle injection, introduces a price monitoring polling loop, and extends the execution context with resolved price data. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant AutoSvc as AutomationService
participant Oracle as OracleAggregator
participant Monitor as Polling Loop
participant Evaluator as ConditionEvaluator
participant Rules as Rules DB
App->>AutoSvc: startPriceMonitoring(30000)
activate AutoSvc
AutoSvc->>Monitor: initialize interval
activate Monitor
Monitor->>AutoSvc: runPriceMonitoringCycle()
AutoSvc->>Rules: find active PRICE rules
Rules-->>AutoSvc: [rule1, rule2, ...]
AutoSvc->>AutoSvc: collectPriceAssets([rule1, rule2])
AutoSvc->>Oracle: getAggregatedPrices(['XLM', 'USDC'])
Oracle-->>AutoSvc: {XLM: 0.15, USDC: 1.0, timestamp}
AutoSvc->>AutoSvc: buildPriceContext() → priceContext
loop For each rule
AutoSvc->>AutoSvc: attachLivePrices(rule, baseContext)
AutoSvc->>Evaluator: evaluateConditionGroup(conditions, context+priceContext)
rect rgba(100, 150, 255, 0.5)
Evaluator->>Evaluator: evaluatePriceCondition(PRICE_ABOVE, priceContext)
Evaluator->>Evaluator: resolvePrice('XLM', priceContext)
end
Evaluator-->>AutoSvc: true/false
alt Condition matched
AutoSvc->>AutoSvc: executeRule(rule, context)
end
end
deactivate Monitor
deactivate AutoSvc
App->>AutoSvc: stopPriceMonitoring()
Monitor->>Monitor: clearInterval()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/core/automation/src/services/automation.service.ts (1)
21-21: Cross-package relative import is fragile.The relative import path
../../../stellar-sdk/src/utils/supabase-client.jstraverses outside the current package and directly imports from another package'ssrcdirectory. This couples the build/runtime to a specific monorepo directory structure and bypasses the package's public API.Consider importing from the published package name (e.g.,
@galaxy-kj/core-stellar-sdk) or re-exporting the client from an appropriate location within the automation package.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/automation/src/services/automation.service.ts` at line 21, The current cross-package relative import of supabaseClient (import of '../../../stellar-sdk/src/utils/supabase-client.js') is fragile; replace it by importing the client from the stellar SDK's public package export (e.g., import { supabaseClient } from '@galaxy-kj/core-stellar-sdk') or consume a re-export from the automation package; update the import in automation.service.ts to reference the published package name (or add a re-export in the stellar SDK package index and import that) so you no longer reference another package's src tree directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/automation/src/services/automation.service.ts`:
- Around line 355-362: The current Promise.all over activePriceRules causes all
executeRule(rule.id, {...}) calls to start concurrently and can bypass the
per-rule maxConcurrentExecutions check (because activeExecutions is incremented
concurrently); change this to a controlled-concurrency pattern by serializing or
limiting parallel invocations: either use a concurrency limiter like p-limit and
wrap each call with limit(() => this.executeRule(...)) or iterate the
activePriceRules with an async for-loop and await each executeRule (or process
in configurable batches) so that the effective number of concurrent executeRule
executions respects maxConcurrentExecutions and updates to activeExecutions
occur under the intended limit. Ensure references to activePriceRules,
executeRule, maxConcurrentExecutions, and activeExecutions are updated
accordingly.
In `@packages/core/automation/src/test/automation.test.ts`:
- Around line 549-571: The test's use of expect.not.objectContaining({
priceContext: expect.anything() }) is too lenient because it treats a
present-but-undefined priceContext the same as absent; change the assertion to
explicitly verify the argument has no priceContext property: after calling
serviceWithoutOracle.executeRule, read the first call arg from
mockConditionEvaluator.evaluateConditionGroup.mock.calls (e.g., const arg =
mockConditionEvaluator.evaluateConditionGroup.mock.calls[0][0]) and assert
expect(arg).not.toHaveProperty('priceContext') so the test guarantees
priceContext is strictly absent when evaluateConditionGroup is invoked for
serviceWithoutOracle.
---
Nitpick comments:
In `@packages/core/automation/src/services/automation.service.ts`:
- Line 21: The current cross-package relative import of supabaseClient (import
of '../../../stellar-sdk/src/utils/supabase-client.js') is fragile; replace it
by importing the client from the stellar SDK's public package export (e.g.,
import { supabaseClient } from '@galaxy-kj/core-stellar-sdk') or consume a
re-export from the automation package; update the import in
automation.service.ts to reference the published package name (or add a
re-export in the stellar SDK package index and import that) so you no longer
reference another package's src tree directly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 74998809-b0a8-421a-995b-66fc07693f31
📒 Files selected for processing (8)
packages/core/automation/README.mdpackages/core/automation/jest.config.cjspackages/core/automation/package.jsonpackages/core/automation/src/services/automation.service.tspackages/core/automation/src/test/automation.test.tspackages/core/automation/src/test/condition.test.tspackages/core/automation/src/types/automation-types.tspackages/core/automation/src/utils/condition-evaluator.ts
| return Promise.all( | ||
| activePriceRules.map(rule => | ||
| this.executeRule(rule.id, { | ||
| ...contextData, | ||
| priceContext, | ||
| }) | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Concurrent execution of price rules may bypass maxConcurrentExecutions limit.
Promise.all executes all active price rules simultaneously. If there are many active price rules, this could exceed maxConcurrentExecutions (checked per-rule in executeRule but each call increments activeExecutions concurrently before any checks complete).
Consider using a controlled concurrency pattern (e.g., p-limit or sequential execution) or batching to respect the configured limit.
🛠️ Example using sequential execution
- return Promise.all(
- activePriceRules.map(rule =>
- this.executeRule(rule.id, {
- ...contextData,
- priceContext,
- })
- )
- );
+ const results: ExecutionResult[] = [];
+ for (const rule of activePriceRules) {
+ const result = await this.executeRule(rule.id, {
+ ...contextData,
+ priceContext,
+ });
+ results.push(result);
+ }
+ return results;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/automation/src/services/automation.service.ts` around lines 355
- 362, The current Promise.all over activePriceRules causes all
executeRule(rule.id, {...}) calls to start concurrently and can bypass the
per-rule maxConcurrentExecutions check (because activeExecutions is incremented
concurrently); change this to a controlled-concurrency pattern by serializing or
limiting parallel invocations: either use a concurrency limiter like p-limit and
wrap each call with limit(() => this.executeRule(...)) or iterate the
activePriceRules with an async for-loop and await each executeRule (or process
in configurable batches) so that the effective number of concurrent executeRule
executions respects maxConcurrentExecutions and updates to activeExecutions
occur under the intended limit. Ensure references to activePriceRules,
executeRule, maxConcurrentExecutions, and activeExecutions are updated
accordingly.
| it('should fall back cleanly when a price rule has no oracle configured', async () => { | ||
| const serviceWithoutOracle = new AutomationService({ | ||
| network: testNetwork, | ||
| sourceSecret: 'SBXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX', | ||
| }); | ||
| const rule = createPriceRule({ id: 'price-rule-no-oracle' }); | ||
|
|
||
| mockConditionEvaluator.evaluateConditionGroup.mockResolvedValue(false); | ||
|
|
||
| await serviceWithoutOracle.registerRule(rule); | ||
| const result = await serviceWithoutOracle.executeRule(rule.id); | ||
|
|
||
| expect(result.success).toBe(false); | ||
| expect(result.error?.message).toBe('Conditions not met'); | ||
| expect( | ||
| mockConditionEvaluator.evaluateConditionGroup | ||
| ).toHaveBeenCalledWith( | ||
| rule.conditionGroup, | ||
| expect.not.objectContaining({ | ||
| priceContext: expect.anything(), | ||
| }) | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how expect.not.objectContaining behaves with undefined values
cat << 'EOF' > /tmp/jest-matcher-test.js
const obj1 = { ruleId: 'test', priceContext: undefined };
const obj2 = { ruleId: 'test' };
// Test if expect.not.objectContaining catches undefined
try {
expect(obj1).toEqual(expect.not.objectContaining({ priceContext: expect.anything() }));
console.log("PASS: obj1 with priceContext: undefined passed not.objectContaining");
} catch (e) {
console.log("FAIL: obj1 with priceContext: undefined failed not.objectContaining");
}
try {
expect(obj2).toEqual(expect.not.objectContaining({ priceContext: expect.anything() }));
console.log("PASS: obj2 without priceContext passed not.objectContaining");
} catch (e) {
console.log("FAIL: obj2 without priceContext failed not.objectContaining");
}
EOF
node -e "
const obj1 = { ruleId: 'test', priceContext: undefined };
const obj2 = { ruleId: 'test' };
// expect.anything() matches any value except null/undefined
console.log('priceContext in obj1:', 'priceContext' in obj1);
console.log('obj1.priceContext:', obj1.priceContext);
console.log('priceContext in obj2:', 'priceContext' in obj2);
"Repository: Galaxy-KJ/Galaxy-DevKit
Length of output: 150
🏁 Script executed:
npm list jest 2>/dev/null | head -5Repository: Galaxy-KJ/Galaxy-DevKit
Length of output: 118
🏁 Script executed:
# Find the test file mentioned in the review
find . -type f -name "automation.test.ts" | head -5Repository: Galaxy-KJ/Galaxy-DevKit
Length of output: 121
🏁 Script executed:
# Check the test file around lines 549-571
sed -n '540,575p' ./packages/core/automation/src/test/automation.test.tsRepository: Galaxy-KJ/Galaxy-DevKit
Length of output: 1209
🏁 Script executed:
# Check Jest version and test setup
cat ./packages/core/package.json | grep -A 5 '"jest"' | head -10Repository: Galaxy-KJ/Galaxy-DevKit
Length of output: 127
🏁 Script executed:
# Find package.json files in the packages directory
find ./packages -name "package.json" -type f | head -10Repository: Galaxy-KJ/Galaxy-DevKit
Length of output: 456
🏁 Script executed:
# Check Jest version in the automation package
cat ./packages/core/automation/package.json | grep -E '"jest"|"@jest' -A 1Repository: Galaxy-KJ/Galaxy-DevKit
Length of output: 178
🏁 Script executed:
# Create a Jest test to verify the matcher behavior
mkdir -p /tmp/jest-test && cd /tmp/jest-test
# Create a simple test file
cat > test-matcher.test.js << 'EOF'
describe('expect.not.objectContaining matcher behavior', () => {
test('should FAIL when priceContext is undefined', () => {
const objWithUndefined = { ruleId: 'test', priceContext: undefined };
expect(objWithUndefined).toEqual(
expect.not.objectContaining({
priceContext: expect.anything(),
})
);
});
test('should PASS when priceContext is absent', () => {
const objWithout = { ruleId: 'test' };
expect(objWithout).toEqual(
expect.not.objectContaining({
priceContext: expect.anything(),
})
);
});
});
EOF
# Create minimal jest.config.js
cat > jest.config.js << 'EOF'
module.exports = {
testEnvironment: 'node',
};
EOF
# Create package.json
cat > package.json << 'EOF'
{
"name": "jest-test",
"version": "1.0.0",
"devDependencies": {
"jest": "^30.0.5"
}
}
EOF
# Install jest
npm install --silent 2>&1 | grep -v "npm warn"
# Run the test
npm test -- test-matcher.test.js 2>&1 | grep -A 50 "FAIL\|PASS\|●"Repository: Galaxy-KJ/Galaxy-DevKit
Length of output: 49
🏁 Script executed:
# Let's check if we can run the actual test file from the repo
cd ./packages/core/automation
npm test -- --testNamePattern="should fall back cleanly when a price rule has no oracle configured" 2>&1 | tail -50Repository: Galaxy-KJ/Galaxy-DevKit
Length of output: 224
🌐 Web query:
Jest expect.objectContaining expect.anything() undefined behavior matcher
💡 Result:
In Jest, expect.anything matches any value except null or undefined. When used within expect.objectContaining({ prop: expect.anything }), it requires the object to have a prop property whose value is neither null nor undefined. If the property is undefined (i.e., absent or explicitly set to undefined), the matcher fails. This is the documented behavior: official Jest docs state expect.anything matches "anything but null or undefined" [3,4,14]. Stack Overflow confirms it does not match undefined, even in object contexts [2]. Blogs note expect.objectContaining does not natively support undefined properties; workarounds like jest-extended's expect.toBeOneOf([expect.anything, null, undefined]) are needed [1,9]. Examples: - Passes: { prop: "value" } matches { prop: expect.anything } - Fails: { prop: undefined } or {} does not match { prop: expect.anything } For optional/undefined properties, omit from objectContaining, use toHaveProperty('prop') without value, or extend with custom matchers.
Citations:
- 1: https://zirkelc.dev/posts/use-expectobjectcontaining-with-null-and-undefined
- 2: https://stackoverflow.com/questions/47027011/using-expect-anyobject-or-expect-anything-does-not-work-to-match-undefi
- 3: https://jestjs.io/docs/en/expect
- 4: https://jestjs.io/docs/expect
- 5: https://archive.jestjs.io/docs/en/23.x/expect
- 6: https://dev.to/zirkelc/how-to-use-expectobjectcontaining-with-null-and-undefined-3iba
- 7: https://jest-archive-august-2023.netlify.app/docs/25.x/expect/
The test assertion is subtly lenient with expect.anything().
The use of expect.not.objectContaining({ priceContext: expect.anything() }) will pass when priceContext is either absent OR explicitly undefined. Since expect.anything() matches any value except null/undefined, the objectContaining part won't match either case, making expect.not.objectContaining pass in both scenarios.
If the intent is to ensure priceContext is strictly absent (not present-but-undefined), consider using a more explicit assertion:
expect.not.objectContaining({ priceContext: expect.anything() })— current (accepts both absent and undefined)- Alternative: Check the exact object structure or use a custom matcher if stricter behavior is needed
This doesn't necessarily indicate a bug, but it's worth confirming whether the current behavior aligns with the intended guarantee that no price context is provided to the condition evaluator.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/automation/src/test/automation.test.ts` around lines 549 - 571,
The test's use of expect.not.objectContaining({ priceContext: expect.anything()
}) is too lenient because it treats a present-but-undefined priceContext the
same as absent; change the assertion to explicitly verify the argument has no
priceContext property: after calling serviceWithoutOracle.executeRule, read the
first call arg from mockConditionEvaluator.evaluateConditionGroup.mock.calls
(e.g., const arg =
mockConditionEvaluator.evaluateConditionGroup.mock.calls[0][0]) and assert
expect(arg).not.toHaveProperty('priceContext') so the test guarantees
priceContext is strictly absent when evaluateConditionGroup is invoked for
serviceWithoutOracle.
2 participants
Summary
This PR connects live oracle prices to automation price triggers so active
TriggerType.PRICErules can evaluate against current market data instead of static context.Closes #155
What Changed
OracleAggregatorsupport intoAutomationServicePriceConditionContexttoExecutionContextConditionEvaluatorto consume live context instead of querying infrastructure directlycheckAndExecute(),startPriceMonitoring(), andstopPriceMonitoring()for active price rulesDesign Notes
ConditionEvaluatoris now context-driven for price conditions. This keeps evaluation logic pure and letsAutomationServiceown oracle access, price prefetching, and monitoring lifecycle management.When no oracle is configured, non-price automations continue to work as before. Price conditions evaluate to
falseunless a caller providespriceContextexplicitly.Validation
npx jest packages/core/automation/src/test/condition.test.ts packages/core/automation/src/test/automation.test.ts --runInBandcd packages/core/automation && npm run lintcd packages/core/automation && npm run test:coverageNotes For Maintainers
The repository currently has unrelated pre-existing build/type-check issues in other packages (
core-invisible-walletandcore-stellar-sdk). This PR does not modify those areas.Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests