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

fix 1inch trade pair #6236

Closed
wants to merge 2 commits into from

Conversation

mendesfabio
Copy link
Contributor

Thank you for contributing to Spellbook 🪄

Contribution type

Please check the type of contribution this pull request is for:

  • New spell(s)
  • Adding to existing spell lineage
  • Bug fix

Note: You can safely discard any section below which doesn't apply based on selection above


For bug fixes

If you are fixing a bug, please provide the following information:

  • Description: 1inch LOP trades return trade_pair with symbols incorrectly sorted.
  • Implementation details: Implemented the same logic as in dex.trades spells.

Thank you for your contribution!

@dune-eng
Copy link

Workflow run id 9618842469 approved.

@dune-eng
Copy link

Workflow run id 9618842880 approved.

@dune-eng
Copy link

Workflow run id 9618842865 approved.

@@ -19,7 +19,10 @@ select
, block_number
, coalesce(dst_token_symbol, '') as token_bought_symbol
, coalesce(src_token_symbol, '') as token_sold_symbol
, array_join(array_sort(array[coalesce(src_token_symbol, ''), coalesce(dst_token_symbol, '')]), '-') as token_pair
, case
Copy link
Contributor

Choose a reason for hiding this comment

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

@grkhr does this look good to you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as mentioned in the PR I just applied the same logic as in DEX trades. It seems the current logic doesn't work well with mixed-case token symbols - see example: https://dune.com/queries/3864620

@grkhr
Copy link
Contributor

grkhr commented Jul 1, 2024

will be fixed here with other spells #6285
so current pr can be closed

@aalan3 aalan3 closed this Jul 2, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jul 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants