Update CoW AMM service fee bounty query to include all chains #51
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fleupold
reviewed
Sep 27, 2024
| from aggregate_results_per_solver_all_chains as arps cross join total_surplus as ts | ||
| ), | ||
|
|
||
| named_results as ( |
There was a problem hiding this comment.
nit
Suggested change
| named_results as ( | |
| reward_targets as ( |
is clearer imo (generally there are a lot of little meaning table names in this query like temp, final, results etc)
| @@ -1,12 +1,11 @@ | |||
| -- This query computes how much surplus has been provided to CoW AMMs, when trading with other user orders | |||
There was a problem hiding this comment.
Can we move this file into a bounty subfolder to keep the main cow amm folder lean?
| @@ -19,38 +18,92 @@ with cow_amm_surplus as ( | |||
| where istrade | |||
| ), | |||
|
|
|||
| cow_surplus_per_batch as ( | |||
| cow_surplus_per_batch_ethereum as ( | |||
There was a problem hiding this comment.
Shouldn't we also add arbitrum?
Maybe we should keep the current query that takes blockchain as an arg and gives us solver_name, total_cow_surplus_in_usd and then have the first query in this file be something like
with
aggregate_results_per_solver_all_chains as (
select
solver_name,
sum(total_cow_surplus_in_usd) as total_cow_surplus_in_usd
from (
select * from queryX(blockchain='ethereum')
union all
select * from queryX(blockchain='gnosis')
union all
select * from queryX(blockchain='arbitrum')
)
group by solver_name
),
| from "query_1541516(end_time='{{end_time}}',vouch_cte_name='named_results')" | ||
| ), | ||
|
|
||
| final_results_per_solver as ( |
There was a problem hiding this comment.
last cte is not needed and in fact the final select seems to be expecting another table (and not use this query at all)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates query 4031724 so that it aggregates surplus over all chains, which is generated from CoWs between CoW AMMs and user orders. The query on Dune is already updated as we needed to use it to payout this week's bounty. Still a proper review would be ideal.