Skip to content

fix:(PRO-264) apply pricing model to fee estimation#207

Merged
dev-jodee merged 1 commit intomainfrom
fix/estimate-fee-pricing
Aug 29, 2025
Merged

fix:(PRO-264) apply pricing model to fee estimation#207
dev-jodee merged 1 commit intomainfrom
fix/estimate-fee-pricing

Conversation

@amilz
Copy link
Copy Markdown
Contributor

@amilz amilz commented Aug 28, 2025

• Problem: The estimateTransactionFee RPC method was not applying the configured price model (margin/fixed/free), while signTransactionIfPaid was correctly applying it, leading to fee estimates that were insufficient on sign if paid
• Solution: Modified estimateTransactionFee to call get_required_lamports() with the price model configuration, matching the behavior in signTransactionIfPaid.

todo--add pricing scenarios to kora.toml test fixtures


Important

Apply pricing model to estimateTransactionFee to ensure consistent fee estimation with signTransactionIfPaid.

  • Behavior:
    • Modified estimateTransactionFee in estimate_transaction_fee.rs to apply pricing model using get_required_lamports().
    • Ensures fee estimates match those in signTransactionIfPaid by including margin or fixed amounts.
  • Misc:
    • Renamed fee_in_lamports to min_transaction_fee for clarity in estimate_transaction_fee.rs.

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

📊 Test Coverage

Coverage

Coverage: 86.3%

View Detailed Coverage Report

@amilz amilz requested a review from dev-jodee August 28, 2025 21:57
@amilz amilz self-assigned this Aug 28, 2025
@amilz amilz added the bug Something isn't working label Aug 28, 2025
@linear
Copy link
Copy Markdown

linear bot commented Aug 28, 2025

Copy link
Copy Markdown
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 af3223f in 38 seconds. Click for details.
  • Reviewed 30 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 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/rpc_server/method/estimate_transaction_fee.rs:53
  • Draft comment:
    Renaming the fee variable to 'min_transaction_fee' clarifies that it represents the preliminary fee before pricing adjustments. Consider if a name like 'base_fee' might be even clearer.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. crates/lib/src/rpc_server/method/estimate_transaction_fee.rs:61
  • Draft comment:
    The updated code applies the pricing model via get_required_lamports, aligning with signTransactionIfPaid. Verify that passing Some(rpc_client) and Some(price_source.clone()) meets the expected type and behavior.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_DB9w3jiWYVUlazsl

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

Fee estimation now uses the price model to determine the required lamports, including margin or fixed amount, instead of returning the minimum transaction fee directly.
@dev-jodee dev-jodee force-pushed the fix/estimate-fee-pricing branch from af3223f to 5761c61 Compare August 29, 2025 16:29
@dev-jodee dev-jodee merged commit 2b3b408 into main Aug 29, 2025
2 checks passed
@amilz amilz deleted the fix/estimate-fee-pricing branch August 29, 2025 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants