-
Notifications
You must be signed in to change notification settings - Fork 1.4k
tokens.transfers_from_traces fix duplicates issue in deposit
#9123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
tokens.transfers_from_traces fix duplicates issue in deposit
#9123
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a duplicate records issue in tokens.transfers_from_traces for deposit transactions caused by inconsistencies between dune.blockchains and tokens.erc20 tables. The primary issue stems from the null address being used as a native token address in both tables, creating unwanted duplicates when joined.
Key changes:
- Removed
prod_excludetag from hyperevm transfers model - Added filter condition to exclude cases where
contract_addressequalstoaddress in deposit transfers
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tokens_hyperevm_transfers_from_traces.sql | Removed prod_exclude tag to enable production deployment |
| transfers_from_traces_base_wrapper_deposits_macro.sql | Added deduplication logic to filter out duplicate deposits caused by null address inconsistency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...ts/tokens/macros/transfers_from_traces/transfers_from_traces_base_wrapper_deposits_macro.sql
Show resolved
Hide resolved
PR SummaryExclude wrapper-deposit rows where
Written by Cursor Bugbot for commit 1f0700e. Configure here. |
jeff-dude
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i hear you on the 0x000.. usage. at this time, that is pretty entrenched in the data and use cases for queries on the app/api/etc so it is difficult to just "switch" to 0xeee... address. this will need to be a bigger org wide discussion. if you wanted to flag that, feel free in the shared slack for support to consider.
as for this change, thank you for quick fix. here is quick validation on ethereum, no results as expected.
with ci as (
select *
from test_schema.git_dunesql_094b851_tokens_ethereum_transfers_from_traces_base_wrapper_deposits
)
, prod as (
select *
from tokens_ethereum.transfers_from_traces_base
)
, unioned as (
select * from ci
union all
select * from prod
)
select block_date, unique_key, count(1)
from unioned
group by block_date, unique_key
having count(1) > 1
it makes sense that full history ran here, as it's scoped down to just the wrapper models rather than entire lineage.
|
wait, prior to merge (which will likely be early next week), can you also push a commit to remove this workaround? |
|
canceled CI, as it will now likely fail and we don't need to waste the compute for that. |
Solving the issue 9122.
The issue due to inconsistency in
dune.blockchains&tokens.erc20.The duplicates issue is due to using a
nulladdress as anativeone indune.blockchains.Some context here
tokens.erc20contains addresses that are not contracts.The
nulladdress has been added to this table as a syntheticnativetoken.success run even without the easy dates
cc @jeff-dude