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

Fix service fee query #140

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix service fee query #140

wants to merge 1 commit into from

Conversation

harisang
Copy link
Contributor

The query assumed, implicitly (this was not really intended), that every colocated solver with a reduced pool had initially joined the CoW DAO bonding pool at least 3 months before creating the reduced pool. This caused some issues, with Portus, for example. This is a bug, and this PR adds an explicit check so as to ensure that solvers that joined the CoW DAO bonding pool AND created a subpool within an interval of 3 months will start paying a service fee after 3 months. Specifically for Portus, there is no ambiguity around this as they joined a long time ago and only got renamed (which revealed the issue in the first place)

Copy link
Contributor

@bram-vdberg bram-vdberg left a comment

Choose a reason for hiding this comment

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

LGTM

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.

The change looks fine.

The previous behavior makes more sense to me. It only does not work due to using names for determining joining dates. This seems like one more reason to just hard-code values instead of maintaining that query.

@harisang
Copy link
Contributor Author

The change looks fine.

The previous behavior makes more sense to me. It only does not work due to using names for determining joining dates. This seems like one more reason to just hard-code values instead of maintaining that query.

Actually you are right. The previous version is the most reasonable in terms of interpreting the CIP and this "fix" just works because of the given situation of the existing solvers. Will rethink a bit more about this

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.

3 participants