Skip to content

refactor(dex): split 1inch LOP dedup models per chain to isolate blast radius - #9905

Open
jeff-dude wants to merge 2 commits into
mainfrom
refactor/oneinch-lop-per-chain
Open

refactor(dex): split 1inch LOP dedup models per chain to isolate blast radius#9905
jeff-dude wants to merge 2 commits into
mainfrom
refactor/oneinch-lop-per-chain

Conversation

@jeff-dude

Copy link
Copy Markdown
Member

Thank you for contributing to Spellbook 🪄

Please open the PR in draft and mark as ready when you want to request a review.

Description:

What & why

The 1inch LOP venue-settled dedup was one cross-chain model reading every chain's dex_<chain>_base_trades and feeding back into every chain's dex_<chain>_trades, so touching one chain's base trades rebuilt all EVM chains' dex + dex_aggregator lineage. The dedup join is strictly per-chain, so the logic now lives in per-chain models and each dex_<chain>_trades refs only its own chain's model. Adding a DEX to one chain now rebuilds only that chain's stack plus the cross-chain final unions. Output data is unchanged; oneinch.lop_own_trades stays as a cross-chain union view.

Architecture

  • macros/models/oneinch_lop_dex_trades_passthrough.sql - the decoupling edge: refs oneinch_<chain>_lop_own_trades instead of the cross-chain view; macro change triggers a one-time rebuild of all dex_<chain>_trades on this PR
  • macros/.../LO/oneinch_lop_venue_settled_fills_macro.sql, oneinch_lop_own_trades_macro.sql - former model bodies parameterized by chain; venue co-occurrence now scans only that chain's base trades
  • outside/oneinch_lop_own_trades.sql - now a union view of the 13 per-chain views (user-facing name, tests, and seed check preserved)
  • outside/oneinch_lop_aggregator_trades.sql - unions the per-chain fills inline; feeds dex_aggregator.trades as before
  • outside/oneinch_lop_venue_settled_fills.sql - deleted; prod table oneinch.lop_venue_settled_fills needs a manual drop after merge
  • +29 files: 26 generated per-chain models (13 chains × fills incremental + own-trades view), schema entries, LINEAGE.md
  • Tested: full compile of all new/changed models; dbt ls dex_ethereum_base_trades+ confirms no other chain's trades/MEV models remain downstream. Not tested: prod data parity (views are definitionally equivalent; fills tables rebuild from full history on first run)
  • Needs human eyes: first prod build of the 13 fills tables is full-history (heavy, per-chain parallel); evt_index numbering is now per-chain, which removes the documented ~0-probability cross-chain tx_hash collision quirk but would shift merge keys for any such historical row
  • Blast radius: dex.trades / dex_aggregator.trades consumers see identical data; CI/deploy selection scope shrinks for future per-chain DEX additions (checked via dbt lineage)
Agent context
  • No agent review yet
  • Repro fan-out check: uv run dbt ls --select dex_ethereum_base_trades+ --project-dir dbt_subprojects/dex/ --resource-type model — no dex_<other_chain>_trades in output
  • Prod materiality baseline: oneinch.lop_venue_settled_fills = ~4.0M rows / 13 chains (2021-06-11 → 2026-07-27), ~270k rows in last 90d; dex.trades holds ~7.0M 1inch-LOP rows
  • Chains split: ethereum, bnb, polygon, arbitrum, optimism, avalanche_c, gnosis, base, fantom, zksync, linea, sonic, unichain (all with lo in exposed; solana excluded as before)

quick links for more information:

🤖 Generated with Claude Code

…t radius

The venue-settled fills dedup (oneinch_lop_venue_settled_fills ->
oneinch_lop_own_trades) was a single cross-chain node that read every
dex_<blockchain>_base_trades and fed back into every dex_<blockchain>_trades,
so any change to one chain's base trades rebuilt all chains' dex +
dex_aggregator lineage.

The venue co-occurrence join is strictly per-chain, so the logic now lives in
per-chain models under blockchains/<chain>/ (parameterized macros, following
the existing oneinch per-chain pattern), and oneinch_lop_dex_trades_passthrough
refs the calling chain's model only. Adding a DEX on one chain now rebuilds
only that chain's stack plus the cross-chain final unions.

