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

Dex Aggregator Migration - #2 - Yield yak #4063

Merged
merged 3 commits into from
Aug 22, 2023

Conversation

henrystats
Copy link
Contributor

Thank you for contributing to Spellbook!

Please refer to the top of the readme in the root of Spellbook to learn how to contribute to Spellbook on DuneSQL.

@dune-eng
Copy link

Workflow run id 5843934042 approved.

@dune-eng
Copy link

Workflow run id 5843934097 approved.

@henrystats henrystats changed the title Dex Aggregator Migration - #2 - Yield yak Dex Aggregator Migration - #2 - Yield yak - ready for review Aug 12, 2023
@antonio-mendes antonio-mendes self-requested a review August 17, 2023 13:26
@antonio-mendes antonio-mendes added dex ready-for-review this PR development is complete, please review labels Aug 17, 2023
@antonio-mendes antonio-mendes changed the title Dex Aggregator Migration - #2 - Yield yak - ready for review Dex Aggregator Migration - #2 - Yield yak Aug 17, 2023
Copy link
Contributor

@antonio-mendes antonio-mendes left a comment

Choose a reason for hiding this comment

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

Thanks for the work here @henrystats ! I've left a few notes on updates to keep things neat

CAST(dexs.token_bought_amount_raw AS DECIMAL(38,0)) AS token_bought_amount_raw,
CAST(dexs.token_sold_amount_raw AS DECIMAL(38,0)) AS token_sold_amount_raw,
CAST(dexs.token_bought_amount_raw AS UINT256) AS token_bought_amount_raw,
CAST(dexs.token_sold_amount_raw AS UINT256) AS token_sold_amount_raw,
Copy link
Contributor

Choose a reason for hiding this comment

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

These fields are already UINT256 in the decoded table so no need to cast

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oke ser... will remove cast.

dexs.trace_address,
dexs.evt_index
FROM dexs
INNER JOIN {{ source('avalanche_c', 'transactions') }} tx
ON tx.hash = dexs.tx_hash
{% if not is_incremental() %}
AND tx.block_time >= '{{project_start_date}}'
AND tx.block_time >= DATE '{{project_start_date}}'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might work as is but we should swap DATE here for TIMESTAMP to match the block_time data type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oke ser

@@ -89,19 +91,18 @@ LEFT JOIN {{ source('prices', 'usd') }} p_bought
AND p_bought.contract_address = dexs.token_bought_address
AND p_bought.blockchain = 'avalanche_c'
{% if not is_incremental() %}
AND p_bought.minute >= '{{project_start_date}}'
AND p_bought.minute >= DATE '{{project_start_date}}'
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as before

{% endif %}
LEFT JOIN {{ source('prices', 'usd') }} p_sold
ON p_sold.minute = date_trunc('minute', dexs.block_time)
AND p_sold.contract_address = dexs.token_sold_address
AND p_sold.blockchain = 'avalanche_c'
{% if not is_incremental() %}
AND p_sold.minute >= '{{project_start_date}}'
AND p_sold.minute >= DATE '{{project_start_date}}'
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as before

tx.from as tx_from,
tx.to AS tx_to,
tx."from" as tx_from,
tx."to" AS tx_to,
Copy link
Contributor

Choose a reason for hiding this comment

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

Here you don't need the "" since to isn't a special keyword

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oke ser.

@henrystats
Copy link
Contributor Author

Thank you ser, I'll make these changes across all the other pr's as well!

@dune-eng
Copy link

Workflow run id 5906189792 approved.

@dune-eng
Copy link

Workflow run id 5906189599 approved.

@dune-eng
Copy link

Workflow run id 5906192581 approved.

@dune-eng
Copy link

Workflow run id 5906192603 approved.

@henrystats
Copy link
Contributor Author

already made the changes here too ser @jeff-dude

@jeff-dude jeff-dude self-assigned this Aug 21, 2023
@jeff-dude jeff-dude added ready-for-merging and removed ready-for-review this PR development is complete, please review labels Aug 21, 2023
@jeff-dude jeff-dude merged commit 738ae41 into duneanalytics:main Aug 22, 2023
3 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 22, 2023
@henrystats henrystats deleted the migrate-yield-yak branch August 23, 2023 03:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants