Skip to content

fix(xrp): Don't set LastLedgerSequence when not provided in SigningInput#4732

Open
MJamalian wants to merge 1 commit intotrustwallet:masterfrom
MJamalian:fix/xrp-last-ledger-sequence-zero
Open

fix(xrp): Don't set LastLedgerSequence when not provided in SigningInput#4732
MJamalian wants to merge 1 commit intotrustwallet:masterfrom
MJamalian:fix/xrp-last-ledger-sequence-zero

Conversation

@MJamalian
Copy link
Copy Markdown

Summary

  • Guard LastLedgerSequence assignment in both build_tx_json and prepare_builder to only set the field when the proto input value is non-zero
  • When not provided (proto default 0), the field stays None and is omitted from serialization, matching the old C++ behavior

Problem

After the C++ → Rust migration in #4250, XRP transactions that don't explicitly set lastLedgerSequence in SigningInput get LastLedgerSequence = 0 in the signed output. This means "expire at ledger 0", causing immediate transaction expiry on the XRP Ledger.

The root cause is in protobuf_builder.rs:

// build_tx_json path:
if self.input.last_ledger_sequence != 0 || tx.common_fields.last_ledger_sequence.is_none() {
    tx.common_fields.last_ledger_sequence = Some(self.input.last_ledger_sequence);
}

// prepare_builder path:
builder.last_ledger_sequence(self.input.last_ledger_sequence)

When lastLedgerSequence is not provided, the proto default is 0. The condition false || true = true sets Some(0), and prepare_builder unconditionally sets it. Since CommonFields.last_ledger_sequence is Option<u32> with skip_serializing_if = "Option::is_none", Some(0) gets serialized into the transaction.

Fix

Only set the field when the input value is non-zero:

// build_tx_json path:
if self.input.last_ledger_sequence != 0 {
    tx.common_fields.last_ledger_sequence = Some(self.input.last_ledger_sequence);
}

// prepare_builder path:
if self.input.last_ledger_sequence != 0 {
    builder.last_ledger_sequence(self.input.last_ledger_sequence);
}

Note: the same pattern (!= 0 guard) is already correctly used for source_tag on line 339.

Test plan

  • All existing Ripple tests pass unchanged (they all use non-zero last_ledger_sequence values)
  • Verified with real XRP mainnet transactions that the fix produces valid output

Fixes #4731

When `lastLedgerSequence` is not set in SigningInput, the proto default
is 0. The Rust migration (trustwallet#4250) unconditionally set this value on the
transaction, producing `LastLedgerSequence = 0` which means "expire at
ledger 0" — causing immediate transaction expiry on the XRP Ledger.

The old C++ implementation correctly omitted the field when not provided.

This fix guards both `build_tx_json` and `prepare_builder` paths to only
set `LastLedgerSequence` when the input value is non-zero.

Fixes trustwallet#4731
@MJamalian MJamalian requested a review from a team as a code owner April 11, 2026 14:08
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.

XRP: LastLedgerSequence set to 0 when not provided, causing immediate transaction expiry

1 participant