Skip to content

fix(discovery): use actual slab size in memcmp fallback, not hardcoded V12_1 large#183

Open
6figpsolseeker wants to merge 1 commit intodcccrypto:mainfrom
6figpsolseeker:fix/discovery-memcmp-fallback-actual-size
Open

fix(discovery): use actual slab size in memcmp fallback, not hardcoded V12_1 large#183
6figpsolseeker wants to merge 1 commit intodcccrypto:mainfrom
6figpsolseeker:fix/discovery-memcmp-fallback-actual-size

Conversation

@6figpsolseeker
Copy link
Copy Markdown

@6figpsolseeker 6figpsolseeker commented Apr 9, 2026

Summary

Both memcmp fallback paths in discoverMarkets hardcoded maxAccounts: 4096 and dataSize: SLAB_TIERS.large.dataSize for all returned accounts. This caused detectSlabLayout to fail for any slab that wasn't V12_1 large, silently dropping valid markets with an "unrecognized layout" warning.

The root cause: memcmp queries used dataSlice (1940 bytes), making data.length always 1940. The fake dataSize hint then fed a V12_1-large size to detectSlabLayout, which only matches actual V12_1-large slabs.

Changes

Both paths now use detectSlabLayout(data.length, data) to derive maxAccounts and dataSize from the actual slab data.

Trade-off

Higher bandwidth for the memcmp path (full slab data vs 1940-byte slice). Acceptable because this is already a last-resort fallback (tiers 1 and 2 both failed), and correct layout detection is worth the extra bytes.

Test plan

  • npm run lint clean
  • Full vitest run: same 14 pre-existing failures, zero new failures
  • Recommended: add test covering memcmp fallback with non-large slab sizes

Summary by CodeRabbit

  • Bug Fixes
    • Improved market discovery fallback logic when initial queries return no results. The system now fetches complete market data and dynamically calculates account sizing for better accuracy, replacing previously hardcoded size assumptions.

…d V12_1 large

Both memcmp fallback paths in discoverMarkets (empty-result fallback at
line 686 and error-catch fallback at line 712) hardcoded maxAccounts=4096
and dataSize=SLAB_TIERS.large.dataSize for all returned accounts. This
caused detectSlabLayout to fail for any slab that wasn't V12_1 large
(the vast majority of slabs), silently dropping them with an
"unrecognized layout" warning.

The root cause was that the memcmp queries used dataSlice to fetch only
1940 bytes, making data.length always 1940 regardless of actual slab
size. With the fake dataSize hint, detectSlabLayout looked for a layout
matching V12_1 large — which only matches actual V12_1 large slabs.

The fix removes dataSlice from both memcmp queries so the full account
data is returned. This makes data.length equal to the actual slab size,
which is then passed to detectSlabLayout for correct layout detection.
maxAccounts is derived from the detected layout instead of hardcoded.

The trade-off is higher bandwidth for the memcmp path (full slab data
vs 1940-byte slice). This is acceptable because:
  1. The memcmp path is already a last-resort fallback (tier 1 dataSize
     filters and tier 2 REST API both failed)
  2. The accounts already passed the PERCOLAT magic bytes filter
  3. Correct layout detection is worth the extra bandwidth — the
     alternative is silently dropping valid markets

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 9, 2026

📝 Walkthrough

Walkthrough

The fallback logic in discoverMarkets now fetches full slab accounts and dynamically detects their layout per account using detectSlabLayout, replacing hardcoded large-tier sizing. The resulting entries derive maxAccounts from the detected layout and set dataSize to the computed per-account size, applied consistently across both fallback paths.

Changes

Cohort / File(s) Summary
Discovery Fallback Refactor
src/solana/discovery.ts
Modified tiered fallback and memcmp fallback paths to fetch full slab accounts (no dataSlice), compute per-account actualSize from account.data.length, call detectSlabLayout(actualSize, data) for each account, and derive RawEntry objects using layout?.maxAccounts (default 4096) and dataSize=actualSize, removing hardcoded large-tier sizing.

Sequence Diagram

sequenceDiagram
    participant discoverMarkets
    participant RPC as RPC / getProgramAccounts
    participant detectLayout as detectSlabLayout
    participant results as RawEntry Results

    discoverMarkets->>RPC: Tiered queries with dataSlice
    RPC-->>discoverMarkets: Zero results
    
    discoverMarkets->>RPC: Fetch full slab accounts (no slice)
    RPC-->>discoverMarkets: Full account data

    loop For each account
        discoverMarkets->>discoverMarkets: Compute actualSize from account.data.length
        discoverMarkets->>detectLayout: detectSlabLayout(actualSize, data)
        detectLayout-->>discoverMarkets: Detected layout with maxAccounts
        discoverMarkets->>results: Build RawEntry with layout.maxAccounts & actualSize
    end
    
    results-->>discoverMarkets: Fallback discovery results
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 A rabbit hops through slabs so deep,
No more hardcoded tiers to keep,
Each account reveals its shape,
With layouts drawn from data's drape, 🔍
The fallback bounds now fit just right!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main fix: using actual slab size in memcmp fallback instead of hardcoded large-tier values, which aligns with the core problem and solution described in the PR objectives.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/solana/discovery.ts (1)

728-736: LGTM — consistent fix applied to the error-catch fallback path.

The same pattern is correctly applied here, ensuring both fallback paths behave identically.

Minor note: the mapping logic on lines 702-710 and 728-736 is now duplicated. Consider extracting to a small helper function for DRY, though this is optional given the limited scope.

,

♻️ Optional: Extract shared mapping logic
// Helper near the top of discoverMarkets or as a module-level function
function toRawEntries(
  accounts: Awaited<ReturnType<Connection["getProgramAccounts"]>>,
): RawEntry[] {
  return accounts.map(e => {
    const actualSize = e.account.data.length;
    const layout = detectSlabLayout(actualSize, new Uint8Array(e.account.data));
    return {
      ...e,
      maxAccounts: layout?.maxAccounts ?? 4096,
      dataSize: actualSize,
    };
  });
}

Then both fallback paths become:

rawAccounts = toRawEntries(fallback);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/solana/discovery.ts` around lines 728 - 736, The mapping that converts
getProgramAccounts results into RawEntry objects is duplicated (lines mapping to
rawAccounts in discoverMarkets); extract that logic into a helper (e.g.,
toRawEntries) that accepts the accounts array, computes actualSize, calls
detectSlabLayout(new Uint8Array(...)), sets maxAccounts = layout?.maxAccounts ??
4096 and dataSize = actualSize, and returns RawEntry[]; then replace both inline
map usages with rawAccounts = toRawEntries(fallback) (and the other occurrence)
to remove duplication while preserving behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/solana/discovery.ts`:
- Around line 728-736: The mapping that converts getProgramAccounts results into
RawEntry objects is duplicated (lines mapping to rawAccounts in
discoverMarkets); extract that logic into a helper (e.g., toRawEntries) that
accepts the accounts array, computes actualSize, calls detectSlabLayout(new
Uint8Array(...)), sets maxAccounts = layout?.maxAccounts ?? 4096 and dataSize =
actualSize, and returns RawEntry[]; then replace both inline map usages with
rawAccounts = toRawEntries(fallback) (and the other occurrence) to remove
duplication while preserving behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c44929be-488a-4dd1-81d2-c6f760a9ac91

📥 Commits

Reviewing files that changed from the base of the PR and between 2f80d79 and fda3e6a.

📒 Files selected for processing (1)
  • src/solana/discovery.ts

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.

1 participant