Skip to content

Fix/fee table for fee bump tx #2108

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Aug 6, 2025
Merged

Fix/fee table for fee bump tx #2108

merged 11 commits into from
Aug 6, 2025

Conversation

@elizabethengelman
Copy link
Collaborator

@elizabethengelman elizabethengelman commented Aug 1, 2025 •
edited
Loading

What

Closes #2101

Why

We were previously miscalculating the resource fees for fee bump transactions that wrap soroban transactions. This PR fixes that by looking at the inner transaction to get the proposed resource fee.

This PR also includes some refactoring to make the fee calculation easier to understand & test.

Before:

❯ stellar tx fetch fee --hash e7a6268b66d6fe2c8a89941062c4012d4b020b91c2d98cd18d98b3b34ed296d4 --network main
┌──────────────────────────────────────────────────────────────────────────────────────┐
│ Fee Proposed: 255364                                                                 │
├──────────────────────────────────────────────────────────────────────────────────────┤
│ Inclusion Fee: 255364 │ Resource Fee: 0                                              │
├──────────────────────────────────────────────────────────────────────────────────────┤
│ Inclusion Fee: 255364 │ Non-Refundable: 165209 │ Refundable: -165209                 │
│                       │                        │                                     │
│                       │ cpu instructions       │ return value                        │
│                       │ storage read/write     │ storage rent                        │
│                       │ tx size                │ events                              │
├──────────────────────────────────────────────────────────────────────────────────────┤
│                            👆 Proposed Fee  👇 Final Fee                             │
├──────────────────────────────────────────────────────────────────────────────────────┤
│ Inclusion Fee: 200    │ Non-Refundable: 165209 │ Refundable: 23118 │ Refunded: 66837 │
├──────────────────────────────────────────────────────────────────────────────────────┤
│ Inclusion Fee: 200    │ Resource Fee: 188327                       │ Refunded: 66837 │
├──────────────────────────────────────────────────────────────────────────────────────┤
│ Fee Charged: 188527                                                │ Refunded: 66837 │
└──────────────────────────────────────────────────────────────────────────────────────┘

After:

❯ cargo run tx fetch fee --hash e7a6268b66d6fe2c8a89941062c4012d4b020b91c2d98cd18d98b3b34ed296d4 --network main
┌───────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Fee Proposed: 255364                                                                                      │
├───────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Inclusion Fee: 201         │ Resource Fee: 255163                                                         │
├───────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Inclusion Fee: 201         │ Non-Refundable: 165209         │ Refundable: 89954                           │
│                            │                                │                                             │
│                            │ cpu instructions               │ return value                                │
│                            │ storage read/write             │ storage rent                                │
│                            │ tx size                        │ events                                      │
├───────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│                                       👆 Proposed Fee  👇 Final Fee                                       │
├───────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Inclusion Fee Charged: 200 │ Non-Refundable Charged: 165209 │ Refundable Charged: 23118 │ Refunded: 66837 │
├───────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Inclusion Fee Charged: 200 │ Resource Fee Charged: 188327                               │ Refunded: 66837 │
├───────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Fee Charged: 188527                                                                     │ Refunded: 66837 │
└───────────────────────────────────────────────────────────────────────────────────────────────────────────┘

Known limitations

n/a

@github-project-automation github-project-automation bot moved this to Backlog (Not Ready) in DevX Aug 1, 2025
@elizabethengelman elizabethengelman force-pushed the fix/fee-table-for-fee-bump-tx branch from 5bfb62a to 5321337 Compare August 1, 2025 15:25
@elizabethengelman elizabethengelman self-assigned this Aug 4, 2025
elizabethengelman
elizabethengelman commented Aug 4, 2025
View reviewed changes
@elizabethengelman elizabethengelman marked this pull request as ready for review August 4, 2025 15:03
@fnando fnando requested a review from Copilot August 5, 2025 17:59
Copilot AI reviewed Aug 5, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug in fee calculation for fee bump transactions that wrap Soroban transactions. Previously, the resource fees were being miscalculated because the code wasn't looking at the inner transaction's resource fee for fee bump transactions.

Key changes include:

  • Updated fee bump transaction handling to extract resource fees from the inner transaction
  • Refactored fee calculation logic into separate ProposedFees and ChargedFees structs for better organization
  • Updated table display constants to distinguish between proposed and charged fees

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
cmd/soroban-cli/src/commands/tx/fetch/fee.rs Main fee calculation logic fix for fee bump transactions and structural refactoring
cmd/crates/soroban-test/tests/it/integration/tx/fetch.rs Test updates to match new fee structure

@elizabethengelman elizabethengelman merged commit 5df51d8 into main Aug 6, 2025
35 checks passed
@elizabethengelman elizabethengelman deleted the fix/fee-table-for-fee-bump-tx branch August 6, 2025 13:55
@github-project-automation github-project-automation bot moved this from Backlog (Not Ready) to Done in DevX Aug 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@leighmcculloch leighmcculloch leighmcculloch approved these changes

@fnando fnando Awaiting requested review from fnando

Assignees

@elizabethengelman elizabethengelman

Labels

None yet

Projects

DevX
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

tx fetch fee miscalculating resource fees for fee bump txns that wrap soroban txns

4 participants

@elizabethengelman @leighmcculloch @fnando