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.

Sign up for GitHub

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

Jump to bottom

add missing factories to curve models #6277

Merged
merged 28 commits into from
Jul 17, 2024
Merged

add missing factories to curve models #6277

merged 28 commits into from
Jul 17, 2024

Conversation

viniabussafi
Copy link
Contributor

@viniabussafi viniabussafi commented Jun 27, 2024 •
edited
Loading

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 adding to existing spell lineage

If you are adding to an existing spell lineage, please provide the following information:

  • Description: add missing factory to curve pools #6218 added a missing factory to curvefi_ethereum.view_pools. Now, this PR adds it to curve_ethereum_base_trades, in order to also update dex.trades. It also adds the same factory to optimism to curvefi_optimism_pools

@dune-eng
Copy link

Workflow run id 9693474098 approved.

@dune-eng
Copy link

Workflow run id 9693474367 approved.

@dune-eng
Copy link

Workflow run id 9693474371 approved.

@viniabussafi viniabussafi mentioned this pull request Jun 27, 2024
Closed
@viniabussafi viniabussafi marked this pull request as ready for review June 27, 2024 08:53
@dune-eng
Copy link

Workflow run id 9694317774 approved.

@dune-eng
Copy link

Workflow run id 9694317917 approved.

@dune-eng
Copy link

Workflow run id 9694317921 approved.

@dune-eng
Copy link

Workflow run id 9694377395 approved.

@dune-eng
Copy link

Workflow run id 9694377669 approved.

@dune-eng
Copy link

Workflow run id 9694377675 approved.

@viniabussafi viniabussafi marked this pull request as draft June 27, 2024 09:54
@viniabussafi viniabussafi changed the title add missing factory to curve_ethereum_base_trades add missing factories to curve models Jun 27, 2024
@viniabussafi viniabussafi marked this pull request as ready for review June 27, 2024 10:54
@dune-eng
Copy link

Workflow run id 9696833811 approved.

@dune-eng
Copy link

Workflow run id 9696833955 approved.

@dune-eng
Copy link

Workflow run id 9696833950 approved.

@dune-eng
Copy link

Workflow run id 9697393143 approved.

@dune-eng
Copy link

Workflow run id 9697393422 approved.

@dune-eng
Copy link

Workflow run id 9697393429 approved.

@dune-eng
Copy link

Workflow run id 9697396793 approved.

@dune-eng
Copy link

Workflow run id 9697396659 approved.

@dune-eng
Copy link

Workflow run id 9891901739 approved.

@dune-eng
Copy link

Workflow run id 9892879847 approved.

@dune-eng
Copy link

Workflow run id 9892880186 approved.

@dune-eng
Copy link

Workflow run id 9892880194 approved.

@dune-eng
Copy link

Workflow run id 9893478418 approved.

@dune-eng
Copy link

Workflow run id 9893478714 approved.

@dune-eng
Copy link

Workflow run id 9893478734 approved.

@dune-eng
Copy link

Workflow run id 9893630386 approved.

@dune-eng
Copy link

Workflow run id 9893630882 approved.

@dune-eng
Copy link

Workflow run id 9893630880 approved.

@dune-eng
Copy link

Workflow run id 9893788167 approved.

@dune-eng
Copy link

Workflow run id 9893788363 approved.

@dune-eng
Copy link

Workflow run id 9893788376 approved.

@viniabussafi
Copy link
Contributor Author

viniabussafi commented Jul 11, 2024 •
edited
Loading

your suggestion seems to work @Hosuke! Looks like we're good to go now. Thank you very much once again

@jeff-dude
Copy link
Member

@Hosuke give this one last glance after latest changes, then i can help get it moving along if approved

Hosuke
Hosuke approved these changes Jul 14, 2024
View reviewed changes
Copy link
Collaborator

@Hosuke Hosuke left a comment

Choose a reason for hiding this comment

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

LGTM.✅
Thank you @viniabussafi

dex/models/trades/ethereum/platforms/curvefi_ethereum_base_trades.sql
when l.topic0 != 0x8b3e96f2b889fa771c53c981b40daf005f63f637f1869f707052d15a3dd97140
and cast(bytearray_to_uint256(bytearray_substring(l.data, 65, 32)) as int) + 1 <= CARDINALITY(p.undercoins)
then p.undercoins[cast(bytearray_to_uint256(bytearray_substring(l.data, 65, 32)) as int) + 1]
else NULL
Copy link
Collaborator

Choose a reason for hiding this comment

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

NULL value may be introduced here.

@Hosuke
Copy link
Collaborator

Hosuke commented Jul 14, 2024

Here are records with null token_*_address:
https://dune.com/queries/3916398
All of them (13 in total) with version Factory V1 Stableswap Plain NG.

@viniabussafi
Copy link
Contributor Author

hey, @jeff-dude! can we please move forward with this still this week? would be really helpful to have the trades spell complete for curve

jeff-dude
jeff-dude approved these changes Jul 17, 2024
View reviewed changes
Copy link
Member

@jeff-dude jeff-dude left a comment

Choose a reason for hiding this comment

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

lets do it

@jeff-dude jeff-dude merged commit 9ae7cca into duneanalytics:main Jul 17, 2024
3 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 17, 2024
@viniabussafi viniabussafi deleted the crv2 branch September 17, 2024 19:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Hosuke Hosuke Hosuke approved these changes

@jeff-dude jeff-dude jeff-dude approved these changes

Assignees

@Hosuke Hosuke

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@viniabussafi @dune-eng @jeff-dude @Hosuke