Skip to content

feat(dex): wire 1inch-LOP aggregator trades into dex_aggregator.trades (CUR2-2693, 2/2) - #9759

Merged
tomfutago merged 3 commits into
mainfrom
tal/cur2-2693-reland-1inch-lop-agg-wiring
Jun 12, 2026
Merged

feat(dex): wire 1inch-LOP aggregator trades into dex_aggregator.trades (CUR2-2693, 2/2)#9759
tomfutago merged 3 commits into
mainfrom
tal/cur2-2693-reland-1inch-lop-agg-wiring

Conversation

@thevaizman

Copy link
Copy Markdown
Contributor

Part 2 of 2 re-landing #9747 (CUR2-2693). Stacked on #9758 — merge only after it.

One line: adds ref('oneinch_lop_aggregator_trades') to the as_is_models list, so venue-settled 1inch-LOP fills (excluded from dex.trades by #9758) get their intent-layer record in dex_aggregator.trades — exactly how oneinch_ar_trades rows are served today.

Why this can't reproduce the #9752 revert

The revert's failure mode was generic erc20-decimals enrichment applied to lineage raw amounts. The as_is path skips enrichment entirely: rows carry 1inch's own enriched amounts and guarded amount_usd. Full-history validation across all 13 chains: 0 rows with amount_usd > $1B, 0 negative, max $38.3M (top-5 all plausible large fills). Additionally, this PR's own CI rebuilds dex_aggregator_trades with these rows and runs the exact dbt_utils_accepted_range test that failed in prod — pre-merge proof, not post-deploy discovery.

CI expectations

While #9758 is unmerged, this PR's CI builds the full stack (CI compares against the prod S3 manifest, not the base branch): venue_settled (~9 min) + views + dex_aggregator_trades full build (28.6–75 min observed variance for the same build) + tests. After #9758 merges and this branch rebases, the diff collapses to one line and CI builds only dex_aggregator_trades + its tests.

Post-merge

Full refresh of dex_aggregator_trades (backfills historical '1inch-LOP' rows), then re-run the audit query — the full-history gate says it passes. Note open PRs #9690/#9357 also touch this file (trivial append rebase for whoever lands second).

🤖 Generated with Claude Code

thevaizman and others added 3 commits June 11, 2026 21:11
…gregator wiring

Re-land of #9747 (reverted in #9752) — part 1 of 2.

The revert's root cause was the base-tier wiring: generic
enrich_dex_aggregator_trades divides raw amounts by tokens.erc20.decimals
and recomputes USD, but this lineage's raw amounts are not erc20-consistent
(executed-transfer amounts can be denominated in a different token —
the $48T audit rows; and the complement fill amounts are order-level
mis-scaled on some fills — a $78 USDT/SFUND fill implying $235M, verified
live). Neither raw source survives generic enrichment, so the view now
serves fully-enriched values incl. 1inch's own guarded amount_usd, exactly
like oneinch_ar_trades (full-history audit gate: 0 rows > $1B on all 13
chains, max $38.3M).

This PR intentionally leaves the view UNWIRED (dex_aggregator_base_trades
restored byte-identical to main; dex_aggregator_trades untouched) so the
dex.trades dedup — the actual CUR2-2693 bug — lands without rebuilding the
739M-row table; the one-line as_is hookup follows in part 2.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s (CUR2-2693, 2/2)

One-line as_is hookup of oneinch_lop_aggregator_trades (introduced unwired
in part 1). The view serves 1inch's own enriched values incl. its guarded
amount_usd — full-history audit gate: 0 rows > $1B on all 13 chains, max
$38.3M — so the accepted_range failure that reverted #9747 cannot recur via
generic enrichment (this PR's CI rebuilds dex_aggregator_trades with these
rows and runs that exact test pre-merge).

Stacked on #9758; merge only after it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Jun 11, 2026

Copy link
Copy Markdown

PR Summary

High Risk
Changes core DEX trade attribution and incremental merge behavior across dex.trades and dex_aggregator.trades; misclassification would affect volume/USD metrics at scale.

Overview
Fixes double-counting for 1inch limit-order fills that a resolver settles on an underlying DEX in the same transaction: the venue row stays in dex.trades, while the intent-layer fill moves to dex_aggregator.trades.

A shared oneinch_lop_evt_index() macro assigns evt_index over all limits fills before exclusions, so merge keys stay stable when rows are reclassified (merges upsert only). oneinch_lop_own_trades drops venue-settled fills via oneinch_lop_venue_settled_fills, applies a 6-hour maturity delay so venue decoded events land first, and adds a seed check. New oneinch_lop_venue_settled_fills (incremental) flags fills that share a tx with dex_*_base_trades, with keep-guards for nested classic/fusion/cross-chain and direct fills. oneinch_lop_aggregator_trades exposes those fills for aggregators (as-is amounts, like oneinch_ar_trades). dex_aggregator_trades unions ref('oneinch_lop_aggregator_trades') into as_is_models, skipping generic ERC20 enrichment. Seeds and a regression test guard the split.

Reviewed by Cursor Bugbot for commit e2bec45. Configure here.

@github-actions
github-actions Bot marked this pull request as draft June 11, 2026 18:22
@github-actions github-actions Bot added WIP work in progress dbt: dex covers the DEX dbt subproject labels Jun 11, 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 2 potential issues.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Six-hour merge race double-count
    • Delayed oneinch_lop_own_trades until fills age out of the venue classifier window and extended the passthrough incremental overlap so newly mature rows still merge once.

Create PR

Or push these changes by commenting:

@cursor push 3659104cd6
Preview (3659104cd6)
diff --git a/dbt_subprojects/dex/macros/models/oneinch_lop_dex_trades_passthrough.sql b/dbt_subprojects/dex/macros/models/oneinch_lop_dex_trades_passthrough.sql
--- a/dbt_subprojects/dex/macros/models/oneinch_lop_dex_trades_passthrough.sql
+++ b/dbt_subprojects/dex/macros/models/oneinch_lop_dex_trades_passthrough.sql
@@ -46,7 +46,9 @@
 	AND o.block_date > current_date - interval '3' day -- dev_dates mode for dev, to prevent full scan
 {%- else -%}
 	{% if is_incremental() %}
-	AND {{ incremental_predicate('o.block_time') }}
+	-- oneinch_lop_own_trades only emits after classifier finality; keep a one-unit
+	-- overlap so newly mature rows are still picked up by dex_<blockchain>_trades.
+	AND {{ incremental_predicate("o.block_time + interval '1' " ~ var('DBT_ENV_INCREMENTAL_TIME_UNIT')) }}
 	{% endif %}
 {%- endif %}
 

diff --git a/dbt_subprojects/dex/models/_projects/oneinch/outside/oneinch_lop_own_trades.sql b/dbt_subprojects/dex/models/_projects/oneinch/outside/oneinch_lop_own_trades.sql
--- a/dbt_subprojects/dex/models/_projects/oneinch/outside/oneinch_lop_own_trades.sql
+++ b/dbt_subprojects/dex/models/_projects/oneinch/outside/oneinch_lop_own_trades.sql
@@ -10,7 +10,6 @@
 {% set src_symbol = "coalesce(src_executed_symbol, '')" %}
 {% set dst_symbol = "coalesce(dst_executed_symbol, '')" %}
 {% set placeholder_tokens = oneinch_cross_chain_placeholder_tokens_cfg_macro() | join(', ') %}
-{% set maturity_delay = "interval '6' hour" %}
 
 
 
@@ -70,6 +69,6 @@
     )
     -- maturity delay: the 1inch lineage builds from raw traces while venue base trades
     -- build from decoded events; a fill merged into dex_<blockchain>_trades before its
-    -- venue's event decodes would never be removed (merge can't delete), so fills only
-    -- pass through once old enough for the venue side to have landed
-    and f.block_time <= now() - {{ maturity_delay }}
+    -- venue classification finalizes would never be removed (merge can't delete), so
+    -- fills only pass through once they age out of the classifier's incremental window
+    and not ({{ incremental_predicate('f.block_time') }})

You can send follow-ups to the cloud agent here.

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

Reviewed by Cursor Bugbot for commit e2bec45. Configure here.

@thevaizman
thevaizman marked this pull request as ready for review June 12, 2026 08:16
@github-actions github-actions Bot added ready-for-review this PR development is complete, please review and removed WIP work in progress labels Jun 12, 2026

@tomfutago tomfutago left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Regression Report: PR #9759

Overall status: PASS / no blocker

Scope: targeted regression for dex_aggregator_trades, focused on the new 1inch-LOP as-is hookup and the prior amount_usd accepted-range failure.

Accepted Range Check

dbt_utils_accepted_range_dex_aggregator_trades_amount_usd__1000000000 passed in CI.

Independent Dune scan of CI relation dune.dune_spellbook_ci__tmp_pr9759_27368340570_1.dex_aggregator_trades:

  • Rows with amount_usd > 1B: 0
  • Rows with negative amount_usd: 0
  • Max amount_usd overall: $714.17M
  • 1inch-LOP rows with amount_usd > 1B: 0
  • 1inch-LOP max amount_usd: $38.35M

1inch-LOP Regression

Relations checked:

  • Source: dune.dune_spellbook_ci__tmp_pr9759_27368340570_1.oneinch_lop_aggregator_trades
  • Final: dune.dune_spellbook_ci__tmp_pr9759_27368340570_1.dex_aggregator_trades
  • Prod: dex_aggregator.trades

Results:

  • CI source 1inch-LOP rows: 3,881,477
  • CI final 1inch-LOP rows: 3,881,477
  • Prod 1inch-LOP rows: 0 as expected pre-merge
  • Source-only keys: 0
  • Final-only keys: 0
  • Source duplicate keys: 0
  • Final duplicate keys: 0

Note

There are 191 matched rows on 2026-06-11 where the live source view now has amount_usd populated but the materialized CI final table has NULL, totaling $380,378 of source-vs-CI drift. Since keys match exactly and the drift is confined to fresh rows after the CI snapshot, this looks like upstream/view enrichment drift rather than a PR regression.

Conclusion: safe to proceed from regression perspective.

@tomfutago tomfutago added ready-for-merging and removed ready-for-review this PR development is complete, please review labels Jun 12, 2026
@tomfutago
tomfutago merged commit 0a4377c into main Jun 12, 2026
14 checks passed
@tomfutago
tomfutago deleted the tal/cur2-2693-reland-1inch-lop-agg-wiring branch June 12, 2026 09:21
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 12, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

dbt: dex covers the DEX dbt subproject ready-for-merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants