ERS: sort the hash lists once instead of inserting into a LinkedList by index

SortedHashList and SortedIndexedHashList kept their hashes in a LinkedList and
found each insertion point by walking it with get(index). LinkedList.get(i) is
O(i), so one add was O(n^2) and building a list of n hashes was O(n^3): 10,000
data objects took 347 s in generateTimeStampRequest, rising by about a factor of
eight per doubling.

Both lists are on the evidence record generation path - SortedIndexedHashList
from ERSArchiveTimeStampGenerator.getPartialHashtrees(), SortedHashList from
BinaryTreeRootCalculator.computeRootHash() - and neither can be replaced from
outside the library, since getPartialHashtrees() is private.

Both classes expose only size(), getFirst() and toList(), so collect into an
ArrayList and sort once, lazily, in the accessors. The order is unchanged: the
old add inserted each hash after every element comparing <= to it, which is
where a stable sort of the insertion sequence puts it, and Collections.sort is
stable. getFirst() still throws NoSuchElementException on an empty list.

Tests: testSortedHashListOrder reproduces the original insertion algorithm and
asserts the list matches it element for element over 1,200 pseudo-random values
including duplicates and shared prefixes; testLargeDataObjectSet builds a
reduced hash tree over 2,000 data objects, reaching both lists, and asserts the
root does not depend on the order the objects were added in.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0137cntJ35EeSoLRzoBaUVEK

Author: dejmekradim

pkix/src/main/java/org/bouncycastle/tsp/ers/SortedHashList.java

@@ -1,9 +1,10 @@
 package org.bouncycastle.tsp.ers;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Comparator;
-import java.util.LinkedList;
 import java.util.List;
+import java.util.NoSuchElementException;
 
 /**
  * A sorting list - byte[] are sorted in ascending order.
@@ -12,47 +13,30 @@ public class SortedHashList
 {
     private static final Comparator<byte[]> hashComp = new ByteArrayComparator();
 
-    private final LinkedList<byte[]> baseList = new LinkedList<byte[]>();
+    private final List<byte[]> baseList = new ArrayList<byte[]>();
+
+    private boolean isSorted = true;
 
     public SortedHashList()
     {
     }
 
     public byte[] getFirst()
     {
-        return (byte[])baseList.getFirst();
+        if (baseList.isEmpty())
+        {
+            throw new NoSuchElementException();
+        }
+
+        sort();
+
+        return (byte[])baseList.get(0);
     }
 
     public void add(byte[] hash)
     {
-        if (baseList.size() == 0)
-        {
-             baseList.addFirst(hash);
-        }
-        else
-        {
-            if (hashComp.compare(hash, baseList.get(0)) < 0)
-            {
-                baseList.addFirst(hash);
-            }
-            else
-            {
-                int index = 1;
-                while(index < baseList.size() && hashComp.compare(baseList.get(index), hash) <= 0)
-                {
-                    index++;
-                }
-
-                if (index == baseList.size())
-                {
-                    baseList.add(hash);
-                }
-                else
-                {
-                    baseList.add(index, hash);
-                }
-            }
-        }
+        baseList.add(hash);
+        isSorted = false;
     }
 
     public int size()
@@ -62,6 +46,25 @@ public int size()
 
     public List<byte[]> toList()
     {
+        sort();
+
         return new ArrayList<byte[]>(baseList);
     }
+
+    /**
+     * Sorting is deferred to the accessors. Inserting each hash on add() meant searching a
+     * LinkedList for the insertion point with get(index), which is O(index), so a single add()
+     * was O(n^2) and building a list of n hashes was O(n^3).
+     * <p>
+     * Collections.sort() is stable, so hashes comparing equal keep the order they were added
+     * in - which is where inserting after the last equal element used to put them.
+     */
+    private void sort()
+    {
+        if (!isSorted)
+        {
+            Collections.sort(baseList, hashComp);
+            isSorted = true;
+        }
+    }
 }

pkix/src/main/java/org/bouncycastle/tsp/ers/SortedIndexedHashList.java

@@ -1,9 +1,10 @@
 package org.bouncycastle.tsp.ers;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Comparator;
-import java.util.LinkedList;
 import java.util.List;
