fix(quartz): repair equals/hashCode contracts in OTS ops and VerifyResult

Op instances key Timestamp.ops (MutableMap<Op, Timestamp>), so contract violations corrupt hash-map behavior: - OpKECCAK256 defined equals without hashCode, so equal instances hashed by identity — two equal keys could land in different buckets, producing duplicate branches or failed lookups in keccak256 timestamp trees. Add hashCode = TAG, mirroring OpSHA1/OpSHA256/OpRIPEMD160. - OpBinary defined hashCode without equals — and its TAG referenced Op.TAG (0x00), a no-op XOR. Define the equals/hashCode pair once on OpBinary using tag() and drop the duplicated overrides from OpAppend/OpPrepend (behavior unchanged: same tag + same arg content). - VerifyResult.equals cast without a type test (ClassCastException on foreign types instead of false) and hashCode force-cast the nullable timestamp (NPE for null-timestamp results). Convert to a data class; the custom toString and compareTo stay.
2026-08-12 01:07:46 +00:00 · 2026-07-04 21:37:24 +02:00
parent b7912244fb
commit 772b4ea8ed
6 changed files with 116 additions and 29 deletions
@@ -23,7 +23,7 @@ package com.vitorpamplona.quartz.nip03Timestamp.ots
 /**
 * Class that lets us compare, sort, store and print timestamps.
 */
-class VerifyResult(
+data class VerifyResult(
    val timestamp: Long?,
    val height: Int,
 ) : Comparable<VerifyResult> {
@@ -41,11 +41,4 @@ class VerifyResult(
    }

    override fun compareTo(other: VerifyResult): Int = this.height - other.height
-
-    override fun equals(other: Any?): Boolean {
-        val vr = other as VerifyResult
-        return this.timestamp == vr.timestamp && this.height == vr.height
-    }
-
-    override fun hashCode(): Int = (((this.timestamp) as Long).toInt()) xor this.height
 }
@@ -37,16 +37,6 @@ class OpAppend(

    public override fun call(msg: ByteArray): ByteArray = msg + this.arg

-    override fun equals(other: Any?): Boolean {
-        if (other !is OpAppend) {
-            return false
-        }
-
-        return this.arg.contentEquals(other.arg)
-    }
-
-    override fun hashCode(): Int = TAG.toInt() xor this.arg.contentHashCode()
-
    companion object {
        val TAG: Byte = 0xf0.toByte()

@@ -53,7 +53,9 @@ abstract class OpBinary(
        return this.tag() - other.tag()
    }

-    override fun hashCode(): Int = TAG.toInt() xor this.arg.contentHashCode()
+    override fun equals(other: Any?): Boolean = other is OpBinary && this.tag() == other.tag() && this.arg.contentEquals(other.arg)
+
+    override fun hashCode(): Int = this.tag().toInt() xor this.arg.contentHashCode()

    companion object {
        @Throws(DeserializationException::class)
@@ -51,6 +51,8 @@ class OpKECCAK256 : OpCrypto() {

    override fun equals(other: Any?): Boolean = (other is OpKECCAK256)

+    override fun hashCode(): Int = TAG.toInt()
+
    companion object {
        val TAG: Byte = 103.toByte()

@@ -37,16 +37,6 @@ class OpPrepend(

    public override fun call(msg: ByteArray): ByteArray = this.arg + msg

-    override fun equals(other: Any?): Boolean {
-        if (other !is OpPrepend) {
-            return false
-        }
-
-        return this.arg.contentEquals(other.arg)
-    }
-
-    public override fun hashCode(): Int = TAG.toInt() xor this.arg.contentHashCode()
-
    companion object {
        val TAG: Byte = 0xf1.toByte()

@@ -0,0 +1,110 @@
+/*
+ * Copyright (c) 2025 Vitor Pamplona
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
+ * Software, and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+ * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
+ * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+package com.vitorpamplona.quartz.nip03Timestamp.ots
+
+import com.vitorpamplona.quartz.nip03Timestamp.ots.op.Op
+import com.vitorpamplona.quartz.nip03Timestamp.ots.op.OpAppend
+import com.vitorpamplona.quartz.nip03Timestamp.ots.op.OpKECCAK256
+import com.vitorpamplona.quartz.nip03Timestamp.ots.op.OpPrepend
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertNotEquals
+import kotlin.test.assertTrue
+
+/**
+ * Equals/hashCode contract tests for the types keying [Timestamp.ops]
+ * (a MutableMap<Op, Timestamp>): equal ops MUST hash equally or map
+ * lookups produce duplicate branches and failed upgrades.
+ */
+class OtsEqualsContractTest {
+    // --- OpKECCAK256: equals existed without hashCode; equal instances hashed by identity ---
+
+    @Test
+    fun keccakInstancesAreEqualAndHashEqually() {
+        val a = OpKECCAK256()
+        val b = OpKECCAK256()
+
+        assertEquals(a, b)
+        assertEquals(a.hashCode(), b.hashCode())
+    }
+
+    @Test
+    fun keccakWorksAsMapKey() {
+        val map = mutableMapOf<Op, String>(OpKECCAK256() to "branch")
+
+        assertEquals("branch", map[OpKECCAK256()])
+    }
+
+    // --- OpBinary: equals is defined once on the superclass, keyed on tag() + arg content ---
+
+    @Test
+    fun binaryOpsWithSameArgAndClassAreEqual() {
+        val arg = byteArrayOf(1, 2, 3)
+
+        assertEquals(OpAppend(arg), OpAppend(byteArrayOf(1, 2, 3)))
+        assertEquals(OpAppend(arg).hashCode(), OpAppend(byteArrayOf(1, 2, 3)).hashCode())
+    }
+
+    @Test
+    fun binaryOpsWithDifferentArgAreNotEqual() {
+        assertNotEquals(OpAppend(byteArrayOf(1, 2, 3)), OpAppend(byteArrayOf(9)))
+    }
+
+    @Test
+    fun appendAndPrependWithSameArgAreNotEqual() {
+        // Same arg content, different tag — the superclass equals must not conflate them.
+        val arg = byteArrayOf(1, 2, 3)
+
+        assertNotEquals<Op>(OpAppend(arg), OpPrepend(arg))
+    }
+
+    @Test
+    fun binaryOpsWorkAsMapKeys() {
+        val map = mutableMapOf<Op, String>()
+        map[OpAppend(byteArrayOf(1, 2, 3))] = "append"
+        map[OpPrepend(byteArrayOf(1, 2, 3))] = "prepend"
+
+        assertEquals(2, map.size)
+        assertEquals("append", map[OpAppend(byteArrayOf(1, 2, 3))])
+        assertEquals("prepend", map[OpPrepend(byteArrayOf(1, 2, 3))])
+    }
+
+    // --- VerifyResult: equals used to throw on null/foreign types; hashCode NPEd on null timestamp ---
+
+    @Test
+    fun verifyResultEqualsNullIsFalse() {
+        assertFalse(VerifyResult(1234L, 100).equals(null))
+    }
+
+    @Test
+    fun verifyResultEqualsForeignTypeIsFalse() {
+        assertFalse(VerifyResult(1234L, 100).equals("not a VerifyResult"))
+    }
+
+    @Test
+    fun verifyResultWithNullTimestampHashesWithoutThrowing() {
+        val result = VerifyResult(null, 100)
+
+        result.hashCode()
+        assertTrue(result == VerifyResult(null, 100))
+    }
+}