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

Include setFee GraphQL mutation in updateAsset instead #2036

Closed
JoblersTune opened this issue Oct 16, 2023 · 1 comment
Closed

Include setFee GraphQL mutation in updateAsset instead #2036

JoblersTune opened this issue Oct 16, 2023 · 1 comment

Comments

@JoblersTune
Copy link
Collaborator

Prior to feat: liquidity webhooks #1782 there was an updateWithdrawlThreshold mutation for assets. But since liquidity updates were added as well now there's an updateAsset mutation. Since there is a general updateAsset mutation it would make sense to move the setFee mutation into the updateAsset mutation as well.

The createAsset mutation should also have an option to add fees immediately then as well.

@JoblersTune JoblersTune self-assigned this Oct 16, 2023
@github-project-automation github-project-automation bot moved this to Backlog in Rafiki Oct 16, 2023
@JoblersTune JoblersTune moved this from Backlog to In Progress in Rafiki Oct 16, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in Rafiki Oct 18, 2023
@JoblersTune
Copy link
Collaborator Author

The decision is to leave setFee separate from creating and updating assets. Bringing fees into the asset creation and update functionality adds complexity. Assets and fees are stored in separate database tables, so it's logical to have separate mutations that respect the independence of these entities, as they can succeed or fail independently. This also simplifies error handling on both the frontend and backend as well as simplifying testing and debugging. Ultimately I think the code is cleaner if we keep the stricter separation of concerns.

@JoblersTune JoblersTune removed their assignment Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

2 participants