Add block range, bonding pool and vouch queries #19
Conversation
|
Tested them and they seem to work. |
...otocol/accounting/rewards/mainnet/block_number_interval_from_time_interval_query_3333356.sql
Show resolved
Hide resolved
| last_block_before_timestamp as ( | ||
| select max(number) from ethereum.blocks | ||
| where time < cast('{{end_time}}' as timestamp) | ||
| ), |
There was a problem hiding this comment.
This could in principle call out to query 3333356. Then it would be easier to make sure that the convention on which boundary points are included is consistent across queries.
There was a problem hiding this comment.
Yeah thought about it. There is a start_time in query query 3333356, but i guess i could hardcode some very old one to work around it.
cowprotocol/accounting/rewards/mainnet/vouch_registry_query_1541516.sql
Outdated
Show resolved
Hide resolved
Good question. Ok so 2 of the 3 queries are very straightforward and easy to test. The vouching query is essentially the one we currently use, but i believe it actually might not be 100% correct, as it might not take into account cases where a solver left a bonding pool and then rejoined during the accounting period we are interested in. I will open an issue for this one, as i don't have the capacity to fix this at the moment. |
|
Opened issue #21 in order to revisit the logic of the vouching query. |
cowprotocol/accounting/rewards/mainnet/full_bonding_pools_query_4056263.sql
Outdated
Show resolved
Hide resolved
|
@fhenneke I did some review of the query and it makes sense. I am not very comfortable with the Let me know if you want to do another pass |
There was a problem hiding this comment.
We can add this as is to stay compatible with its current function.
An alternative, which I prefer, is to simplify it to just get the latest pool and reward address, irrespective of any invalidations later on. Solvers changing bonding pools or getting unvouched before running the script have been a pain.
There was a problem hiding this comment.
Just to clarify, are you saying we should simply fetch the latest vouching event of every solver, and then don't worry about the rest, as in case the solver is later unvouched, we will have in any case blacklisted as well and then that solver would have no effect in the competition?
There was a problem hiding this comment.
As far as uses in solver rewards, I think we should only care about latest reward addresses (and bonding pools). If a solver gets blacklisted and removed, solver rewards should still be rewarded as if they had not been removed. Blacklisting should take care of barring solvers from gaining additional rewards if they are not eligible.
Other places might use that query differently. But for our main use case, this simplification should be useful.
There was a problem hiding this comment.
yeah makes sense, will proceed with the simplification
There was a problem hiding this comment.
Actually, i think i changed my mind on this again. This query also shows in the main dashboard, and i think it is nice to be able to see all "active" solvers, with their names etc. So if you don't mind, i would keep it as is and would actually even revert to the previous state where there are two views of the results, so that we can use the other view in the main dashboard.
* add block range, bonding pool and vouch queries * small fix * bug fixing * add description to block range query * add description to full bonding pools query * Add descritpion to vouch registry query * small updates * Update cowprotocol/accounting/rewards/mainnet/.sqlfluff Co-authored-by: Felix Henneke <[email protected]> * add hardcoded timestamps * add back alternative view * fix typo * bug fixes * minor edits --------- Co-authored-by: Felix Henneke <[email protected]>
This PR migrates queries 3333356 and 1541516 to the repo. There are also some changes proposed for 1541516, that get rid of the parameter that passes the list of full bonding pools, and proposes instead to use a new query 4056263 to hardcode the full bonding pools.