oneinch.lop_own_trades stays as a cross-chain union view for users;
oneinch_lop_aggregator_trades unions the per-chain fills inline. The old
oneinch.lop_venue_settled_fills model is removed (its prod table needs a
manual drop).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added WIP work in progress dbt: dex covers the DEX dbt subproject labels Jul 27, 2026
@jeff-dude

jeff-dude commented Jul 27, 2026

Copy link
Copy Markdown
Member Author

Regression: CI vs prod — passed

All comparisons run against dune.dune_spellbook_ci__tmp_pr9905_30298442119_1 with the unsettled tail excluded (block_date < current_date - interval '2' day). Expectation was exact match (structure-only refactor), and that's what we got:

Comparison Window Rows compared Diff
13 per-chain fills (union) vs prod oneinch.lop_venue_settled_fills — full outer join on (blockchain, block_month, block_date, execution_id), incl. evt_index equality full history (2021-06 →) ~4.0M 0
CI oneinch.lop_own_trades view vs prod — join on (blockchain, tx_hash, evt_index) + amount_usd last 30d 146,421 0
CI oneinch.lop_aggregator_trades view vs prod — join incl. trace_address + amount_usd last 30d 70,613 0
All 13 CI dex_<chain>_trades vs prod, project = '1inch-LOP' — row presence + token amounts + amount_usd CI build range ~4.9M 0

Notes:

  • evt_index renumbering is a non-issue empirically: the full-history fills comparison found zero evt_index shifts, so no cross-chain tx_hash collisions exist in practice.
  • The only mismatches ever observed were ~20 rows inside the excluded 3-day tail: identical amount_usd, token_bought_amount NULL on one side — prod rows merged before token decimals landed in oneinch.swaps.complement, CI recomputed from current swaps. Timing artifact, converges via prod's own incremental window, unrelated to this refactor.
  • CI built the 13 fills tables full-history (2021-06 → today), so the comparison covers the entire lifetime of the dedup, not a truncated window.

Merge plan

  1. Merge, then run the override with state:modified only — no downstream (+). No data changes, so downstream (dex.trades, MEV models, dex_aggregator.trades) doesn't need a rebuild.
  2. Note state:modified inherently includes the 13 dex_<chain>_trades via the changed passthrough macro — intended: their incremental runs validate the new per-chain refs in prod and are no-op merges (history proven identical above).
  3. DAG order builds the per-chain fills/views before the chain trades, so there's no window with dangling refs.
  4. The heavy step is the first build of the 13 fills tables (full history, no incremental window on first run; ethereum/bnb/base largest). CI just completed the identical build successfully.

🤖 Generated with Claude Code

@jeff-dude
jeff-dude marked this pull request as ready for review July 27, 2026 21:10
@cursor

cursor Bot commented Jul 27, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches core dex.trades / dex_aggregator.trades routing for ~7M 1inch-LOP rows; first prod build of 13 incremental fills tables is full-history heavy, and evt_index is now per-chain (possible merge-key shift for rare cross-chain tx_hash edge cases).

Overview
Splits 1inch LOP venue-settled dedup and dex.trades inputs by chain so updates to one chain’s dex_<chain>_base_trades no longer rebuild every EVM chain’s dex_<chain>_trades lineage.

The former monolithic oneinch_lop_venue_settled_fills and inline oneinch_lop_own_trades logic move into oneinch_lop_venue_settled_fills_macro and oneinch_lop_own_trades_macro, backed by 13 per-chain incremental lop_venue_settled_fills models (each joins only that chain’s dex_<chain>_base_trades) and per-chain lop_own_trades views. oneinch_lop_dex_trades_passthrough now refs oneinch_<chain>_lop_own_trades instead of the cross-chain view.

oneinch.lop_own_trades becomes a union of per-chain views (same external contract/tests). oneinch_lop_aggregator_trades unions per-chain venue-settled fills. The global oneinch.lop_venue_settled_fills model is removed (prod table drop expected post-merge). Schema and LINEAGE.md document the new per-chain layer; per-chain fills drop blockchain from the unique key where chain is implicit.

Reviewed by Cursor Bugbot for commit c816675. Configure here.

@github-actions github-actions Bot added ready-for-review this PR development is complete, please review and removed WIP work in progress labels Jul 27, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit c816675. Configure here.

hide_spells() is reserved for clearing properties on already-materialized
models; net-new models omit it, matching the sibling per-chain oneinch models.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dbt: dex covers the DEX dbt subproject ready-for-review this PR development is complete, please review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant