From a53b508049d0febf59dfd3f7e5d75776a0b3f7a8 Mon Sep 17 00:00:00 2001 From: Amichai Rothman Date: Tue, 7 Jul 2015 14:24:25 +0300 Subject: [PATCH] Fix various bugs and documented unintuitive/suspicious behavior in equals/hashCode/compareTo implementations. --- core/src/main/java/org/bitcoinj/core/DumpedPrivateKey.java | 2 +- core/src/main/java/org/bitcoinj/core/GetBlocksMessage.java | 4 ++-- core/src/main/java/org/bitcoinj/core/GetHeadersMessage.java | 4 ++-- core/src/main/java/org/bitcoinj/core/Sha256Hash.java | 3 ++- core/src/main/java/org/bitcoinj/core/TransactionInput.java | 2 -- core/src/main/java/org/bitcoinj/core/TransactionOutPoint.java | 2 +- core/src/main/java/org/bitcoinj/core/TransactionOutput.java | 2 +- core/src/main/java/org/bitcoinj/core/Utils.java | 1 + core/src/main/java/org/bitcoinj/core/Wallet.java | 1 + core/src/main/java/org/bitcoinj/crypto/ChildNumber.java | 1 + core/src/main/java/org/bitcoinj/utils/ExponentialBackoff.java | 1 + 11 files changed, 13 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/core/DumpedPrivateKey.java b/core/src/main/java/org/bitcoinj/core/DumpedPrivateKey.java index ac7cc387..8145ed84 100644 --- a/core/src/main/java/org/bitcoinj/core/DumpedPrivateKey.java +++ b/core/src/main/java/org/bitcoinj/core/DumpedPrivateKey.java @@ -90,6 +90,6 @@ public class DumpedPrivateKey extends VersionedChecksummedBytes { @Override public int hashCode() { - return Objects.hashCode(bytes, version, compressed); + return Objects.hashCode(version, compressed, Arrays.hashCode(bytes)); } } diff --git a/core/src/main/java/org/bitcoinj/core/GetBlocksMessage.java b/core/src/main/java/org/bitcoinj/core/GetBlocksMessage.java index c2e43042..fbbcedab 100644 --- a/core/src/main/java/org/bitcoinj/core/GetBlocksMessage.java +++ b/core/src/main/java/org/bitcoinj/core/GetBlocksMessage.java @@ -102,14 +102,14 @@ public class GetBlocksMessage extends Message { GetBlocksMessage other = (GetBlocksMessage) o; return version == other.version && locator.size() == other.locator.size() && - locator.containsAll(other.locator) && + locator.containsAll(other.locator) && // ignores locator ordering stopHash.equals(other.stopHash); } @Override public int hashCode() { int hashCode = (int) version ^ "getblocks".hashCode(); - for (Sha256Hash aLocator : locator) hashCode ^= aLocator.hashCode(); + for (Sha256Hash aLocator : locator) hashCode ^= aLocator.hashCode(); // ignores locator ordering hashCode ^= stopHash.hashCode(); return hashCode; } diff --git a/core/src/main/java/org/bitcoinj/core/GetHeadersMessage.java b/core/src/main/java/org/bitcoinj/core/GetHeadersMessage.java index a93c7290..7df17063 100644 --- a/core/src/main/java/org/bitcoinj/core/GetHeadersMessage.java +++ b/core/src/main/java/org/bitcoinj/core/GetHeadersMessage.java @@ -49,14 +49,14 @@ public class GetHeadersMessage extends GetBlocksMessage { GetHeadersMessage other = (GetHeadersMessage) o; return version == other.version && locator.size() == other.locator.size() && - locator.containsAll(other.locator) && + locator.containsAll(other.locator) && // ignores locator ordering stopHash.equals(other.stopHash); } @Override public int hashCode() { int hashCode = (int) version ^ "getheaders".hashCode(); - for (Sha256Hash aLocator : locator) hashCode ^= aLocator.hashCode(); + for (Sha256Hash aLocator : locator) hashCode ^= aLocator.hashCode(); // ignores locator ordering hashCode ^= stopHash.hashCode(); return hashCode; } diff --git a/core/src/main/java/org/bitcoinj/core/Sha256Hash.java b/core/src/main/java/org/bitcoinj/core/Sha256Hash.java index 70f96f67..4c35e1e6 100644 --- a/core/src/main/java/org/bitcoinj/core/Sha256Hash.java +++ b/core/src/main/java/org/bitcoinj/core/Sha256Hash.java @@ -265,6 +265,7 @@ public class Sha256Hash implements Serializable, Comparable { @Override public int compareTo(Sha256Hash o) { - return this.hashCode() - o.hashCode(); + // note that in this implementation compareTo() is not consistent with equals() + return this.hashCode() - o.hashCode(); // arbitrary but consistent } } diff --git a/core/src/main/java/org/bitcoinj/core/TransactionInput.java b/core/src/main/java/org/bitcoinj/core/TransactionInput.java index a31697c0..1341a17d 100644 --- a/core/src/main/java/org/bitcoinj/core/TransactionInput.java +++ b/core/src/main/java/org/bitcoinj/core/TransactionInput.java @@ -479,7 +479,6 @@ public class TransactionInput extends ChildMessage implements Serializable { if (sequence != input.sequence) return false; if (!outpoint.equals(input.outpoint)) return false; if (!Arrays.equals(scriptBytes, input.scriptBytes)) return false; - if (scriptSig != null ? !scriptSig.equals(input.scriptSig) : input.scriptSig != null) return false; if (parent != input.parent) return false; return true; @@ -490,7 +489,6 @@ public class TransactionInput extends ChildMessage implements Serializable { int result = (int) (sequence ^ (sequence >>> 32)); result = 31 * result + outpoint.hashCode(); result = 31 * result + (scriptBytes != null ? Arrays.hashCode(scriptBytes) : 0); - result = 31 * result + (scriptSig != null ? scriptSig.hashCode() : 0); return result; } } diff --git a/core/src/main/java/org/bitcoinj/core/TransactionOutPoint.java b/core/src/main/java/org/bitcoinj/core/TransactionOutPoint.java index 0f8df74f..8f410d29 100644 --- a/core/src/main/java/org/bitcoinj/core/TransactionOutPoint.java +++ b/core/src/main/java/org/bitcoinj/core/TransactionOutPoint.java @@ -239,6 +239,6 @@ public class TransactionOutPoint extends ChildMessage implements Serializable { @Override public int hashCode() { - return 31 * hash.hashCode() + (int) (index ^ (index >>> 32)); + return 31 * getHash().hashCode() + (int) (getIndex() ^ (getIndex() >>> 32)); } } diff --git a/core/src/main/java/org/bitcoinj/core/TransactionOutput.java b/core/src/main/java/org/bitcoinj/core/TransactionOutput.java index ced0a7d5..79f30814 100644 --- a/core/src/main/java/org/bitcoinj/core/TransactionOutput.java +++ b/core/src/main/java/org/bitcoinj/core/TransactionOutput.java @@ -444,7 +444,7 @@ public class TransactionOutput extends ChildMessage implements Serializable { int result = (int) (value ^ (value >>> 32)); result = 31 * result + Arrays.hashCode(scriptBytes); if (parent != null) - result *= parent.getHash().hashCode() + getIndex(); + result *= parent.getHash().hashCode(); return result; } } diff --git a/core/src/main/java/org/bitcoinj/core/Utils.java b/core/src/main/java/org/bitcoinj/core/Utils.java index 407404b6..c6782edb 100644 --- a/core/src/main/java/org/bitcoinj/core/Utils.java +++ b/core/src/main/java/org/bitcoinj/core/Utils.java @@ -578,6 +578,7 @@ public class Utils { private static class Pair implements Comparable { int item, count; public Pair(int item, int count) { this.count = count; this.item = item; } + // note that in this implementation compareTo() is not consistent with equals() @Override public int compareTo(Pair o) { return -Ints.compare(count, o.count); } } diff --git a/core/src/main/java/org/bitcoinj/core/Wallet.java b/core/src/main/java/org/bitcoinj/core/Wallet.java index 2c5ed9e9..a9272455 100644 --- a/core/src/main/java/org/bitcoinj/core/Wallet.java +++ b/core/src/main/java/org/bitcoinj/core/Wallet.java @@ -4050,6 +4050,7 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha } @Override public int compareTo(TxOffsetPair o) { + // note that in this implementation compareTo() is not consistent with equals() return Ints.compare(offset, o.offset); } } diff --git a/core/src/main/java/org/bitcoinj/crypto/ChildNumber.java b/core/src/main/java/org/bitcoinj/crypto/ChildNumber.java index 7e912af5..d26d5a8b 100644 --- a/core/src/main/java/org/bitcoinj/crypto/ChildNumber.java +++ b/core/src/main/java/org/bitcoinj/crypto/ChildNumber.java @@ -91,6 +91,7 @@ public class ChildNumber implements Comparable { @Override public int compareTo(ChildNumber other) { + // note that in this implementation compareTo() is not consistent with equals() return Ints.compare(this.num(), other.num()); } } diff --git a/core/src/main/java/org/bitcoinj/utils/ExponentialBackoff.java b/core/src/main/java/org/bitcoinj/utils/ExponentialBackoff.java index 0be4449f..9b6f127a 100644 --- a/core/src/main/java/org/bitcoinj/utils/ExponentialBackoff.java +++ b/core/src/main/java/org/bitcoinj/utils/ExponentialBackoff.java @@ -90,6 +90,7 @@ public class ExponentialBackoff implements Comparable { @Override public int compareTo(ExponentialBackoff other) { + // note that in this implementation compareTo() is not consistent with equals() if (retryTime < other.retryTime) return -1; if (retryTime > other.retryTime)