Skip to content
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 integration fee recipient #94

Merged
merged 23 commits into from
Apr 15, 2024
Merged

Add integration fee recipient #94

merged 23 commits into from
Apr 15, 2024

Conversation

harisang
Copy link
Contributor

@harisang harisang commented Apr 3, 2024

This PR adds one more field in the raw_order_rewards table on Dune; namely, what is the recipient of the protocol fee collected, if any, for an order.

Note that here we slightly abuse the term "protocol fee" as integrator fees are not really protocol fees, but from a solvers and backend point of view, they behave identically.

@harisang
Copy link
Contributor Author

harisang commented Apr 4, 2024

Side comment: it ended up being extremely annoying to pass the integration test. Hadn't connected to the database locally so was relying on github for debugging. The block/block_deadline business is quite confusing, so I need to do at least some sanity checks to convince myself everything is good. Opening an issue to address this.

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.

LGTM

src/models/order_rewards_schema.py Outdated Show resolved Hide resolved
- added fee kind to order book query
- added fee kind to uploaded data
- added fee kind to tests
to stay consistent with the naming in appdata
@fhenneke fhenneke merged commit 47d19c1 into main Apr 15, 2024
6 checks passed
@fhenneke fhenneke deleted the add_integrator_fees branch April 15, 2024 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants