Various fixes, part 3

CHANGELOG.md

@@ -22,10 +22,6 @@
   StrKey strings.
 - `Transaction` no longer has generic type parameters `<TMemo, TOps>`. Code like
   `Transaction<Memo<MemoType.Text>>` will no longer compile.
-- The default `networkPassphrase` for `authorizeEntry` and
-  `authorizeInvocation` has changed from `Networks.FUTURENET` to
-  `Networks.TESTNET`. Callers omitting this argument will silently produce
-  signatures for a different network.
 - `Operation.isValidAmount()`, `Operation.constructAmountRequirementsError()`, and `Operation.setSourceAccount()` have been removed from the runtime `Operation` class. They now exist only as internal standalone functions in operations.ts and are not re-exported. These methods were never part of the published TypeScript declarations, but JavaScript callers could still access them before.
 - The revoke sponsorship operation `type` field has been split from a single
   `"revokeSponsorship"` into 7 specific strings: `"revokeAccountSponsorship"`,
@@ -84,6 +80,20 @@
   `parseFloat()` to `Number()`. Strings with trailing non-numeric characters
   (e.g., `"123abc"`) that previously silently succeeded will now be rejected.
 
+- `authorizeInvocation()` now takes a single `AuthorizeInvocationParams`
+  object instead of positional arguments. Callers must switch from
+  `authorizeInvocation(signer, validUntilLedgerSeq, invocation, publicKey,
+  networkPassphrase)` to
+  `authorizeInvocation({ signer, validUntilLedgerSeq, invocation,
+  networkPassphrase, publicKey })`.
+- `authorizeEntry()` no longer defaults `networkPassphrase` to
+  `Networks.TESTNET`. Callers that relied on the default must now pass the
+  network passphrase explicitly.
+- `TransactionBase.tx` now returns a defensive copy of the underlying XDR
+  object. External mutations on the returned value no longer affect the
+  transaction that will be signed or serialized. Code that relied on
+  mutating `tx` directly will need to be updated.
+
 ### Added
 
 - `Address` class now has a `type` getter and `AddressType` type export.
@@ -107,6 +117,25 @@
 
 ### Fixed
 
+- `nativeToScVal` now uses `Object.getPrototypeOf(val) !== Object.prototype`
+  instead of `val.constructor?.name !== "Object"` to detect plain objects,
+  fixing false negatives for objects created across different realms or with
+  overridden `constructor` properties.
+- `nativeToScVal` now uses `Object.hasOwn` when looking up per-key type
+  specs in map conversions, preventing inherited properties from the
+  prototype chain from being used as type hints.
+- `nativeToScVal` now validates bounds for `u32`/`i32` types, throwing
+  `TypeError` for out-of-range values that previously passed through to the XDR
+  layer.
+- `best_r` (continued fraction approximation) no longer throws for small
+  numbers whose reciprocal exceeds `MAX_INT`. It now computes a
+  semi-convergent to recover a valid rational approximation.
+- `MuxedAccount` constructor and `setId` now validate that the `id` string
+  represents a valid uint64 value (0 to 2^64 - 1), throwing immediately on
+  out-of-range or non-numeric input.
+- `Transaction` constructor now reads `sourceAccountEd25519()` and
+  `sourceAccount()` from the local `tx` parameter instead of `this.tx`,
+  avoiding the overhead of a defensive copy during construction.
 - `authorizeInvocation` is now stricter when called with a callback signer
   and no `publicKey` — it throws
   `"authorizeInvocation requires publicKey parameter"` instead of failing with
@@ -126,9 +155,6 @@
   `opts.signer.sha256Hash` in place.
 - `allow_trust` now throws `"authorize is required"` when the `authorize` option
   is null or undefined, instead of silently passing through.
-- `nativeToScVal` now validates bounds for `u32`/`i32` types, throwing
-  `TypeError` for out-of-range values that previously passed through to the XDR
-  layer.
 - `scvSortedMap` sort comparator now correctly returns `0` for equal values,
   fixing a violation of the sort contract.
 - `createCustomContract` error message for invalid salt now correctly prints the

src/auth.ts

@@ -2,6 +2,7 @@ import xdr from "./xdr.js";
 
 import { Keypair } from "./keypair.js";
 import { StrKey } from "./strkey.js";
+// eslint-disable-next-line @typescript-eslint/no-unused-vars
 import { Networks } from "./network.js";
 import { hash } from "./hashing.js";
 
