diff --git a/CHANGELOG.md b/CHANGELOG.md index ae86d871f..62d2dec01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,16 +8,17 @@ As this project is pre 1.0, breaking changes may happen for minor version bumps. * Support resource leeway parameter when simulating Soroban transactions. ([#561](https://github.com/stellar/java-stellar-sdk/pull/561)) * Support for the new, optional `diagnosticEventsXdr` field on the `SorobanServer.sendTransaction` method. ([#564](https://github.com/stellar/java-stellar-sdk/pull/564)) * Remove deprecated classes and methods. ([#565](https://github.com/stellar/java-stellar-sdk/pull/565)) +* Fix the `hashCode` and `equals` methods in `Transaction` and `FeeBumpTransaction`. ([#566](https://github.com/stellar/java-stellar-sdk/pull/566)) * Add `TransactionBuilder#TransactionBuilder(Transaction)` constructor. ([#567](https://github.com/stellar/java-stellar-sdk/pull/567)) ### Breaking changes +* 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)) * The types of the following fields have changed. ([#560](https://github.com/stellar/java-stellar-sdk/pull/560)) | field | before | now | | --------------------------------------- | ------- | ---- | | GetEventsRequest.startLedger | String | Long | | GetEventsResponse.EventInfo.ledger | Integer | Long | | GetLatestLedgerResponse.protocolVersion | Integer | Long | - * The following classes and methods have been marked as deprecated in previous releases, and now they have been removed. ([#565](https://github.com/stellar/java-stellar-sdk/pull/565)) * `AccountResponse.Signer#getAccountId()` has been removed, use `AccountResponse.Signer#getKey()` instead. * `OffersRequestBuilder#forAccount(String)` has been removed, use `OffersRequestBuilder#forSeller(String)` instead. diff --git a/src/main/java/org/stellar/sdk/AbstractTransaction.java b/src/main/java/org/stellar/sdk/AbstractTransaction.java index c1c4748f9..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,8 +14,6 @@ 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 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..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 lombok.EqualsAndHashCode; import lombok.Getter; import lombok.NonNull; import org.stellar.sdk.xdr.DecoratedSignature; @@ -20,7 +19,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 +200,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 Arrays.equals(signatureBase(), that.signatureBase()); + } + + @Override + public int hashCode() { + 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 5d80fd366..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 lombok.EqualsAndHashCode; import lombok.Getter; import lombok.NonNull; import org.stellar.sdk.xdr.AccountID; @@ -29,7 +28,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; @@ -313,6 +311,25 @@ public TransactionEnvelope toEnvelopeXdr() { return xdr; } + /** + * Maintain backwards compatibility references to Transaction.Builder + * + * @see org.stellar.sdk.TransactionBuilder + * @deprecated will be removed in upcoming releases. Use TransactionBuilder instead. + */ + public static class Builder extends TransactionBuilder { + public Builder( + AccountConverter accountConverter, + TransactionBuilderAccount sourceAccount, + Network network) { + super(accountConverter, sourceAccount, network); + } + + public Builder(TransactionBuilderAccount sourceAccount, Network network) { + super(sourceAccount, network); + } + } + /** * Returns true if this transaction is a Soroban transaction. * @@ -328,4 +345,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 Arrays.equals(signatureBase(), that.signatureBase()); + } + + @Override + public int hashCode() { + 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 ad4942b7f..b2b160c5f 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", @@ -33,7 +33,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 @@ -222,7 +230,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()); @@ -245,4 +252,42 @@ 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(); + + // they get different account converters + 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); + + // they get different base fee + 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); + + // 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/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()) diff --git a/src/test/java/org/stellar/sdk/TransactionTest.java b/src/test/java/org/stellar/sdk/TransactionTest.java index 52b63bf67..eae1cf514 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,72 @@ 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); + + // they get different account converters + Transaction transaction2 = + new Transaction( + AccountConverter.disableMuxed(), + 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); + + // they get different memo + 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); + + // 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); + } }