diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cca137d2..11e68f751 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,23 @@ ## Unreleased +### Breaking Changes + +* `TransactionBase.networkPassphrase` setter now throws an error to enforce immutability ([#891](https://github.com/stellar/js-stellar-base/pull/891)). + +### Fixed + +* `Keypair.verify` now returns `false` instead of throwing when the signature is invalid ([#892](https://github.com/stellar/js-stellar-base/pull/892)). +* `Memo.id` now correctly rejects negative values, decimal values, and values exceeding the uint64 maximum (`2^64 - 1`); the error message now correctly says `uint64` ([#892](https://github.com/stellar/js-stellar-base/pull/892)). +* `Operation._toXDRPrice` now accepts price objects with `n: 0` (a zero numerator was previously treated as falsy and fell through to float approximation) ([#892](https://github.com/stellar/js-stellar-base/pull/892)). +* `SignerKey.decodeSignerKey` now reads the exact payload length from the 4-byte length prefix when decoding `signedPayload` signer keys, preventing data truncation or over-read ([#892](https://github.com/stellar/js-stellar-base/pull/892)). +* `TransactionBuilder.cloneFrom` now correctly re-encodes `extraSigners` as StrKey strings (they were previously passed as raw XDR objects) ([#892](https://github.com/stellar/js-stellar-base/pull/892)). +* `TransactionBuilder.cloneFrom` now uses `Math.floor` when computing `unscaledFee` to prevent fractional fee values ([#892](https://github.com/stellar/js-stellar-base/pull/892)). +* `TransactionBuilder` now floors `Date` timebounds to integer UNIX timestamps ([#892](https://github.com/stellar/js-stellar-base/pull/892)). +* `Auth.bytesToInt64` now correctly handles bytes with upper-32-bit values set by processing each 32-bit half independently ([#891](https://github.com/stellar/js-stellar-base/pull/891)). +* `ScInt` constructor now correctly handles string input ([#891](https://github.com/stellar/js-stellar-base/pull/891)). +* `Soroban.parseTokenAmount` now throws when the input value has more decimal places than the specified `decimals` argument ([#891](https://github.com/stellar/js-stellar-base/pull/891)). + ## [`v14.1.0`](https://github.com/stellar/js-stellar-base/compare/v14.0.4...v14.1.0): ### Added diff --git a/src/keypair.js b/src/keypair.js index 8d36b3852..29c111f76 100644 --- a/src/keypair.js +++ b/src/keypair.js @@ -226,7 +226,11 @@ export class Keypair { * @returns {boolean} */ verify(data, signature) { - return verify(data, signature, this._publicKey); + try { + return verify(data, signature, this._publicKey); + } catch (e) { + return false; + } } /** diff --git a/src/memo.js b/src/memo.js index 1547f2be8..d9d0bfa55 100644 --- a/src/memo.js +++ b/src/memo.js @@ -96,7 +96,7 @@ export class Memo { } static _validateIdValue(value) { - const error = new Error(`Expects a int64 as a string. Got ${value}`); + const error = new Error(`Expects a uint64 as a string. Got ${value}`); if (typeof value !== 'string') { throw error; @@ -118,6 +118,21 @@ export class Memo { if (number.isNaN()) { throw error; } + + // Negative + if (number.isNegative()) { + throw error; + } + + // Decimal + if (!number.isInteger()) { + throw error; + } + + // Exceeds uint64 max (2^64 - 1) + if (number.isGreaterThan('18446744073709551615')) { + throw error; + } } static _validateTextValue(value) { diff --git a/src/operation.js b/src/operation.js index bb5950071..fd0a0ddef 100644 --- a/src/operation.js +++ b/src/operation.js @@ -525,7 +525,7 @@ export class Operation { */ static _toXDRPrice(price) { let xdrObject; - if (price.n && price.d) { + if (price.n !== undefined && price.d !== undefined) { xdrObject = new xdr.Price(price); } else { const approx = best_r(price); diff --git a/src/signerkey.js b/src/signerkey.js index 8512138ba..2115a18ae 100644 --- a/src/signerkey.js +++ b/src/signerkey.js @@ -40,7 +40,7 @@ export class SignerKey { return encoder( new xdr.SignerKeyEd25519SignedPayload({ ed25519: raw.slice(0, 32), - payload: raw.slice(32 + 4) + payload: raw.slice(36, 36 + raw.readUInt32BE(32)) }) ); diff --git a/src/transaction.js b/src/transaction.js index 5a0ecab9c..47d63255b 100644 --- a/src/transaction.js +++ b/src/transaction.js @@ -190,8 +190,9 @@ export class Transaction extends TransactionBase { } /** - * array of extra signers ({@link StrKey}s) - * @type {string[]} + * array of extra signers as XDR objects; use {@link SignerKey.encodeSignerKey} + * to convert to StrKey strings + * @type {xdr.SignerKey[]} * @readonly */ get extraSigners() { diff --git a/src/transaction_builder.js b/src/transaction_builder.js index 48fe454d9..3ad2bdcf0 100644 --- a/src/transaction_builder.js +++ b/src/transaction_builder.js @@ -209,7 +209,7 @@ export class TransactionBuilder { // the initial fee passed to the builder gets scaled up based on the number // of operations at the end, so we have to down-scale first - const unscaledFee = parseInt(tx.fee, 10) / tx.operations.length; + const unscaledFee = Math.floor(parseInt(tx.fee, 10) / tx.operations.length); const builder = new TransactionBuilder(source, { fee: (unscaledFee || BASE_FEE).toString(), @@ -220,7 +220,7 @@ export class TransactionBuilder { minAccountSequence: tx.minAccountSequence, minAccountSequenceAge: tx.minAccountSequenceAge, minAccountSequenceLedgerGap: tx.minAccountSequenceLedgerGap, - extraSigners: tx.extraSigners, + extraSigners: tx.extraSigners?.map(SignerKey.encodeSignerKey), ...opts }); @@ -815,10 +815,14 @@ export class TransactionBuilder { } if (isValidDate(this.timebounds.minTime)) { - this.timebounds.minTime = this.timebounds.minTime.getTime() / 1000; + this.timebounds.minTime = Math.floor( + this.timebounds.minTime.getTime() / 1000 + ); } if (isValidDate(this.timebounds.maxTime)) { - this.timebounds.maxTime = this.timebounds.maxTime.getTime() / 1000; + this.timebounds.maxTime = Math.floor( + this.timebounds.maxTime.getTime() / 1000 + ); } this.timebounds.minTime = UnsignedHyper.fromString( diff --git a/test/unit/keypair_test.js b/test/unit/keypair_test.js index 3f9ebd955..a195615e0 100644 --- a/test/unit/keypair_test.js +++ b/test/unit/keypair_test.js @@ -196,3 +196,27 @@ describe('Keypair.sign*Decorated', function () { }); }); }); + +describe('Keypair.verify', function () { + const kp = StellarBase.Keypair.random(); + const data = Buffer.from('hello'); + const validSig = kp.sign(data); + + it('returns true for a valid signature', function () { + expect(kp.verify(data, validSig)).to.be.true; + }); + + it('returns false for a wrong-length signature (32 bytes instead of 64)', function () { + const shortSig = Buffer.alloc(32, 0xab); + expect(kp.verify(data, shortSig)).to.be.false; + }); + + it('returns false for an empty signature', function () { + expect(kp.verify(data, Buffer.alloc(0))).to.be.false; + }); + + it('returns false for a correctly-sized but invalid signature', function () { + const badSig = Buffer.alloc(64, 0xab); + expect(kp.verify(data, badSig)).to.be.false; + }); +}); diff --git a/test/unit/memo_test.js b/test/unit/memo_test.js index 60cde0e04..dbb69816b 100644 --- a/test/unit/memo_test.js +++ b/test/unit/memo_test.js @@ -110,6 +110,8 @@ describe('Memo.id()', function () { it('returns a value for a correct argument', function () { expect(() => StellarBase.Memo.id('1000')).to.not.throw(); expect(() => StellarBase.Memo.id('0')).to.not.throw(); + // max uint64 + expect(() => StellarBase.Memo.id('18446744073709551615')).to.not.throw(); }); it('converts to/from xdr object', function () { @@ -123,11 +125,19 @@ describe('Memo.id()', function () { }); it('throws an error when invalid argument was passed', function () { - expect(() => StellarBase.Memo.id()).to.throw(/Expects a int64/); - expect(() => StellarBase.Memo.id({})).to.throw(/Expects a int64/); - expect(() => StellarBase.Memo.id(Infinity)).to.throw(/Expects a int64/); - expect(() => StellarBase.Memo.id(NaN)).to.throw(/Expects a int64/); - expect(() => StellarBase.Memo.id('test')).to.throw(/Expects a int64/); + expect(() => StellarBase.Memo.id()).to.throw(/Expects a uint64/); + expect(() => StellarBase.Memo.id({})).to.throw(/Expects a uint64/); + expect(() => StellarBase.Memo.id(Infinity)).to.throw(/Expects a uint64/); + expect(() => StellarBase.Memo.id(NaN)).to.throw(/Expects a uint64/); + expect(() => StellarBase.Memo.id('test')).to.throw(/Expects a uint64/); + // negative + expect(() => StellarBase.Memo.id('-1')).to.throw(/Expects a uint64/); + // decimal + expect(() => StellarBase.Memo.id('1.5')).to.throw(/Expects a uint64/); + // overflow: 2^64 silently became 0 before this fix + expect(() => StellarBase.Memo.id('18446744073709551616')).to.throw( + /Expects a uint64/ + ); }); }); diff --git a/test/unit/operations/classic_ops_test.js b/test/unit/operations/classic_ops_test.js index bfdf121e6..c1fbc995d 100644 --- a/test/unit/operations/classic_ops_test.js +++ b/test/unit/operations/classic_ops_test.js @@ -972,6 +972,27 @@ describe('Operation', function () { ); }); + it('creates a manageSellOfferOp (price fraction with zero numerator)', function () { + var opts = {}; + opts.selling = new StellarBase.Asset( + 'USD', + 'GDGU5OAPHNPU5UCLE5RDJHG7PXZFQYWKCFOEXSXNMR6KRQRI5T6XXCD7' + ); + opts.buying = new StellarBase.Asset( + 'USD', + 'GDGU5OAPHNPU5UCLE5RDJHG7PXZFQYWKCFOEXSXNMR6KRQRI5T6XXCD7' + ); + opts.amount = '3.123456'; + opts.price = { n: 0, d: 1 }; + opts.offerId = '1'; + const op = StellarBase.Operation.manageSellOffer(opts); + const xdr = op.toXDR('hex'); + const obj = StellarBase.Operation.fromXDRObject( + StellarBase.xdr.Operation.fromXDR(Buffer.from(xdr, 'hex')) + ); + expect(obj.price).to.equal('0'); + }); + it('creates an invalid manageSellOfferOp (price fraction)', function () { var opts = {}; opts.selling = new StellarBase.Asset( diff --git a/test/unit/signerkey_test.js b/test/unit/signerkey_test.js index 92e0672a3..9e968be18 100644 --- a/test/unit/signerkey_test.js +++ b/test/unit/signerkey_test.js @@ -33,6 +33,28 @@ describe('SignerKey', function () { expect(address).to.equal(testCase.strkey); }); }); + + it('roundtrip is identity for signed payloads with non-multiple-of-4 lengths', function () { + const ed25519 = Buffer.alloc(32, 0x01); + // 29 % 4 = 1 (3 padding bytes), 30 % 4 = 2 (2 bytes), 31 % 4 = 3 (1 byte) + [29, 30, 31].forEach((len) => { + const payload = Buffer.alloc(len, 0xab); + const signerKey = + StellarBase.xdr.SignerKey.signerKeyTypeEd25519SignedPayload( + new StellarBase.xdr.SignerKeyEd25519SignedPayload({ + ed25519, + payload + }) + ); + const address = StellarBase.SignerKey.encodeSignerKey(signerKey); + const decoded = StellarBase.SignerKey.decodeAddress(address); + + expect(decoded.ed25519SignedPayload().payload()).to.have.lengthOf(len); + expect(StellarBase.SignerKey.encodeSignerKey(decoded)).to.equal( + address + ); + }); + }); }); describe('error cases', function () { diff --git a/test/unit/transaction_builder_test.js b/test/unit/transaction_builder_test.js index b497e025c..749a63436 100644 --- a/test/unit/transaction_builder_test.js +++ b/test/unit/transaction_builder_test.js @@ -927,6 +927,38 @@ describe('TransactionBuilder', function () { ); done(); }); + + it('floors sub-second precision Date timebounds', function () { + let source = new StellarBase.Account( + 'GCEZWKCA5VLDNRLN3RPRJMRZOX3Z6G5CHCGSNFHEYVXM3XOJMDS674JZ', + '0' + ); + let timebounds = { + minTime: new Date(1528145519500), // 500ms sub-second + maxTime: new Date(1528231982999) // 999ms sub-second + }; + + let transaction; + expect(() => { + transaction = new StellarBase.TransactionBuilder(source, { + timebounds, + fee: 100, + networkPassphrase: StellarBase.Networks.TESTNET + }) + .addOperation( + StellarBase.Operation.payment({ + destination: + 'GDJJRRMBK4IWLEPJGIE6SXD2LP7REGZODU7WDC3I2D6MR37F4XSHBKX2', + asset: StellarBase.Asset.native(), + amount: '1000' + }) + ) + .build(); + }).not.to.throw(); + + expect(transaction.timeBounds.minTime).to.equal('1528145519'); + expect(transaction.timeBounds.maxTime).to.equal('1528231982'); + }); }); describe('timebounds', function () { @@ -1581,3 +1613,69 @@ describe('TransactionBuilder', function () { }); }); }); + +describe('TransactionBuilder.cloneFrom', function () { + const networkPassphrase = StellarBase.Networks.TESTNET; + const source = new StellarBase.Account( + 'GCEZWKCA5VLDNRLN3RPRJMRZOX3Z6G5CHCGSNFHEYVXM3XOJMDS674JZ', + '0' + ); + const destination = + 'GDJJRRMBK4IWLEPJGIE6SXD2LP7REGZODU7WDC3I2D6MR37F4XSHBKX2'; + const op = StellarBase.Operation.payment({ + destination, + asset: StellarBase.Asset.native(), + amount: '100' + }); + + it('handles a total fee not evenly divisible by the operation count', function () { + // TransactionBuilder always produces total fees divisible by op count, + // but arbitrary network transactions can have any fee. Simulate one by + // patching the XDR fee field directly to 1000 across 3 ops. + // Previously: 1000/3 = 333.333..., scaled back up to 999.999... which + // crashed with "XDR Write Error: invalid u32 value". + const builtTx = new StellarBase.TransactionBuilder(source, { + fee: '100', + timebounds: { minTime: 0, maxTime: 0 }, + networkPassphrase + }) + .addOperation(op) + .addOperation(op) + .addOperation(op) + .build(); + + const envelope = builtTx.toEnvelope(); + envelope.v1().tx().fee(1000); // 1000 is not divisible by 3 + const tx = new StellarBase.Transaction(envelope, networkPassphrase); + + let cloneTx; + expect(() => { + cloneTx = StellarBase.TransactionBuilder.cloneFrom(tx).build(); + }).not.to.throw(); + + // Math.floor(1000/3) = 333 per-op → 333 * 3 = 999 + expect(cloneTx.fee).to.equal('999'); + }); + + it('preserves extraSigners', function () { + const extraSigner = + 'GA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVSGZ'; + const tx = new StellarBase.TransactionBuilder(source, { + fee: '100', + timebounds: { minTime: 0, maxTime: 0 }, + networkPassphrase + }) + .addOperation(op) + .setExtraSigners([extraSigner]) + .build(); + + let cloneTx; + expect(() => { + cloneTx = StellarBase.TransactionBuilder.cloneFrom(tx).build(); + }).not.to.throw(); + + expect( + cloneTx.extraSigners.map(StellarBase.SignerKey.encodeSignerKey) + ).to.eql([extraSigner]); + }); +});