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

chore: Executed surplus fee in surplus token #2963

Open
sunce86 opened this issue Sep 9, 2024 · 3 comments
Open

chore: Executed surplus fee in surplus token #2963

sunce86 opened this issue Sep 9, 2024 · 3 comments

Comments

@sunce86
Copy link
Contributor

sunce86 commented Sep 9, 2024

Background

Both solver team and frontend team would like to have this field expressed over surplus token, instead of sell token. This would help a lot with showing complete breakdown of fees (network fee, protocol fee, total fee) as protocol fees are already calculated in surplus tokens.

Details

Steps:

  1. Add executed_surplus_fee_token to order_execution database table and populate historic entries with sell token of the order.
  2. Expose the field executed_surplus_fee_token over API whenever executed_surplus_fee is exposed.
  3. Switch to saving surplus fee in surplus token instead of sell token

So, bottom line, there will be a timestamp after which all fees will be in surplus token. For historic entries, executed surplus fee in sell token will remain.
Both solver and frontend team need to cope with this. I assume frontend will have to have a special IF to show things differently depending on if executed_surplus_fee is in sell token (for historic entries) or in surplus token (for new entries). There is also an alternative to try to convert executed_surplus_fee to surplus token for historic entries on the frontend side, where traded sell/buy amounts could be decent approximates (at least in most cases).

Acceptance criteria

Complete breakdown of taken fees is exposed on get_trades and get_order endpoints:

  1. network fee in surplus token
  2. protocol fees per fee policy, in surplus token
  3. total fee in surplus token (or maybe omitted since it can be calculated by summarizing (1) and (2))
@fleupold
Copy link
Contributor

Not sure about the naming (maybe we can finally drop the "surplus" from the field, given that there is no non surplus fee anymore and "surplus fee token" = surplus token is 🤯)

Otherwise I think the plan is sound since the surplus token can either be buy or sell token (and so the frontend in any case needs to handle both cases). It's just that for historic orders it will displayed in sell token regardless of order kind.

I'm just not sure in terms of priority if this is something we should do this quarter. cc @MartinquaXD

@sunce86
Copy link
Contributor Author

sunce86 commented Sep 11, 2024

Not sure about the naming

Agreed, just wasn't sure if it's the right time for renaming. But why not, will adjust the field.

I'm just not sure in terms of priority if this is something we should do this quarter.

I was almost finished with the implementation when you posted the comment. Will finish it since already started, frontend will be happy about it.

Copy link

This issue has been marked as stale because it has been inactive a while. Please update this issue or it will be automatically closed.

@github-actions github-actions bot added the stale label Nov 11, 2024
@sunce86 sunce86 reopened this Nov 29, 2024
@github-actions github-actions bot removed the stale label Nov 30, 2024
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

No branches or pull requests

2 participants