-
Notifications
You must be signed in to change notification settings - Fork 0
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
Update CoW AMM service fee bounty query to include all chains #51
base: main
Are you sure you want to change the base?
Conversation
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
last cte is not needed and in fact the final select seems to be expecting another table (and not use this query at all)
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.