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

312 use stellar fee statistics to derive transaction fee #474

Merged
merged 4 commits into from
Jan 11, 2024

Conversation

b-yap
Copy link
Contributor

@b-yap b-yap commented Jan 9, 2024

closes #312

General overview of the changes:

  1. Remove DEFAULT_STROOP_FEE_PER_OPERATION constant in production code.
  2. Remove stroop_fee_per_operation parameters on method fn send_to_address(); since
  3. fee will be queried from horizon; the method is calledfn get_fee_stats()
  4. Memoization using the cached lib to return the same result until 10 minutes (or 600 seconds) have elapsed.

todo: only memoize for Ok() results.


How to begin the review:

  1. clients/wallet/src/types.rs -> has the FeeAttribute enum, where you can pick which one to base the operation on
  2. clients/wallet/src/horizon/responses.rs -> Defining the response into a struct of FeeStats and FeeDistribution
  3. clients/wallet/src/horizon/serde.rs -> Implements deserialization of the fields in FeeStats and FeeDistribution to u32
  4. clients/wallet/src/horizon/traits.rs -> add another method to the trait: fn get_fee_stats() and should return the struct FeeStats
  5. clients/wallet/src/horizon/horizon.rs -> implements fn get_fee_stats()
  6. clients/wallet/src/horizon/tests.rs -> testing the method fn get_fee_stats()
  7. clients/wallet/src/stellar_wallet.rs
    • the method fn get_fee() is introduced, with caching:
         #[cached(result = true, time = 600)]
      
    • the method fn get_fee() is called inside fn send_to_address(...), removing the stroop_fee_per_operation param
  8. ALL calls of fn send_to_address(...) will not need to pass a stroop_fee_per_operation

@b-yap b-yap marked this pull request as ready for review January 10, 2024 06:40
@b-yap b-yap requested a review from a team January 10, 2024 06:40
Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

I love it, these are some really great changes 🎉 Left some minor comments

@b-yap b-yap requested a review from ebma January 10, 2024 13:06
Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

LGTM ready to merge once CI passed

@b-yap b-yap merged commit 1928496 into main Jan 11, 2024
@b-yap b-yap deleted the 312-use-stellar-fee-statistics-to-derive-transaction-fee branch January 11, 2024 07:14
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.

Use Stellar fee statistics to derive transaction fee
2 participants