From 51c0fb13174b23d1b119bf3e1042395349badfd9 Mon Sep 17 00:00:00 2001 From: overcat <4catcode@gmail.com> Date: Fri, 12 Jan 2024 12:00:30 +0800 Subject: [PATCH 1/3] Add `accountConverter` to the `equals` and `hashCode` of `AbstractTransaction`. --- CHANGELOG.md | 3 +++ src/main/java/org/stellar/sdk/AbstractTransaction.java | 3 +-- src/test/java/org/stellar/sdk/Sep10ChallengeTest.java | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2334a2fbd..036945746 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ As this project is pre 1.0, breaking changes may happen for minor version bumps. ### Update * Support resource leeway parameter when simulating Soroban transactions. ([#561](https://github.com/stellar/java-stellar-sdk/pull/561)) +* Add `accountConverter` to the `equals` and `hashCode` of `AbstractTransaction`. ([#566](https://github.com/stellar/java-stellar-sdk/pull/566)) + ### Breaking changes * The types of the following fields have changed. ([#560](https://github.com/stellar/java-stellar-sdk/pull/560)) | field | before | now | @@ -13,6 +15,7 @@ As this project is pre 1.0, breaking changes may happen for minor version bumps. | GetEventsRequest.startLedger | String | Long | | GetEventsResponse.EventInfo.ledger | Integer | Long | | GetLatestLedgerResponse.protocolVersion | Integer | Long | +* Add `accountConverter` to the `equals` and `hashCode` of `AbstractTransaction`, this will affect `Transaction` and `FeeBumpTransaction`, in previous versions, they would ignore `accountConverter`. ([#566](https://github.com/stellar/java-stellar-sdk/pull/566)) ## 0.42.0 * Make `StrKey` public, this allows users to conveniently encode and decode Stellar keys to/from strings. ([#548](https://github.com/stellar/java-stellar-sdk/pull/548)) diff --git a/src/main/java/org/stellar/sdk/AbstractTransaction.java b/src/main/java/org/stellar/sdk/AbstractTransaction.java index c1c4748f9..ecbdd4339 100644 --- a/src/main/java/org/stellar/sdk/AbstractTransaction.java +++ b/src/main/java/org/stellar/sdk/AbstractTransaction.java @@ -15,8 +15,7 @@ import org.stellar.sdk.xdr.TransactionSignaturePayload; /** Abstract class for transaction classes. */ -@EqualsAndHashCode(exclude = {"accountConverter"}) -// TODO: maybe we should not exclude accountConverter from equals and hashCode +@EqualsAndHashCode public abstract class AbstractTransaction { /** The network that the transaction is to be submitted to. */ @NonNull @Getter protected final Network network; diff --git a/src/test/java/org/stellar/sdk/Sep10ChallengeTest.java b/src/test/java/org/stellar/sdk/Sep10ChallengeTest.java index 035c44e89..7d3032e6c 100644 --- a/src/test/java/org/stellar/sdk/Sep10ChallengeTest.java +++ b/src/test/java/org/stellar/sdk/Sep10ChallengeTest.java @@ -1286,7 +1286,7 @@ public void testReadChallengeTransactionInvalidWebAuthDomainOperationValueIsNull Operation[] operations = new Operation[] {domainNameOperation, webAuthDomainOperation, otherDomainOperation}; Transaction transaction = - new TransactionBuilder(AccountConverter.disableMuxed(), sourceAccount, network) + new TransactionBuilder(AccountConverter.enableMuxed(), sourceAccount, network) .setBaseFee(100 * operations.length) .addOperations(Arrays.asList(operations)) .addMemo(Memo.none()) From c273bf0492326614576b9ca5a42f006099f3ab14 Mon Sep 17 00:00:00 2001 From: overcat <4catcode@gmail.com> Date: Sat, 13 Jan 2024 11:36:07 +0800 Subject: [PATCH 2/3] Fix the `hashCode` and `equals` methods in `Transaction` and `FeeBumpTransaction` --- CHANGELOG.md | 4 +- .../org/stellar/sdk/AbstractTransaction.java | 2 - .../org/stellar/sdk/FeeBumpTransaction.java | 20 +++++++- .../java/org/stellar/sdk/Transaction.java | 22 ++++++-- .../stellar/sdk/FeeBumpTransactionTest.java | 26 ++++++++++ .../java/org/stellar/sdk/TransactionTest.java | 51 +++++++++++++++++++ 6 files changed, 116 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 036945746..c9306755a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ As this project is pre 1.0, breaking changes may happen for minor version bumps. ### Update * Support resource leeway parameter when simulating Soroban transactions. ([#561](https://github.com/stellar/java-stellar-sdk/pull/561)) -* Add `accountConverter` to the `equals` and `hashCode` of `AbstractTransaction`. ([#566](https://github.com/stellar/java-stellar-sdk/pull/566)) +* Fix the `hashCode` and `equals` methods in `Transaction` and `FeeBumpTransaction`. ([#566](https://github.com/stellar/java-stellar-sdk/pull/566)) ### Breaking changes * The types of the following fields have changed. ([#560](https://github.com/stellar/java-stellar-sdk/pull/560)) @@ -15,7 +15,7 @@ As this project is pre 1.0, breaking changes may happen for minor version bumps. | GetEventsRequest.startLedger | String | Long | | GetEventsResponse.EventInfo.ledger | Integer | Long | | GetLatestLedgerResponse.protocolVersion | Integer | Long | -* Add `accountConverter` to the `equals` and `hashCode` of `AbstractTransaction`, this will affect `Transaction` and `FeeBumpTransaction`, in previous versions, they would ignore `accountConverter`. ([#566](https://github.com/stellar/java-stellar-sdk/pull/566)) +* Fix the `hashCode` and `equals` methods in `Transaction` and `FeeBumpTransaction`, now they will compare based on the final generated XDR objects. If the XDR objects are the same, then the original instances are considered equal. ([#566](https://github.com/stellar/java-stellar-sdk/pull/566)) ## 0.42.0 * Make `StrKey` public, this allows users to conveniently encode and decode Stellar keys to/from strings. ([#548](https://github.com/stellar/java-stellar-sdk/pull/548)) diff --git a/src/main/java/org/stellar/sdk/AbstractTransaction.java b/src/main/java/org/stellar/sdk/AbstractTransaction.java index ecbdd4339..a958fa294 100644 --- a/src/main/java/org/stellar/sdk/AbstractTransaction.java +++ b/src/main/java/org/stellar/sdk/AbstractTransaction.java @@ -5,7 +5,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; -import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.NonNull; import org.stellar.sdk.xdr.DecoratedSignature; @@ -15,7 +14,6 @@ import org.stellar.sdk.xdr.TransactionSignaturePayload; /** Abstract class for transaction classes. */ -@EqualsAndHashCode public abstract class AbstractTransaction { /** The network that the transaction is to be submitted to. */ @NonNull @Getter protected final Network network; diff --git a/src/main/java/org/stellar/sdk/FeeBumpTransaction.java b/src/main/java/org/stellar/sdk/FeeBumpTransaction.java index a8d433635..68c9999fa 100644 --- a/src/main/java/org/stellar/sdk/FeeBumpTransaction.java +++ b/src/main/java/org/stellar/sdk/FeeBumpTransaction.java @@ -2,7 +2,7 @@ import java.util.ArrayList; import java.util.Arrays; -import lombok.EqualsAndHashCode; +import java.util.Objects; import lombok.Getter; import lombok.NonNull; import org.stellar.sdk.xdr.DecoratedSignature; @@ -20,7 +20,6 @@ * Transactions */ @Getter -@EqualsAndHashCode(callSuper = true) public class FeeBumpTransaction extends AbstractTransaction { /** The max fee willing to be paid for this transaction. */ private final long fee; @@ -202,4 +201,21 @@ public FeeBumpTransaction build() { this.accountConverter, this.feeAccount, this.baseFee, this.innerTransaction); } } + + @Override + public boolean equals(Object object) { + if (this == object) { + return true; + } + if (object == null || getClass() != object.getClass()) { + return false; + } + FeeBumpTransaction that = (FeeBumpTransaction) object; + return Objects.equals(toEnvelopeXdr(), that.toEnvelopeXdr()); + } + + @Override + public int hashCode() { + return Objects.hash(toEnvelopeXdr()); + } } diff --git a/src/main/java/org/stellar/sdk/Transaction.java b/src/main/java/org/stellar/sdk/Transaction.java index 2b28aaddd..12603c9fd 100644 --- a/src/main/java/org/stellar/sdk/Transaction.java +++ b/src/main/java/org/stellar/sdk/Transaction.java @@ -3,7 +3,7 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.util.Arrays; -import lombok.EqualsAndHashCode; +import java.util.Objects; import lombok.Getter; import lombok.NonNull; import org.stellar.sdk.xdr.AccountID; @@ -29,7 +29,6 @@ * Represents Transaction in Stellar network. */ -@EqualsAndHashCode(callSuper = true) public class Transaction extends AbstractTransaction { /** fee paid for transaction in stroops (1 stroop = 0.0000001 XLM). */ @Getter private final long fee; @@ -316,8 +315,8 @@ public TransactionEnvelope toEnvelopeXdr() { /** * Maintain backwards compatibility references to Transaction.Builder * - * @deprecated will be removed in upcoming releases. Use TransactionBuilder instead. * @see org.stellar.sdk.TransactionBuilder + * @deprecated will be removed in upcoming releases. Use TransactionBuilder instead. */ public static class Builder extends TransactionBuilder { public Builder( @@ -347,4 +346,21 @@ public boolean isSorobanTransaction() { || op instanceof ExtendFootprintTTLOperation || op instanceof RestoreFootprintOperation; } + + @Override + public boolean equals(Object object) { + if (this == object) { + return true; + } + if (object == null || getClass() != object.getClass()) { + return false; + } + Transaction that = (Transaction) object; + return Objects.equals(toEnvelopeXdr(), that.toEnvelopeXdr()); + } + + @Override + public int hashCode() { + return Objects.hash(toEnvelopeXdr()); + } } diff --git a/src/test/java/org/stellar/sdk/FeeBumpTransactionTest.java b/src/test/java/org/stellar/sdk/FeeBumpTransactionTest.java index f845915ca..4bb5958f6 100644 --- a/src/test/java/org/stellar/sdk/FeeBumpTransactionTest.java +++ b/src/test/java/org/stellar/sdk/FeeBumpTransactionTest.java @@ -244,4 +244,30 @@ public void testFeeBumpUpgradesInnerToV1() throws IOException { assertEquals(1, feeBump.getSignatures().size()); signer.verify(feeBump.hash(), feeBump.getSignatures().get(0).getSignature().getSignature()); } + + @Test + public void testHashCodeAndEquals() { + Transaction inner = createInnerTransaction(); + + FeeBumpTransaction feeBump0 = + new FeeBumpTransaction.Builder(AccountConverter.enableMuxed(), inner) + .setBaseFee(Transaction.MIN_BASE_FEE * 2) + .setFeeAccount("GDQNY3PBOJOKYZSRMK2S7LHHGWZIUISD4QORETLMXEWXBI7KFZZMKTL3") + .build(); + FeeBumpTransaction feeBump1 = + new FeeBumpTransaction.Builder(AccountConverter.disableMuxed(), inner) + .setBaseFee(Transaction.MIN_BASE_FEE * 2) + .setFeeAccount("GDQNY3PBOJOKYZSRMK2S7LHHGWZIUISD4QORETLMXEWXBI7KFZZMKTL3") + .build(); + assertEquals(feeBump0.hashCode(), feeBump1.hashCode()); + assertEquals(feeBump0, feeBump1); + + FeeBumpTransaction feeBump2 = + new FeeBumpTransaction.Builder(AccountConverter.enableMuxed(), inner) + .setBaseFee(Transaction.MIN_BASE_FEE * 3) + .setFeeAccount("GDQNY3PBOJOKYZSRMK2S7LHHGWZIUISD4QORETLMXEWXBI7KFZZMKTL3") + .build(); + System.out.println(feeBump2.toEnvelopeXdr()); + assertNotEquals(feeBump0, feeBump2); + } } diff --git a/src/test/java/org/stellar/sdk/TransactionTest.java b/src/test/java/org/stellar/sdk/TransactionTest.java index 52b63bf67..5ff59559b 100644 --- a/src/test/java/org/stellar/sdk/TransactionTest.java +++ b/src/test/java/org/stellar/sdk/TransactionTest.java @@ -3,6 +3,7 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -436,4 +437,54 @@ public void testIsSorobanTransactionBumpSequenceOperation() { Network.TESTNET); assertFalse(transaction.isSorobanTransaction()); } + + @Test + public void testHashCodeAndEquals() { + KeyPair source = + KeyPair.fromSecretSeed("SCH27VUZZ6UAKB67BDNF6FA42YMBMQCBKXWGMFD5TZ6S5ZZCZFLRXKHS"); + + Account account = new Account(source.getAccountId(), 2908908335136768L); + BumpSequenceOperation operation = new BumpSequenceOperation.Builder(0L).build(); + + Transaction transaction1 = + new Transaction( + AccountConverter.enableMuxed(), + account.getAccountId(), + Transaction.MIN_BASE_FEE, + account.getIncrementedSequenceNumber(), + new org.stellar.sdk.Operation[] {operation}, + null, + new TransactionPreconditions( + null, null, BigInteger.ZERO, 0, new ArrayList(), null), + null, + Network.TESTNET); + Transaction transaction2 = + new Transaction( + AccountConverter.disableMuxed(), // they get different account converters + account.getAccountId(), + Transaction.MIN_BASE_FEE, + account.getIncrementedSequenceNumber(), + new org.stellar.sdk.Operation[] {operation}, + null, + new TransactionPreconditions( + null, null, BigInteger.ZERO, 0, new ArrayList(), null), + null, + Network.TESTNET); + assertEquals(transaction1.hashCode(), transaction2.hashCode()); + assertEquals(transaction1, transaction2); + + Transaction transaction3 = + new Transaction( + AccountConverter.enableMuxed(), + account.getAccountId(), + Transaction.MIN_BASE_FEE, + account.getIncrementedSequenceNumber(), + new org.stellar.sdk.Operation[] {operation}, + Memo.text("not equal tx"), + new TransactionPreconditions( + null, null, BigInteger.ZERO, 0, new ArrayList(), null), + null, + Network.TESTNET); + assertNotEquals(transaction1, transaction3); + } } From 2b1eb841f615b40542cec9e67e712e18c6da3f70 Mon Sep 17 00:00:00 2001 From: overcat <4catcode@gmail.com> Date: Tue, 16 Jan 2024 10:19:26 +0800 Subject: [PATCH 3/3] Fix the `hashCode` and `equals` methods in `Transaction` and `FeeBumpTransaction`, now they will compare based on the `signatureBase()`. --- CHANGELOG.md | 2 +- .../org/stellar/sdk/FeeBumpTransaction.java | 5 ++-- .../java/org/stellar/sdk/Transaction.java | 5 ++-- .../stellar/sdk/FeeBumpTransactionTest.java | 27 ++++++++++++++++--- .../java/org/stellar/sdk/TransactionTest.java | 20 +++++++++++++- 5 files changed, 47 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9306755a..cb194dc38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ As this project is pre 1.0, breaking changes may happen for minor version bumps. | GetEventsRequest.startLedger | String | Long | | GetEventsResponse.EventInfo.ledger | Integer | Long | | GetLatestLedgerResponse.protocolVersion | Integer | Long | -* Fix the `hashCode` and `equals` methods in `Transaction` and `FeeBumpTransaction`, now they will compare based on the final generated XDR objects. If the XDR objects are the same, then the original instances are considered equal. ([#566](https://github.com/stellar/java-stellar-sdk/pull/566)) +* Fix the `hashCode` and `equals` methods in `Transaction` and `FeeBumpTransaction`, now they will compare based on the `signatureBase()`. ([#566](https://github.com/stellar/java-stellar-sdk/pull/566)) ## 0.42.0 * Make `StrKey` public, this allows users to conveniently encode and decode Stellar keys to/from strings. ([#548](https://github.com/stellar/java-stellar-sdk/pull/548)) diff --git a/src/main/java/org/stellar/sdk/FeeBumpTransaction.java b/src/main/java/org/stellar/sdk/FeeBumpTransaction.java index 68c9999fa..8f8a38021 100644 --- a/src/main/java/org/stellar/sdk/FeeBumpTransaction.java +++ b/src/main/java/org/stellar/sdk/FeeBumpTransaction.java @@ -2,7 +2,6 @@ import java.util.ArrayList; import java.util.Arrays; -import java.util.Objects; import lombok.Getter; import lombok.NonNull; import org.stellar.sdk.xdr.DecoratedSignature; @@ -211,11 +210,11 @@ public boolean equals(Object object) { return false; } FeeBumpTransaction that = (FeeBumpTransaction) object; - return Objects.equals(toEnvelopeXdr(), that.toEnvelopeXdr()); + return Arrays.equals(signatureBase(), that.signatureBase()); } @Override public int hashCode() { - return Objects.hash(toEnvelopeXdr()); + return Arrays.hashCode(signatureBase()); } } diff --git a/src/main/java/org/stellar/sdk/Transaction.java b/src/main/java/org/stellar/sdk/Transaction.java index 12603c9fd..99b95b94b 100644 --- a/src/main/java/org/stellar/sdk/Transaction.java +++ b/src/main/java/org/stellar/sdk/Transaction.java @@ -3,7 +3,6 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.util.Arrays; -import java.util.Objects; import lombok.Getter; import lombok.NonNull; import org.stellar.sdk.xdr.AccountID; @@ -356,11 +355,11 @@ public boolean equals(Object object) { return false; } Transaction that = (Transaction) object; - return Objects.equals(toEnvelopeXdr(), that.toEnvelopeXdr()); + return Arrays.equals(signatureBase(), that.signatureBase()); } @Override public int hashCode() { - return Objects.hash(toEnvelopeXdr()); + return Arrays.hashCode(signatureBase()); } } diff --git a/src/test/java/org/stellar/sdk/FeeBumpTransactionTest.java b/src/test/java/org/stellar/sdk/FeeBumpTransactionTest.java index 4bb5958f6..ef0af9f3d 100644 --- a/src/test/java/org/stellar/sdk/FeeBumpTransactionTest.java +++ b/src/test/java/org/stellar/sdk/FeeBumpTransactionTest.java @@ -10,13 +10,13 @@ public class FeeBumpTransactionTest { - private Transaction createInnerTransaction(int baseFee) { + private Transaction createInnerTransaction(int baseFee, Network network) { KeyPair source = KeyPair.fromSecretSeed("SCH27VUZZ6UAKB67BDNF6FA42YMBMQCBKXWGMFD5TZ6S5ZZCZFLRXKHS"); Account account = new Account(source.getAccountId(), 2908908335136768L); Transaction inner = - new TransactionBuilder(AccountConverter.enableMuxed(), account, Network.TESTNET) + new TransactionBuilder(AccountConverter.enableMuxed(), account, network) .addOperation( new PaymentOperation.Builder( "GA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVSGZ", @@ -32,7 +32,15 @@ private Transaction createInnerTransaction(int baseFee) { } private Transaction createInnerTransaction() { - return createInnerTransaction(Transaction.MIN_BASE_FEE); + return createInnerTransaction(Transaction.MIN_BASE_FEE, Network.TESTNET); + } + + private Transaction createInnerTransaction(int baseFee) { + return createInnerTransaction(baseFee, Network.TESTNET); + } + + private Transaction createInnerTransaction(Network network) { + return createInnerTransaction(Transaction.MIN_BASE_FEE, network); } @Test @@ -221,7 +229,6 @@ public void testFeeBumpUpgradesInnerToV1() throws IOException { assertEquals(0, feeBump.getSignatures().size()); assertEquals(EnvelopeType.ENVELOPE_TYPE_TX_V0, innerV0.toEnvelopeXdr().getDiscriminant()); - assertNotEquals(innerV0, feeBump.getInnerTransaction()); innerV0.setEnvelopeType(EnvelopeType.ENVELOPE_TYPE_TX); assertEquals(innerV0, feeBump.getInnerTransaction()); @@ -254,6 +261,8 @@ public void testHashCodeAndEquals() { .setBaseFee(Transaction.MIN_BASE_FEE * 2) .setFeeAccount("GDQNY3PBOJOKYZSRMK2S7LHHGWZIUISD4QORETLMXEWXBI7KFZZMKTL3") .build(); + + // they get different account converters FeeBumpTransaction feeBump1 = new FeeBumpTransaction.Builder(AccountConverter.disableMuxed(), inner) .setBaseFee(Transaction.MIN_BASE_FEE * 2) @@ -262,6 +271,7 @@ public void testHashCodeAndEquals() { assertEquals(feeBump0.hashCode(), feeBump1.hashCode()); assertEquals(feeBump0, feeBump1); + // they get different base fee FeeBumpTransaction feeBump2 = new FeeBumpTransaction.Builder(AccountConverter.enableMuxed(), inner) .setBaseFee(Transaction.MIN_BASE_FEE * 3) @@ -269,5 +279,14 @@ public void testHashCodeAndEquals() { .build(); System.out.println(feeBump2.toEnvelopeXdr()); assertNotEquals(feeBump0, feeBump2); + + // they get different network + FeeBumpTransaction feeBump3 = + new FeeBumpTransaction.Builder( + AccountConverter.enableMuxed(), createInnerTransaction(Network.PUBLIC)) + .setBaseFee(Transaction.MIN_BASE_FEE * 2) + .setFeeAccount("GDQNY3PBOJOKYZSRMK2S7LHHGWZIUISD4QORETLMXEWXBI7KFZZMKTL3") + .build(); + assertNotEquals(feeBump0, feeBump3); } } diff --git a/src/test/java/org/stellar/sdk/TransactionTest.java b/src/test/java/org/stellar/sdk/TransactionTest.java index 5ff59559b..eae1cf514 100644 --- a/src/test/java/org/stellar/sdk/TransactionTest.java +++ b/src/test/java/org/stellar/sdk/TransactionTest.java @@ -458,9 +458,11 @@ public void testHashCodeAndEquals() { null, null, BigInteger.ZERO, 0, new ArrayList(), null), null, Network.TESTNET); + + // they get different account converters Transaction transaction2 = new Transaction( - AccountConverter.disableMuxed(), // they get different account converters + AccountConverter.disableMuxed(), account.getAccountId(), Transaction.MIN_BASE_FEE, account.getIncrementedSequenceNumber(), @@ -473,6 +475,7 @@ public void testHashCodeAndEquals() { assertEquals(transaction1.hashCode(), transaction2.hashCode()); assertEquals(transaction1, transaction2); + // they get different memo Transaction transaction3 = new Transaction( AccountConverter.enableMuxed(), @@ -486,5 +489,20 @@ public void testHashCodeAndEquals() { null, Network.TESTNET); assertNotEquals(transaction1, transaction3); + + // they get different network + Transaction transaction4 = + new Transaction( + AccountConverter.enableMuxed(), + account.getAccountId(), + Transaction.MIN_BASE_FEE, + account.getIncrementedSequenceNumber(), + new org.stellar.sdk.Operation[] {operation}, + null, + new TransactionPreconditions( + null, null, BigInteger.ZERO, 0, new ArrayList(), null), + null, + Network.PUBLIC); + assertNotEquals(transaction1, transaction4); } }