Fix TransactionBuilder to exclude resource fee when deriving base fee in cloneFrom (#1478)

Author: Ryang-21

CHANGELOG.md

@@ -6,6 +6,15 @@ A breaking change will get clearly marked in this log.
 
 ## Unreleased
 
+### Fixed
+- `TransactionBuilder.cloneFrom` now accounts for Soroban transactions: the
+  resource fee is excluded from the total fee before deriving the per-operation
+  base fee (previously it was treated as part of the inclusion fee and re-added
+  on `build()`, doubling it), and the transaction's `sorobanData` is carried
+  over into the new builder. If the declared resource fee equals or exceeds the
+  total fee (a malformed transaction), the subtraction is skipped and the full
+  fee is used as-is.
+
 ## [v16.0.0-rc.2](https://github.com/stellar/js-stellar-sdk/compare/v16.0.0-rc.1...v16.0.0-rc.2)
 
 ### Changed

docs/reference/core-transactions.md

@@ -2964,7 +2964,7 @@ static buildFeeBumpTransaction(feeSource: string | Keypair, baseFee: string, inn
 
 - https://developers.stellar.org/docs/glossary/fee-bumps/#replace-by-fee
 
-**Source:** [src/base/transaction_builder.ts:1092](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L1092)
+**Source:** [src/base/transaction_builder.ts:1114](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L1114)
 
 ### `TransactionBuilder.cloneFrom(tx, opts)`
 
@@ -3014,7 +3014,7 @@ static fromXDR(envelope: string | TransactionEnvelope, networkPassphrase: string
       Stellar network (e.g. "Public Global Stellar Network ; September
       2015"), see `Networks`.
 
-**Source:** [src/base/transaction_builder.ts:1203](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L1203)
+**Source:** [src/base/transaction_builder.ts:1225](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L1225)
 
 ### `transactionBuilder.baseFee`
 
@@ -3124,7 +3124,7 @@ addMemo(memo: Memo): TransactionBuilder;
 
 - **`memo`** — `Memo` (required) — `Memo` object
 
-**Source:** [src/base/transaction_builder.ts:394](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L394)
+**Source:** [src/base/transaction_builder.ts:416](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L416)
 
 ### `transactionBuilder.addOperation(operation)`
 
@@ -3138,7 +3138,7 @@ addOperation(operation: Operation2): TransactionBuilder;
 
 - **`operation`** — `Operation2` (required) — The xdr operation object, use `Operation` static methods.
 
-**Source:** [src/base/transaction_builder.ts:356](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L356)
+**Source:** [src/base/transaction_builder.ts:378](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L378)
 
 ### `transactionBuilder.addOperationAt(operation, index)`
 
@@ -3153,7 +3153,7 @@ addOperationAt(operation: Operation2, index: number): TransactionBuilder;
 - **`operation`** — `Operation2` (required) — The xdr operation object to add, use `Operation` static methods.
 - **`index`** — `number` (required) — The index at which to insert the operation.
 
-**Source:** [src/base/transaction_builder.ts:367](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L367)
+**Source:** [src/base/transaction_builder.ts:389](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L389)
 
 ### `transactionBuilder.addSacTransferOperation(destination, asset, amount, sorobanFees)`
 
@@ -3172,7 +3172,7 @@ addSacTransferOperation(destination: string, asset: Asset, amount: string | bigi
 - **`amount`** — `string | bigint` (required) — the amount of tokens to be transferred in 7 decimals. IE 1 token with 7 decimals of precision would be represented as "1_0000000"
 - **`sorobanFees`** — `SorobanFees` (optional) — optional Soroban fees for the transaction to override the default fees used
 
-**Source:** [src/base/transaction_builder.ts:701](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L701)
+**Source:** [src/base/transaction_builder.ts:723](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L723)
 
 ### `transactionBuilder.build()`
 
@@ -3183,7 +3183,7 @@ number by 1.
 build(): Transaction;
 ```
 
-**Source:** [src/base/transaction_builder.ts:921](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L921)
+**Source:** [src/base/transaction_builder.ts:943](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L943)
 
 ### `transactionBuilder.clearOperationAt(index)`
 
@@ -3197,7 +3197,7 @@ clearOperationAt(index: number): TransactionBuilder;
 
 - **`index`** — `number` (required) — The index of the operation to remove.
 
-**Source:** [src/base/transaction_builder.ts:385](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L385)
+**Source:** [src/base/transaction_builder.ts:407](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L407)
 
 ### `transactionBuilder.clearOperations()`
 
@@ -3207,7 +3207,7 @@ Removes the operations from the builder (useful when cloning).
 clearOperations(): TransactionBuilder;
 ```
 
-**Source:** [src/base/transaction_builder.ts:375](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L375)
+**Source:** [src/base/transaction_builder.ts:397](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L397)
 
 ### `transactionBuilder.hasV2Preconditions()`
 
@@ -3217,7 +3217,7 @@ Checks whether any v2 preconditions have been set on this builder.
 hasV2Preconditions(): boolean;
 ```
 
-**Source:** [src/base/transaction_builder.ts:1060](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L1060)
+**Source:** [src/base/transaction_builder.ts:1082](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L1082)
 
 ### `transactionBuilder.setExtraSigners(extraSigners)`
 
@@ -3234,7 +3234,7 @@ setExtraSigners(extraSigners: string[]): TransactionBuilder;
 
 - **`extraSigners`** — `string[]` (required) — required extra signers (as `StrKey`s)
 
-**Source:** [src/base/transaction_builder.ts:636](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L636)
+**Source:** [src/base/transaction_builder.ts:658](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L658)
 
 ### `transactionBuilder.setLedgerbounds(minLedger, maxLedger)`
 
@@ -3255,7 +3255,7 @@ setLedgerbounds(minLedger: number, maxLedger: number): TransactionBuilder;
       before. Cannot be negative. If the value is `0`, the transaction is
       valid indefinitely.
 
-**Source:** [src/base/transaction_builder.ts:524](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L524)
+**Source:** [src/base/transaction_builder.ts:546](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L546)
 
 ### `transactionBuilder.setMinAccountSequence(minAccountSequence)`
 
@@ -3279,7 +3279,7 @@ setMinAccountSequence(minAccountSequence: string): TransactionBuilder;
       default), the transaction is valid when `sourceAccount`'s sequence
       number `== tx.seqNum - 1`.
 
-**Source:** [src/base/transaction_builder.ts:561](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L561)
+**Source:** [src/base/transaction_builder.ts:583](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L583)
 
 ### `transactionBuilder.setMinAccountSequenceAge(durationInSeconds)`
 
@@ -3298,7 +3298,7 @@ setMinAccountSequenceAge(durationInSeconds: bigint): TransactionBuilder;
       will become valid. If the value is `0`, the transaction is unrestricted
       by the account sequence age. Cannot be negative.
 
-**Source:** [src/base/transaction_builder.ts:583](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L583)
+**Source:** [src/base/transaction_builder.ts:605](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L605)
 
 ### `transactionBuilder.setMinAccountSequenceLedgerGap(gap)`
 
@@ -3317,7 +3317,7 @@ setMinAccountSequenceLedgerGap(gap: number): TransactionBuilder;
       If the value is `0`, the transaction is unrestricted by the account
       sequence ledger. Cannot be negative.
 
-**Source:** [src/base/transaction_builder.ts:612](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L612)
+**Source:** [src/base/transaction_builder.ts:634](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L634)
 
 ### `transactionBuilder.setNetworkPassphrase(networkPassphrase)`
 
@@ -3332,7 +3332,7 @@ setNetworkPassphrase(networkPassphrase: string): TransactionBuilder;
 - **`networkPassphrase`** — `string` (required) — passphrase of the target Stellar
       network (e.g. "Public Global Stellar Network ; September 2015").
 
-**Source:** [src/base/transaction_builder.ts:662](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L662)
+**Source:** [src/base/transaction_builder.ts:684](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L684)
 
 ### `transactionBuilder.setSorobanData(sorobanData)`
 
@@ -3360,7 +3360,7 @@ setSorobanData(sorobanData: string | SorobanTransactionData): TransactionBuilder
 
 - `SorobanDataBuilder`
 
-**Source:** [src/base/transaction_builder.ts:684](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L684)
+**Source:** [src/base/transaction_builder.ts:706](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L706)
 
 ### `transactionBuilder.setTimebounds(minEpochOrDate, maxEpochOrDate)`
 
@@ -3384,7 +3384,7 @@ setTimebounds(minEpochOrDate: number | Date, maxEpochOrDate: number | Date): Tra
       Can't be negative. If the value is `0`, the transaction is valid
       indefinitely.
 
-**Source:** [src/base/transaction_builder.ts:475](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L475)
+**Source:** [src/base/transaction_builder.ts:497](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L497)
 
 ### `transactionBuilder.setTimeout(timeoutSeconds)`
 
@@ -3424,7 +3424,7 @@ setTimeout(timeoutSeconds: number): TransactionBuilder;
 - - `TimeoutInfinite`
  - https://developers.stellar.org/docs/tutorials/handling-errors/
 
-**Source:** [src/base/transaction_builder.ts:428](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L428)
+**Source:** [src/base/transaction_builder.ts:450](https://github.com/stellar/js-stellar-sdk/blob/main/src/base/transaction_builder.ts#L450)
 
 ## Uint128
 

src/base/transaction_builder.ts

@@ -306,16 +306,38 @@ export class TransactionBuilder {
       );
     }
 
+    // if this is a Soroban transaction, the resource fee is folded into the
+    // total fee and gets re-added on build(), so it has to be excluded before
+    // deriving the per-operation base fee
+    let sorobanData: xdr.SorobanTransactionData | undefined;
+    const envelope = tx.toEnvelope();
+    if (envelope.switch() === xdr.EnvelopeType.envelopeTypeTx()) {
+      sorobanData = envelope.v1().tx().ext().value() ?? undefined;
+    }
+    let totalFee = parseInt(tx.fee, 10);
+    if (sorobanData) {
+      const resourceFee = Number(sorobanData.resourceFee().toBigInt());
+      // only subtract if it leaves a positive inclusion fee: a malformed tx
+      // can declare a resource fee that eats the entire fee (or more)
+      if (totalFee - resourceFee > 0) {
+        totalFee -= resourceFee;
+      }
+    }
+
     // the initial fee passed to the builder gets scaled up based on the number
     // of operations at the end, so we have to down-scale first
-    const unscaledFee = Math.floor(parseInt(tx.fee, 10) / tx.operations.length);
+    const unscaledFee = Math.floor(totalFee / tx.operations.length);
 
     const builderOpts: TransactionBuilderOptions = {
       fee: (unscaledFee || BASE_FEE).toString(),
       memo: tx.memo,
       networkPassphrase: tx.networkPassphrase,
     };
 
+    if (sorobanData) {
+      builderOpts.sorobanData = sorobanData;
+    }
+
     if (tx.timeBounds) {
       builderOpts.timebounds = tx.timeBounds;
     }

test/unit/base/transaction_builder.test.ts

@@ -1704,6 +1704,56 @@ describe("TransactionBuilder.cloneFrom", () => {
     );
   });
 
+  it("excludes the resource fee when deriving the base fee", () => {
+    const resourceFee = "50000";
+    const tx = new TransactionBuilder(source, {
+      fee: "150",
+      timebounds: { minTime: 0, maxTime: 0 },
+      networkPassphrase,
+    })
+      .addOperation(op)
+      .setSorobanData(
+        new SorobanDataBuilder().setResourceFee(resourceFee).build(),
+      )
+      .build();
+
+    // total fee = baseFee * ops + resourceFee
+    expect(tx.fee).toBe("50150");
+
+    const builder = TransactionBuilder.cloneFrom(tx);
+    // base fee must be derived from the inclusion fee only
+    expect(builder.baseFee).toBe("150");
+
+    const cloneTx = builder.build();
+    expect(cloneTx.fee).toBe(tx.fee);
+    expect(
+      expectDefined(cloneTx.toEnvelope().v1().tx().ext().value())
+        .resourceFee()
+        .toString(),
+    ).toBe(resourceFee);
+  });
+
+  it("skips the resource fee subtraction when it would zero out the fee", () => {
+    const tx = new TransactionBuilder(source, {
+      fee: "150",
+      timebounds: { minTime: 0, maxTime: 0 },
+      networkPassphrase,
+    })
+      .addOperation(op)
+      .setSorobanData(new SorobanDataBuilder().setResourceFee("50000").build())
+      .build();
+
+    // simulate a malformed tx whose declared resource fee exceeds the total
+    // fee by patching the XDR fee field directly
+    const envelope = tx.toEnvelope();
+    envelope.v1().tx().fee(100);
+    const malformedTx = new Transaction(envelope, networkPassphrase);
+
+    const builder = TransactionBuilder.cloneFrom(malformedTx);
+    // subtraction would go negative, so the full fee is used instead
+    expect(builder.baseFee).toBe("100");
+  });
+
   it("preserves extraSigners", () => {
     const extraSigner =
       "GA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVSGZ";