diff --git a/src/main/java/org/qortal/controller/Controller.java b/src/main/java/org/qortal/controller/Controller.java index e2ae6ca4..d693f1fd 100644 --- a/src/main/java/org/qortal/controller/Controller.java +++ b/src/main/java/org/qortal/controller/Controller.java @@ -675,7 +675,7 @@ public class Controller extends Thread { public static final Predicate hasInferiorChainTip = peer -> { final PeerChainTipData peerChainTipData = peer.getChainTipData(); final List inferiorChainTips = Synchronizer.getInstance().inferiorChainSignatures; - return peerChainTipData == null || peerChainTipData.getLastBlockSignature() == null || inferiorChainTips.contains(new ByteArray(peerChainTipData.getLastBlockSignature())); + return peerChainTipData == null || peerChainTipData.getLastBlockSignature() == null || inferiorChainTips.contains(ByteArray.wrap(peerChainTipData.getLastBlockSignature())); }; public static final Predicate hasOldVersion = peer -> { @@ -1203,7 +1203,7 @@ public class Controller extends Thread { byte[] signature = getBlockMessage.getSignature(); this.stats.getBlockMessageStats.requests.incrementAndGet(); - ByteArray signatureAsByteArray = new ByteArray(signature); + ByteArray signatureAsByteArray = ByteArray.wrap(signature); CachedBlockMessage cachedBlockMessage = this.blockMessageCache.get(signatureAsByteArray); int blockCacheSize = Settings.getInstance().getBlockCacheSize(); @@ -1283,7 +1283,7 @@ public class Controller extends Thread { if (getChainHeight() - blockData.getHeight() <= blockCacheSize) { this.stats.getBlockMessageStats.cacheFills.incrementAndGet(); - this.blockMessageCache.put(new ByteArray(blockData.getSignature()), blockMessage); + this.blockMessageCache.put(ByteArray.wrap(blockData.getSignature()), blockMessage); } } catch (DataException e) { LOGGER.error(String.format("Repository issue while send block %s to peer %s", Base58.encode(signature), peer), e); diff --git a/src/main/java/org/qortal/controller/Synchronizer.java b/src/main/java/org/qortal/controller/Synchronizer.java index c6e730fd..63a48888 100644 --- a/src/main/java/org/qortal/controller/Synchronizer.java +++ b/src/main/java/org/qortal/controller/Synchronizer.java @@ -314,7 +314,7 @@ public class Synchronizer extends Thread { case INFERIOR_CHAIN: { // Update our list of inferior chain tips - ByteArray inferiorChainSignature = new ByteArray(peer.getChainTipData().getLastBlockSignature()); + ByteArray inferiorChainSignature = ByteArray.wrap(peer.getChainTipData().getLastBlockSignature()); if (!inferiorChainSignatures.contains(inferiorChainSignature)) inferiorChainSignatures.add(inferiorChainSignature); @@ -343,7 +343,7 @@ public class Synchronizer extends Thread { // fall-through... case NOTHING_TO_DO: { // Update our list of inferior chain tips - ByteArray inferiorChainSignature = new ByteArray(peer.getChainTipData().getLastBlockSignature()); + ByteArray inferiorChainSignature = ByteArray.wrap(peer.getChainTipData().getLastBlockSignature()); if (!inferiorChainSignatures.contains(inferiorChainSignature)) inferiorChainSignatures.add(inferiorChainSignature); @@ -419,7 +419,7 @@ public class Synchronizer extends Thread { public void addInferiorChainSignature(byte[] inferiorSignature) { // Update our list of inferior chain tips - ByteArray inferiorChainSignature = new ByteArray(inferiorSignature); + ByteArray inferiorChainSignature = ByteArray.wrap(inferiorSignature); if (!inferiorChainSignatures.contains(inferiorChainSignature)) inferiorChainSignatures.add(inferiorChainSignature); } diff --git a/src/main/java/org/qortal/controller/tradebot/TradeBot.java b/src/main/java/org/qortal/controller/tradebot/TradeBot.java index 4a44eaa9..6d7ac942 100644 --- a/src/main/java/org/qortal/controller/tradebot/TradeBot.java +++ b/src/main/java/org/qortal/controller/tradebot/TradeBot.java @@ -402,7 +402,7 @@ public class TradeBot implements Listener { long now = NTP.getTime(); long newExpiry = generateExpiry(now); - ByteArray pubkeyByteArray = ByteArray.of(tradeNativeAccount.getPublicKey()); + ByteArray pubkeyByteArray = ByteArray.wrap(tradeNativeAccount.getPublicKey()); // If map entry's timestamp is missing, or within early renewal period, use the new expiry - otherwise use existing timestamp. synchronized (this.ourTradePresenceTimestampsByPubkey) { @@ -489,7 +489,7 @@ public class TradeBot implements Listener { int knownCount = entriesUnknownToPeer.size(); for (TradePresenceData peersTradePresence : peersTradePresences) { - ByteArray pubkeyByteArray = ByteArray.of(peersTradePresence.getPublicKey()); + ByteArray pubkeyByteArray = ByteArray.wrap(peersTradePresence.getPublicKey()); TradePresenceData ourEntry = entriesUnknownToPeer.get(pubkeyByteArray); @@ -546,7 +546,7 @@ public class TradeBot implements Listener { continue; } - ByteArray pubkeyByteArray = ByteArray.of(peersTradePresence.getPublicKey()); + ByteArray pubkeyByteArray = ByteArray.wrap(peersTradePresence.getPublicKey()); // Ignore if we've previously verified this timestamp+publickey combo or sent timestamp is older TradePresenceData existingTradeData = this.safeAllTradePresencesByPubkey.get(pubkeyByteArray); @@ -589,7 +589,7 @@ public class TradeBot implements Listener { continue; } - ByteArray atCodeHash = new ByteArray(atData.getCodeHash()); + ByteArray atCodeHash = ByteArray.wrap(atData.getCodeHash()); Supplier acctSupplier = acctSuppliersByCodeHash.get(atCodeHash); if (acctSupplier == null) { LOGGER.trace("Ignoring trade presence {} from peer {} as AT isn't a known ACCT?", @@ -642,7 +642,7 @@ public class TradeBot implements Listener { public void bridgePresence(long timestamp, byte[] publicKey, byte[] signature, String atAddress) { long expiry = generateExpiry(timestamp); - ByteArray pubkeyByteArray = ByteArray.of(publicKey); + ByteArray pubkeyByteArray = ByteArray.wrap(publicKey); TradePresenceData fakeTradePresenceData = new TradePresenceData(expiry, publicKey, signature, atAddress); diff --git a/src/main/java/org/qortal/crosschain/SupportedBlockchain.java b/src/main/java/org/qortal/crosschain/SupportedBlockchain.java index 5bff7ac9..a26e0e01 100644 --- a/src/main/java/org/qortal/crosschain/SupportedBlockchain.java +++ b/src/main/java/org/qortal/crosschain/SupportedBlockchain.java @@ -62,7 +62,7 @@ public enum SupportedBlockchain { private static final Map> supportedAcctsByCodeHash = Arrays.stream(SupportedBlockchain.values()) .map(supportedBlockchain -> supportedBlockchain.supportedAccts) .flatMap(List::stream) - .collect(Collectors.toUnmodifiableMap(triple -> new ByteArray(triple.getB()), Triple::getC)); + .collect(Collectors.toUnmodifiableMap(triple -> ByteArray.wrap(triple.getB()), Triple::getC)); private static final Map> supportedAcctsByName = Arrays.stream(SupportedBlockchain.values()) .map(supportedBlockchain -> supportedBlockchain.supportedAccts) @@ -94,7 +94,7 @@ public enum SupportedBlockchain { return getAcctMap(); return blockchain.supportedAccts.stream() - .collect(Collectors.toUnmodifiableMap(triple -> new ByteArray(triple.getB()), Triple::getC)); + .collect(Collectors.toUnmodifiableMap(triple -> ByteArray.wrap(triple.getB()), Triple::getC)); } public static Map> getFilteredAcctMap(String specificBlockchain) { @@ -109,7 +109,7 @@ public enum SupportedBlockchain { } public static ACCT getAcctByCodeHash(byte[] codeHash) { - ByteArray wrappedCodeHash = new ByteArray(codeHash); + ByteArray wrappedCodeHash = ByteArray.wrap(codeHash); Supplier acctInstanceSupplier = supportedAcctsByCodeHash.get(wrappedCodeHash); diff --git a/src/main/java/org/qortal/transaction/PresenceTransaction.java b/src/main/java/org/qortal/transaction/PresenceTransaction.java index 9bdbe3c7..d0f54548 100644 --- a/src/main/java/org/qortal/transaction/PresenceTransaction.java +++ b/src/main/java/org/qortal/transaction/PresenceTransaction.java @@ -185,7 +185,7 @@ public class PresenceTransaction extends Transaction { String signerAddress = Crypto.toAddress(this.transactionData.getCreatorPublicKey()); for (ATData atData : atsData) { - ByteArray atCodeHash = new ByteArray(atData.getCodeHash()); + ByteArray atCodeHash = ByteArray.wrap(atData.getCodeHash()); Supplier acctSupplier = acctSuppliersByCodeHash.get(atCodeHash); if (acctSupplier == null) continue; diff --git a/src/main/java/org/qortal/utils/ByteArray.java b/src/main/java/org/qortal/utils/ByteArray.java index d3464c9f..30eec3e9 100644 --- a/src/main/java/org/qortal/utils/ByteArray.java +++ b/src/main/java/org/qortal/utils/ByteArray.java @@ -8,12 +8,16 @@ public class ByteArray implements Comparable { private int hash; public final byte[] value; - public ByteArray(byte[] value) { - this.value = Objects.requireNonNull(value); + private ByteArray(byte[] value) { + this.value = value; } - public static ByteArray of(byte[] value) { - return new ByteArray(value); + public static ByteArray wrap(byte[] value) { + return new ByteArray(Objects.requireNonNull(value)); + } + + public static ByteArray copyOf(byte[] value) { + return new ByteArray(Arrays.copyOf(value, value.length)); } @Override @@ -36,12 +40,7 @@ public class ByteArray implements Comparable { byte[] val = this.value; if (h == 0 && val.length > 0) { - h = 1; - - for (int i = 0; i < val.length; ++i) - h = 31 * h + val[i]; - - this.hash = h; + this.hash = h = Arrays.hashCode(val); } return h; } @@ -53,24 +52,7 @@ public class ByteArray implements Comparable { } public int compareToPrimitive(byte[] otherValue) { - byte[] val = this.value; - - if (val.length < otherValue.length) - return -1; - - if (val.length > otherValue.length) - return 1; - - for (int i = 0; i < val.length; ++i) { - int a = val[i] & 0xFF; - int b = otherValue[i] & 0xFF; - if (a < b) - return -1; - if (a > b) - return 1; - } - - return 0; + return Arrays.compareUnsigned(this.value, otherValue); } public String toString() { diff --git a/src/test/java/org/qortal/test/ByteArrayTests.java b/src/test/java/org/qortal/test/ByteArrayTests.java index 8fb6f1cf..f954a367 100644 --- a/src/test/java/org/qortal/test/ByteArrayTests.java +++ b/src/test/java/org/qortal/test/ByteArrayTests.java @@ -32,19 +32,40 @@ public class ByteArrayTests { private static void fillMap(Map map) { for (byte[] testValue : testValues) - map.put(new ByteArray(testValue), String.valueOf(map.size())); + map.put(ByteArray.wrap(testValue), String.valueOf(map.size())); } private static byte[] dup(byte[] value) { return Arrays.copyOf(value, value.length); } + @Test + @SuppressWarnings("unlikely-arg-type") + public void testOriginatingIssue() { + Map testMap = new HashMap<>(); + + byte[] someValue = testValues.get(3); + testMap.put(someValue, "someValue"); + + byte[] copiedValue = dup(someValue); + + // Show that a byte[] with same values is not found + System.out.printf("byte[] hashCode: 0x%08x%n", someValue.hashCode()); + System.out.printf("duplicated byte[] hashCode: 0x%08x%n", copiedValue.hashCode()); + + /* + * Unfortunately this doesn't work because HashMap::containsKey compares hashCodes first, + * followed by object references, and copiedValue.hashCode() will never match someValue.hashCode(). + */ + assertFalse("byte[] with same values, but difference reference, not found", testMap.containsKey(copiedValue)); + } + @Test public void testSameContentReference() { - // Create two objects, which will have different references, but same content. + // Create two objects, which will have different references, but same content references. byte[] testValue = testValues.get(0); - ByteArray ba1 = new ByteArray(testValue); - ByteArray ba2 = new ByteArray(testValue); + ByteArray ba1 = ByteArray.wrap(testValue); + ByteArray ba2 = ByteArray.wrap(testValue); // Confirm JVM-assigned references are different assertNotSame(ba1, ba2); @@ -58,13 +79,31 @@ public class ByteArrayTests { } @Test - public void testSameContentValue() { - // Create two objects, which will have different references, but same content. + public void testSameWrappedContentValue() { + // Create two objects, which will have different references, and different content references, but same content values. byte[] testValue = testValues.get(0); - ByteArray ba1 = new ByteArray(testValue); + ByteArray ba1 = ByteArray.wrap(testValue); byte[] copiedValue = dup(testValue); - ByteArray ba2 = new ByteArray(copiedValue); + ByteArray ba2 = ByteArray.wrap(copiedValue); + + // Confirm JVM-assigned references are different + assertNotSame(ba1, ba2); + + // Confirm "equals" works as intended + assertTrue("equals did not return true", ba1.equals(ba2)); + assertEquals("ba1 not equal to ba2", ba1, ba2); + + // Confirm "hashCode" results match + assertEquals("hashCodes do not match", ba1.hashCode(), ba2.hashCode()); + } + + @Test + public void testSameCopiedContentValue() { + // Create two objects, which will have different references, and different content references, but same content values. + byte[] testValue = testValues.get(0); + ByteArray ba1 = ByteArray.wrap(testValue); + ByteArray ba2 = ByteArray.copyOf(testValue); // Confirm JVM-assigned references are different assertNotSame(ba1, ba2); @@ -81,13 +120,17 @@ public class ByteArrayTests { @SuppressWarnings("unlikely-arg-type") public void testCompareBoxedWithPrimitive() { byte[] testValue = testValues.get(0); - ByteArray ba1 = new ByteArray(testValue); + ByteArray wrappedByteArray = ByteArray.wrap(testValue); byte[] copiedValue = dup(testValue); + ByteArray copiedByteArray = ByteArray.copyOf(copiedValue); // Confirm "equals" works as intended - assertTrue("equals did not return true", ba1.equals(copiedValue)); - assertEquals("boxed not equal to primitive", ba1, copiedValue); + assertTrue("equals did not return true", wrappedByteArray.equals(copiedValue)); + assertEquals("boxed not equal to primitive", wrappedByteArray, copiedValue); + + assertTrue("equals did not return true", copiedByteArray.equals(testValue)); + assertEquals("boxed not equal to primitive", copiedByteArray, testValue); } @Test @@ -98,7 +141,7 @@ public class ByteArrayTests { // Create new ByteArray object with an existing value. byte[] copiedValue = dup(testValues.get(3)); - ByteArray ba = new ByteArray(copiedValue); + ByteArray ba = ByteArray.wrap(copiedValue); // Confirm object can be found in map assertTrue("ByteArray not found in map", testMap.containsKey(ba)); @@ -120,7 +163,7 @@ public class ByteArrayTests { // Create new ByteArray object with an existing value. byte[] copiedValue = dup(testValues.get(3)); - ByteArray ba = new ByteArray(copiedValue); + ByteArray ba = ByteArray.wrap(copiedValue); // Confirm object can be found in map assertTrue("ByteArray not found in map", testMap.containsKey(ba)); @@ -128,7 +171,7 @@ public class ByteArrayTests { assertTrue("boxed not equal to primitive", ba.equals(copiedValue)); /* - * Unfortunately this doesn't work because TreeMap::containsKey(x) wants to cast x to + * Unfortunately this doesn't work because TreeMap::containsKey(byte[]) wants to cast byte[] to * Comparable and byte[] does not fit * so this throws a ClassCastException. */ @@ -145,7 +188,7 @@ public class ByteArrayTests { public void testArrayListContains() { // Create new ByteArray object with an existing value. byte[] copiedValue = dup(testValues.get(3)); - ByteArray ba = new ByteArray(copiedValue); + ByteArray ba = ByteArray.wrap(copiedValue); // Confirm object can be found in list assertTrue("ByteArray not found in map", testValues.contains(ba)); @@ -154,7 +197,7 @@ public class ByteArrayTests { /* * Unfortunately this doesn't work because ArrayList::contains performs - * copiedValue.equals(x) for each x in testValues, and byte[].equals() + * copiedValue.equals(byte[]) for each byte[] in testValues, and byte[].equals() * simply compares object references, so will never match any ByteArray. */ assertFalse("Primitive shouldn't be found in ArrayList", testValues.contains(copiedValue)); @@ -163,23 +206,25 @@ public class ByteArrayTests { @Test public void debugBoxedVersusPrimitive() { byte[] testValue = testValues.get(0); - ByteArray ba1 = new ByteArray(testValue); + ByteArray ba1 = ByteArray.wrap(testValue); byte[] copiedValue = dup(testValue); - System.out.println(String.format("Primitive hashCode: 0x%08x", testValue.hashCode())); - System.out.println(String.format("Boxed hashCode: 0x%08x", ba1.hashCode())); - System.out.println(String.format("Duplicated primitive hashCode: 0x%08x", copiedValue.hashCode())); + System.out.printf("Primitive hashCode: 0x%08x%n", testValue.hashCode()); + System.out.printf("Boxed hashCode: 0x%08x%n", ba1.hashCode()); + System.out.printf("Duplicated primitive hashCode: 0x%08x%n", copiedValue.hashCode()); } @Test public void testCompareTo() { - ByteArray testValue0 = new ByteArray(new byte[] { 0x00 }); - ByteArray testValue1 = new ByteArray(new byte[] { 0x01 }); + ByteArray testValue0 = ByteArray.wrap(new byte[] { 0x00 }); + ByteArray testValue1 = ByteArray.wrap(new byte[] { 0x01 }); + ByteArray testValueFf = ByteArray.wrap(new byte[] {(byte) 0xFF}); - assertEquals("0 should be the same as 0", 0, testValue0.compareTo(testValue0)); - assertEquals("0 should be before 1", -1, testValue0.compareTo(testValue1)); - assertEquals("1 should be after 0", 1, testValue1.compareTo(testValue0)); + assertTrue("0 should be the same as 0", testValue0.compareTo(testValue0) == 0); + assertTrue("0 should be before 1", testValue0.compareTo(testValue1) < 0); + assertTrue("1 should be after 0", testValue1.compareTo(testValue0) > 0); + assertTrue("FF should be after 0", testValueFf.compareTo(testValue0) > 0); } } diff --git a/src/test/java/org/qortal/test/crosschain/TradeBotPresenceTests.java b/src/test/java/org/qortal/test/crosschain/TradeBotPresenceTests.java index c60a046b..0dd049d5 100644 --- a/src/test/java/org/qortal/test/crosschain/TradeBotPresenceTests.java +++ b/src/test/java/org/qortal/test/crosschain/TradeBotPresenceTests.java @@ -55,7 +55,7 @@ public class TradeBotPresenceTests { @Test public void testEnforceLatestTimestamp() { - ByteArray pubkeyByteArray = ByteArray.of("publickey".getBytes(StandardCharsets.UTF_8)); + ByteArray pubkeyByteArray = ByteArray.wrap("publickey".getBytes(StandardCharsets.UTF_8)); Map timestampsByPublicKey = new HashMap<>();