Skip to content
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

Mass decoding non dynamic #6630

Open
wants to merge 97 commits into
base: main
Choose a base branch
from
Open

Mass decoding non dynamic #6630

wants to merge 97 commits into from

Conversation

0xBoxer
Copy link
Collaborator

@0xBoxer 0xBoxer commented Aug 29, 2024

scaleable evm decode function usage

@0xBoxer 0xBoxer added WIP work in progress dune team created by dune team labels Aug 30, 2024
@0xBoxer
Copy link
Collaborator Author

0xBoxer commented Sep 24, 2024

Hey @Hosuke

I just talked to @jeff-dude and he showed me the amount_usd issues that arises within a few trades.

this query in particular: https://dune.com/queries/4093790

The problem here lies within the fact that swap events can be arbitrarily and maliciously emitted by smart contracts, without any connection to reality.

The culprit here is not 1inch, but rather the factory, which somehow also is the Safe token.

The trade it is emitting what looks like a WETH-XXX trade because the factory event was already wrong/didn't make sense.

A way for us to correct this is to simply check if there is corresponding transfer of token_bought or token_sold.

@Hosuke
Copy link
Collaborator

Hosuke commented Sep 26, 2024

dex_automation_beta.trades preview There are 358950146(99.6%) records matched with the existing records in dex.trades, and the rest are trades from newly added uncovered dexes.

I think we can have some initial review feedbacks.

The new implementation has:
uni-v2 ethereum trades: 424567703
uni-v3 ethereum trades: 124873690

@Hosuke
Copy link
Collaborator

Hosuke commented Sep 29, 2024

Now back to incremental error:
https://github.com/duneanalytics/spellbook/actions/runs/11092757311/job/30818194907?pr=6630#step:14:49

This may be an unclear problem.

@jeff-dude
Copy link
Member

good progress here!

brainstorming what we still need to do, not in any particular order:

  • run tests again of data built in CI here vs. prod dex.trades -- ensure data matches and/or is built as expected, if not exists in prod version (how do we test new DEXs added? can we find a simple, repeatable query to run?)
  • naming standards for each spell within the new pipeline, updating file names to match
  • macro naming standards for the pipeline, directory structure of where they live, file names of macros, which ones make sense to live in same file
  • determine if we stick to just uniswap v2 ethereum for this PR, or open scope to uniswap v2 and v3, both ethereum and arbitrum
  • how to name project and version columns when we aren't sure -- do we left join to evms.contracts and look for match? default to unknown if not match?

...maybe more, stopping for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dune team created by dune team in review Assignee is currently reviewing the PR WIP work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants