Some fixes 2

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

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;
+    }
   }
 
   /**

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) {

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);

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))
           })
         );
 

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() {

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(

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;
+  });
+});

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/
+    );
   });
 });
 

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(

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 () {

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]);
+  });
+});