From bc71e1714556bce2a44abcc70c05e63a0edfa5df Mon Sep 17 00:00:00 2001 From: Julian Len Date: Fri, 13 Dec 2024 18:35:10 -0300 Subject: [PATCH 1/6] feat: added a test to check a tx with negative block height is invalid --- .../java/co/rsk/peg/RegisterBtcTransactionIT.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java b/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java index 1f7ed3484c..54eaa0ae5b 100644 --- a/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java +++ b/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java @@ -147,6 +147,20 @@ void registerBtcTransaction_forARepeatedLegacyBtcTransaction_shouldNotPerformAny assertEquals(expectedReceiverBalance, repository.getBalance(rskReceiver)); } + @Test + void registerBtcTransaction_forALegacyBtcTransactionWithNegativeHeight_shouldNotPerformAnyChange() throws Exception { + // Arrange + co.rsk.core.Coin expectedReceiverBalance = repository.getBalance(rskReceiver); + List expectedFederationUTXOs = federationSupport.getActiveFederationBtcUTXOs(); + + // Act + bridgeSupport.registerBtcTransaction(rskTx, bitcoinTransaction.bitcoinSerialize(), -1, pmtWithTransactions.bitcoinSerialize()); + bridgeSupport.save(); + + // Assert + assertEquals(expectedFederationUTXOs, federationSupport.getActiveFederationBtcUTXOs()); + assertEquals(expectedReceiverBalance, repository.getBalance(rskReceiver)); + } private static UTXO utxoOf(BtcTransaction bitcoinTransaction, TransactionOutput output) { return new UTXO( bitcoinTransaction.getHash(), From f8b37f54aab348b346c933037bf933739def64db Mon Sep 17 00:00:00 2001 From: Julian Len Date: Thu, 19 Dec 2024 11:15:35 -0300 Subject: [PATCH 2/6] fix: improved title for the test --- .../src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java b/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java index 54eaa0ae5b..03f987de25 100644 --- a/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java +++ b/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java @@ -148,7 +148,7 @@ void registerBtcTransaction_forARepeatedLegacyBtcTransaction_shouldNotPerformAny } @Test - void registerBtcTransaction_forALegacyBtcTransactionWithNegativeHeight_shouldNotPerformAnyChange() throws Exception { + void registerBtcTransaction_whenLegacyBtcTransactionWithNegativeHeight_shouldNotPerformAnyChange() throws Exception { // Arrange co.rsk.core.Coin expectedReceiverBalance = repository.getBalance(rskReceiver); List expectedFederationUTXOs = federationSupport.getActiveFederationBtcUTXOs(); From a80be0c7b2ab93df052784f1c42ed0ea3ec5ab5d Mon Sep 17 00:00:00 2001 From: Julian Len Date: Thu, 19 Dec 2024 11:44:37 -0300 Subject: [PATCH 3/6] fix: federationBtcUTXOs are passed as reference, now it makes a copy --- .../src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java b/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java index 03f987de25..42f91afb1e 100644 --- a/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java +++ b/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java @@ -136,7 +136,7 @@ void registerBtcTransaction_forARepeatedLegacyBtcTransaction_shouldNotPerformAny bridgeSupport.save(); co.rsk.core.Coin expectedReceiverBalance = repository.getBalance(rskReceiver); - List expectedFederationUTXOs = federationSupport.getActiveFederationBtcUTXOs(); + List expectedFederationUTXOs = List.copyOf(federationSupport.getActiveFederationBtcUTXOs()); // Act bridgeSupport.registerBtcTransaction(rskTx, bitcoinTransaction.bitcoinSerialize(), btcBlockWithPmtHeight, pmtWithTransactions.bitcoinSerialize()); @@ -151,7 +151,7 @@ void registerBtcTransaction_forARepeatedLegacyBtcTransaction_shouldNotPerformAny void registerBtcTransaction_whenLegacyBtcTransactionWithNegativeHeight_shouldNotPerformAnyChange() throws Exception { // Arrange co.rsk.core.Coin expectedReceiverBalance = repository.getBalance(rskReceiver); - List expectedFederationUTXOs = federationSupport.getActiveFederationBtcUTXOs(); + List expectedFederationUTXOs = List.copyOf(federationSupport.getActiveFederationBtcUTXOs()); // Act bridgeSupport.registerBtcTransaction(rskTx, bitcoinTransaction.bitcoinSerialize(), -1, pmtWithTransactions.bitcoinSerialize()); From 97d1f9b500c900ffcd190109317f1a8b1f936d74 Mon Sep 17 00:00:00 2001 From: Julian Len Date: Thu, 19 Dec 2024 11:44:55 -0300 Subject: [PATCH 4/6] fix: changed to List.of instead of Collections.singletonList --- .../src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java b/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java index 42f91afb1e..f66d91d7a6 100644 --- a/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java +++ b/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java @@ -120,7 +120,7 @@ void registerBtcTransaction_forALegacyBtcTransaction_shouldRegisterTheNewUtxoAnd int outputIndex = 0; TransactionOutput output = bitcoinTransaction.getOutput(outputIndex); - List expectedFederationUtxos = Collections.singletonList(utxoOf(bitcoinTransaction, output)); + List expectedFederationUtxos = List.of(utxoOf(bitcoinTransaction, output)); assertEquals(expectedFederationUtxos, federationSupport.getActiveFederationBtcUTXOs()); co.rsk.core.Coin expectedReceiverBalance = co.rsk.core.Coin.fromBitcoin(output.getValue()); From 28ccc6c7b85adc7c5946cbf3cf304c44cccdec03 Mon Sep 17 00:00:00 2001 From: Julian Len Date: Thu, 19 Dec 2024 13:39:31 -0300 Subject: [PATCH 5/6] fix: extra space --- rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java | 1 + 1 file changed, 1 insertion(+) diff --git a/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java b/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java index f66d91d7a6..b1a79ee5f0 100644 --- a/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java +++ b/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java @@ -161,6 +161,7 @@ void registerBtcTransaction_whenLegacyBtcTransactionWithNegativeHeight_shouldNot assertEquals(expectedFederationUTXOs, federationSupport.getActiveFederationBtcUTXOs()); assertEquals(expectedReceiverBalance, repository.getBalance(rskReceiver)); } + private static UTXO utxoOf(BtcTransaction bitcoinTransaction, TransactionOutput output) { return new UTXO( bitcoinTransaction.getHash(), From ea055ed1c1dc7cbccfab1881cec3aa18de2e6516 Mon Sep 17 00:00:00 2001 From: Julian Len Date: Thu, 19 Dec 2024 13:42:28 -0300 Subject: [PATCH 6/6] fix: no magic numbers, extracted them as variables --- .../java/co/rsk/peg/RegisterBtcTransactionIT.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java b/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java index b1a79ee5f0..38dbf100d2 100644 --- a/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java +++ b/rskj-core/src/test/java/co/rsk/peg/RegisterBtcTransactionIT.java @@ -154,7 +154,8 @@ void registerBtcTransaction_whenLegacyBtcTransactionWithNegativeHeight_shouldNot List expectedFederationUTXOs = List.copyOf(federationSupport.getActiveFederationBtcUTXOs()); // Act - bridgeSupport.registerBtcTransaction(rskTx, bitcoinTransaction.bitcoinSerialize(), -1, pmtWithTransactions.bitcoinSerialize()); + int height = -1; + bridgeSupport.registerBtcTransaction(rskTx, bitcoinTransaction.bitcoinSerialize(), height, pmtWithTransactions.bitcoinSerialize()); bridgeSupport.save(); // Assert @@ -163,11 +164,12 @@ void registerBtcTransaction_whenLegacyBtcTransactionWithNegativeHeight_shouldNot } private static UTXO utxoOf(BtcTransaction bitcoinTransaction, TransactionOutput output) { + int height = 0; return new UTXO( bitcoinTransaction.getHash(), output.getIndex(), output.getValue(), - 0, + height, bitcoinTransaction.isCoinBase(), output.getScriptPubKey() ); @@ -175,7 +177,9 @@ private static UTXO utxoOf(BtcTransaction bitcoinTransaction, TransactionOutput private BtcTransaction createPegInTransaction(Address federationAddress, Coin coin, BtcECKey pubKey) { BtcTransaction btcTx = new BtcTransaction(btcNetworkParams); - btcTx.addInput(BitcoinTestUtils.createHash(0), 0, ScriptBuilder.createInputScript(null, pubKey)); + int outputIndex = 0; + int nHash = 0; + btcTx.addInput(BitcoinTestUtils.createHash(nHash), outputIndex, ScriptBuilder.createInputScript(null, pubKey)); btcTx.addOutput(new TransactionOutput(btcNetworkParams, btcTx, coin, federationAddress)); return btcTx; @@ -184,7 +188,8 @@ private BtcTransaction createPegInTransaction(Address federationAddress, Coin co private void assertLogPegInBtc() { Sha256Hash peginTransactionHash = bitcoinTransaction.getHash(); List encodedTopics = getEncodedTopics(BridgeEvents.PEGIN_BTC.getEvent(), rskReceiver.toString(), peginTransactionHash.getBytes()); - byte[] encodedData = getEncodedData(BridgeEvents.PEGIN_BTC.getEvent(), minimumPeginValue.getValue(), 0); + int protocolVersion = 0; + byte[] encodedData = getEncodedData(BridgeEvents.PEGIN_BTC.getEvent(), minimumPeginValue.getValue(), protocolVersion); assertEventWasEmittedWithExpectedTopics(encodedTopics, logs); assertEventWasEmittedWithExpectedData(encodedData, logs);