-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add mainnet solver rewards dashboard #32
Conversation
cowprotocol/accounting/rewards/mainnet/mainnet_dashboard_query_2510345.sql
Show resolved
Hide resolved
cowprotocol/accounting/rewards/mainnet/mainnet_dashboard_query_2510345.sql
Outdated
Show resolved
Hide resolved
cowprotocol/accounting/rewards/mainnet/mainnet_dashboard_query_2510345.sql
Show resolved
Hide resolved
cowprotocol/accounting/rewards/mainnet/mainnet_dashboard_query_2510345.sql
Outdated
Show resolved
Hide resolved
cowprotocol/accounting/rewards/mainnet/mainnet_dashboard_query_2510345.sql
Outdated
Show resolved
Hide resolved
where | ||
blockchain = 'ethereum' | ||
and contract_address = 0xdef1ca1fb7fbcdc777520aa7f396b4e015f497ab | ||
and date(minute) = cast('{{end_time}}' as timestamp) - interval '1' day |
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.
This is fine for now. In the future I would expect this to handle more general accounting periods more gracefully.
A minor improvement would be
and date(minute) = cast('{{end_time}}' as timestamp) - interval '1' day | |
and date(minute) = date(cast('{{end_time}}' as timestamp)) - interval '1' day |
here and below. (Not super sure though.)
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.
Will leave it as is for now
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.
A comment regarding the parameter at the top of the file would be helpful. In the sense of
-- {{end_time}} - the timestamp for which the analysis should end (exclusively); only choices with with date(end_time) = end_time are allowed
Running the query
select
date(timestamp '2024-09-16 12:00:00') = timestamp '2024-09-17 00:00:00' - interval '1' day,
date(timestamp '2024-09-16 12:00:00') = timestamp '2024-09-17 00:00:01' - interval '1' day,
date(timestamp '2024-09-17 12:00:00') = timestamp '2024-09-17 00:00:01' - interval '1' day
gives true, false, false
. So not choosing end_time
incorrectly will make this price unavailable.
cowprotocol/accounting/rewards/mainnet/mainnet_dashboard_query_2510345.sql
Outdated
Show resolved
Hide resolved
cowprotocol/accounting/rewards/mainnet/mainnet_dashboard_query_2510345.sql
Outdated
Show resolved
Hide resolved
cowprotocol/accounting/rewards/mainnet/mainnet_dashboard_query_2510345.sql
Outdated
Show resolved
Hide resolved
when service_fee is true then 0.85 | ||
else 1 | ||
end as service_fee_factor | ||
from "query_4017925" |
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.
This will need to be passed a start_time
parameter since we need the status on the first day of the accounting period.
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.
Oh yeah, technically you are right here. The payouts script doesn't need that but we might run into edge cases (i.e., one week for each solver) where this might incorrectly charge a fee as it currently checks roughly the final day of the accounting and not the first day.
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.
In any case, the service fee query requires fixing but it seems its results are fine for today so i would leave it as is in order not to get stuck at this
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.
cowprotocol/accounting/rewards/mainnet/mainnet_dashboard_query_2510345.sql
Outdated
Show resolved
Hide resolved
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.
Looks good to me.
I have not tested the query, though. Is there a test version or is the original query already updated?
This PR adds the mainnet payouts dashboard (2510345) to the repo, and simplifies it quite a bit on the way