Repository navigation
Conversation
- Add docker-compose.devnet.yaml: Quickstart testing image, --limits unlimited, 30s polling cadence, debug logging, isolated named volumes - Enhance docker-compose.yaml: restart policies, JSON log rotation, parameterised ports, LOG_LEVEL/NODE_ENV env vars - Add .env.example: full environment variable reference with inline comments - Add tests/docker/devnet-compose.test.ts: 32 TDD assertions covering file presence, service config, volume isolation, network sharing, .env.example, and compose merge compatibility - Update .dockerignore: exclude compose files, systemd/, docs/, templates/ - Update .gitignore: allow .env.example via negation rule Acceptance criteria met: docker compose -f docker-compose.yaml -f docker-compose.devnet.yaml up boots daemon and mock RPC environment successfully. All 530 tests pass, 63 docker-specific tests, 5 skipped TODOs.
…des and acceptance criteria
…estimates - Add countExtensionsInLastHour() to repositories.ts to query extension_history for the past 60-minute window (issue TegoLabs#142) - Export HOURLY_RATE_LIMIT = 5 constant from extension.ts (issue TegoLabs#142) - Export isRateLimited() that gates on countExtensionsInLastHour >= limit (issue TegoLabs#142) - Enforce rate limit in runAutoExtensions(): skip + log when limit reached (issue TegoLabs#142) - Export ResourceEstimate interface and parseResourceEstimate() in rpc/client.ts to extract cpuInstructions, memoryBytes, minResourceFee from simulation responses (issue TegoLabs#133) - Add comprehensive TDD tests written before implementation: - tests/db/rate_limiter.test.ts: countExtensionsInLastHour edge cases - tests/core/rate_limiter.test.ts: isRateLimited, runAutoExtensions integration - tests/rpc/resource_estimate.test.ts: parseResourceEstimate + failure edge cases Closes TegoLabs#133 Closes TegoLabs#137 Closes TegoLabs#142
Preserves existing behavior exactly: same target flags, same missing- target error text, same lazy dynamic import for discord/telegram, same webhook-only HMAC signing. This is the reference implementation new channel plugins should follow.
Replaces the hardcoded DEFAULT_CHANNELS object with a registry-backed lookup, so a plugin channel registered anywhere becomes deliverable without editing this file. Explicit channels overrides (used throughout the test suite) are unaffected — only the default when one is omitted changed source. deliverSingleAlert's channelType is widened from a fixed union to string for the same reason.
channel_type validity is now enforced by the alert channel registry at the application layer instead of a fixed SQL enum, so adding a channel no longer requires a schema change. The CHECK now only guards against an empty string.
…ration The SCHEMA comment-stripper (`--.*\n`) silently failed to match comments ending in \r\n, since JS's `.` excludes all line terminators including \r. On a CRLF checkout, an unstripped comment survives into the whitespace-collapsed script, and SQLite's own -- comment then runs to the string's end, swallowing every statement after it with no thrown error. Switched to `--[^\n]*\n`, which matches either line ending. Latent since schema.sql had no comments before this change. Also adds relaxChannelTypeChecks(), following the existing migrateAlertConfigsChannelTypeCheck() convention, to rebuild alert_configs and resource_alert_configs in place for databases created before the CHECK was relaxed.
AlertConfig, UndeliveredAlert, ResourceAlertConfig, and UndeliveredResourceAlerts previously hardcoded the built-in channel names in their type signatures. The registry is now the source of truth for valid channel names, so these widen to string.
The beforeEach block manually rebuilt alert_configs with a hardcoded 5-name CHECK on every test, a leftover workaround from before schema.sql had these columns natively. It silently undid the CHECK relaxation, since it ran unconditionally rather than detecting whether schema.sql already had the change. getDatabaseForTesting() already execs the current schema.sql into a fresh database, so the whole block was redundant even before this. Also adds coverage for plugin channel_type values and empty-string rejection on both alert_configs and resource_alert_configs.
Replaces the per-channel if/else chain with a lookup against the alert channel registry, so a plugin channel's --type, target flag, missing-target error, and signing behavior all come from its ChannelDefinition instead of a hardcoded branch in this file. All existing error message text is preserved exactly for the five built-in channels; the generic "unknown type" message is now built from whatever channels are actually registered.
…nsion submission - Add MINIMUM_BALANCE_XLM constant (5 XLM) — before submitting an extension through a channel account, verify it holds enough XLM to cover the base reserve and transaction fee. - If balance is insufficient or unknown, skip submission, log a clear warning, and fire an alert through the contract's configured alert channels rather than attempting a failing transaction. - Surface low-balance channel accounts in "sorokeep channels list" output with a visual balance indicator and LOW warning. Resolves TegoLabs#504
…dundant back-to-back extensions - Add EXTENSION_COOLDOWN_MS constant (5 minutes) — prevent the same entry from being extended twice in quick succession by checking the most recent extension timestamp per entry before auto-extending. - Cooldown runs after the rate-limit check; both are complementary safeguards — HOURLY_RATE_LIMIT is not weakened or removed. - An entry extended within the cooldown window is skipped with a clear log message; entries outside the window extend normally. Resolves TegoLabs#510
|
@EthTobi Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change adds a five-minute extension cooldown, enforces a minimum channel-account balance before submission, sends alerts for low balances, and displays balance status in the channels list. Tests cover cooldown, balance validation, alerts, successful extension, and unknown balances. ChangesExtension safety checks
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant runAutoExtensions
participant channel_accounts repository
participant alert dispatcher
participant extension submission
runAutoExtensions->>channel_accounts repository: read balance and extension history
runAutoExtensions->>runAutoExtensions: filter cooldown-ineligible entries
runAutoExtensions->>channel_accounts repository: validate channel-account balance
runAutoExtensions->>alert dispatcher: deliver low-balance alert
runAutoExtensions->>extension submission: submit eligible extensions
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| - | - | Generic High Entropy Secret | ded54f4 | tests/commands/guard-cli-export-import.test.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
43ad363 to
8692701
Compare
# Conflicts: # .github/workflows/ci.yml # CONTRIBUTING.md # README.md # docs/ARCHITECTURE.md # docs/adding-an-alert-channel.md # package-lock.json # package.json # src/alerts/builtins.ts # src/alerts/dispatcher.ts # src/alerts/registry.ts # src/commands/alerts.ts # src/commands/daemon.ts # src/commands/db.ts # src/core/extension.ts # src/core/monitor.ts # src/daemon/loop.ts # src/db/backup.ts # src/db/database.ts # src/db/migrator.ts # src/db/repositories.ts # src/db/schema.sql # src/index.ts # src/lib.ts # src/rpc/client.ts # src/utils/config.ts # tests/alerts/builtins.test.ts # tests/alerts/dispatcher.test.ts # tests/alerts/slack.test.ts # tests/alerts/webhook.test.ts # tests/commands/alerts.test.ts # tests/commands/db.test.ts # tests/core/extension.test.ts # tests/core/monitor.test.ts # tests/core/rate_limiter.test.ts # tests/daemon/loop.test.ts # tests/db/backup.test.ts # tests/db/database.test.ts # tests/db/repositories.test.ts # tests/docker/docker-compose.test.ts # tests/e2e/sandbox-network.test.ts # tests/mcp/lifecycle.test.ts # tests/rpc/resource_estimate.test.ts
# Conflicts: # .github/workflows/ci.yml # CONTRIBUTING.md # README.md # docs/ARCHITECTURE.md # docs/adding-an-alert-channel.md # package-lock.json # package.json # src/alerts/builtins.ts # src/alerts/dispatcher.ts # src/alerts/registry.ts # src/commands/alerts.ts # src/commands/channels.ts # src/commands/daemon.ts # src/commands/db.ts # src/core/extension.ts # src/core/monitor.ts # src/daemon/loop.ts # src/db/backup.ts # src/db/database.ts # src/db/migrator.ts # src/db/repositories.ts # src/db/schema.sql # src/index.ts # src/lib.ts # src/rpc/client.ts # src/utils/config.ts # tests/alerts/builtins.test.ts # tests/alerts/dispatcher.test.ts # tests/alerts/slack.test.ts # tests/alerts/webhook.test.ts # tests/commands/alerts.test.ts # tests/commands/channels.test.ts # tests/commands/db.test.ts # tests/core/extension.test.ts # tests/core/monitor.test.ts # tests/daemon/loop.test.ts # tests/db/backup.test.ts # tests/db/database.test.ts # tests/db/repositories.test.ts # tests/docker/docker-compose.test.ts # tests/e2e/sandbox-network.test.ts # tests/mcp/lifecycle.test.ts # tests/rpc/resource_estimate.test.ts
# Conflicts: # src/core/extension.ts # tests/core/extension.test.ts
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/extension.ts`:
- Around line 450-471: Reuse the in-scope channelAccounts collection instead of
calling getChannelAccounts inside the extension loop, and find the matching
account from that collection. Replace the getContract(db, contract.id) re-fetch
with the existing contract object when configuring the alert, preserving the
current alert behavior and contract data usage.
- Around line 473-486: Update the threshold_crossed event construction in the
low-balance alert path to pass config.threshold_ledgers as configuredLedgers
instead of MINIMUM_BALANCE_XLM, preserving TTL severity calculations against the
configured ledger threshold. Extend the low-balance test to verify the delivered
event payload fields, including configuredLedgers and the TTL-related values.
- Around line 449-460: The unknown-balance warning in the slot handling flow
should direct users to the operation that actually refreshes balances, rather
than `sorokeep channels list`. Update the message constructed in the `balance
=== null || balance === undefined` branch while preserving the existing logging,
error collection, pool release, and return behavior.
- Around line 487-497: Update the low-balance alert path around
deliverSingleAlert to use the existing alerts_fired state and suppress repeat
notifications across extension cycles, matching the TTL threshold behavior.
Preserve error logging for failed deliveries, and ensure the alert state is
checked and recorded consistently before dispatch.
- Around line 39-57: Replace the hardcoded MINIMUM_BALANCE_XLM value with
configuration loading through loadConfig(), using 5 as the fallback when no
threshold is configured. Update the balance-checking flow to consume this
configured value while preserving the existing reserve and fee validation
behavior.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e474b520-741b-41c7-9876-191ca3a306b7
📒 Files selected for processing (5)
src/commands/channels.tssrc/core/extension.tstests/commands/channels.test.tstests/core/extension.test.tstests/core/rate_limiter.test.ts
📜 Review details
🧰 Additional context used
🪛 OpenGrep (1.26.0)
tests/core/extension.test.ts
[ERROR] 783-786: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔇 Additional comments (8)
src/core/extension.ts (2)
384-421: LGTM!
504-504: LGTM!src/commands/channels.ts (1)
6-6: LGTM!Also applies to: 60-73
tests/commands/channels.test.ts (1)
6-6: LGTM!Also applies to: 227-267
tests/core/extension.test.ts (3)
11-13: LGTM!Also applies to: 39-45
782-787: LGTM!
897-1161: LGTM!tests/core/rate_limiter.test.ts (1)
94-101: LGTM!
| /** | ||
| * Minimum time (in milliseconds) that must pass between two consecutive | ||
| * extensions of the same contract entry. Prevents the same entry being | ||
| * extended twice in quick succession when the threshold is very tight | ||
| * relative to the target TTL. | ||
| * | ||
| * Default 5 minutes — slightly below the typical polling interval so it | ||
| * does not interfere with normal operation. | ||
| */ | ||
| export const EXTENSION_COOLDOWN_MS = 5 * 60 * 1000; | ||
|
|
||
| /** | ||
| * Minimum XLM balance a channel account must hold before Sorokeep will submit | ||
| * an extension transaction through it. Covers the base reserve (1 XLM) plus | ||
| * a safety margin for transaction fees. Accounts below this threshold are | ||
| * skipped and an alert is fired rather than attempting a failing transaction. | ||
| */ | ||
| export const MINIMUM_BALANCE_XLM = 5; | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if loadConfig is used elsewhere in extension.ts
rg -n 'loadConfig' src/core/extension.ts
# Check the config schema/type for a balance-threshold-like field
rg -n -i 'balance|minimum|threshold|reserve' src/utils/config.tsRepository: AbdulmalikAlayande/sorokeep
Length of output: 262
🏁 Script executed:
#!/bin/bash
set -u
printf '--- extension.ts around imports/config uses ---\n'
sed -n '1,90p' src/core/extension.ts
printf '\n--- extension.ts around config line 724 ---\n'
sed -n '700,745p' src/core/extension.ts
printf '\n--- config schema/type files ---\n'
fd -a 'config\.(ts|tsx|js)$' . | sed 's#^\./##' | sort
rg -n -i 'interface .*config|type .*config|class .*config|loaded|load|balance|minimum|threshold|reserve|channel' src/utils/config.ts src -g '*.ts' -g '*.js' | head -200Repository: AbdulmalikAlayande/sorokeep
Length of output: 24227
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- src/utils/config.ts ---\n'
sed -n '1,110p' src/utils/config.ts
printf '\n--- occurrences of MINIMUM_BALANCE_XLM ---\n'
rg -n 'MINIMUM_BALANCE_XLM\b' src
printf '\n--- channel list/list balance command files ---\n'
fd -a '(^channels\.ts$|.*channels.*)' src/commands src | sort | sed 's#^\./##'
rg -n -i 'list|refresh|balance_xlm|run .channels|sorokeep channels' src/commands src -g '*.ts' | head -200Repository: AbdulmalikAlayande/sorokeep
Length of output: 256
Make the minimum balance threshold configurable.
MINIMUM_BALANCE_XLM is a hardcoded constant, while issue #504 requires a configurable threshold covering transaction fees and the Stellar base reserve. Read the threshold from loadConfig() with 5 as the default instead of fixing it in source.
♻️ Proposed direction
-export const MINIMUM_BALANCE_XLM = 5;
+export function getMinimumBalanceXlm(): number {
+ const config = loadConfig();
+ return config.minimumChannelBalanceXlm ?? 5;
+}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/extension.ts` around lines 39 - 57, Replace the hardcoded
MINIMUM_BALANCE_XLM value with configuration loading through loadConfig(), using
5 as the fallback when no threshold is configured. Update the balance-checking
flow to consume this configured value while preserving the existing reserve and
fee validation behavior.
| if (slot) { | ||
| const accounts = getChannelAccounts(db, network); | ||
| const channelAccount = accounts.find(a => a.public_key === slot!.publicKey); | ||
| const balance = channelAccount?.balance_xlm; | ||
|
|
||
| if (balance === null || balance === undefined) { | ||
| const msg = `Contract ${contract.id}: Channel account ${slot.publicKey} balance is unknown — skipping extension. Run 'sorokeep channels list' to refresh balances.`; | ||
| logger.warn(msg); | ||
| result.errors.push(msg); | ||
| pool!.release(slot.publicKey); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find where balance_xlm actually gets updated from network data
rg -n 'updateChannelBalance\(' src -g '!**/tests/**'
# Confirm the actual behavior of 'channels list'
rg -n -A5 'command\("list"\)' src/commands/channels.tsRepository: AbdulmalikAlayande/sorokeep
Length of output: 722
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
git ls-files | rg 'src/(core/extension\.ts|commands/channels\.ts|db/repositories\.ts|core/channels\.ts)$' || true
echo
echo "== extension warning context =="
sed -n '430,475p' src/core/extension.ts 2>/dev/null || true
echo
echo "== channels list implementation =="
sed -n '1,90p' src/commands/channels.ts 2>/dev/null || true
echo
echo "== getChannelAccounts / updateChannelBalance refs =="
rg -n "getChannelAccounts|updateChannelBalance|balance_xlm|xlm|SorobanRPC|getLedgerEntries|getBalance" src -g '!**/tests/**' | head -n 200 || trueRepository: AbdulmalikAlayande/sorokeep
Length of output: 19176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== core/channels.ts balance refresh implementation =="
sed -n '1,190p' src/core/channels.ts
echo
echo "== updateChannelBalance definition =="
sed -n '990,1012p' src/db/repositories.ts
echo
echo "== all updateChannelBalance call sites outside tests =="
rg -n 'updateChannelBalance\(' src -g '!**/tests/**'Repository: AbdulmalikAlayande/sorokeep
Length of output: 7543
Correct the unknown-balance guidance.
channels list only reads stored balance_xlm; it does not call the RPC or call refreshBalances(). If balance_xlm is null/undefined, the warning should reference the actual refresh operation instead of sorokeep channels list.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/extension.ts` around lines 449 - 460, The unknown-balance warning in
the slot handling flow should direct users to the operation that actually
refreshes balances, rather than `sorokeep channels list`. Update the message
constructed in the `balance === null || balance === undefined` branch while
preserving the existing logging, error collection, pool release, and return
behavior.
| const accounts = getChannelAccounts(db, network); | ||
| const channelAccount = accounts.find(a => a.public_key === slot!.publicKey); | ||
| const balance = channelAccount?.balance_xlm; | ||
|
|
||
| if (balance === null || balance === undefined) { | ||
| const msg = `Contract ${contract.id}: Channel account ${slot.publicKey} balance is unknown — skipping extension. Run 'sorokeep channels list' to refresh balances.`; | ||
| logger.warn(msg); | ||
| result.errors.push(msg); | ||
| pool!.release(slot.publicKey); | ||
| return; | ||
| } | ||
|
|
||
| if (balance < MINIMUM_BALANCE_XLM) { | ||
| const msg = `Contract ${contract.id}: Channel account ${slot.publicKey} balance ${balance} XLM is below minimum ${MINIMUM_BALANCE_XLM} XLM — skipping extension.`; | ||
| logger.warn(msg); | ||
| result.errors.push(msg); | ||
|
|
||
| // Fire alert through the contract's configured alert channels | ||
| // using the first entry that needed extension as context | ||
| const sampleEntry = needsExtension[0]!; | ||
| const contractRecord = getContract(db, contract.id); | ||
| const alertConfigs = getAlertConfigsForContract(db, contract.id); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Avoid re-fetching data already available in scope.
getChannelAccounts(db, network) at line 450 re-queries the full channel-account table even though channelAccounts was already fetched at line 341 and is in scope. getContract(db, contract.id) at line 470 likewise re-fetches a record that is likely already present on the contract object from eligibleContracts (sourced from getAllContracts).
Reuse the already-fetched values instead of issuing new queries per eligible task.
♻️ Proposed fix
if (slot) {
- const accounts = getChannelAccounts(db, network);
- const channelAccount = accounts.find(a => a.public_key === slot!.publicKey);
+ const channelAccount = channelAccounts.find(a => a.public_key === slot!.publicKey);
const balance = channelAccount?.balance_xlm; const sampleEntry = needsExtension[0]!;
- const contractRecord = getContract(db, contract.id);
+ const contractRecord = contract;
const alertConfigs = getAlertConfigsForContract(db, contract.id);Confirm contract already carries a name field matching Contract:
#!/bin/bash
ast-grep run --pattern 'interface Contract { $$$ }' --lang typescript src🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/extension.ts` around lines 450 - 471, Reuse the in-scope
channelAccounts collection instead of calling getChannelAccounts inside the
extension loop, and find the matching account from that collection. Replace the
getContract(db, contract.id) re-fetch with the existing contract object when
configuring the alert, preserving the current alert behavior and contract data
usage.
| const event = buildAlertEvent({ | ||
| type: "threshold_crossed", | ||
| contractId: contract.id, | ||
| contractName: contractRecord?.name ?? null, | ||
| network, | ||
| entryKeyXdr: sampleEntry.entry_key_xdr, | ||
| entryType: sampleEntry.entry_type, | ||
| entryLabel: sampleEntry.label, | ||
| configuredLedgers: MINIMUM_BALANCE_XLM, | ||
| remainingTTL: sampleEntry.live_until_ledger | ||
| ? Math.max(0, sampleEntry.live_until_ledger - latestLedger) | ||
| : 0, | ||
| firedAtLedger: latestLedger, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
sed -n '1,60p' src/alerts/types.ts
rg -n 'threshold_crossed|alert_resolved|low_balance' src/alerts --type tsRepository: AbdulmalikAlayande/sorokeep
Length of output: 6912
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== extension relevant section =="
sed -n '430,520p' src/core/extension.ts
echo
echo "== constants and config usages =="
rg -n "MINIMUM_BALANCE_XLM|threshold_ledgers|buildAlertEvent|deliverSingleAlert|alert_config" src/core/extension.ts src -g '*.ts'
echo
echo "== alerts builder/contracts =="
sed -n '70,130p' src/alerts/types.ts
rg -n "function buildAlertEvent|type AlertEvent|interface .*Event|makeAlertEvent" src/alerts --type ts
echo
echo "== test coverage section =="
sed -n '995,1095p' tests/core/extension.test.tsRepository: AbdulmalikAlayande/sorokeep
Length of output: 31145
Pass the TTL threshold into the low-balance alert event.
buildAlertEvent expects configuredLedgers to match the configured ledger threshold for threshold_crossed/alert_resolved events. This path uses MINIMUM_BALANCE_XLM for an XLM balance alert, but the event type is still TTL; this makes the payload read “configured ledgers: 5” and computes severity against that value instead of config.threshold_ledgers. Use the config’s ledger threshold here or create a balance-alert event type instead of overloading threshold_crossed. Also extend the low-balance test to assert the delivered event fields, not just that delivery was called.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/extension.ts` around lines 473 - 486, Update the threshold_crossed
event construction in the low-balance alert path to pass
config.threshold_ledgers as configuredLedgers instead of MINIMUM_BALANCE_XLM,
preserving TTL severity calculations against the configured ledger threshold.
Extend the low-balance test to verify the delivered event payload fields,
including configuredLedgers and the TTL-related values.
| deliverSingleAlert( | ||
| config.channel_type, | ||
| config.channel_target, | ||
| event, | ||
| config.webhook_secret, | ||
| ).catch((err: unknown) => { | ||
| logger.warn( | ||
| `Low-balance alert delivery failed for channel ${config.channel_type}: ${err instanceof Error ? err.message : String(err)}`, | ||
| ); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check other call sites of deliverSingleAlert/buildAlertEvent for a dedup/state-tracking wrapper
rg -n -B3 -A15 'deliverSingleAlert\(' src --type ts
rg -n 'alert_history|already_fired|last_alert|alert_state' src --type tsRepository: AbdulmalikAlayande/sorokeep
Length of output: 6511
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files 'src/**/*.ts' | sed -n '1,220p'
echo
echo "== extension outline/section =="
wc -l src/core/extension.ts
ast-grep outline src/core/extension.ts --match runAutoExtensions --view expanded || true
sed -n '440,510p' src/core/extension.ts
echo
echo "== promises/runAutoExtensions call sites =="
rg -n -B4 -A12 'runAutoExtensions|Promise\.all\(eligibleTasks|runPolling|cron|process\.on\(' src --type ts
echo
echo "== alert state/dedup/search =="
rg -n 'alert_history|already_fired|last_alert|alert_state|sentAlert|alert_sent|cooldown|ttl|firedAtLedger|FiredAtLedger|buildAlertEvent|buildTestEvent|send\(.*event|send\(' src --type ts | sed -n '1,260p'
echo
echo "== dispatcher relevant section =="
sed -n '150,210p' src/alerts/dispatcher.tsRepository: AbdulmalikAlayande/sorokeep
Length of output: 26494
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== daemon startDaemon section =="
wc -l src/daemon/loop.ts
sed -n '1,260p' src/daemon/loop.ts
echo
echo "== monitor full alert resolution loop for dedupe =="
sed -n '220,310p' src/core/monitor.ts
echo
echo "== extension task loop full and alert firing section =="
sed -n '360,502p' src/core/extension.ts
echo
echo "== repositories alert fired helpers =="
sed -n '330,380p' src/db/repositories.ts
rg -n -B8 -A20 'function readAlertFiredRows|const readAlertFiredRows|export .*readAlertFiredRows|firedAtLedger|fired_at_ledger|ttl_at_fire' src/db/repositories.ts src/core/monitor.ts src/core/extension.ts | sed -n '1,240p'Repository: AbdulmalikAlayande/sorokeep
Length of output: 38844
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== daemon commands usage =="
sed -n '1,130p' src/commands/daemon.ts
echo
echo "== entrypoint / program references =="
rg -n 'startDaemon|command\("\s*daemon|run.*daemon|executeCycle|process\.exit|process\.on\("SIG|SIGTERM|SIGINT' src --type ts
echo
echo "== CLI entrypoint =="
sed -n '1,180p' src/cli/program.tsRepository: AbdulmalikAlayande/sorokeep
Length of output: 16440
Check low-balance alert spam before making this a blocking await.
This path calls deliverSingleAlert(...).catch(...) without adding the promise to a collectible set before Promise.all(eligibleTasks.map(...)), so delivery errors are still logged, but the warning should not be framed as silently dropped. The larger actionable concern is that this code does not insert/check an alerts_fired state like the TTL threshold path, so low-balance alerts can fire every extension cycle.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/extension.ts` around lines 487 - 497, Update the low-balance alert
path around deliverSingleAlert to use the existing alerts_fired state and
suppress repeat notifications across extension cycles, matching the TTL
threshold behavior. Preserve error logging for failed deliveries, and ensure the
alert state is checked and recorded consistently before dispatch.
…504, PR #587/#656) Adds MINIMUM_BALANCE_XLM (5 XLM) — before submitting an extension through a channel account, verify it holds enough XLM to cover the base reserve and transaction fee. If the balance is insufficient or unknown, skip submission, log a warning, and fire an alert through the contract's configured channels rather than attempting a transaction that would likely fail or drain the account below reserve. Surfaces low-balance channel accounts in 'sorokeep channels list' with a balance indicator and LOW warning. Ported from PR #656 (a reconciled combination of #587/min-balance and #586/cooldown, both by the same contributor) rather than either individual PR, since #656 was explicitly built to merge conflict-free regardless of order and represented the contributor's own final, tested state — including two real fixes they'd already made (UTC timestamp parsing in the cooldown check, and clamping remainingTTL to zero in the low-balance alert payload). Applied only the min-balance delta here; the cooldown feature (#510) is a separate issue handled on its own.
|
Merged via 3cb8517 on main (ported from PR #656, your reconciled combination branch, since it represented your own final tested state including two real fixes — UTC timestamp parsing and remainingTTL clamping — beyond the original #587). The design was exactly right: pre-flight balance check against the base reserve + fee margin, alert-fire-and-skip instead of attempt-and-fail, low-balance indicator in Only merged the min-balance delta here — the cooldown feature (#510/#586) is a separate issue and will be handled on its own. Thanks for reconciling the two branches so cleanly, that made this much easier to review. |
Summary
Adds a minimum-balance safety check () that verifies channel accounts have sufficient funds to cover transaction fees and base reserves before submitting extension transactions. When the balance is insufficient or unknown, the extension is skipped and alerts are fired through configured alert channels.
The CLI command is also enhanced to display balance information and highlight low-balance accounts.
Closes #504
What Changed
Testing
Checklist