Various fixes, part 3

[quietbits]

Addresses items 1-9 for js-stellar-base in this doc.

[quietbits]

This is a breaking change, but I think we should make it. This is the only place where we silently set the network passphrase to testnet. Everywhere else it is required.

[github-actions]

Size Change: +5.14 kB (+0.29%)

Total Size: 1.76 MB

📦 View Changed

Filename	Size	Change
dist/stellar-base.cjs.js	696 kB	+2.02 kB (+0.29%)
dist/stellar-base.js	715 kB	+2.09 kB (+0.29%)
dist/stellar-base.min.js	344 kB	+1.04 kB (+0.3%)

_{compressed-size-action}

[Copilot]

Pull request overview

This PR bundles multiple correctness and safety fixes across rational approximation utilities, transaction immutability guarantees, Soroban SCVal conversion hardening, muxed account ID validation, and Soroban auth signing.

Changes:

• Improve best_r continued-fraction approximation to recover near int32 boundaries (instead of throwing).
• Add optional immutableTx support so TransactionBase.tx can return a defensive XDR copy to prevent external mutation.
• Harden nativeToScVal against prototype-pollution edge cases and add regression tests; validate muxed-account uint64 IDs for overflow.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/util/continued_fraction.ts	Adds semi-convergent recovery when convergents degenerate near int32 bounds.
test/unit/util/continued_fraction.test.ts	Updates expectations and adds boundary/regression coverage for best_r.
src/transaction_base.ts	Implements defensive-copy behavior for tx getter behind immutableTx.
src/transaction.ts	Threads immutableTx option into TransactionBase.
src/fee_bump_transaction.ts	Threads immutableTx option into TransactionBase and inner transaction construction.
src/transaction_builder.ts	Adds immutableTx option and propagates it when building/fromXDR.
test/unit/transaction.test.ts	Adds tests asserting tx getter immutability behavior.
src/scval.ts	Uses prototype checks and Object.hasOwn to avoid prototype key pitfalls in map type hints.
test/unit/scval.test.ts	Adds prototype-pollution regression tests and cleans up some unsafe casts/formatting.
src/muxed_account.ts	Adds uint64-range validation for muxed account IDs (constructor + setId).
test/unit/muxed_account.test.ts	Adds overflow and max-uint64 acceptance tests.
src/auth.ts	Updates Soroban auth signing APIs around network passphrase handling.
test/unit/auth.test.ts	Updates auth tests to pass explicit network passphrase and adds network-differentiation test.
test/unit/operation.test.ts	Adds toXDRPrice zero-denominator regression test.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

authorizeInvocation declares publicKey with a default value (making it optional) but then has a required networkPassphrase parameter after it. TypeScript does not allow required parameters to follow optional ones, so this signature should be reordered and/or networkPassphrase should also have a default (or be moved into an options object).

[quietbits]

We'll keep the current order to make the breaking change "lighter". We'll add a note in JSDoc about why we have this order.

[Ryang-21]

I think we would need to change the order right since the previous param is optional / has a default

[Ryang-21]

It might make sense to just make publicKey required as well. Anyone seriously using this has to since the default passphrase was future net.

[quietbits]

If we make publicKey required, it will be a breaking change and everyone who is relying on signer to be used as publicKey will be affected. There is no real benefit making it required, I think. It might cause more headache than good.

If we change the order of the arguments, everyone will need to update now (swap public key and network). Since both are strings, it will be easy to miss. If we do update, I'd rather pass an object instead. It's a bigger change, but it would be clear what needs to be updated. Thoughs, @Ryang-21 ?

[Ryang-21]

We would already be making a breaking change since the default for the passphrase is removed. Users now need to do something like

authorizeInvocation(
  signer: Keypair.random(),
  validUntilLedgerSeq: 123,
  invocation: {},
  publicKey: undefined,
  networkPassphrase: Networks.Testnet,
)

So if they need to pass undefined anyways we might as well make it required?

[quietbits]

We'll go with the object approach for args, which feels like the best option considering everything. 🤞