Fix double-counting resourceFee

CHANGELOG.md

@@ -6,6 +6,10 @@ A breaking change will get clearly marked in this log.
 
 ## Unreleased
 
+### Fixed
+* Fix `assembleTransaction` double-counting the resource fee when the input transaction already has Soroban data attached (e.g. when re-assembling a previously simulated transaction).
+* Removed `resourceFee` adding in `assembleTransaction` as its now handled by `TransactionBuilder.build()`.
+
 ## [v14.6.0](https://github.com/stellar/js-stellar-sdk/compare/v14.5.0...v14.6.0)
 
 ### Added

src/rpc/transaction.ts

@@ -66,8 +66,23 @@ export function assembleTransaction(
     throw new Error(`simulation incorrect: ${JSON.stringify(success)}`);
   }
 
-  const classicFeeNum = parseInt(raw.fee) || 0;
-  const minResourceFeeNum = parseInt(success.minResourceFee) || 0;
+  let classicFeeNum;
+  try {
+    classicFeeNum = BigInt(raw.fee);
+  } catch {
+    classicFeeNum = BigInt(0);
+  }
+
+  const rawSorobanData = raw.toEnvelope().v1().tx().ext().value();
+
+  // If the incoming raw transaction already has Soroban data,
+  // we need to be careful to handle the fee to prevent double-counting,
+  if (rawSorobanData) {
+    if (classicFeeNum - rawSorobanData.resourceFee().toBigInt() > BigInt(0)) {
+      classicFeeNum -= rawSorobanData.resourceFee().toBigInt();
+    }
+  }
+
   const txnBuilder = TransactionBuilder.cloneFrom(raw, {
     // automatically update the tx fee that will be set on the resulting tx to
     // the sum of 'classic' fee provided from incoming tx.fee and minResourceFee
@@ -77,7 +92,7 @@ export function assembleTransaction(
     // operations', In soroban contract tx, there can only be single operation
     // in the tx, so can make simplification of total classic fees for the
     // soroban transaction will be equal to incoming tx.fee + minResourceFee.
-    fee: (classicFeeNum + minResourceFeeNum).toString(),
+    fee: classicFeeNum.toString(),
     // apply the pre-built Soroban Tx Data from simulation onto the Tx
     sorobanData: success.transactionData.build(),
     networkPassphrase: raw.networkPassphrase,

test/unit/transaction.test.ts

@@ -88,7 +88,7 @@
 
       // validate it auto updated the tx fees from sim response fees
       // since it was greater than tx.fee
       expect(result.toEnvelope().v1().tx().fee()).toBe(215);
 
       // validate it updated sorobantransactiondata block in the tx ext
       expect(result.toEnvelope().v1().tx().ext().sorobanData()).toEqual(
@@ -220,5 +220,82 @@
         `auths aren't preserved after simulation: ${simulationResponse}, ${tx}`,
       ).toEqual(3);
     });
+
+    describe("fee handling with pre-existing soroban data", () => {
+      const oldResourceFee = 500;
+      const newResourceFee = 115; // from simulationResponse.minResourceFee
+
+      // Build soroban data with a non-zero resourceFee to simulate
+      // a transaction that was previously assembled/simulated
+      const preExistingSorobanData = new StellarSdk.SorobanDataBuilder()
+        .setResources(0, 5, 0)
+        .setResourceFee(oldResourceFee)
+        .build();
+
+      function txWithPreExistingSorobanData(fee: string) {
+        return new StellarSdk.TransactionBuilder(source, {
+          fee,
+          networkPassphrase,
+        })
+          .setTimeout(StellarSdk.TimeoutInfinite)
+          .setSorobanData(preExistingSorobanData)
+          .addOperation(
+            StellarSdk.Operation.invokeHostFunction({
+              func: xdr.HostFunction.hostFunctionTypeInvokeContract(
+                new xdr.InvokeContractArgs({
+                  contractAddress: scAddress,
+                  functionName: "hello",
+                  args: [xdr.ScVal.scvString("hello")],
+                }),
+              ),
+              auth: [],
+            }),
+          )
+          .build();
+      }
+
+      it("subtracts old resourceFee from raw.fee to avoid double-counting", () => {
+        // Build with fee=100. After build(), the tx.fee = 100 + oldResourceFee = 600.
+        const txn = txWithPreExistingSorobanData("100");
+        expect(parseInt(txn.fee)).toBe(100 + oldResourceFee);
+
+        const result = rpc.assembleTransaction(txn, simulationResponse).build();
+        const finalFee = result.toEnvelope().v1().tx().fee();
+
+        // assembleTransaction should strip old resourceFee from tx.fee (600 - 500 = 100),
+        // then build() adds the new resourceFee (100 + 115 = 215)
+        expect(finalFee).toBe(100 + newResourceFee);
+      });
+
+      it("handles re-assembly with a higher classic fee", () => {
+        const txn = txWithPreExistingSorobanData("300");
+        // tx.fee = 300 + oldResourceFee = 800
+        expect(parseInt(txn.fee)).toBe(300 + oldResourceFee);
+
+        const result = rpc.assembleTransaction(txn, simulationResponse).build();
+        const finalFee = result.toEnvelope().v1().tx().fee();
+
+        // 800 - 500 = 300 classic, then build() adds 115 = 415
+        expect(finalFee).toBe(300 + newResourceFee);
+      });
+
+      it("assembling twice produces the same fee as assembling once", () => {
+        // First assembly: no pre-existing soroban data
+        const freshTxn = singleContractFnTransaction(undefined);
+        const firstAssembly = rpc
+          .assembleTransaction(freshTxn, simulationResponse)
+          .build();
+        const feeAfterFirst = firstAssembly.toEnvelope().v1().tx().fee();
+
+        // Second assembly: re-assemble the already-assembled transaction
+        const secondAssembly = rpc
+          .assembleTransaction(firstAssembly, simulationResponse)
+          .build();
+        const feeAfterSecond = secondAssembly.toEnvelope().v1().tx().fee();
+
+        // Should be identical — no double-counting
+        expect(feeAfterSecond).toBe(feeAfterFirst);
+      });
+    });
   });
 });