fix: Fixing Stellar transaction retry logic with fee bump and delay#654
Closed
NicoMolinaOZ wants to merge 1 commit intomainfrom
Closed
fix: Fixing Stellar transaction retry logic with fee bump and delay#654NicoMolinaOZ wants to merge 1 commit intomainfrom
NicoMolinaOZ wants to merge 1 commit intomainfrom
Conversation
WalkthroughThis PR introduces support for Stellar transaction fee-bump retry logic when encountering insufficient fee errors. It adds retry configuration constants, extends transaction data structures to track retry attempts, implements fee escalation based on retry count, and adds detection and handling of insufficient fee errors during transaction submission with automatic retries and sequence resynchronization. Changes
Sequence Diagram(s)sequenceDiagram
participant Submit as submit_core
participant ErrorDetect as Error Detection
participant FeeCalc as Fee Calculation
participant Chain as Chain Sync
participant Retry as Reset & Retry
Submit->>ErrorDetect: Receive error from Stellar
ErrorDetect->>ErrorDetect: is_insufficient_fee_error()?
alt Insufficient Fee Detected
ErrorDetect->>ErrorDetect: Check retries < max?
alt Retries Available
ErrorDetect->>FeeCalc: compute_escalated_inclusion_fee(retry_count)
FeeCalc-->>ErrorDetect: escalated_fee
ErrorDetect->>Chain: sync_sequence_from_chain()
Chain-->>ErrorDetect: sequence synced
ErrorDetect->>ErrorDetect: Sleep STELLAR_FEE_RETRY_DELAY_SECONDS
ErrorDetect->>Retry: increment retry counter
Retry->>Retry: reset_transaction_for_retry()
Retry-->>Submit: return for re-submission
else Max Retries Reached
ErrorDetect->>Submit: mark Failed, enqueue next
end
else Other Error
ErrorDetect->>Submit: mark Failed, enqueue next
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.16.0)src/constants/stellar_transaction.rs┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m src/domain/transaction/stellar/prepare/common.rs┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m src/domain/transaction/stellar/prepare/fee_bump.rs┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
NicoMolinaOZ
changed the title
NicoMolinaOZ
commented
Feb 23, 2026
| "waiting before fee escalation retry" | ||
| ); | ||
|
|
||
| // Alternative consideration: Instead of sleeping in the handler, you could reset the transaction immediately and schedule the recovery job with a delay (there's already send_submit_transaction_job with a delay_seconds parameter |
Contributor
Author
There was a problem hiding this comment.
@zeljkoX do you think this suggestion should be the best approach?
Codecov Report❌ Patch coverage is Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. @@ Coverage Diff @@
## main #654 +/- ##
==========================================
- Coverage 91.75% 88.86% -2.89%
==========================================
Files 278 289 +11
Lines 114297 117994 +3697
==========================================
- Hits 104872 104859 -13
- Misses 9425 13135 +3710
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/models/transaction/repository.rs (1)
828-834:⚠️ Potential issue | 🟠 MajorGuard against fee overflow when escalating inclusion fees.
operations_count * feeand+ resource_feecan overflowu64and wrap in release builds, leading to under-priced fees or conversion errors. Use saturating/checked math before theu32conversion.Proposed fix
- let inclusion_fee = operations_count - * compute_escalated_inclusion_fee(self.insufficient_fee_retries) as u64; + let inclusion_fee = operations_count.saturating_mul(u64::from( + compute_escalated_inclusion_fee(self.insufficient_fee_retries), + )); let resource_fee = sim_response.min_resource_fee; - let updated_fee = u32::try_from(inclusion_fee + resource_fee) - .map_err(|_| SignerError::ConversionError("Fee too high".to_string()))? + let updated_fee = inclusion_fee + .checked_add(resource_fee) + .and_then(|total| u32::try_from(total).ok()) + .ok_or_else(|| SignerError::ConversionError("Fee too high".to_string()))? .max(STELLAR_DEFAULT_TRANSACTION_FEE);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/transaction/repository.rs` around lines 828 - 834, The fee arithmetic can overflow on the u64 operations; replace the direct multiplication/addition with checked/saturating operations: compute inclusion_fee_u64 = (operations_count as u64).checked_mul(compute_escalated_inclusion_fee(self.insufficient_fee_retries) as u64).and_then(|v| v.checked_add(sim_response.min_resource_fee)).ok_or_else(|| SignerError::ConversionError("Fee overflow".to_string()))? and then try converting to u32 and applying .max(STELLAR_DEFAULT_TRANSACTION_FEE) as before. Update references: operations_count, compute_escalated_inclusion_fee, self.insufficient_fee_retries, sim_response.min_resource_fee, and updated_fee to use this checked arithmetic and return the ConversionError on overflow.
🧹 Nitpick comments (4)
src/models/transaction/repository.rs (1)
826-827: Move the local import to module scope.Please lift
compute_escalated_inclusion_feeto the top-leveluseblock to align with the repo rule on header imports.
As per coding guidelines, "Prefer header imports over function-level imports of dependencies".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/transaction/repository.rs` around lines 826 - 827, The local function-level import of compute_escalated_inclusion_fee should be moved to the module/header use block: remove the inline `use crate::domain::transaction::stellar::utils::compute_escalated_inclusion_fee;` from inside the function in repository.rs and add a top-level `use crate::domain::transaction::stellar::utils::compute_escalated_inclusion_fee;` alongside the other module imports so the symbol is imported in the file header and no function-local use remains.src/domain/transaction/stellar/submit.rs (1)
304-317: Consider scheduling delayed retry instead of sleeping in-handler.Sleeping here keeps the handler task alive; using the existing delayed submit job would free the worker and be more crash‑resilient.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/domain/transaction/stellar/submit.rs` around lines 304 - 317, Replace the in-handler sleep (tokio::time::sleep with STELLAR_FEE_RETRY_DELAY_SECONDS) by scheduling the existing delayed retry job: reset or mark the transaction for retry and call send_submit_transaction_job(...) with delay_seconds = STELLAR_FEE_RETRY_DELAY_SECONDS (instead of awaiting the sleep) so the handler can return immediately and the retry runs resiliently outside this task.src/domain/transaction/stellar/utils.rs (1)
283-288: Move the XDR import to module scope.
decode_transaction_result_codepullsTransactionResultvia a function-level import; prefer module-level imports for consistency.♻️ Proposed change
- TransactionEnvelope, TransactionMeta, Uint256, VecM, + TransactionEnvelope, TransactionMeta, TransactionResult, Uint256, VecM, }; pub fn decode_transaction_result_code(xdr_base64: &str) -> Option<String> { - use soroban_rs::xdr::{Limits, ReadXdr, TransactionResult}; let result = TransactionResult::from_xdr_base64(xdr_base64, Limits::none()).ok()?;As per coding guidelines, prefer header imports over function-level imports of dependencies.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/domain/transaction/stellar/utils.rs` around lines 283 - 288, The function decode_transaction_result_code currently imports soroban_rs::xdr::{Limits, ReadXdr, TransactionResult} inside the function; move that import to the module header so the types are imported at module scope (add use soroban_rs::xdr::{Limits, ReadXdr, TransactionResult}; near the top of the file) and remove the in-function use statement in decode_transaction_result_code so the function simply references TransactionResult and Limits directly.src/domain/transaction/stellar/prepare/common.rs (1)
222-223: Move compute_escalated_inclusion_fee import to module scope.Function-local import breaks the repo’s import convention; promote it to the header.
♻️ Proposed change
+use crate::domain::transaction::stellar::utils::compute_escalated_inclusion_fee; ... pub async fn calculate_fee_bump_required_fee<P>( inner_envelope: &TransactionEnvelope, max_fee: i64, provider: &P, insufficient_fee_retries: u32, ) -> Result<u32, TransactionError> where P: StellarProviderTrait + Send + Sync, { - use crate::domain::transaction::stellar::utils::compute_escalated_inclusion_fee; let escalated_inclusion = compute_escalated_inclusion_fee(insufficient_fee_retries) as i64;As per coding guidelines, prefer header imports over function-level imports of dependencies.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/domain/transaction/stellar/prepare/common.rs` around lines 222 - 223, The function-local import of compute_escalated_inclusion_fee should be moved to module/header scope: remove the in-function "use crate::domain::transaction::stellar::utils::compute_escalated_inclusion_fee;" and add that use declaration with the other top-level imports in this module, leaving the call "compute_escalated_inclusion_fee(insufficient_fee_retries) as i64" unchanged; update any file-level import ordering to match project convention.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/domain/transaction/stellar/validation.rs`:
- Around line 1002-1009: The validation currently calls estimate_fee(envelope,
provider, None, 0) which hardcodes retries=0 and underestimates fees; update
validate_user_fee_payment_amounts to accept an insufficient_fee_retries: u32
parameter and pass that value into estimate_fee (i.e., estimate_fee(envelope,
provider, None, insufficient_fee_retries)). Propagate this new parameter through
the caller chain (or add it to upstream function signatures) so the real retry
count used by insufficient_fee_retries is threaded into
validate_user_fee_payment_amounts and into any other call sites that invoke it;
ensure the type is u32 where declared and update all call sites accordingly.
---
Outside diff comments:
In `@src/models/transaction/repository.rs`:
- Around line 828-834: The fee arithmetic can overflow on the u64 operations;
replace the direct multiplication/addition with checked/saturating operations:
compute inclusion_fee_u64 = (operations_count as
u64).checked_mul(compute_escalated_inclusion_fee(self.insufficient_fee_retries)
as u64).and_then(|v| v.checked_add(sim_response.min_resource_fee)).ok_or_else(||
SignerError::ConversionError("Fee overflow".to_string()))? and then try
converting to u32 and applying .max(STELLAR_DEFAULT_TRANSACTION_FEE) as before.
Update references: operations_count, compute_escalated_inclusion_fee,
self.insufficient_fee_retries, sim_response.min_resource_fee, and updated_fee to
use this checked arithmetic and return the ConversionError on overflow.
---
Nitpick comments:
In `@src/domain/transaction/stellar/prepare/common.rs`:
- Around line 222-223: The function-local import of
compute_escalated_inclusion_fee should be moved to module/header scope: remove
the in-function "use
crate::domain::transaction::stellar::utils::compute_escalated_inclusion_fee;"
and add that use declaration with the other top-level imports in this module,
leaving the call "compute_escalated_inclusion_fee(insufficient_fee_retries) as
i64" unchanged; update any file-level import ordering to match project
convention.
In `@src/domain/transaction/stellar/submit.rs`:
- Around line 304-317: Replace the in-handler sleep (tokio::time::sleep with
STELLAR_FEE_RETRY_DELAY_SECONDS) by scheduling the existing delayed retry job:
reset or mark the transaction for retry and call
send_submit_transaction_job(...) with delay_seconds =
STELLAR_FEE_RETRY_DELAY_SECONDS (instead of awaiting the sleep) so the handler
can return immediately and the retry runs resiliently outside this task.
In `@src/domain/transaction/stellar/utils.rs`:
- Around line 283-288: The function decode_transaction_result_code currently
imports soroban_rs::xdr::{Limits, ReadXdr, TransactionResult} inside the
function; move that import to the module header so the types are imported at
module scope (add use soroban_rs::xdr::{Limits, ReadXdr, TransactionResult};
near the top of the file) and remove the in-function use statement in
decode_transaction_result_code so the function simply references
TransactionResult and Limits directly.
In `@src/models/transaction/repository.rs`:
- Around line 826-827: The local function-level import of
compute_escalated_inclusion_fee should be moved to the module/header use block:
remove the inline `use
crate::domain::transaction::stellar::utils::compute_escalated_inclusion_fee;`
from inside the function in repository.rs and add a top-level `use
crate::domain::transaction::stellar::utils::compute_escalated_inclusion_fee;`
alongside the other module imports so the symbol is imported in the file header
and no function-local use remains.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
Cargo.tomlsrc/constants/stellar_transaction.rssrc/domain/relayer/stellar/gas_abstraction.rssrc/domain/transaction/stellar/prepare/common.rssrc/domain/transaction/stellar/prepare/fee_bump.rssrc/domain/transaction/stellar/prepare/mod.rssrc/domain/transaction/stellar/prepare/operations.rssrc/domain/transaction/stellar/prepare/soroban_gas_abstraction.rssrc/domain/transaction/stellar/prepare/unsigned_xdr.rssrc/domain/transaction/stellar/submit.rssrc/domain/transaction/stellar/test_helpers.rssrc/domain/transaction/stellar/utils.rssrc/domain/transaction/stellar/validation.rssrc/models/transaction/repository.rssrc/models/transaction/response.rssrc/models/transaction/stellar/conversion.rssrc/services/signer/stellar/aws_kms_signer.rssrc/services/signer/stellar/google_cloud_kms_signer.rssrc/services/signer/stellar/local_signer.rssrc/services/signer/stellar/turnkey_signer.rs
Comment on lines
+1002
to
+1009
| let mut required_xlm_fee = | ||
| estimate_fee(envelope, provider, None, 0) | ||
| .await | ||
| .map_err(|e| { | ||
| StellarTransactionValidationError::ValidationError(format!( | ||
| "Failed to estimate fee: {e}", | ||
| )) | ||
| })?; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find validation function definitions and their signatures
rg -n "fn validate_user_fee_payment" -A 5
# Find where these validation functions are called
rg -n "validate_user_fee_payment_amounts|validate_user_fee_payment_transaction" -B 2 -A 2
# Check for insufficient_fee_retries usage
rg -n "insufficient_fee_retries" -B 3 -A 3Repository: OpenZeppelin/openzeppelin-relayer
Length of output: 50389
🏁 Script executed:
#!/bin/bash
# Get full signature of validate_user_fee_payment_amounts including parameters
sed -n '963,1020p' src/domain/transaction/stellar/validation.rs | head -60
# Check validate_user_fee_payment_transaction full signature
sed -n '912,945p' src/domain/transaction/stellar/validation.rs
# Check call sites in fee_bump.rs and stellar_relayer.rs
sed -n '50,80p' src/domain/transaction/stellar/prepare/fee_bump.rs
sed -n '945,965p' src/domain/relayer/stellar/stellar_relayer.rsRepository: OpenZeppelin/openzeppelin-relayer
Length of output: 5651
🏁 Script executed:
#!/bin/bash
# Check estimate_fee signature and usage of retries parameter
sed -n '881,950p' src/domain/transaction/stellar/utils.rs
# Check if compute_escalated_inclusion_fee is used in estimate_fee
rg -n "compute_escalated_inclusion_fee" src/domain/transaction/stellar/utils.rsRepository: OpenZeppelin/openzeppelin-relayer
Length of output: 3581
Thread insufficient_fee_retries through validation to ensure correct fee estimation during retries.
The estimate_fee(..., 0) call always uses the base inclusion fee (100 stroops). During transaction retries, insufficient_fee_retries escalates this fee by 10x per retry (1,000 → 10,000 → 100,000 stroops). Validating with retries=0 will underestimate the required fee during retries and allow insufficient fee payments to pass validation.
Modify validate_user_fee_payment_amounts to accept insufficient_fee_retries: u32 and pass it to estimate_fee, or add it to the caller's signature chain.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/domain/transaction/stellar/validation.rs` around lines 1002 - 1009, The
validation currently calls estimate_fee(envelope, provider, None, 0) which
hardcodes retries=0 and underestimates fees; update
validate_user_fee_payment_amounts to accept an insufficient_fee_retries: u32
parameter and pass that value into estimate_fee (i.e., estimate_fee(envelope,
provider, None, insufficient_fee_retries)). Propagate this new parameter through
the caller chain (or add it to upstream function signatures) so the real retry
count used by insufficient_fee_retries is threaded into
validate_user_fee_payment_amounts and into any other call sites that invoke it;
ensure the type is u32 where declared and update all call sites accordingly.
zeljkoX
reviewed
Feb 25, 2026
Collaborator
zeljkoX
left a comment
zeljkoX
left a comment
There was a problem hiding this comment.
Thanks for working on this one.
I think we need slightly different approach.
I think we do not need to bump fee by 10x because tx was never accepted to mempool, it is not duplicate or similar.
We should reuse get fee stats and decide on next bump value.
Let me share more details via DM.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@zeljkoX if you can take a look 👀 on this PR, I will leave this as a draft for now.
Testing Process
Checklist
Note
If you are using Relayer in your stack, consider adding your team or organization to our list of Relayer Users in the Wild!
Summary by CodeRabbit
New Features
Chores