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

refactor(suite): optimize how often fees are fetched in send form #15469

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

Conversation

senky
Copy link

@senky senky commented Nov 20, 2024

!!! WORK IN PROGRESS !!! do not merge yet

This commit introduces a new approach to fetching fees.

Previous state:
Fees for every wallet account were fetched periodically after 10 blocks. This was problematic for various reasons:

  1. The data weren't often needed.
  2. Useless network requests.
  3. Some fees were outdated, because fees aren't connected to blocks in any way - in fact, fee can update multiple times in between two blocks!

New state:
This solution mainly builds on the idea that fees aren't connected to blocks. This link is now removed. There is a dedicated useFees hook that is supposed to be called for every route that needs to display fees. The hook is responsible for decision on how often the fee should be fetched (decided manually in networkToInterval function). That way all logic behind fee fetching is located in one file. Consequence of this decision is that updateFeeInfoThunk is now "dumb" - it fetches new fees whenever it is called. While this solution is more verbose (useFees has to be called everywhere fees are needed), I think its explicit relationship improves readability of the code.

What needs to be done before this commit is mergeable:

  1. Set network refresh intervals in networkToInterval function.
  2. Call useFees for every route that needs fees (swap, sell, etc.).
  3. Move useFees hook to better location so that it can be used in suite-native/module-send/src/screens/SendOutputsScreen.tsx:174, too.

Relates to #15087

!!! WORK IN PROGRESS !!! do not merge yet

This commit introduces a new approach to fetching fees.

Previous state:
Fees for every wallet account were fetched periodically after 10 blocks. This was problematic for various reasons:
1. The data weren't often needed.
2. Useless network requests.
3. Some fees were outdated, because fees aren't connected to blocks in any way - in fact, fee can update multiple times in between two blocks!

New state:
This solution mainly builds on the idea that fees aren't connected to blocks. This link is now removed. There is a dedicated `useFees` hook that is supposed to be called for every route that needs to display fees. The hook is responsible for decision on how often the fee should be fetched (decided manually in `networkToInterval` function). That way all logic behind fee fetching is located in one file. Consequence of this decision is that `updateFeeInfoThunk` is now "dumb" - it fetches new fees whenever it is called. While this solution is more verbose (`useFees` has to be called everywhere fees are needed), I think its explicit relationship improves readability of the code.

What needs to be done before this commit is mergeable:
1. Set network refresh intervals in `networkToInterval` function.
2. Call `useFees` for every route that needs fees (swap, sell, etc.).
3. Move `useFees` hook to better location so that it can be used in `suite-native/module-send/src/screens/SendOutputsScreen.tsx:174`, too.

Relates to trezor#15087
@tomasklim tomasklim self-assigned this Nov 22, 2024
@tomasklim tomasklim self-requested a review November 22, 2024 09:59
@tomasklim
Copy link
Member

Thank you @senky for your contribution. The POC works really great, love it.

Screen.Recording.2024-11-23.at.12.57.34.mov

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