Various fixes, part 5

CHANGELOG.md

@@ -179,6 +179,18 @@
 - `TransactionBuilder.addSacTransferOperation` now supports muxed (M...)
   addresses for the destination and source. Previously, passing muxed addresses
   caused `Keypair.fromPublicKey` to throw.
+- `ScInt` auto-type selection now correctly classifies negative boundary values
+  (e.g., `-(2^63)` fits `i64`, not `i128`). The previous bit-length calculation
+  was off by one for negative `BigInt` values.
+- `changeTrust` now handles `line` internally as a fallback for `asset`,
+  fixing round-trip compatibility when feeding the output of
+  `Operation.fromXDRObject` (which returns `line`) back into the builder.
+- `manageData` now accepts `undefined` for `opts.value` (treated as a
+  delete-entry), fixing round-trip compatibility with `Operation.fromXDRObject`
+  which returns `undefined` for absent optional fields.
+- `TransactionBuilder` constructor now validates `timebounds` and
+  `ledgerbounds`: negative values and `min > max` now throw immediately instead
+  of producing silently invalid transactions.
 
 ## [`v14.1.0`](https://github.com/stellar/js-stellar-base/compare/v14.0.4...v14.1.0):
 

src/numbers/sc_int.ts

@@ -108,9 +108,17 @@ export class ScInt extends XdrLargeInt {
 }
 
 function nearestBigIntSize(bigI: bigint): number {
-  // Note: Even though BigInt.toString(2) includes the negative sign for
-  // negative values (???), the following is still accurate, because the
-  // negative sign would be represented by a sign bit.
+  if (bigI < 0n) {
+    // Two's complement: N bits represent -(2^(N-1)) to 2^(N-1)-1.
+    // For negative values, compute the signed bit width as
+    // (bitlen of abs-1) + 1 to account for the sign bit. This correctly
+    // classifies -(2^63) as 64 bits (fits i64) and -(2^63)-1 as 65 bits
+    // (needs i128).
+    const abs = -bigI;
+    const bitlen = (abs - 1n).toString(2).length + 1;
+    return [64, 128, 256].find((len) => bitlen <= len) ?? bitlen;
+  }
+
   const bitlen = bigI.toString(2).length;
   return [64, 128, 256].find((len) => bitlen <= len) ?? bitlen;
 }

src/operations/change_trust.ts

