fix(lighthouse): use depth tracking to avoid CPI false positives#180
fix(lighthouse): use depth tracking to avoid CPI false positives#1806figpsolseeker wants to merge 1 commit intodcccrypto:mainfrom
Conversation
isLighthouseFailureInLogs used a boolean insideLighthouse flag that was set to true on any Lighthouse invoke and only cleared on success. When Lighthouse CPIs into a sub-program (e.g. Pyth oracle) and that sub- program fails, the log contains "Program Pyth failed: ..." while insideLighthouse is still true. The old code matched /failed/i on that line and incorrectly returned true — even though Lighthouse itself may succeed on the very next log line. This caused false Lighthouse failure classifications on transactions where a CPI target errored but Lighthouse handled the error gracefully. The fix replaces the boolean with a depth counter (lighthouseDepth). Only explicit "Program <LIGHTHOUSE_ID> failed" lines are now treated as Lighthouse failures. The generic "any failed line while inside Lighthouse" check is removed because it cannot distinguish sub-program failures from Lighthouse's own failures. The depth counter also correctly handles nested re-entrant invocations (unlikely but defensive). All 37 existing lighthouse tests continue to pass — they test direct Lighthouse failure lines (which still match) and non-Lighthouse logs (which still don't match). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughModified Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/runtime/lighthouse.ts (1)
162-173: Add a regression test for “sub-program failed, Lighthouse success”.This exact log shape is the behavior this patch is protecting; a dedicated test case will prevent future regressions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/lighthouse.ts` around lines 162 - 173, Add a regression test that reproduces the log sequence where Lighthouse (module src/runtime/lighthouse.ts) invokes a sub-program which fails (e.g., "Program Pyth failed: ...") but the Lighthouse program later succeeds, to ensure lighthouseDepth tracking prevents misattribution; write a test that simulates or mocks a Lighthouse invocation calling a CPI sub-program, emits logs in the order shown in the comment, asserts that the failure is attributed to the sub-program (not Lighthouse) and that lighthouseDepth increments/decrements correctly (reference the lighthouseDepth tracking logic and any helpers around insideLighthouse or log-attribute functions), and include this test in the relevant test suite so future changes to lighthouseDepth behavior will fail the test if regressed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/runtime/lighthouse.ts`:
- Around line 179-193: The current substring checks on lines containing
LIGHTHOUSE_PROGRAM_ID_STR produce false positives; instead update the logic in
the block that manipulates lighthouseDepth and returns on failure (the checks
using line.includes and the variable LIGHTHOUSE_PROGRAM_ID_STR) to only match
anchored runtime status lines like "Program <id> <status>" — e.g., first verify
the line starts with "Program " (or use a regex anchored at line start) then
parse the program id and status and only increment lighthouseDepth when id ===
LIGHTHOUSE_PROGRAM_ID_STR and status === "invoke", decrement when status ===
"success", and return true only when status === "failed". Ensure you modify the
three checks that currently use includes(...) so they only trigger on these
anchored matches.
---
Nitpick comments:
In `@src/runtime/lighthouse.ts`:
- Around line 162-173: Add a regression test that reproduces the log sequence
where Lighthouse (module src/runtime/lighthouse.ts) invokes a sub-program which
fails (e.g., "Program Pyth failed: ...") but the Lighthouse program later
succeeds, to ensure lighthouseDepth tracking prevents misattribution; write a
test that simulates or mocks a Lighthouse invocation calling a CPI sub-program,
emits logs in the order shown in the comment, asserts that the failure is
attributed to the sub-program (not Lighthouse) and that lighthouseDepth
increments/decrements correctly (reference the lighthouseDepth tracking logic
and any helpers around insideLighthouse or log-attribute functions), and include
this test in the relevant test suite so future changes to lighthouseDepth
behavior will fail the test if regressed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| // Lighthouse invoke — increment depth | ||
| if (line.includes(`Program ${LIGHTHOUSE_PROGRAM_ID_STR} invoke`)) { | ||
| insideLighthouse = true; | ||
| lighthouseDepth++; | ||
| continue; | ||
| } | ||
|
|
||
| // Lighthouse program returned success — reset | ||
| // Lighthouse success — decrement depth | ||
| if (line.includes(`Program ${LIGHTHOUSE_PROGRAM_ID_STR} success`)) { | ||
| insideLighthouse = false; | ||
| lighthouseDepth = Math.max(0, lighthouseDepth - 1); | ||
| continue; | ||
| } | ||
|
|
||
| // Error while inside a Lighthouse invocation | ||
| if (insideLighthouse && /failed/i.test(line)) { | ||
| return true; | ||
| } | ||
|
|
||
| // Explicit Lighthouse failure log | ||
| // Explicit Lighthouse failure (the program ID is in the "failed" line) | ||
| if (line.includes(`Program ${LIGHTHOUSE_PROGRAM_ID_STR} failed`)) { | ||
| return true; |
There was a problem hiding this comment.
Use anchored program-status matching instead of substring matching.
At Line 192 (and similarly Line 180 and Line 186), includes(...) can match "Program log: Program <LIGHTHOUSE_ID> failed" emitted by arbitrary programs, which reintroduces false positives. Match only runtime status lines (Program <id> ...) with anchored checks.
Proposed fix
- if (line.includes(`Program ${LIGHTHOUSE_PROGRAM_ID_STR} invoke`)) {
+ if (line.startsWith(`Program ${LIGHTHOUSE_PROGRAM_ID_STR} invoke`)) {
lighthouseDepth++;
continue;
}
- if (line.includes(`Program ${LIGHTHOUSE_PROGRAM_ID_STR} success`)) {
+ if (line.startsWith(`Program ${LIGHTHOUSE_PROGRAM_ID_STR} success`)) {
lighthouseDepth = Math.max(0, lighthouseDepth - 1);
continue;
}
- if (line.includes(`Program ${LIGHTHOUSE_PROGRAM_ID_STR} failed`)) {
+ if (line.startsWith(`Program ${LIGHTHOUSE_PROGRAM_ID_STR} failed`)) {
return true;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Lighthouse invoke — increment depth | |
| if (line.includes(`Program ${LIGHTHOUSE_PROGRAM_ID_STR} invoke`)) { | |
| insideLighthouse = true; | |
| lighthouseDepth++; | |
| continue; | |
| } | |
| // Lighthouse program returned success — reset | |
| // Lighthouse success — decrement depth | |
| if (line.includes(`Program ${LIGHTHOUSE_PROGRAM_ID_STR} success`)) { | |
| insideLighthouse = false; | |
| lighthouseDepth = Math.max(0, lighthouseDepth - 1); | |
| continue; | |
| } | |
| // Error while inside a Lighthouse invocation | |
| if (insideLighthouse && /failed/i.test(line)) { | |
| return true; | |
| } | |
| // Explicit Lighthouse failure log | |
| // Explicit Lighthouse failure (the program ID is in the "failed" line) | |
| if (line.includes(`Program ${LIGHTHOUSE_PROGRAM_ID_STR} failed`)) { | |
| return true; | |
| // Lighthouse invoke — increment depth | |
| if (line.startsWith(`Program ${LIGHTHOUSE_PROGRAM_ID_STR} invoke`)) { | |
| lighthouseDepth++; | |
| continue; | |
| } | |
| // Lighthouse success — decrement depth | |
| if (line.startsWith(`Program ${LIGHTHOUSE_PROGRAM_ID_STR} success`)) { | |
| lighthouseDepth = Math.max(0, lighthouseDepth - 1); | |
| continue; | |
| } | |
| // Explicit Lighthouse failure (the program ID is in the "failed" line) | |
| if (line.startsWith(`Program ${LIGHTHOUSE_PROGRAM_ID_STR} failed`)) { | |
| return true; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime/lighthouse.ts` around lines 179 - 193, The current substring
checks on lines containing LIGHTHOUSE_PROGRAM_ID_STR produce false positives;
instead update the logic in the block that manipulates lighthouseDepth and
returns on failure (the checks using line.includes and the variable
LIGHTHOUSE_PROGRAM_ID_STR) to only match anchored runtime status lines like
"Program <id> <status>" — e.g., first verify the line starts with "Program " (or
use a regex anchored at line start) then parse the program id and status and
only increment lighthouseDepth when id === LIGHTHOUSE_PROGRAM_ID_STR and status
=== "invoke", decrement when status === "success", and return true only when
status === "failed". Ensure you modify the three checks that currently use
includes(...) so they only trigger on these anchored matches.
Summary
isLighthouseFailureInLogsused a booleaninsideLighthouseflag that was set on any Lighthouse invoke and only cleared on success. When Lighthouse CPIs into a sub-program (e.g. Pyth oracle) and that sub-program fails, the log contains"Program Pyth failed: ..."whileinsideLighthouseis stilltrue. The old code matched/failed/ion that line and incorrectly returnedtrue— even though Lighthouse itself may succeed on the very next line.Example false-positive scenario:
Changes
lighthouseDepthcounter. Only explicit"Program <LIGHTHOUSE_ID> failed"lines are now treated as failures. The generic "any failed line while inside Lighthouse" check is removed.Test plan
npm run lintcleannpx vitest run test/lighthouse.test.ts— all 37 tests passvitest run: same 14 pre-existing failures, zero new failuresSummary by CodeRabbit