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

Removed auction_transaction #334

Merged
merged 6 commits into from
Feb 5, 2024
Merged

Removed auction_transaction #334

merged 6 commits into from
Feb 5, 2024

Conversation

sunce86
Copy link
Contributor

@sunce86 sunce86 commented Jan 26, 2024

In the PR we removed the auction_transaction table and added a new column auction_id to settlements table, so we need to redirect all queries to fetch the auction_id from settlements going forward.

When to merge this?

The db change is already applied to staging (migration of data from auction_transaction to settlements is also done) so we should merge the PR before the next script run on Tuesday (otherwise the accounting will try to join on <from,nonce> which are not populated anymore).

OTOH, the db change is not applied to prod yet (also it won't be applied until next script run on Tuesday), so we don't want to have this PR merged before the next script run on Tuesday.

Can we use two different versions of script for staging and prod?

Copy link

github-actions bot commented Jan 26, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@sunce86
Copy link
Contributor Author

sunce86 commented Jan 26, 2024

I have read the CLA Document and I hereby sign the CLA

github-actions bot added a commit that referenced this pull request Jan 26, 2024
@harisang harisang requested a review from fhenneke January 26, 2024 13:28
@fhenneke
Copy link
Collaborator

The changes look good.

As to when this should be merged: We do not have separate code for staging and production. So next payments should be run with the old code I think. This might require a correction for staging or we ignore the inconsistency there. With the next release we should include this change so that the following week is run with this PR applied.

We still plan to merge the protocol fee change before the next release. So we need to wait for the protocol fee change and adapt those queries as well.

More generally: Should we change how we handle production and staging in solver rewards? At the moment there is only one version of the code and one total payment is generated. Similar things hold for dune-sync.

@fhenneke
Copy link
Collaborator

fhenneke commented Feb 5, 2024

I added additional changes due to new queries for protocol fees. The changes are consistent with what dune-sync does now. Most (but not all) parts of the adapted queries are just copied from from cowprotocol/dune-sync#76.

@fhenneke fhenneke requested a review from harisang February 5, 2024 13:01
@harisang
Copy link
Contributor

harisang commented Feb 5, 2024

Looks good. Added a small comment

@harisang harisang merged commit 6319c81 into main Feb 5, 2024
6 checks passed
@harisang harisang deleted the removed-auction-transaction branch February 5, 2024 17:51
@github-actions github-actions bot locked and limited conversation to collaborators Feb 5, 2024
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