@@ -28,12 +28,17 @@ const MAX_INT64 = "9223372036854775807";
 export function changeTrust(
   opts: ChangeTrustOpts,
 ): xdr.Operation<ChangeTrustResult> {
+  // Accept `line` as an alias for `asset` so that the output of
+  // fromXDRObject (which uses `line`) can round-trip back through here.
+  const asset =
+    opts.asset ??
+    (opts as unknown as { line?: Asset | LiquidityPoolAsset }).line;
   let line: xdr.ChangeTrustAsset;
 
-  if (opts.asset instanceof Asset) {
-    line = opts.asset.toChangeTrustXDRObject();
-  } else if (opts.asset instanceof LiquidityPoolAsset) {
-    line = opts.asset.toXDRObject();
+  if (asset instanceof Asset) {
+    line = asset.toChangeTrustXDRObject();
+  } else if (asset instanceof LiquidityPoolAsset) {
+    line = asset.toXDRObject();
   } else {
     throw new TypeError("asset must be Asset or LiquidityPoolAsset");
   }

src/operations/manage_data.ts

@@ -20,10 +20,14 @@ export function manageData(
     throw new Error("name must be a string, up to 64 characters");
   }
 
+  // undefined is accepted (treated as null/delete) for internal round-trip
+  // compatibility: fromXDRObject returns undefined for absent optional fields.
+  // The public API contract is null for delete-entry.
   if (
     typeof opts.value !== "string" &&
     !Buffer.isBuffer(opts.value) &&
-    opts.value !== null
+    opts.value !== null &&
+    opts.value !== undefined
   ) {
     throw new Error("value must be a string, Buffer or null");
   }
@@ -32,7 +36,7 @@ export function manageData(
   if (typeof opts.value === "string") {
     dataValue = Buffer.from(opts.value);
   } else {
-    dataValue = opts.value;
+    dataValue = opts.value ?? null;
   }
 
   if (dataValue !== null && dataValue.length > 64) {

src/transaction_builder.ts

@@ -185,8 +185,57 @@ export class TransactionBuilder {
     this.operations = [];
 
     this.baseFee = opts.fee;
-    this.timebounds = opts.timebounds ? { ...opts.timebounds } : null;
-    this.ledgerbounds = opts.ledgerbounds ? { ...opts.ledgerbounds } : null;
+    if (opts.timebounds) {
+      const minTime = toEpochSeconds(opts.timebounds.minTime);
+      const maxTime = toEpochSeconds(opts.timebounds.maxTime);
+
+      if (minTime !== undefined && minTime < 0) {
+        throw new Error("min_time cannot be negative");
+      }
+
+      if (maxTime !== undefined && maxTime < 0) {
+        throw new Error("max_time cannot be negative");
+      }
+
+      if (
+        minTime !== undefined &&
+        maxTime !== undefined &&
+        maxTime > 0 &&
+        minTime > maxTime
+      ) {
+        throw new Error("min_time cannot be greater than max_time");
+      }
+
+      this.timebounds = { ...opts.timebounds };
+    } else {
+      this.timebounds = null;
+    }
+
+    if (opts.ledgerbounds) {
+      const minLedger = opts.ledgerbounds.minLedger;
+      const maxLedger = opts.ledgerbounds.maxLedger;
+
+      if (minLedger !== undefined && minLedger < 0) {
+        throw new Error("min_ledger cannot be negative");
+      }
+
+      if (maxLedger !== undefined && maxLedger < 0) {
+        throw new Error("max_ledger cannot be negative");
+      }
+
+      if (
+        minLedger !== undefined &&
+        maxLedger !== undefined &&
+        maxLedger > 0 &&
+        minLedger > maxLedger
+      ) {
+        throw new Error("min_ledger cannot be greater than max_ledger");
+      }
+
+      this.ledgerbounds = { ...opts.ledgerbounds };
+    } else {
+      this.ledgerbounds = null;
+    }
     this.minAccountSequence = opts.minAccountSequence || null;
     this.minAccountSequenceAge =
       opts.minAccountSequenceAge !== undefined
@@ -1150,3 +1199,24 @@ export class TransactionBuilder {
 export function isValidDate(d: Date | number | string): d is Date {
   return d instanceof Date && !Number.isNaN(d.getTime());
 }
+
+/**
+ * Converts a Date, number, or string time value to epoch seconds for
+ * validation. Returns undefined if the value is undefined.
+ */
+function toEpochSeconds(
+  value: Date | number | string | undefined,
+): number | undefined {
+  if (value === undefined) {
+    return undefined;
+  }
+
+  const num =
+    value instanceof Date ? Math.floor(value.getTime() / 1000) : Number(value);
+
+  if (!Number.isFinite(num) || num % 1 !== 0) {
+    throw new Error("timebounds value must be a finite integer or Date");
+  }
+
+  return num;
+}

test/unit/numbers/sc_int.test.ts

@@ -161,6 +161,43 @@ describe("ScInt", () => {
       expect(sci.type).toBe("i64");
     });
 
+    it("selects i64 for the exact i64 minimum (-(2^63))", () => {
+      const min = -(2n ** 63n);
+      const sci = new ScInt(min);
+      expect(sci.type).toBe("i64");
+    });
+
+    it("selects i128 for the exact i128 minimum (-(2^127))", () => {
+      const min = -(2n ** 127n);
+      const sci = new ScInt(min);
+      expect(sci.type).toBe("i128");
+    });
+
+    it("selects i256 for the exact i256 minimum (-(2^255))", () => {
+      const min = -(2n ** 255n);
+      const sci = new ScInt(min);
+      expect(sci.type).toBe("i256");
+    });
+
+    it("selects i128 for -(2^63)-1 (just below i64 minimum)", () => {
+      const val = -(2n ** 63n) - 1n;
+      const sci = new ScInt(val);
+      expect(sci.type).toBe("i128");
+      expect(sci.toBigInt()).toBe(val);
+    });
+
+    it("selects i256 for -(2^127)-1 (just below i128 minimum)", () => {
+      const val = -(2n ** 127n) - 1n;
+      const sci = new ScInt(val);
+      expect(sci.type).toBe("i256");
+      expect(sci.toBigInt()).toBe(val);
+    });
+
+    it("throws for -(2^255)-1 (below i256 minimum)", () => {
+      const val = -(2n ** 255n) - 1n;
+      expect(() => new ScInt(val)).toThrow(RangeError);
+    });
+
     it("selects i128 for negative numbers beyond i64 range", () => {
       const val = -(1n << 64n);
       const sci = new ScInt(val);

test/unit/operations/change_trust.test.ts

@@ -103,6 +103,34 @@ describe("Operation.changeTrust()", () => {
     ).toThrow(TypeError);
   });
 
+  it("round-trips an Asset changeTrust through fromXDRObject and back", () => {
+    const op = Operation.changeTrust({ asset: usd, limit: "50.0000000" });
+    const xdrHex = op.toXDR("hex");
+    const parsed = expectOperationType(
+      Operation.fromXDRObject(xdr.Operation.fromXDR(xdrHex, "hex")),
+      "changeTrust",
+    );
+
+    // parsed has `line` (not `asset`); changeTrust accepts both
+    const rebuilt = Operation.changeTrust(parsed);
+    expect(rebuilt).toBeInstanceOf(xdr.Operation);
+    expect(rebuilt.toXDR("hex")).toBe(xdrHex);
+  });
+
+  it("round-trips a LiquidityPoolAsset changeTrust through fromXDRObject and back", () => {
+    const op = Operation.changeTrust({ asset: lpAsset });
+    const xdrHex = op.toXDR("hex");
+    const parsed = expectOperationType(
+      Operation.fromXDRObject(xdr.Operation.fromXDR(xdrHex, "hex")),
+      "changeTrust",
+    );
+
+    // parsed has `line` (not `asset`); changeTrust accepts both
+    const rebuilt = Operation.changeTrust(parsed);
+    expect(rebuilt).toBeInstanceOf(xdr.Operation);
+    expect(rebuilt.toXDR("hex")).toBe(xdrHex);
+  });
+
   it("preserves an optional source account", () => {
     const source = "GCEZWKCA5VLDNRLN3RPRJMRZOX3Z6G5CHCGSNFHEYVXM3XOJMDS674JZ";
     const op = Operation.changeTrust({ asset: usd, source });

test/unit/operations/manage_data.test.ts

@@ -46,6 +46,24 @@ describe("Operation.manageData()", () => {
     expect(obj.value).toBeUndefined();
   });
 
+  it("round-trips a null-value (delete) manageData through fromXDRObject and back", () => {
+    const op = Operation.manageData({ name: "test", value: null });
+    const xdrHex = op.toXDR("hex");
+    const operation = xdr.Operation.fromXDR(Buffer.from(xdrHex, "hex"));
+    const parsed = expectOperationType(
+      Operation.fromXDRObject(operation),
+      "manageData",
+    );
+
+    // Rebuilding from the parsed result must not throw
+    const rebuilt = Operation.manageData({
+      name: parsed.name,
+      value: parsed.value,
+    });
+    expect(rebuilt).toBeInstanceOf(xdr.Operation);
+    expect(rebuilt.toXDR("hex")).toBe(xdrHex);
+  });
+
   it("creates a manageData operation with source account", () => {
     const source = "GCEZWKCA5VLDNRLN3RPRJMRZOX3Z6G5CHCGSNFHEYVXM3XOJMDS674JZ";
     const opts = { name: "test", value: "data", source };