Skip to content

Conversation

@dev-jodee
Copy link
Contributor

@dev-jodee dev-jodee commented Sep 25, 2025

…ype (Free -> instant, Fixed -> just calculate price), Margin -> the full calculation


Important

Improves fee estimation efficiency by handling different price models (Free, Fixed, Margin) with new methods in fee.rs and price.rs.

  • Behavior:
    • estimate_kora_fee() in fee.rs now handles PriceModel::Free, PriceModel::Fixed, and PriceModel::Margin more efficiently by using new methods get_required_lamports_with_fixed() and get_required_lamports_with_margin().
    • TotalFeeCalculation struct in fee.rs now has a new_fixed() constructor for fixed fee scenarios.
  • Models:
    • PriceConfig in price.rs now includes get_required_lamports_with_fixed() and get_required_lamports_with_margin() for calculating fees based on PriceModel.
  • Tests:
    • Updated tests in price.rs and fee.rs to cover new fee calculation methods and scenarios.
  • Misc:
    • Minor change in transfer_transaction.rs to fix a test case by renaming source to invalid.

This description was created by Ellipsis for 0c437c6. You can customize this summary. It will automatically update as commits are pushed.

📊 Unit Test Coverage

Coverage

Unit Test Coverage: 84.6%

View Detailed Coverage Report

…ype (Free -> instant, Fixed -> just calculate price), Margin -> the full calculation
@dev-jodee dev-jodee requested a review from amilz September 25, 2025 19:28
@linear
Copy link

linear bot commented Sep 25, 2025

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to 0c437c6 in 1 minute and 30 seconds. Click for details.
  • Reviewed 362 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. crates/lib/src/fee/fee.rs:39
  • Draft comment:
    Consider adding doc comments for the new() and new_fixed() constructors for clarity.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50% None
2. crates/lib/src/fee/fee.rs:289
  • Draft comment:
    The match on PriceModel (Free, Fixed, Margin) cleanly separates fee calculation paths. Confirm that using new_fixed for Fixed and new for Margin correctly represents the fee breakdown.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50% None
3. crates/lib/src/fee/price.rs:29
  • Draft comment:
    In get_required_lamports_with_fixed, consider providing a more descriptive error if the PriceModel is not Fixed.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50% None
4. crates/lib/src/fee/price.rs:51
  • Draft comment:
    The margin calculation correctly checks for overflow. Consider enforcing or documenting that margin values should be non-negative to avoid unintended fee reductions.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50% None
5. crates/lib/src/rpc_server/method/transfer_transaction.rs:170
  • Draft comment:
    Test update: Changing the source string from 'invalid_pubkey' to 'invalid' properly triggers Pubkey::from_str error.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, describing a change in a test case without suggesting any action or asking for clarification. It doesn't provide any actionable feedback or raise any concerns about the code.

Workflow ID: wflow_5F82XkaGSOXFhQLr

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@dev-jodee dev-jodee merged commit ec05044 into release/feature-freeze-for-audit Sep 29, 2025
9 checks passed
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