Skip to content
This repository was archived by the owner on Dec 20, 2024. It is now read-only.

Conversation

@harisang
Copy link
Contributor

@harisang harisang commented Oct 11, 2024

This PR mirrors the changes in the solver rewards repo from the following two PRs:

Concretely, the calculation of protocol fees in the sql query is simplified by a lot, and consistency rewards are removed as they are no longer used.

Note: some tests needed updating since the new way of fetching protocol fees is not available for the block range the test was set up for. The tests were updated accordingly, and one (bit strange) way to verify the numbers is to run the following dune query, as the data for that block range on Dune have been uploaded via the old (and NOT the proposed) and tested way. The tiny discrepancies are a result of rounding "errors", but they are very small.

For the batch rewards test, this query should be used

select
    block_number,
    block_deadline,
    tx_hash,
    solver,
    data.execution_cost,
    data.surplus,
    data.protocol_fee,
    data.fee,
    data.uncapped_payment_eth,
    data.capped_payment,
    data.winning_score,
    data.reference_score
from cowswap.raw_batch_rewards where block_deadline >= 20936269 and block_deadline <= 20936280

while for the order rewards test, this query should be used

select
    block_number,
    order_uid,
    solver,
    data.quote_solver,
    tx_hash,
    data.surplus_fee,
    data.amount,
    data.protocol_fee,
    data.protocol_fee_token,
    data.protocol_fee_native_price,
    data.quote_sell_amount,
    data.quote_buy_amount,
    data.quote_gas_cost,
    data.quote_sell_token_price,
    data.partner_fee,
    data.partner_fee_recipient,
    data.protocol_fee_kind
from cowswap.raw_order_rewards where block_number>= 20934809 and block_number <= 20934808 + 17 order by block_number ASC

As for the integrator fees test, this range on Dune should be user: block_number>= 20934805 and block_number <= 20934809

@harisang harisang marked this pull request as ready for review October 11, 2024 14:24
Copy link
Contributor

@fhenneke fhenneke left a comment

Choose a reason for hiding this comment

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

Generally looks good.

The only thing missing are new tests for order rewards, right?

@harisang
Copy link
Contributor Author

Generally looks good.

The only thing missing are new tests for order rewards, right?

This is now addressed. It would be good to check old vs new behavior (by recovering the results from Dune). It seems there are some discrepancies, which i believe should be due to rounding/slightly different ways of computing protocol fees, but would like to check 1-2 examples in more detail.

@harisang
Copy link
Contributor Author

I ran the proposed order rewards query for the accounting period of Oct 8-15, 2024, and it seems that indeed results look fine.

https://dune.com/queries/4186410

The only non-trivial discrepancies where showing up for trades involving this token: https://etherscan.io/token/0x30ae41d5f9988d359c733232c6c693c0e645c77e

which is a token with 0 decimals. Not sure how we handle such tokens throughout our scripts tbh.

So i am fine merging this PR, if you also agree @fhenneke

@harisang harisang merged commit a35f5fb into main Oct 23, 2024
6 checks passed
@harisang harisang deleted the remove_consistency_rewards_from_sql branch October 23, 2024 10:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants