Zy0n/fix nullifier bug

src/__tests__/railgun-engine-nullifier-collision.test.ts

@@ -0,0 +1,139 @@
+import chai from 'chai';
+import chaiAsPromised from 'chai-as-promised';
+import memdown from 'memdown';
+import sinon from 'sinon';
+import { RailgunEngine } from '../railgun-engine';
+import { UTXOMerkletree } from '../merkletree/utxo-merkletree';
+import { Database } from '../database/database';
+import { Chain, ChainType } from '../models/engine-types';
+import { TXIDVersion } from '../models/poi-types';
+import { ByteLength, ByteUtils } from '../utils/bytes';
+
+chai.use(chaiAsPromised);
+const { expect } = chai;
+
+const txidVersion = TXIDVersion.V2_PoseidonMerkle;
+const chain: Chain = { type: ChainType.EVM, id: 1 };
+
+describe('railgun-engine-nullifier-collision', () => {
+    let engine: RailgunEngine;
+    let db: Database;
+    let utxoMerkletree: UTXOMerkletree;
+
+    beforeEach(async () => {
+        const artifactGetter = {
+            assertArtifactExists: () => {},
+            getArtifacts: async () => ({ zkey: [], wasm: undefined, dat: undefined, vkey: {} }),
+            getArtifactsPOI: async () => ({ zkey: [], wasm: undefined, dat: undefined, vkey: {} }),
+        };
+        const quickSyncEvents = async () => ({
+            commitmentEvents: [],
+            nullifierEvents: [],
+            unshieldEvents: [],
+        });
+        const quickSyncRailgunTransactionsV2 = async () => [];
+
+        engine = await RailgunEngine.initForWallet(
+            'testwallet',
+            memdown(),
+            artifactGetter,
+            quickSyncEvents,
+            quickSyncRailgunTransactionsV2,
+            async () => true, // validateRailgunTxidMerkleroot
+            async () => ({ txidIndex: undefined, merkleroot: undefined }), // getLatestValidatedRailgunTxid
+            undefined, // engineDebugger
+            false, // skipMerkletreeScans
+        );
+
+        db = new Database(memdown());
+        utxoMerkletree = await UTXOMerkletree.create(db, chain, txidVersion, async () => true);
+
+        sinon.stub(engine as any, 'getUTXOMerkletree').returns(utxoMerkletree);
+        sinon.stub(utxoMerkletree, 'latestTree').resolves(1);
+    });
+
+    afterEach(() => {
+        sinon.restore();
+    });
+
+    it('Should find completed txid across trees correctly', async () => {
+        // Scenario:
+        // Transaction 0 (TxID 1000): Nullifiers A, B in Tree 0
+        // Transaction 1 (TxID 1001): Nullifiers A, B in Tree 1
+
+        await utxoMerkletree.nullify([
+            { nullifier: 'A', treeNumber: 0, txid: '1000', blockNumber: 0 },
+            { nullifier: 'B', treeNumber: 0, txid: '1000', blockNumber: 0 },
+            { nullifier: 'A', treeNumber: 1, txid: '1001', blockNumber: 0 },
+            { nullifier: 'B', treeNumber: 1, txid: '1001', blockNumber: 0 },
+        ]);
+
+        const result = await engine.getCompletedTxidFromNullifiers(txidVersion, chain, ['A', 'B']);
+
+        const expected = ByteUtils.formatToByteLength('1001', ByteLength.UINT_256, true);
+        expect(result).to.equal(expected);
+    });
+
+    it('Should fallback to older trees if nullifiers match there', async () => {
+        // Scenario:
+        // Tree 1: A->1001, B->undefined (Partial / Mismatch)
+        // Tree 0: A->1000, B->1000 (Complete match)
+
+        await utxoMerkletree.nullify([
+            { nullifier: 'A', treeNumber: 0, txid: '1000', blockNumber: 0 },
+            { nullifier: 'B', treeNumber: 0, txid: '1000', blockNumber: 0 },
+            { nullifier: 'A', treeNumber: 1, txid: '1001', blockNumber: 0 },
+        ]);
+
+        const result = await engine.getCompletedTxidFromNullifiers(txidVersion, chain, ['A', 'B']);
+        const expected = ByteUtils.formatToByteLength('1000', ByteLength.UINT_256, true);
+
+        expect(result).to.equal(expected);
+    });
+
+    it('Should return undefined if nullifiers match in different trees but not same tree', async () => {
+        // Scenario:
+        // Nullifier A in Tree 1 (1001)
+        // Nullifier B in Tree 0 (1000)
+
+        await utxoMerkletree.nullify([
+            { nullifier: 'B', treeNumber: 0, txid: '1000', blockNumber: 0 },
+            { nullifier: 'A', treeNumber: 1, txid: '1001', blockNumber: 0 },
+        ]);
+
+        const result = await engine.getCompletedTxidFromNullifiers(txidVersion, chain, ['A', 'B']);
+        // eslint-disable-next-line no-unused-expressions
+        expect(result).to.be.undefined;
+    });
+
+    it('Should handle sparse trees correctly (skip empty trees)', async () => {
+        // Scenario:
+        // Tree 2: Empty / Undefined (mocked via latestTree=2)
+        // Tree 1: Match found
+
+        (utxoMerkletree.latestTree as sinon.SinonStub).resolves(2);
+
+        await utxoMerkletree.nullify([
+            { nullifier: 'A', treeNumber: 1, txid: '1001', blockNumber: 0 },
+            { nullifier: 'B', treeNumber: 1, txid: '1001', blockNumber: 0 },
+        ]);
+
+        const result = await engine.getCompletedTxidFromNullifiers(txidVersion, chain, ['A', 'B']);
+        const expected = ByteUtils.formatToByteLength('1001', ByteLength.UINT_256, true);
+        expect(result).to.equal(expected);
+    });
+
+    it('Should fail if nullifiers map to different TXIDs within the same tree', async () => {
+        // Scenario:
+        // Tree 1: A -> 1001, B -> 1002 (Different transactions in same tree)
+
+        await utxoMerkletree.nullify([
+            { nullifier: 'A', treeNumber: 1, txid: '1001', blockNumber: 0 },
+            { nullifier: 'B', treeNumber: 1, txid: '1002', blockNumber: 0 },
+        ]);
+
+        const result = await engine.getCompletedTxidFromNullifiers(txidVersion, chain, ['A', 'B']);
+        // eslint-disable-next-line no-unused-expressions
+        expect(result).to.be.undefined;
+    });
+});

src/merkletree/__tests__/utxo-merkletree-nullifier-collision.test.ts

@@ -0,0 +1,101 @@
+import chai from 'chai';
+import chaiAsPromised from 'chai-as-promised';
+import memdown from 'memdown';
+import { Database } from '../../database/database';
+import { UTXOMerkletree } from '../utxo-merkletree';
+import { Chain } from '../../models/engine-types';
+import { getTestTXIDVersion } from '../../test/helper.test';
+
+chai.use(chaiAsPromised);
+const { expect } = chai;
+
+const txidVersion = getTestTXIDVersion();
+
+describe('utxo-merkletree-nullifier-collision', () => {
+    let db: Database;
+    let merkletree: UTXOMerkletree;
+    const chain: Chain = { type: 0, id: 0 };
+
+    beforeEach(async () => {
+        db = new Database(memdown());
+        merkletree = await UTXOMerkletree.create(db, chain, txidVersion, async () => true);
+
+        // @ts-ignore
+        merkletree.latestTree = async () => 1;
+    });
+
+    it('Should retrieve nullifier txid from specific tree', async () => {
+        await merkletree.nullify([{ nullifier: 'COLLISION', treeNumber: 0, txid: '1000', blockNumber: 0 }]);
+        await merkletree.nullify([{ nullifier: 'COLLISION', treeNumber: 1, txid: '1001', blockNumber: 0 }]);
+
+        expect(await merkletree.getNullifierTxid('COLLISION', 0)).to.equal('1000');
+        expect(await merkletree.getNullifierTxid('COLLISION', 1)).to.equal('1001');
+    });
+
+    it('Should return correct txid when searching without tree parameter (latest tree priority)', async () => {
+        await merkletree.nullify([{ nullifier: 'COLLISION', treeNumber: 0, txid: '1000', blockNumber: 0 }]);
+        await merkletree.nullify([{ nullifier: 'COLLISION', treeNumber: 1, txid: '1001', blockNumber: 0 }]);
+
+        // Should return '1001' because it searches tree 1 first (latestTree = 1)
+        expect(await merkletree.getNullifierTxid('COLLISION')).to.equal('1001');
+    });
+
+    it('Should find nullifiers that exist only in older trees', async () => {
+        await merkletree.nullify([{ nullifier: 'UNIQUE0', treeNumber: 0, txid: '2000', blockNumber: 0 }]);
+
+        // Should start at tree 1 (empty), fail, then go to tree 0 and find it
+        expect(await merkletree.getNullifierTxid('UNIQUE0')).to.equal('2000');
+    });
+
+    it('Should return undefined for non-existent nullifiers', async () => {
+        // eslint-disable-next-line no-unused-expressions
+        expect(await merkletree.getNullifierTxid('NONEXISTENT')).to.be.undefined;
+        // eslint-disable-next-line no-unused-expressions
+        expect(await merkletree.getNullifierTxid('NONEXISTENT', 0)).to.be.undefined;
+        // eslint-disable-next-line no-unused-expressions
+        expect(await merkletree.getNullifierTxid('NONEXISTENT', 1)).to.be.undefined;
+    });
+
+    it('Should handle batch insertion of nullifiers', async () => {
+        await merkletree.nullify([
+            { nullifier: 'A', treeNumber: 0, txid: '00A0', blockNumber: 0 },
+            { nullifier: 'B', treeNumber: 0, txid: '00B0', blockNumber: 0 },
+            { nullifier: 'C', treeNumber: 1, txid: '00C0', blockNumber: 0 }
+        ]);
+
+        expect(await merkletree.getNullifierTxid('A', 0)).to.equal('00a0');
+        expect(await merkletree.getNullifierTxid('B', 0)).to.equal('00b0');
+        expect(await merkletree.getNullifierTxid('C', 1)).to.equal('00c0');
+
+        // Also check via iteration
+        expect(await merkletree.getNullifierTxid('A')).to.equal('00a0');
+        expect(await merkletree.getNullifierTxid('C')).to.equal('00c0');
+    });
+
+    it('Should overwrite nullifier txid if added again to same tree', async () => {
+        await merkletree.nullify([{ nullifier: 'OVERWRITE', treeNumber: 0, txid: '1111', blockNumber: 0 }]);
+        expect(await merkletree.getNullifierTxid('OVERWRITE', 0)).to.equal('1111');
+
+        await merkletree.nullify([{ nullifier: 'OVERWRITE', treeNumber: 0, txid: '2222', blockNumber: 1 }]);
+        expect(await merkletree.getNullifierTxid('OVERWRITE', 0)).to.equal('2222');
+    });
+
+    it('Should respect sparse tree usage', async () => {
+        // If we have data in Tree 0 and Tree 2, but skip Tree 1
+
+        // @ts-ignore
+        merkletree.latestTree = async () => 2;
+
+        await merkletree.nullify([{ nullifier: 'SPARSE', treeNumber: 0, txid: '3000', blockNumber: 0 }]);
+        await merkletree.nullify([{ nullifier: 'SPARSE', treeNumber: 2, txid: '3002', blockNumber: 0 }]);
+
+        // Search from 2 -> 0
+        expect(await merkletree.getNullifierTxid('SPARSE')).to.equal('3002');
+
+        // Specific checks
+        expect(await merkletree.getNullifierTxid('SPARSE', 0)).to.equal('3000');
+        // eslint-disable-next-line no-unused-expressions
+        expect(await merkletree.getNullifierTxid('SPARSE', 1)).to.be.undefined;
+        expect(await merkletree.getNullifierTxid('SPARSE', 2)).to.equal('3002');
+    });
+});

src/merkletree/utxo-merkletree.ts

@@ -103,11 +103,23 @@ export class UTXOMerkletree extends Merkletree<Commitment> {
   /**
    * Gets nullifier by its id
    * @param {string} nullifier - nullifier to check
+   * @param {number} treeIndex - optional tree to check
    * @returns Nullifier data, including txid of spent transaction
    */
-  async getNullifierTxid(nullifier: string): Promise<Optional<string>> {
+  async getNullifierTxid(nullifier: string, treeIndex?: number): Promise<Optional<string>> {
     // Return if nullifier is set
     let nullifierTxid: Optional<string>;
+
+    // Check specific tree if provided
+    if (isDefined(treeIndex)) {
+      try {
+        nullifierTxid = (await this.db.get(this.getNullifierDBPath(treeIndex, nullifier))) as string;
+      } catch {
+        nullifierTxid = undefined;
+      }
+      return nullifierTxid;
+    }
+
     const latestTree = await this.latestTree();
     for (let tree = latestTree; tree >= 0; tree -= 1) {
       try {

src/railgun-engine.ts

@@ -1934,23 +1934,26 @@ class RailgunEngine extends EventEmitter {
 
     const utxoMerkletree = this.getUTXOMerkletree(txidVersion, chain);
 
-    const firstNullifier = nullifiers[0];
-    const firstTxid = await utxoMerkletree.getNullifierTxid(firstNullifier);
-    if (!isDefined(firstTxid)) {
-      return undefined;
-    }
+    // Iterating backwards from latest tree to find valid txid with matching nullifiers
+    const latestTree = await utxoMerkletree.latestTree();
+    for (let tree = latestTree; tree >= 0; tree -= 1) {
+      // eslint-disable-next-line no-await-in-loop
+      const txids: Optional<string>[] = await Promise.all(
+        nullifiers.map((nullifier) => utxoMerkletree.getNullifierTxid(nullifier, tree)),
+      );
 
-    const otherTxids: Optional<string>[] = await Promise.all(
-      nullifiers
-        .slice(1)
-        .map(async (nullifier) => await utxoMerkletree.getNullifierTxid(nullifier)),
-    );
+      const firstTxid = txids[0];
+      if (!isDefined(firstTxid)) {
+        continue;
+      }
+
+      const allMatch = txids.every((txid) => txid === firstTxid);
+      if (allMatch) {
+        return ByteUtils.formatToByteLength(firstTxid, ByteLength.UINT_256, true);
+      }
+    }
 
-    const matchingTxids = otherTxids.filter((txid) => txid === firstTxid);
-    const allMatch = matchingTxids.length === nullifiers.length - 1;
-    return allMatch
-      ? ByteUtils.formatToByteLength(firstTxid, ByteLength.UINT_256, true)
-      : undefined;
+    return undefined;
   }
 
   private async decryptBalancesAllWallets(

src/wallet/abstract-wallet.ts

@@ -1017,7 +1017,7 @@ abstract class AbstractWallet extends EventEmitter {
 
         // Check if TXO has been spent.
         if (receiveCommitment.spendtxid === false) {
-          const nullifierTxid = await merkletree.getNullifierTxid(receiveCommitment.nullifier);
+          const nullifierTxid = await merkletree.getNullifierTxid(receiveCommitment.nullifier, tree);
           if (isDefined(nullifierTxid)) {
             receiveCommitment.spendtxid = nullifierTxid;
             await this.updateReceiveCommitmentInDB(chain, tree, position, receiveCommitment);