PrefixedChecksummedBytes hierarchy: Add Comparable interface to Address, remove from PrefixedChecksummedBytes.

Requires address subclasses to implement compareTo() and provide the compareAddressPartial() method for comparing the first two fields.

This changes the natural ordering of addresses, and removes the natural ordering entirely for other PrefixedChecksummedBytes subclasses.

This also fixes a compareTo() collision regarding P2SH and non-P2SH addresses with the same bytes[].

core/src/main/java/org/bitcoinj/core/Address.java

@@ -31,7 +31,7 @@ import org.bitcoinj.script.Script.ScriptType;
  * form.
  * </p>
  */
-public abstract class Address extends PrefixedChecksummedBytes {
+public abstract class Address extends PrefixedChecksummedBytes implements Comparable<Address> {
     public Address(NetworkParameters params, byte[] bytes) {
         super(params, bytes);
     }
@@ -101,4 +101,44 @@ public abstract class Address extends PrefixedChecksummedBytes {
      * @return type of output script
      */
     public abstract ScriptType getOutputScriptType();
+
+    /**
+     * Comparison field order for addresses is:
+     * <ol>
+     *     <li>{@link NetworkParameters#getId()}</li>
+     *     <li>Legacy vs. Segwit</li>
+     *     <li>(Legacy only) Version byte</li>
+     *     <li>remaining {@code bytes}</li>
+     * </ol>
+     * <p>
+     * Implementations may use {@code compareAddressPartial} for tests 1 and 2.
+     *
+     * @param o other {@code Address} object
+     * @return comparison result
+     */
+    @Override
+    abstract public int compareTo(Address o);
+
+    /**
+     * Comparator for the first two comparison fields in {@code Address} comparisons, see {@link Address#compareTo(Address)}.
+     * Used by {@link LegacyAddress#compareTo(Address)} and {@link SegwitAddress#compareTo(Address)}.
+     *
+     * @param o other {@code Address} object
+     * @return comparison result
+     */
+    protected int compareAddressPartial(Address o) {
+        // First compare netParams
+        int result = this.params.getId().compareTo(o.params.getId());
+        if (result != 0) return result;
+
+        // Then compare Legacy vs Segwit
+        if (this instanceof LegacyAddress && o instanceof SegwitAddress) {
+            return -1;  // Legacy addresses (starting with 1 or 3) come before Segwit addresses.
+        } else if (this instanceof SegwitAddress && o instanceof LegacyAddress) {
+            return 1;
+        } else {
+            // If both are the same type, then compareTo for that type will finish the comparison
+            return 0;
+        }
+    }
 }

core/src/main/java/org/bitcoinj/core/LegacyAddress.java

@@ -23,6 +23,7 @@ import java.util.Objects;
 
 import javax.annotation.Nullable;
 
+import com.google.common.primitives.UnsignedBytes;
 import org.bitcoinj.params.Networks;
 import org.bitcoinj.script.Script.ScriptType;
 
@@ -213,4 +214,20 @@ public class LegacyAddress extends Address {
     public LegacyAddress clone() throws CloneNotSupportedException {
         return (LegacyAddress) super.clone();
     }
+
+    /**
+     * {@inheritDoc}
+     *
+     * @param o other {@code Address} object
+     * @return comparison result
+     */
+    @Override
+    public int compareTo(Address o) {
+        int result = compareAddressPartial(o);
+        if (result != 0) return result;
+
+        // Compare version byte and finally the {@code bytes} field itself
+        result = Integer.compare(getVersion(), ((LegacyAddress) o).getVersion());
+        return result != 0 ? result : UnsignedBytes.lexicographicalComparator().compare(this.bytes, o.bytes);
+    }
 }

core/src/main/java/org/bitcoinj/core/PrefixedChecksummedBytes.java

@@ -25,7 +25,6 @@ import java.lang.reflect.Field;
 import java.util.Arrays;
 import java.util.Objects;
 
-import com.google.common.primitives.UnsignedBytes;
 import static com.google.common.base.Preconditions.checkNotNull;
 
 /**
@@ -41,7 +40,7 @@ import static com.google.common.base.Preconditions.checkNotNull;
  * keys exported using Bitcoin Core's dumpprivkey command.
  * </p>
  */
-public abstract class PrefixedChecksummedBytes implements Serializable, Cloneable, Comparable<PrefixedChecksummedBytes> {
+public abstract class PrefixedChecksummedBytes implements Serializable, Cloneable {
     protected final transient NetworkParameters params;
     protected final byte[] bytes;
 
@@ -80,15 +79,6 @@ public abstract class PrefixedChecksummedBytes implements Serializable, Cloneabl
         return (PrefixedChecksummedBytes) super.clone();
     }
 
-    /**
-     * This implementation uses an optimized Google Guava method to compare {@code bytes}.
-     */
-    @Override
-    public int compareTo(PrefixedChecksummedBytes o) {
-        int result = this.params.getId().compareTo(o.params.getId());
-        return result != 0 ? result : UnsignedBytes.lexicographicalComparator().compare(this.bytes, o.bytes);
-    }
-
     // Java serialization
 
     private void writeObject(ObjectOutputStream out) throws IOException {

core/src/main/java/org/bitcoinj/core/SegwitAddress.java

@@ -23,6 +23,7 @@ import java.io.ByteArrayOutputStream;
 
 import javax.annotation.Nullable;
 
+import com.google.common.primitives.UnsignedBytes;
 import org.bitcoinj.params.Networks;
 import org.bitcoinj.script.Script;
 
@@ -246,4 +247,19 @@ public class SegwitAddress extends Address {
         }
         return out.toByteArray();
     }
+
+    /**
+     * {@inheritDoc}
+     *
+     * @param o other {@code Address} object
+     * @return comparison result
+     */
+    @Override
+    public int compareTo(Address o) {
+        int result = compareAddressPartial(o);
+        if (result != 0) return result;
+
+        // Compare the bytes
+        return UnsignedBytes.lexicographicalComparator().compare(this.bytes, o.bytes);
+    }
 }

core/src/test/java/org/bitcoinj/core/LegacyAddressTest.java

@@ -247,6 +247,16 @@ public class LegacyAddressTest {
         assertTrue(result > 0);
     }
 
+    @Test
+    public void comparisonNotEquals() {
+        // These addresses only differ by version byte
+        LegacyAddress a = LegacyAddress.fromBase58(MAINNET, "14wivxvNTv9THhewPotsooizZawaWbEKE2");
+        LegacyAddress b = LegacyAddress.fromBase58(MAINNET, "35djrWQp1pTqNsMNWuZUES5vi7EJ74m9Eh");
+
+        int result = a.compareTo(b);
+        assertTrue(result != 0);
+    }
+
     @Test
     public void comparisonBytesVsString() throws Exception {
         BufferedReader dataSetReader = new BufferedReader(

core/src/test/java/org/bitcoinj/core/PrefixedChecksummedBytesTest.java

@@ -93,6 +93,7 @@ public class PrefixedChecksummedBytesTest {
         PrefixedChecksummedBytes a = new PrefixedChecksummedBytesToTest(params, HEX.decode("fda79a24e50ff70ff42f7d89585da5bd19d9e5cc"));
         PrefixedChecksummedBytes b = a.clone();
 
-        assertEquals(0, a.compareTo(b));
+        assertNotSame(a, b);
+        assertEquals(a, b);
     }
 }