diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b7be545..22d20219 100644 --- a/CHANGELOG.md +++ b/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): diff --git a/src/numbers/sc_int.ts b/src/numbers/sc_int.ts index 63ae215c..50f4a107 100644 --- a/src/numbers/sc_int.ts +++ b/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; } diff --git a/src/operations/change_trust.ts b/src/operations/change_trust.ts index 4c550304..e1325df2 100644 --- a/src/operations/change_trust.ts +++ b/src/operations/change_trust.ts @@ -28,12 +28,17 @@ const MAX_INT64 = "9223372036854775807"; export function changeTrust( opts: ChangeTrustOpts, ): xdr.Operation { + // 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"); } diff --git a/src/operations/manage_data.ts b/src/operations/manage_data.ts index e8cc089a..74ac4c2e 100644 --- a/src/operations/manage_data.ts +++ b/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) { diff --git a/src/transaction_builder.ts b/src/transaction_builder.ts index 604471e9..7c20bd6b 100644 --- a/src/transaction_builder.ts +++ b/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; +} diff --git a/test/unit/numbers/sc_int.test.ts b/test/unit/numbers/sc_int.test.ts index 0b1a8e01..56e2ad8d 100644 --- a/test/unit/numbers/sc_int.test.ts +++ b/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); diff --git a/test/unit/operations/change_trust.test.ts b/test/unit/operations/change_trust.test.ts index 1af71fdc..38d3a442 100644 --- a/test/unit/operations/change_trust.test.ts +++ b/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 }); diff --git a/test/unit/operations/manage_data.test.ts b/test/unit/operations/manage_data.test.ts index 71576ffc..e799394e 100644 --- a/test/unit/operations/manage_data.test.ts +++ b/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 }; diff --git a/test/unit/transaction_builder.test.ts b/test/unit/transaction_builder.test.ts index 71f1bc27..06fe16b9 100644 --- a/test/unit/transaction_builder.test.ts +++ b/test/unit/transaction_builder.test.ts @@ -1719,6 +1719,165 @@ describe("TransactionBuilder.cloneFrom", () => { // Additional coverage for setter methods and validation +describe("constructor timebounds/ledgerbounds validation", () => { + const networkPassphrase = Networks.TESTNET; + const source = new Account( + "GCEZWKCA5VLDNRLN3RPRJMRZOX3Z6G5CHCGSNFHEYVXM3XOJMDS674JZ", + "0", + ); + + describe("timebounds", () => { + it("rejects inverted timebounds (minTime > non-zero maxTime)", () => { + expect(() => { + new TransactionBuilder(source, { + fee: "100", + networkPassphrase, + timebounds: { minTime: 9999999999, maxTime: 500 }, + }); + }).toThrow("min_time cannot be greater than max_time"); + }); + + it("rejects inverted Date timebounds", () => { + expect(() => { + new TransactionBuilder(source, { + fee: "100", + networkPassphrase, + timebounds: { + minTime: new Date("2030-01-01"), + maxTime: new Date("2020-01-01"), + }, + }); + }).toThrow("min_time cannot be greater than max_time"); + }); + + it("allows maxTime=0 (indefinite) with non-zero minTime", () => { + const builder = new TransactionBuilder(source, { + fee: "100", + networkPassphrase, + timebounds: { minTime: 1000, maxTime: 0 }, + }); + expect(builder.timebounds?.minTime).toBe(1000); + expect(builder.timebounds?.maxTime).toBe(0); + }); + + it("allows equal minTime and maxTime", () => { + const builder = new TransactionBuilder(source, { + fee: "100", + networkPassphrase, + timebounds: { minTime: 1000, maxTime: 1000 }, + }); + expect(builder.timebounds?.minTime).toBe(1000); + expect(builder.timebounds?.maxTime).toBe(1000); + }); + + it("rejects negative minTime", () => { + expect(() => { + new TransactionBuilder(source, { + fee: "100", + networkPassphrase, + timebounds: { minTime: -1, maxTime: 500 }, + }); + }).toThrow("min_time cannot be negative"); + }); + + it("rejects negative maxTime", () => { + expect(() => { + new TransactionBuilder(source, { + fee: "100", + networkPassphrase, + timebounds: { minTime: 0, maxTime: -1 }, + }); + }).toThrow("max_time cannot be negative"); + }); + + it("rejects non-numeric string minTime", () => { + expect(() => { + new TransactionBuilder(source, { + fee: "100", + networkPassphrase, + timebounds: { minTime: "abc" as unknown as number, maxTime: 500 }, + }); + }).toThrow("timebounds value must be a finite integer or Date"); + }); + + it("rejects invalid Date maxTime", () => { + expect(() => { + new TransactionBuilder(source, { + fee: "100", + networkPassphrase, + timebounds: { minTime: 0, maxTime: new Date("invalid") }, + }); + }).toThrow("timebounds value must be a finite integer or Date"); + }); + + it("rejects Infinity as a timebounds value", () => { + expect(() => { + new TransactionBuilder(source, { + fee: "100", + networkPassphrase, + timebounds: { minTime: 0, maxTime: Infinity }, + }); + }).toThrow("timebounds value must be a finite integer or Date"); + }); + + it("rejects non-integer float as a timebounds value", () => { + expect(() => { + new TransactionBuilder(source, { + fee: "100", + networkPassphrase, + timebounds: { minTime: 1.5, maxTime: 500 }, + }); + }).toThrow("timebounds value must be a finite integer or Date"); + }); + }); + + describe("ledgerbounds", () => { + it("rejects inverted ledgerbounds (minLedger > non-zero maxLedger)", () => { + expect(() => { + new TransactionBuilder(source, { + fee: "100", + networkPassphrase, + timebounds: { minTime: 0, maxTime: 0 }, + ledgerbounds: { minLedger: 5000, maxLedger: 100 }, + }); + }).toThrow("min_ledger cannot be greater than max_ledger"); + }); + + it("allows maxLedger=0 (indefinite) with non-zero minLedger", () => { + const builder = new TransactionBuilder(source, { + fee: "100", + networkPassphrase, + timebounds: { minTime: 0, maxTime: 0 }, + ledgerbounds: { minLedger: 1000, maxLedger: 0 }, + }); + expect(builder.ledgerbounds?.minLedger).toBe(1000); + expect(builder.ledgerbounds?.maxLedger).toBe(0); + }); + + it("rejects negative minLedger", () => { + expect(() => { + new TransactionBuilder(source, { + fee: "100", + networkPassphrase, + timebounds: { minTime: 0, maxTime: 0 }, + ledgerbounds: { minLedger: -1, maxLedger: 100 }, + }); + }).toThrow("min_ledger cannot be negative"); + }); + + it("rejects negative maxLedger", () => { + expect(() => { + new TransactionBuilder(source, { + fee: "100", + networkPassphrase, + timebounds: { minTime: 0, maxTime: 0 }, + ledgerbounds: { minLedger: 0, maxLedger: -1 }, + }); + }).toThrow("max_ledger cannot be negative"); + }); + }); +}); + describe("setTimebounds", () => { const source = new Account( "GCEZWKCA5VLDNRLN3RPRJMRZOX3Z6G5CHCGSNFHEYVXM3XOJMDS674JZ",