+import java.util.NoSuchElementException;
 
 /**
  * A sorting list - byte[] are sorted in ascending order.
@@ -12,47 +13,38 @@ public class SortedIndexedHashList
 {
     private static final Comparator<byte[]> hashComp = new ByteArrayComparator();
 
-    private final LinkedList<IndexedHash> baseList = new LinkedList<IndexedHash>();
+    private static final Comparator<IndexedHash> digestComp = new Comparator<IndexedHash>()
+    {
+        public int compare(IndexedHash l, IndexedHash r)
+        {
+            return hashComp.compare(l.digest, r.digest);
+        }
+    };
+
+    private final List<IndexedHash> baseList = new ArrayList<IndexedHash>();
+
+    private boolean isSorted = true;
 
     public SortedIndexedHashList()
     {
     }
 
     public IndexedHash getFirst()
     {
-        return (IndexedHash)baseList.getFirst();
+        if (baseList.isEmpty())
+        {
+            throw new NoSuchElementException();
+        }
+
+        sort();
+
+        return (IndexedHash)baseList.get(0);
     }
 
     public void add(IndexedHash hash)
     {
-        if (baseList.size() == 0)
-        {
-             baseList.addFirst(hash);
-        }
-        else
-        {
-            if (hashComp.compare(hash.digest, ((IndexedHash)baseList.get(0)).digest) < 0)
-            {
-                baseList.addFirst(hash);
-            }
-            else
-            {
-                int index = 1;
-                while(index < baseList.size() && hashComp.compare(((IndexedHash)baseList.get(index)).digest, hash.digest) <= 0)
-                {
-                    index++;
-                }
-
-                if (index == baseList.size())
-                {
-                    baseList.add(hash);
-                }
-                else
-                {
-                    baseList.add(index, hash);
-                }
-            }
-        }
+        baseList.add(hash);
+        isSorted = false;
     }
 
     public int size()
@@ -62,6 +54,22 @@ public int size()
 
     public List<IndexedHash> toList()
     {
+        sort();
+
         return new ArrayList<IndexedHash>(baseList);
     }
+
+    /**
+     * Sorting is deferred to the accessors, for the reason given on SortedHashList.sort():
+     * finding the insertion point with LinkedList.get(index) made building a list of n hashes
+     * O(n^3). Collections.sort() is stable, so hashes comparing equal keep ascending order.
+     */
+    private void sort()
+    {
+        if (!isSorted)
+        {
+            Collections.sort(baseList, digestComp);
+            isSorted = true;
+        }
+    }
 }

pkix/src/test/java/org/bouncycastle/tsp/test/ERSTest.java

@@ -19,7 +19,9 @@
 import java.util.Collections;
 import java.util.Date;
 import java.util.HashSet;
+import java.util.LinkedList;
 import java.util.List;
+import java.util.Random;
 
 import junit.framework.TestCase;
 import org.bouncycastle.asn1.ASN1EncodableVector;
@@ -68,6 +70,7 @@
 import org.bouncycastle.tsp.ers.ERSException;
 import org.bouncycastle.tsp.ers.ERSFileData;
 import org.bouncycastle.tsp.ers.ERSInputStreamData;
+import org.bouncycastle.tsp.ers.SortedHashList;
 import org.bouncycastle.util.Arrays;
 import org.bouncycastle.util.Store;
 import org.bouncycastle.util.Strings;
@@ -1355,6 +1358,150 @@ private int compare(byte[] a, byte[] b)
         return new BigInteger(1, a).compareTo(new BigInteger(1, b));
     }
 
+    /**
+     * SortedHashList used to find each hash's insertion point by walking a LinkedList with
+     * get(index). The order it produced is the one recorded in every existing evidence record,
+     * so it is reproduced here from the original algorithm and compared against the list's
+     * output, over pseudo-random input including duplicates and arrays of differing lengths.
+     */
+    public void testSortedHashListOrder()
+    {
+        Random random = new Random(0x5eed);
+        List<byte[]> input = new ArrayList<byte[]>();
+
+        for (int i = 0; i != 1000; i++)
+        {
+            byte[] value = new byte[random.nextInt(33)];
+            random.nextBytes(value);
+            input.add(value);
+        }
+        // duplicates, and values sharing a prefix with a longer one
+        for (int i = 0; i != 100; i++)
+        {
+            input.add((byte[])input.get(i));
+            input.add(Arrays.copyOfRange((byte[])input.get(i + 100), 0, ((byte[])input.get(i + 100)).length / 2));
+        }
+
+        SortedHashList list = new SortedHashList();
+        for (int i = 0; i != input.size(); i++)
+        {
+            list.add((byte[])input.get(i));
+        }
+
+        List<byte[]> expected = insertionSorted(input);
+        List<byte[]> actual = list.toList();
+
+        assertEquals(input.size(), list.size());
+        assertEquals(expected.size(), actual.size());
+        for (int i = 0; i != expected.size(); i++)
+        {
+            assertTrue("differs at " + i, Arrays.areEqual((byte[])expected.get(i), (byte[])actual.get(i)));
+        }
+        assertTrue(Arrays.areEqual((byte[])expected.get(0), list.getFirst()));
+    }
+
+    /**
+     * The order SortedHashList produced before sorting was deferred to the accessors.
+     */
+    private List<byte[]> insertionSorted(List<byte[]> hashes)
+    {
+        LinkedList<byte[]> baseList = new LinkedList<byte[]>();
+
+        for (int h = 0; h != hashes.size(); h++)
+        {
+            byte[] hash = (byte[])hashes.get(h);
+
+            if (baseList.size() == 0)
+            {
+                baseList.addFirst(hash);
+            }
+            else if (compareUnsigned(hash, (byte[])baseList.get(0)) < 0)
+            {
+                baseList.addFirst(hash);
+            }
+            else
+            {
+                int index = 1;
+                while (index < baseList.size() && compareUnsigned((byte[])baseList.get(index), hash) <= 0)
+                {
+                    index++;
+                }
+
+                if (index == baseList.size())
+                {
+                    baseList.add(hash);
+                }
+                else
+                {
+                    baseList.add(index, hash);
+                }
+            }
+        }
+
+        return baseList;
+    }
+
+    private int compareUnsigned(byte[] left, byte[] right)
+    {
+        for (int i = 0; i < left.length && i < right.length; i++)
+        {
+            int a = (left[i] & 0xff);
+            int b = (right[i] & 0xff);
+
+            if (a != b)
+            {
+                return a - b;
+            }
+        }
+        return left.length - right.length;
+    }
+
+    /**
+     * A reduced hash tree over a large number of data objects. This reaches both sorted lists -
+     * SortedIndexedHashList from ERSArchiveTimeStampGenerator.getPartialHashtrees(), and
+     * SortedHashList from BinaryTreeRootCalculator.computeRootHash() - and took about 2.5
+     * seconds for these 2,000 objects when the insertion point was found by walking a
+     * LinkedList, rising by roughly a factor of eight per doubling (10,000 objects took 347
+     * seconds). The root is also checked to be independent of the order the objects were added
+     * in, which is what the sorting is there for.
+     */
+    public void testLargeDataObjectSet()
+        throws Exception
+    {
+        DigestCalculatorProvider digestCalculatorProvider = new JcaDigestCalculatorProviderBuilder().build();
+
+        List<ERSData> dataObjects = new ArrayList<ERSData>();
+        for (int i = 0; i != 2000; i++)
+        {
+            dataObjects.add(new ERSByteData(Strings.toByteArray("document " + i)));
+        }
+
+        byte[] ascending = rootOf(dataObjects, digestCalculatorProvider);
+
+        List<ERSData> reversed = new ArrayList<ERSData>(dataObjects);
+        Collections.reverse(reversed);
+
+        assertTrue(Arrays.areEqual(ascending, rootOf(reversed, digestCalculatorProvider)));
+    }
+
+    private byte[] rootOf(List<ERSData> dataObjects, DigestCalculatorProvider digestCalculatorProvider)
+        throws Exception
+    {
+        ERSArchiveTimeStampGenerator ersGen = new ERSArchiveTimeStampGenerator(
+            digestCalculatorProvider.get(new AlgorithmIdentifier(NISTObjectIdentifiers.id_sha256)));
+
+        for (int i = 0; i != dataObjects.size(); i++)
+        {
+            ersGen.addData((ERSData)dataObjects.get(i));
+        }
+
+        TimeStampRequestGenerator tspReqGen = new TimeStampRequestGenerator();
+
+        tspReqGen.setCertReq(true);
+
+        return ersGen.generateTimeStampRequest(tspReqGen).getMessageImprintDigest();
+    }
+
     public void testReducedHashTrees()
         throws Exception
     {