@@ -123,7 +124,7 @@ export async function authorizeEntry(
   entry: xdr.SorobanAuthorizationEntry,
   signer: Keypair | SigningCallback,
   validUntilLedgerSeq: number,
-  networkPassphrase: string = Networks.TESTNET,
+  networkPassphrase: string,
 ): Promise<xdr.SorobanAuthorizationEntry> {
   // no-op if it's source account auth
   if (
@@ -208,32 +209,43 @@ export async function authorizeEntry(
  *
  * This is in contrast to {@link authorizeEntry}, which signs an existing entry.
  *
- * @param signer - either a {@link Keypair} instance (or anything with a
+ * @param params - the parameters for building and signing the authorization
+ * @param params.signer - either a {@link Keypair} instance (or anything with a
  *    `.sign(buf): Buffer-like` method) or a function which takes a payload (a
  *    {@link xdr.HashIdPreimageSorobanAuthorization} instance) input and returns
  *    the signature of the hash of the raw payload bytes (where the signing key
  *    should correspond to the address in the `entry`)
- * @param validUntilLedgerSeq - the (exclusive) future ledger sequence number
- *    until which this authorization entry should be valid (if
+ * @param params.validUntilLedgerSeq - the (exclusive) future ledger sequence
+ *    number until which this authorization entry should be valid (if
  *    `currentLedgerSeq==validUntilLedgerSeq`, this is expired)
- * @param invocation - the invocation tree that we're authorizing (likely, this
- *    comes from transaction simulation)
- * @param publicKey - the public identity of the signer (when providing a
+ * @param params.invocation - the invocation tree that we're authorizing
+ *    (likely, this comes from transaction simulation)
+ * @param params.networkPassphrase - the network passphrase is incorporated into
+ *    the signature (see {@link Networks} for options)
+ * @param params.publicKey - the public identity of the signer (when providing a
  *    {@link Keypair} to `signer`, this can be omitted, as it just uses
  *    {@link Keypair.publicKey})
- * @param networkPassphrase - the network passphrase is incorporated into the
- *    signature (see {@link Networks} for options, default:
- *    {@link Networks.TESTNET})
  *
  * @see authorizeEntry
  */
+export interface AuthorizeInvocationParams {
+  signer: Keypair | SigningCallback;
+  validUntilLedgerSeq: number;
+  invocation: xdr.SorobanAuthorizedInvocation;
+  networkPassphrase: string;
+  publicKey?: string;
+}
+
 export function authorizeInvocation(
-  signer: Keypair | SigningCallback,
-  validUntilLedgerSeq: number,
-  invocation: xdr.SorobanAuthorizedInvocation,
-  publicKey: string = "",
-  networkPassphrase: string = Networks.TESTNET,
+  params: AuthorizeInvocationParams,
 ): Promise<xdr.SorobanAuthorizationEntry> {
+  const {
+    signer,
+    validUntilLedgerSeq,
+    invocation,
+    networkPassphrase,
+    publicKey = "",
+  } = params;
   // We use keypairs as a source of randomness for the nonce to avoid mucking
   // with any crypto dependencies. Note that this just has to be random and
   // unique, not cryptographically secure, so it's fine.

src/muxed_account.ts

@@ -8,6 +8,24 @@ import {
   extractBaseAddress,
 } from "./util/decode_encode_muxed_account.js";
 
+const MAX_UINT64 = BigInt("18446744073709551615"); // 2^64 - 1
+
+function validateUint64Id(id: string): void {
+  let value: bigint;
+
+  try {
+    value = BigInt(id);
+  } catch {
+    throw new Error(`id is not a valid uint64 string: ${id}`);
+  }
+
+  if (value < BigInt(0) || value > MAX_UINT64) {
+    throw new Error(
+      `id value out of range for uint64 [0, ${MAX_UINT64}]: ${id}`,
+    );
+  }
+}
+
 /**
  * Represents a muxed account for transactions and operations.
  *
@@ -57,6 +75,8 @@ export class MuxedAccount {
       throw new Error("accountId is invalid");
     }
 
+    validateUint64Id(id);
+
     this.account = baseAccount;
     this._muxedXdr = encodeMuxedAccount(accountId, id);
     this._mAddress = encodeMuxedAccountToAddress(this._muxedXdr);
@@ -112,6 +132,8 @@ export class MuxedAccount {
       throw new Error("id should be a string representing a number (uint64)");
     }
 
+    validateUint64Id(id);
+
     this._muxedXdr.med25519().id(xdr.Uint64.fromString(id));
     this._mAddress = encodeMuxedAccountToAddress(this._muxedXdr);
     this._id = id;

src/scval.ts

@@ -220,7 +220,7 @@ export function nativeToScVal(
         );
       }
 
-      if (val.constructor?.name !== "Object") {
+      if (Object.getPrototypeOf(val) !== Object.prototype) {
         throw new TypeError(
           `cannot interpret ${
             val.constructor?.name
@@ -240,7 +240,9 @@ export function nativeToScVal(
             // the type can be specified with an entry for the key and the value,
             // e.g. val = { 'hello': 1 } and opts.type = { hello: [ 'symbol',
             // 'u128' ]} or you can use `null` for the default interpretation
-            const [keyType, valType] = mapTypeSpec[k] ?? [null, null];
+            const [keyType, valType] = Object.hasOwn(mapTypeSpec, k)
+              ? (mapTypeSpec[k] ?? [null, null])
+              : [null, null];
             const keyOpts: NativeToScValOpts = keyType ? { type: keyType } : {};
             const valOpts: NativeToScValOpts = valType ? { type: valType } : {};
 

src/transaction.ts

@@ -79,12 +79,12 @@ export class Transaction extends TransactionBase<
     switch (this._envelopeType) {
       case xdr.EnvelopeType.envelopeTypeTxV0():
         this._source = StrKey.encodeEd25519PublicKey(
-          (this.tx as xdr.TransactionV0).sourceAccountEd25519(),
+          (tx as xdr.TransactionV0).sourceAccountEd25519(),
         );
         break;
       default:
         this._source = encodeMuxedAccountToAddress(
-          (this.tx as xdr.Transaction).sourceAccount(),
+          (tx as xdr.Transaction).sourceAccount(),
         );
         break;
     }

src/transaction_base.ts

@@ -40,9 +40,32 @@ export class TransactionBase<
     throw new Error("Transaction is immutable");
   }
 
-  /** The underlying XDR transaction object. */
+  /**
+   * The underlying XDR transaction object.
+   *
+   * Returns a defensive copy so that external mutations cannot alter the
+   * transaction that will be signed or serialized.
+   *
+   * @throws {Error} if the internal transaction is not a recognized XDR type
+   */
   get tx(): TTx {
-    return this._tx;
+    const buf = this._tx.toXDR();
+
+    // Making sure we have the right type here, since the base class doesn't
+    // know which transaction type it is.
+    if (this._tx instanceof xdr.Transaction) {
+      return xdr.Transaction.fromXDR(buf) as TTx;
+    }
+
+    if (this._tx instanceof xdr.TransactionV0) {
+      return xdr.TransactionV0.fromXDR(buf) as TTx;
+    }
+
+    if (this._tx instanceof xdr.FeeBumpTransaction) {
+      return xdr.FeeBumpTransaction.fromXDR(buf) as TTx;
+    }
+
+    throw new Error("Unknown transaction type");
   }
 
   set tx(_value: TTx) {

src/util/continued_fraction.ts

@@ -1,6 +1,7 @@
 import BigNumber from "./bignumber.js";
 
 const MAX_INT = ((1 << 31) >>> 0) - 1;
+const MAX_INT_BN = new BigNumber(MAX_INT);
 
 /**
  * Calculates and returns the best rational approximation of the given real
@@ -55,6 +56,50 @@ export function best_r(
   const [n, d] = lastFraction;
 
   if (n.isZero() || d.isZero()) {
+    // Standard convergents produced a degenerate fraction (e.g. for values
+    // where 1/value > MAX_INT). Recover by computing a semi-convergent: find
+    // the largest coefficient that keeps both n and d within int32 bounds.
+    // Skip recovery for genuinely zero input — there is no valid approximation.
+    const input = new BigNumber(rawNumber);
+
+    if (input.isZero()) {
+      throw new Error("Couldn't find approximation");
+    }
+
+    const prev1 = fractions[fractions.length - 1];
+    const prev2 = fractions[fractions.length - 2];
+
+    if (prev1 && prev2) {
+      let aMax = MAX_INT_BN;
+
+      if (prev1[0].gt(0)) {
+        aMax = BigNumber.min(
+          aMax,
+          MAX_INT_BN.minus(prev2[0])
+            .div(prev1[0])
+            .integerValue(BigNumber.ROUND_FLOOR),
+        );
+      }
+
+      if (prev1[1].gt(0)) {
+        aMax = BigNumber.min(
+          aMax,
+          MAX_INT_BN.minus(prev2[1])
+            .div(prev1[1])
+            .integerValue(BigNumber.ROUND_FLOOR),
+        );
+      }
+
+      if (aMax.gte(1)) {
+        const hn = aMax.times(prev1[0]).plus(prev2[0]);
+        const kn = aMax.times(prev1[1]).plus(prev2[1]);
+
+        if (!hn.isZero() && !kn.isZero()) {
+          return [hn.toNumber(), kn.toNumber()];
+        }
+      }
+    }
+
     throw new Error("Couldn't find approximation");
   }