From b4b7b529f7653441cf8c5728dcb2389fba5aac33 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Mon, 17 Apr 2023 14:12:28 -0400 Subject: [PATCH 01/24] Add RSKIP383 activation code --- .../org/ethereum/config/blockchain/upgrades/ConsensusRule.java | 1 + rskj-core/src/main/resources/expected.conf | 1 + rskj-core/src/main/resources/reference.conf | 1 + .../config/blockchain/upgrades/ActivationConfigTest.java | 1 + 4 files changed, 4 insertions(+) diff --git a/rskj-core/src/main/java/org/ethereum/config/blockchain/upgrades/ConsensusRule.java b/rskj-core/src/main/java/org/ethereum/config/blockchain/upgrades/ConsensusRule.java index e456d3dcd63..4c9216ca398 100644 --- a/rskj-core/src/main/java/org/ethereum/config/blockchain/upgrades/ConsensusRule.java +++ b/rskj-core/src/main/java/org/ethereum/config/blockchain/upgrades/ConsensusRule.java @@ -83,6 +83,7 @@ public enum ConsensusRule { RSKIP374("rskip374"), RSKIP375("rskip375"), RSKIP377("rskip377"), + RSKIP383("rskip383"), RSKIP385("rskip385"), ; diff --git a/rskj-core/src/main/resources/expected.conf b/rskj-core/src/main/resources/expected.conf index 6f8e50452ce..a4347efdabd 100644 --- a/rskj-core/src/main/resources/expected.conf +++ b/rskj-core/src/main/resources/expected.conf @@ -81,6 +81,7 @@ blockchain = { rskip374 = rskip375 = rskip377 = + rskip383 = rskip385 = } } diff --git a/rskj-core/src/main/resources/reference.conf b/rskj-core/src/main/resources/reference.conf index 9d3819a9f69..bd03f37cb4a 100644 --- a/rskj-core/src/main/resources/reference.conf +++ b/rskj-core/src/main/resources/reference.conf @@ -69,6 +69,7 @@ blockchain = { rskip374 = fingerroot500 rskip375 = fingerroot500 rskip377 = fingerroot500 + rskip383 = fingerroot500 rskip385 = fingerroot500 } } diff --git a/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigTest.java b/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigTest.java index d934d44ddf0..b96f8064fc2 100644 --- a/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigTest.java +++ b/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigTest.java @@ -108,6 +108,7 @@ class ActivationConfigTest { " rskip374: fingerroot500", " rskip375: fingerroot500", " rskip377: fingerroot500", + " rskip383: fingerroot500", " rskip385: fingerroot500", "}" )); From 9759d30548db87f640e7a08ffcd2988b2b211a7c Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Mon, 17 Apr 2023 14:52:12 -0400 Subject: [PATCH 02/24] - Added new federationActivationAgeLegacy field - Refactored getFederationActivationAge method to return the value of federationActivationAge when ConsensusRule.RSKIP383 i s active, otherwise returns the legacy --- rskj-core/src/main/java/co/rsk/config/BridgeConstants.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rskj-core/src/main/java/co/rsk/config/BridgeConstants.java b/rskj-core/src/main/java/co/rsk/config/BridgeConstants.java index cc0f248ff88..5d59b76ad1c 100644 --- a/rskj-core/src/main/java/co/rsk/config/BridgeConstants.java +++ b/rskj-core/src/main/java/co/rsk/config/BridgeConstants.java @@ -46,6 +46,7 @@ public abstract class BridgeConstants { protected Coin minimumPegoutTxValueInSatoshis; protected long federationActivationAge; + protected long federationActivationAgeLegacy; protected long fundsMigrationAgeSinceActivationBegin; protected long fundsMigrationAgeSinceActivationEnd; @@ -119,8 +120,8 @@ public int getRsk2BtcMinimumAcceptableConfirmations() { public Coin getMinimumPegoutTxValueInSatoshis() { return minimumPegoutTxValueInSatoshis; } - public long getFederationActivationAge() { - return federationActivationAge; + public long getFederationActivationAge(ActivationConfig.ForBlock activations) { + return activations.isActive(ConsensusRule.RSKIP383)? federationActivationAge: federationActivationAgeLegacy; } public long getFundsMigrationAgeSinceActivationBegin() { From afa5ddc81d5d824f759d374eb8135f1d3c04b3b3 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Mon, 17 Apr 2023 15:51:55 -0400 Subject: [PATCH 03/24] - Set default value --- .../src/main/java/co/rsk/config/BridgeDevNetConstants.java | 1 + .../src/main/java/co/rsk/config/BridgeMainNetConstants.java | 3 ++- .../src/main/java/co/rsk/config/BridgeTestNetConstants.java | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/rskj-core/src/main/java/co/rsk/config/BridgeDevNetConstants.java b/rskj-core/src/main/java/co/rsk/config/BridgeDevNetConstants.java index 6b34969f453..7eb2f590700 100644 --- a/rskj-core/src/main/java/co/rsk/config/BridgeDevNetConstants.java +++ b/rskj-core/src/main/java/co/rsk/config/BridgeDevNetConstants.java @@ -104,6 +104,7 @@ public BridgeDevNetConstants(List federationPublicKeys) { AddressBasedAuthorizer.MinimumRequiredCalculation.ONE ); + federationActivationAgeLegacy = 10L; federationActivationAge = 10L; fundsMigrationAgeSinceActivationBegin = 15L; diff --git a/rskj-core/src/main/java/co/rsk/config/BridgeMainNetConstants.java b/rskj-core/src/main/java/co/rsk/config/BridgeMainNetConstants.java index 5acdaef2d3d..4d631af5ae4 100644 --- a/rskj-core/src/main/java/co/rsk/config/BridgeMainNetConstants.java +++ b/rskj-core/src/main/java/co/rsk/config/BridgeMainNetConstants.java @@ -93,7 +93,8 @@ public class BridgeMainNetConstants extends BridgeConstants { AddressBasedAuthorizer.MinimumRequiredCalculation.ONE ); - federationActivationAge = 18500L; + federationActivationAgeLegacy = 18500L; + federationActivationAge = 40320L; fundsMigrationAgeSinceActivationBegin = 0L; fundsMigrationAgeSinceActivationEnd = 10585L; diff --git a/rskj-core/src/main/java/co/rsk/config/BridgeTestNetConstants.java b/rskj-core/src/main/java/co/rsk/config/BridgeTestNetConstants.java index 649ef3211ae..414576e5b3f 100644 --- a/rskj-core/src/main/java/co/rsk/config/BridgeTestNetConstants.java +++ b/rskj-core/src/main/java/co/rsk/config/BridgeTestNetConstants.java @@ -103,6 +103,7 @@ public class BridgeTestNetConstants extends BridgeConstants { AddressBasedAuthorizer.MinimumRequiredCalculation.ONE ); + federationActivationAgeLegacy = 60L; federationActivationAge = 60L; fundsMigrationAgeSinceActivationBegin = 60L; From 7989e009447e1789f52b898888cc532931927f4c Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Mon, 17 Apr 2023 16:12:08 -0400 Subject: [PATCH 04/24] Refactored places where getFederationActivationAge it's been used to pass the activations as method param --- .../main/java/co/rsk/peg/BridgeSupport.java | 8 ++++---- .../java/co/rsk/peg/BridgeSupportFactory.java | 2 +- .../java/co/rsk/peg/FederationSupport.java | 7 +++++-- .../rsk/peg/utils/BridgeEventLoggerImpl.java | 2 +- .../peg/utils/BrigeEventLoggerLegacyImpl.java | 2 +- .../rsk/remasc/RemascFederationProvider.java | 6 ++++-- .../co/rsk/peg/BridgeSupportFlyoverTest.java | 20 +++++++++---------- ...ridgeSupportProcessFundsMigrationTest.java | 4 ++-- .../java/co/rsk/peg/BridgeSupportTest.java | 4 ++-- .../rsk/peg/BridgeSupportTestIntegration.java | 8 ++++---- .../co/rsk/peg/FederationSupportTest.java | 11 ++++++---- .../java/co/rsk/peg/PowpegMigrationTest.java | 2 +- .../peg/utils/BridgeEventLoggerImplTest.java | 2 +- .../BridgeEventLoggerLegacyImplTest.java | 2 +- .../test/builders/BridgeSupportBuilder.java | 2 +- 15 files changed, 45 insertions(+), 37 deletions(-) diff --git a/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java b/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java index 1d98d19b3ed..077ac14f34c 100644 --- a/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java +++ b/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java @@ -998,7 +998,7 @@ private void processFundsMigration(Transaction rskTx) throws IOException { } private boolean federationIsInMigrationAge(Federation federation) { - long federationActivationAge = bridgeConstants.getFederationActivationAge(); + long federationActivationAge = bridgeConstants.getFederationActivationAge(activations); long federationAge = rskExecutionBlock.getNumber() - federation.getCreationBlockNumber(); long ageBegin = federationActivationAge + bridgeConstants.getFundsMigrationAgeSinceActivationBegin(); long ageEnd = federationActivationAge + bridgeConstants.getFundsMigrationAgeSinceActivationEnd(activations); @@ -1008,7 +1008,7 @@ private boolean federationIsInMigrationAge(Federation federation) { private boolean federationIsPastMigrationAge(Federation federation) { long federationAge = rskExecutionBlock.getNumber() - federation.getCreationBlockNumber(); - long ageEnd = bridgeConstants.getFederationActivationAge() + + long ageEnd = bridgeConstants.getFederationActivationAge(activations) + bridgeConstants.getFundsMigrationAgeSinceActivationEnd(activations); return federationAge >= ageEnd; @@ -1327,7 +1327,7 @@ private void updateFederationCreationBlockHeights() { long nextFederationCreationBlockHeight = nextFederationCreationBlockHeightOpt.get(); long curBlockHeight = rskExecutionBlock.getNumber(); - if (curBlockHeight >= nextFederationCreationBlockHeight + bridgeConstants.getFederationActivationAge()) { + if (curBlockHeight >= nextFederationCreationBlockHeight + bridgeConstants.getFederationActivationAge(activations)) { provider.setActiveFederationCreationBlockHeight(nextFederationCreationBlockHeight); provider.clearNextFederationCreationBlockHeight(); } @@ -2678,7 +2678,7 @@ public long getActiveFederationCreationBlockHeight() { if (nextFederationCreationBlockHeightOpt.isPresent()) { long nextFederationCreationBlockHeight = nextFederationCreationBlockHeightOpt.get(); long curBlockHeight = rskExecutionBlock.getNumber(); - if (curBlockHeight >= nextFederationCreationBlockHeight + bridgeConstants.getFederationActivationAge()) { + if (curBlockHeight >= nextFederationCreationBlockHeight + bridgeConstants.getFederationActivationAge(activations)) { return nextFederationCreationBlockHeight; } } diff --git a/rskj-core/src/main/java/co/rsk/peg/BridgeSupportFactory.java b/rskj-core/src/main/java/co/rsk/peg/BridgeSupportFactory.java index f0c7e65b7ea..dcc9b9b8235 100644 --- a/rskj-core/src/main/java/co/rsk/peg/BridgeSupportFactory.java +++ b/rskj-core/src/main/java/co/rsk/peg/BridgeSupportFactory.java @@ -67,7 +67,7 @@ public BridgeSupport newInstance(Repository repository, Block executionBlock, activations ); - FederationSupport federationSupport = new FederationSupport(bridgeConstants, provider, executionBlock); + FederationSupport federationSupport = new FederationSupport(bridgeConstants, provider, executionBlock, activations); BridgeEventLogger eventLogger; if (logs == null) { diff --git a/rskj-core/src/main/java/co/rsk/peg/FederationSupport.java b/rskj-core/src/main/java/co/rsk/peg/FederationSupport.java index 98d9fea0f53..ea657d14fd5 100644 --- a/rskj-core/src/main/java/co/rsk/peg/FederationSupport.java +++ b/rskj-core/src/main/java/co/rsk/peg/FederationSupport.java @@ -20,6 +20,7 @@ import co.rsk.bitcoinj.core.BtcECKey; import co.rsk.bitcoinj.core.UTXO; import co.rsk.config.BridgeConstants; +import org.ethereum.config.blockchain.upgrades.ActivationConfig; import org.ethereum.core.Block; import javax.annotation.Nullable; @@ -34,11 +35,13 @@ private enum StorageFederationReference { NONE, NEW, OLD, GENESIS } private final BridgeStorageProvider provider; private final BridgeConstants bridgeConstants; private final Block executionBlock; + private final ActivationConfig.ForBlock activations; - public FederationSupport(BridgeConstants bridgeConstants, BridgeStorageProvider provider, Block executionBlock) { + public FederationSupport(BridgeConstants bridgeConstants, BridgeStorageProvider provider, Block executionBlock, ActivationConfig.ForBlock activations) { this.provider = provider; this.bridgeConstants = bridgeConstants; this.executionBlock = executionBlock; + this.activations = activations; } /** @@ -226,6 +229,6 @@ private StorageFederationReference getRetiringFederationReference() { private boolean shouldFederationBeActive(Federation federation) { long federationAge = executionBlock.getNumber() - federation.getCreationBlockNumber(); - return federationAge >= bridgeConstants.getFederationActivationAge(); + return federationAge >= bridgeConstants.getFederationActivationAge(activations); } } diff --git a/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java b/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java index 3da68488d93..ae59b2617df 100644 --- a/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java +++ b/rskj-core/src/main/java/co/rsk/peg/utils/BridgeEventLoggerImpl.java @@ -106,7 +106,7 @@ public void logCommitFederation(Block executionBlock, Federation oldFederation, String oldFederationBtcAddress = oldFederation.getAddress().toBase58(); byte[] newFederationFlatPubKeys = flatKeysAsByteArray(newFederation.getBtcPublicKeys()); String newFederationBtcAddress = newFederation.getAddress().toBase58(); - long newFedActivationBlockNumber = executionBlock.getNumber() + this.bridgeConstants.getFederationActivationAge(); + long newFedActivationBlockNumber = executionBlock.getNumber() + this.bridgeConstants.getFederationActivationAge(activations); CallTransaction.Function event = BridgeEvents.COMMIT_FEDERATION.getEvent(); byte[][] encodedTopicsInBytes = event.encodeEventTopics(); diff --git a/rskj-core/src/main/java/co/rsk/peg/utils/BrigeEventLoggerLegacyImpl.java b/rskj-core/src/main/java/co/rsk/peg/utils/BrigeEventLoggerLegacyImpl.java index 1fc1ecdc380..81d521ea1cb 100644 --- a/rskj-core/src/main/java/co/rsk/peg/utils/BrigeEventLoggerLegacyImpl.java +++ b/rskj-core/src/main/java/co/rsk/peg/utils/BrigeEventLoggerLegacyImpl.java @@ -119,7 +119,7 @@ public void logCommitFederation(Block executionBlock, Federation oldFederation, byte[] newFedFlatPubKeys = flatKeysAsRlpCollection(newFederation.getBtcPublicKeys()); byte[] newFedData = RLP.encodeList(RLP.encodeElement(newFederation.getAddress().getHash160()), RLP.encodeList(newFedFlatPubKeys)); - long newFedActivationBlockNumber = executionBlock.getNumber() + this.bridgeConstants.getFederationActivationAge(); + long newFedActivationBlockNumber = executionBlock.getNumber() + this.bridgeConstants.getFederationActivationAge(activations); byte[] data = RLP.encodeList(oldFedData, newFedData, RLP.encodeString(Long.toString(newFedActivationBlockNumber))); diff --git a/rskj-core/src/main/java/co/rsk/remasc/RemascFederationProvider.java b/rskj-core/src/main/java/co/rsk/remasc/RemascFederationProvider.java index 7b582e41aa0..ae5c8fcc37b 100644 --- a/rskj-core/src/main/java/co/rsk/remasc/RemascFederationProvider.java +++ b/rskj-core/src/main/java/co/rsk/remasc/RemascFederationProvider.java @@ -38,13 +38,15 @@ public RemascFederationProvider( BridgeConstants bridgeConstants, Repository repository, Block processingBlock) { + + ActivationConfig.ForBlock activations = activationConfig.forBlock(processingBlock.getNumber()); BridgeStorageProvider bridgeStorageProvider = new BridgeStorageProvider( repository, PrecompiledContracts.BRIDGE_ADDR, bridgeConstants, - activationConfig.forBlock(processingBlock.getNumber()) + activations ); - this.federationSupport = new FederationSupport(bridgeConstants, bridgeStorageProvider, processingBlock); + this.federationSupport = new FederationSupport(bridgeConstants, bridgeStorageProvider, processingBlock, activations); } public int getFederationSize() { diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportFlyoverTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportFlyoverTest.java index 0b666ce114b..183c14fe378 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportFlyoverTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportFlyoverTest.java @@ -117,7 +117,7 @@ private BigInteger sendFundsToActiveFederation( // For the sake of simplicity, this set the fed activation age value equal to the value in the bridgeRegTestConstants // in order to be able to always get the current retiring federation when it's been mock with no need of creating // unnecessary blocks when testing on mainnet. - doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge()).when(bridgeConstants).getFederationActivationAge(); + doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge(activations)).when(bridgeConstants).getFederationActivationAge(activations); Context btcContext = mock(Context.class); doReturn(bridgeConstants.getBtcParams()).when(btcContext).getParams(); @@ -259,7 +259,7 @@ private BigInteger sendFundsToRetiringFederation( // For the sake of simplicity, this set the fed activation age value equal to the value in the bridgeRegTestConstants // in order to be able to always get the current retiring federation when it's been mock with no need of creating // unnecessary blocks when testing on mainnet. - doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge()).when(bridgeConstants).getFederationActivationAge(); + doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge(activations)).when(bridgeConstants).getFederationActivationAge(activations); Context btcContext = mock(Context.class); doReturn(bridgeConstants.getBtcParams()).when(btcContext).getParams(); @@ -403,7 +403,7 @@ private BigInteger sendFundsToActiveAndRetiringFederation( // For the sake of simplicity, this set the fed activation age value equal to the value in the bridgeRegTestConstants // in order to be able to always get the current retiring federation when it's been mock with no need of creating // unnecessary blocks when testing on mainnet. - doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge()).when(bridgeConstants).getFederationActivationAge(); + doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge(activations)).when(bridgeConstants).getFederationActivationAge(activations); Context btcContext = mock(Context.class); doReturn(bridgeConstants.getBtcParams()).when(btcContext).getParams(); @@ -571,7 +571,7 @@ private BigInteger sendFundsToAnyAddress( // For the sake of simplicity, this set the fed activation age value equal to the value in the bridgeRegTestConstants // in order to be able to always get the current retiring federation when it's been mock with no need of creating // unnecessary blocks when testing on mainnet. - doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge()).when(bridgeConstants).getFederationActivationAge(); + doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge(activations)).when(bridgeConstants).getFederationActivationAge(activations); Context btcContext = mock(Context.class); doReturn(bridgeConstants.getBtcParams()).when(btcContext).getParams(); @@ -1607,7 +1607,7 @@ void registerFlyoverBtcTransaction_failed_when_tx_with_witness_already_saved_in_ // For the sake of simplicity, this set the fed activation age value equal to the value in the bridgeRegTestConstants // in order to be able to always get the current retiring federation when it's been mock with no need of creating // unnecessary blocks when testing on mainnet. - doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge()).when(bridgeConstants).getFederationActivationAge(); + doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge(activations)).when(bridgeConstants).getFederationActivationAge(activations); Context btcContext = mock(Context.class); doReturn(bridgeConstants.getBtcParams()).when(btcContext).getParams(); @@ -1791,7 +1791,7 @@ void registerFlyoverBtcTransaction_failed_when_tx_with_witness_surpassing_lockin // For the sake of simplicity, this set the fed activation age value equal to the value in the bridgeRegTestConstants // in order to be able to always get the current retiring federation when it's been mock with no need of creating // unnecessary blocks when testing on mainnet. - doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge()).when(bridgeConstants).getFederationActivationAge(); + doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge(activations)).when(bridgeConstants).getFederationActivationAge(activations); Context btcContext = mock(Context.class); doReturn(bridgeConstants.getBtcParams()).when(btcContext).getParams(); @@ -1985,7 +1985,7 @@ void registerFlyoverBtcTransaction_failed_when_tx_with_witness_surpassing_lockin // For the sake of simplicity, this set the fed activation age value equal to the value in the bridgeRegTestConstants // in order to be able to always get the current retiring federation when it's been mock with no need of creating // unnecessary blocks when testing on mainnet. - doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge()).when(bridgeConstants).getFederationActivationAge(); + doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge(activations)).when(bridgeConstants).getFederationActivationAge(activations); Context btcContext = mock(Context.class); doReturn(bridgeConstants.getBtcParams()).when(btcContext).getParams(); @@ -2179,7 +2179,7 @@ void registerFlyoverBtcTransaction_failed_when_tx_without_witness_surpassing_loc // For the sake of simplicity, this set the fed activation age value equal to the value in the bridgeRegTestConstants // in order to be able to always get the current retiring federation when it's been mock with no need of creating // unnecessary blocks when testing on mainnet. - doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge()).when(bridgeConstants).getFederationActivationAge(); + doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge(activations)).when(bridgeConstants).getFederationActivationAge(activations); Context btcContext = mock(Context.class); doReturn(bridgeConstants.getBtcParams()).when(btcContext).getParams(); @@ -2346,7 +2346,7 @@ void registerFlyoverBtcTransaction_failed_when_tx_without_witness_surpassing_loc // For the sake of simplicity, this set the fed activation age value equal to the value in the bridgeRegTestConstants // in order to be able to always get the current retiring federation when it's been mock with no need of creating // unnecessary blocks when testing on mainnet. - doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge()).when(bridgeConstants).getFederationActivationAge(); + doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge(activations)).when(bridgeConstants).getFederationActivationAge(activations); Context btcContext = mock(Context.class); doReturn(bridgeConstants.getBtcParams()).when(btcContext).getParams(); @@ -2528,7 +2528,7 @@ void registerFlyoverBtcTransaction_failed_when_tx_without_witness_already_saved_ // For the sake of simplicity, this set the fed activation age value equal to the value in the bridgeRegTestConstants // in order to be able to always get the current retiring federation when it's been mock with no need of creating // unnecessary blocks when testing on mainnet. - doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge()).when(bridgeConstants).getFederationActivationAge(); + doReturn(BridgeRegTestConstants.getInstance().getFederationActivationAge(activations)).when(bridgeConstants).getFederationActivationAge(activations); Context btcContext = mock(Context.class); doReturn(bridgeConstants.getBtcParams()).when(btcContext).getParams(); diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java index 062ef64b2af..0fb03a2b8c2 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java @@ -184,10 +184,10 @@ private void test_processFundsMigration( long federationCreationBlockNumber = 5L; long federationInMigrationAgeHeight = federationCreationBlockNumber + - bridgeConstants.getFederationActivationAge() + + bridgeConstants.getFederationActivationAge(activations) + bridgeConstants.getFundsMigrationAgeSinceActivationBegin() + 1; long federationPastMigrationAgeHeight = federationCreationBlockNumber + - bridgeConstants.getFederationActivationAge() + + bridgeConstants.getFederationActivationAge(activations) + bridgeConstants.getFundsMigrationAgeSinceActivationEnd(activations) + 1; Federation newFederation = new Federation( diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportTest.java index c8a444a596e..da2efd95fac 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportTest.java @@ -6677,7 +6677,7 @@ private void test_migrating_many_utxos(boolean isRskip294Active, int utxosToCrea Block block = mock(Block.class); // Set block right after the migration should start long blockNumber = newFed.getCreationBlockNumber() + - bridgeConstantsRegtest.getFederationActivationAge() + + bridgeConstantsRegtest.getFederationActivationAge(activations) + bridgeConstantsRegtest.getFundsMigrationAgeSinceActivationBegin() + 1; when(block.getNumber()).thenReturn(blockNumber); @@ -7382,7 +7382,7 @@ private BridgeSupport getBridgeSupport(BridgeConstants constants, BridgeStorageP track, executionBlock, new Context(constants.getBtcParams()), - new FederationSupport(constants, provider, executionBlock), + new FederationSupport(constants, provider, executionBlock, activations), blockStoreFactory, activations, signatureCache diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportTestIntegration.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportTestIntegration.java index ff962ebb191..db90f3d2998 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportTestIntegration.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportTestIntegration.java @@ -227,7 +227,7 @@ void testGetBtcBlockchainBlockLocatorWithBtcCheckpoints() throws Exception { track, null, new Context(bridgeConstants.getBtcParams()), - new FederationSupport(bridgeConstants, provider, null), + new FederationSupport(bridgeConstants, provider, null, activationsBeforeForks), btcBlockStoreFactory, mock(ActivationConfig.ForBlock.class), signatureCache @@ -3750,8 +3750,8 @@ private BridgeSupport getBridgeSupportWithMocksForFederationTests( when(constantsMock.getBtcParams()).thenReturn(NetworkParameters.fromID(NetworkParameters.ID_REGTEST)); when(constantsMock.getFederationChangeAuthorizer()).thenReturn(bridgeConstants.getFederationChangeAuthorizer()); - long federationActivationAge = bridgeConstants.getFederationActivationAge(); - when(constantsMock.getFederationActivationAge()).thenReturn(federationActivationAge); + long federationActivationAge = bridgeConstants.getFederationActivationAge(activationsBeforeForks); + when(constantsMock.getFederationActivationAge(activationsBeforeForks)).thenReturn(federationActivationAge); class FederationHolder { private PendingFederation pendingFederation; @@ -3968,7 +3968,7 @@ private BridgeSupport getBridgeSupport( track, executionBlock, new Context(constants.getBtcParams()), - new FederationSupport(constants, provider, executionBlock), + new FederationSupport(constants, provider, executionBlock, activations), blockStoreFactory, activations, signatureCache diff --git a/rskj-core/src/test/java/co/rsk/peg/FederationSupportTest.java b/rskj-core/src/test/java/co/rsk/peg/FederationSupportTest.java index bde6a2a362d..00bbb3b88f6 100644 --- a/rskj-core/src/test/java/co/rsk/peg/FederationSupportTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/FederationSupportTest.java @@ -21,6 +21,7 @@ import co.rsk.bitcoinj.core.NetworkParameters; import co.rsk.config.BridgeConstants; import org.bouncycastle.util.encoders.Hex; +import org.ethereum.config.blockchain.upgrades.ActivationConfig; import org.ethereum.core.Block; import org.ethereum.crypto.ECKey; import org.junit.jupiter.api.Assertions; @@ -43,18 +44,20 @@ class FederationSupportTest { private BridgeConstants bridgeConstants; private BridgeStorageProvider provider; private Block executionBlock; - + private ActivationConfig.ForBlock activations; @BeforeEach void setUp() { provider = mock(BridgeStorageProvider.class); bridgeConstants = mock(BridgeConstants.class); executionBlock = mock(Block.class); + activations = mock(ActivationConfig.ForBlock.class); federationSupport = new FederationSupport( bridgeConstants, provider, - executionBlock + executionBlock, + activations ); } @@ -88,7 +91,7 @@ void whenOldAndNewFederationArePresentReturnOldFederationByActivationAge() { when(provider.getNewFederation()).thenReturn(newFederation); when(provider.getOldFederation()).thenReturn(oldFederation); when(executionBlock.getNumber()).thenReturn(80L); - when(bridgeConstants.getFederationActivationAge()).thenReturn(10L); + when(bridgeConstants.getFederationActivationAge(activations)).thenReturn(10L); assertThat(federationSupport.getActiveFederation(), is(oldFederation)); } @@ -101,7 +104,7 @@ void whenOldAndNewFederationArePresentReturnNewFederationByActivationAge() { when(provider.getNewFederation()).thenReturn(newFederation); when(provider.getOldFederation()).thenReturn(oldFederation); when(executionBlock.getNumber()).thenReturn(80L); - when(bridgeConstants.getFederationActivationAge()).thenReturn(10L); + when(bridgeConstants.getFederationActivationAge(activations)).thenReturn(10L); assertThat(federationSupport.getActiveFederation(), is(newFederation)); } diff --git a/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java b/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java index a757b06b79d..af1dfed3517 100644 --- a/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java @@ -278,7 +278,7 @@ private void testChangePowpeg( // Move the required blocks ahead for the new powpeg to become active // (overriding block number to ensure we don't move beyond the activation phase) - blockNumber = initialBlock.getNumber() + bridgeConstants.getFederationActivationAge(); + blockNumber = initialBlock.getNumber() + bridgeConstants.getFederationActivationAge(activations); Block activationBlock = mock(Block.class); doReturn(blockNumber).when(activationBlock).getNumber(); diff --git a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java index 733e9faf91f..bb4a35a2623 100644 --- a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java @@ -249,7 +249,7 @@ void logCommitFederation() { String oldFederationBtcAddress = oldFederation.getAddress().toBase58(); byte[] newFederationFlatPubKeys = flatKeysAsByteArray(newFederation.getBtcPublicKeys()); String newFederationBtcAddress = newFederation.getAddress().toBase58(); - long newFedActivationBlockNumber = executionBlock.getNumber() + CONSTANTS.getFederationActivationAge(); + long newFedActivationBlockNumber = executionBlock.getNumber() + CONSTANTS.getFederationActivationAge(activations); Object[] data = new Object[]{ oldFederationFlatPubKeys, oldFederationBtcAddress, diff --git a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java index f85ff6e6be0..e30d761e6e4 100644 --- a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java @@ -269,7 +269,7 @@ void testLogCommitFederationBeforeRskip146() { } // Assert new federation activation block number - Assertions.assertEquals(15L + constantsMock.getFederationActivationAge(), Long.valueOf(new String(dataList.get(2).getRLPData(), StandardCharsets.UTF_8)).longValue()); + Assertions.assertEquals(15L + constantsMock.getFederationActivationAge(activations), Long.valueOf(new String(dataList.get(2).getRLPData(), StandardCharsets.UTF_8)).longValue()); } @Test diff --git a/rskj-core/src/test/java/co/rsk/test/builders/BridgeSupportBuilder.java b/rskj-core/src/test/java/co/rsk/test/builders/BridgeSupportBuilder.java index 65dc5402c77..0bc09cb56a7 100644 --- a/rskj-core/src/test/java/co/rsk/test/builders/BridgeSupportBuilder.java +++ b/rskj-core/src/test/java/co/rsk/test/builders/BridgeSupportBuilder.java @@ -99,7 +99,7 @@ public BridgeSupport build() { repository, executionBlock, new Context(bridgeConstants.getBtcParams()), - new FederationSupport(bridgeConstants, provider, executionBlock), + new FederationSupport(bridgeConstants, provider, executionBlock, activations), btcBlockStoreFactory, activations, signatureCache From 5d5e6d03809f5a541482f49d1c1168f7f8b1c9b3 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Mon, 17 Apr 2023 16:20:46 -0400 Subject: [PATCH 05/24] Added tests for getFederationActivationAge method --- .../co/rsk/config/BridgeRegTestConstants.java | 1 + .../co/rsk/config/BridgeConstantsTest.java | 50 ++++++++++++++++--- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/rskj-core/src/main/java/co/rsk/config/BridgeRegTestConstants.java b/rskj-core/src/main/java/co/rsk/config/BridgeRegTestConstants.java index c3258c4e3b3..54257fda66a 100644 --- a/rskj-core/src/main/java/co/rsk/config/BridgeRegTestConstants.java +++ b/rskj-core/src/main/java/co/rsk/config/BridgeRegTestConstants.java @@ -90,6 +90,7 @@ public BridgeRegTestConstants(List federationPublicKeys) { AddressBasedAuthorizer.MinimumRequiredCalculation.MAJORITY ); + federationActivationAgeLegacy = 10L; federationActivationAge = 10L; fundsMigrationAgeSinceActivationBegin = 15L; diff --git a/rskj-core/src/test/java/co/rsk/config/BridgeConstantsTest.java b/rskj-core/src/test/java/co/rsk/config/BridgeConstantsTest.java index d8c4e606735..a6f15c12d60 100644 --- a/rskj-core/src/test/java/co/rsk/config/BridgeConstantsTest.java +++ b/rskj-core/src/test/java/co/rsk/config/BridgeConstantsTest.java @@ -13,8 +13,7 @@ import static org.mockito.Mockito.when; class BridgeConstantsTest { - - private static Stream generator() { + private static Stream generatorFundsMigrationAgeSinceActivationEnd() { return Stream.of( Arguments.of(BridgeMainNetConstants.getInstance(), false), Arguments.of(BridgeTestNetConstants.getInstance(), true), @@ -23,8 +22,8 @@ private static Stream generator() { } @ParameterizedTest() - @MethodSource("generator") - void test_getFundsMigrationAgeSinceActivationEnd(BridgeConstants bridgeConstants, boolean hasSameValueForBothMigrationAges) { + @MethodSource("generatorFundsMigrationAgeSinceActivationEnd") + void test_getFundsMigrationAgeSinceActivationEnd(BridgeConstants bridgeConstants, boolean hasSameValueForBothFields) { // Arrange ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); @@ -33,11 +32,11 @@ void test_getFundsMigrationAgeSinceActivationEnd(BridgeConstants bridgeConstants // assert assertEquals(fundsMigrationAgeSinceActivationEnd, bridgeConstants.fundsMigrationAgeSinceActivationEnd); - assertEquals(hasSameValueForBothMigrationAges, fundsMigrationAgeSinceActivationEnd == bridgeConstants.specialCaseFundsMigrationAgeSinceActivationEnd); + assertEquals(hasSameValueForBothFields, fundsMigrationAgeSinceActivationEnd == bridgeConstants.specialCaseFundsMigrationAgeSinceActivationEnd); } @ParameterizedTest() - @MethodSource("generator") + @MethodSource("generatorFundsMigrationAgeSinceActivationEnd") void test_getFundsMigrationAgeSinceActivationEnd_post_RSKIP357(BridgeConstants bridgeConstants, boolean hasSameValueForBothMigrationAges) { // Arrange ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); @@ -52,7 +51,7 @@ void test_getFundsMigrationAgeSinceActivationEnd_post_RSKIP357(BridgeConstants b } @ParameterizedTest() - @MethodSource("generator") + @MethodSource("generatorFundsMigrationAgeSinceActivationEnd") void test_getFundsMigrationAgeSinceActivationEnd_post_RSKIP374(BridgeConstants bridgeConstants, boolean hasSameValueForBothMigrationAges) { // Arrange ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); @@ -66,4 +65,41 @@ void test_getFundsMigrationAgeSinceActivationEnd_post_RSKIP374(BridgeConstants b assertEquals(fundsMigrationAgeSinceActivationEnd, bridgeConstants.fundsMigrationAgeSinceActivationEnd); assertEquals(hasSameValueForBothMigrationAges, fundsMigrationAgeSinceActivationEnd == bridgeConstants.specialCaseFundsMigrationAgeSinceActivationEnd); } + + private static Stream generatorFederationActivationAge() { + return Stream.of( + Arguments.of(BridgeMainNetConstants.getInstance(), false), + Arguments.of(BridgeTestNetConstants.getInstance(), true), + Arguments.of(BridgeRegTestConstants.getInstance(), true) + ); + } + + @ParameterizedTest() + @MethodSource("generatorFederationActivationAge") + void test_getFederationActivationAge(BridgeConstants bridgeConstants, boolean fieldsHasSameValue) { + // Arrange + ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); + + // Act + long federationActivationAge = bridgeConstants.getFederationActivationAge(activations); + + // assert + assertEquals(federationActivationAge, bridgeConstants.federationActivationAgeLegacy); + assertEquals(fieldsHasSameValue, federationActivationAge == bridgeConstants.federationActivationAge); + } + + @ParameterizedTest() + @MethodSource("generatorFederationActivationAge") + void test_getFederationActivationAge_post_RSKIP383(BridgeConstants bridgeConstants, boolean fieldsHasSameValue) { + // Arrange + ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); + when(activations.isActive(ConsensusRule.RSKIP383)).thenReturn(true); + + // Act + long federationActivationAge = bridgeConstants.getFederationActivationAge(activations); + + // assert + assertEquals(federationActivationAge, bridgeConstants.federationActivationAge); + assertEquals(fieldsHasSameValue, federationActivationAge == bridgeConstants.federationActivationAgeLegacy); + } } From 1ef0932986c8772f440e43d4bfd180b23c7bb78e Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Tue, 18 Apr 2023 10:56:29 -0400 Subject: [PATCH 06/24] Fixed tests failing --- .../test/java/co/rsk/peg/BridgeSupportTestIntegration.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportTestIntegration.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportTestIntegration.java index db90f3d2998..2cc6482e2d9 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportTestIntegration.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportTestIntegration.java @@ -3750,8 +3750,9 @@ private BridgeSupport getBridgeSupportWithMocksForFederationTests( when(constantsMock.getBtcParams()).thenReturn(NetworkParameters.fromID(NetworkParameters.ID_REGTEST)); when(constantsMock.getFederationChangeAuthorizer()).thenReturn(bridgeConstants.getFederationChangeAuthorizer()); - long federationActivationAge = bridgeConstants.getFederationActivationAge(activationsBeforeForks); - when(constantsMock.getFederationActivationAge(activationsBeforeForks)).thenReturn(federationActivationAge); + ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); + long federationActivationAge = bridgeConstants.getFederationActivationAge(activations); + when(constantsMock.getFederationActivationAge(any())).thenReturn(federationActivationAge); class FederationHolder { private PendingFederation pendingFederation; From 40e90de1c150a058b5d0346ca6bc77b8f1f2922d Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Mon, 24 Apr 2023 13:01:04 -0400 Subject: [PATCH 07/24] Added fingerroot activation config test --- .../config/blockchain/upgrades/ActivationConfigsForTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigsForTest.java b/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigsForTest.java index f865a918975..277347a45bb 100644 --- a/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigsForTest.java +++ b/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigsForTest.java @@ -167,7 +167,8 @@ private static List getFingerroot500Rskips() { ConsensusRule.RSKIP326, ConsensusRule.RSKIP374, ConsensusRule.RSKIP375, - ConsensusRule.RSKIP377 + ConsensusRule.RSKIP377, + ConsensusRule.RSKIP383 )); return rskips; From 89b0923f7e27398c0b6cc13ffbb2648a89545639 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Mon, 24 Apr 2023 13:01:49 -0400 Subject: [PATCH 08/24] Set a different value than federationActivationAgeLegacy to federationActivationAge --- .../src/main/java/co/rsk/config/BridgeDevNetConstants.java | 2 +- .../src/main/java/co/rsk/config/BridgeTestNetConstants.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rskj-core/src/main/java/co/rsk/config/BridgeDevNetConstants.java b/rskj-core/src/main/java/co/rsk/config/BridgeDevNetConstants.java index 7eb2f590700..4646b261172 100644 --- a/rskj-core/src/main/java/co/rsk/config/BridgeDevNetConstants.java +++ b/rskj-core/src/main/java/co/rsk/config/BridgeDevNetConstants.java @@ -105,7 +105,7 @@ public BridgeDevNetConstants(List federationPublicKeys) { ); federationActivationAgeLegacy = 10L; - federationActivationAge = 10L; + federationActivationAge = 20L; fundsMigrationAgeSinceActivationBegin = 15L; fundsMigrationAgeSinceActivationEnd = 100L; diff --git a/rskj-core/src/main/java/co/rsk/config/BridgeTestNetConstants.java b/rskj-core/src/main/java/co/rsk/config/BridgeTestNetConstants.java index 414576e5b3f..93a09a1b003 100644 --- a/rskj-core/src/main/java/co/rsk/config/BridgeTestNetConstants.java +++ b/rskj-core/src/main/java/co/rsk/config/BridgeTestNetConstants.java @@ -104,7 +104,7 @@ public class BridgeTestNetConstants extends BridgeConstants { ); federationActivationAgeLegacy = 60L; - federationActivationAge = 60L; + federationActivationAge = 120L; fundsMigrationAgeSinceActivationBegin = 60L; fundsMigrationAgeSinceActivationEnd = 900L; From 166e4c4db480208ab8f23b33e05b16ab2edabbc8 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Mon, 24 Apr 2023 13:37:00 -0400 Subject: [PATCH 09/24] Refactored test to make parameterized --- ...ridgeSupportProcessFundsMigrationTest.java | 25 +++++++- .../co/rsk/peg/FederationSupportTest.java | 63 ++++++++++++++++--- 2 files changed, 78 insertions(+), 10 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java index 0fb03a2b8c2..944e0df4d1a 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java @@ -12,6 +12,7 @@ import org.bouncycastle.util.encoders.Hex; import org.ethereum.config.Constants; import org.ethereum.config.blockchain.upgrades.ActivationConfig; +import org.ethereum.config.blockchain.upgrades.ActivationConfigsForTest; import org.ethereum.config.blockchain.upgrades.ConsensusRule; import org.ethereum.core.Transaction; import org.junit.jupiter.api.Assertions; @@ -137,6 +138,12 @@ void processFundsMigration_in_migration_age_after_rskip_374_activation_mainnet() test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, true); } + @Test + void processFundsMigration_in_migration_age_after_fingerroot_activation_mainnet() throws IOException { + ActivationConfig.ForBlock activations = ActivationConfigsForTest.fingerroot500().forBlock(0); + test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, true); + } + @Test void processFundsMigration_past_migration_age_before_rskip_146_activation_mainnet() throws IOException { ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); @@ -173,6 +180,12 @@ void processFundsMigration_past_migration_age_after_rskip_374_activation_mainnet test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, false); } + @Test + void processFundsMigration_past_migration_age_after_fingerroot_activation_mainnet() throws IOException { + ActivationConfig.ForBlock activations = ActivationConfigsForTest.fingerroot500().forBlock(0); + test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, false); + } + private void test_processFundsMigration( BridgeConstants bridgeConstants, ActivationConfig.ForBlock activations, @@ -181,13 +194,19 @@ private void test_processFundsMigration( BridgeEventLogger bridgeEventLogger = mock(BridgeEventLogger.class); Federation oldFederation = bridgeConstants.getGenesisFederation(); + long federationActivationAge = bridgeConstants.getFederationActivationAge(activations); + + long expectedFederationActivationAge = activations.isActive(ConsensusRule.RSKIP383)? 120L: 60L; + if (bridgeConstants instanceof BridgeMainNetConstants){ + expectedFederationActivationAge = activations.isActive(ConsensusRule.RSKIP383)? 40320L: 18500L; + } + Assertions.assertEquals(expectedFederationActivationAge, federationActivationAge); long federationCreationBlockNumber = 5L; - long federationInMigrationAgeHeight = federationCreationBlockNumber + - bridgeConstants.getFederationActivationAge(activations) + + long federationInMigrationAgeHeight = federationCreationBlockNumber + federationActivationAge + bridgeConstants.getFundsMigrationAgeSinceActivationBegin() + 1; long federationPastMigrationAgeHeight = federationCreationBlockNumber + - bridgeConstants.getFederationActivationAge(activations) + + federationActivationAge + bridgeConstants.getFundsMigrationAgeSinceActivationEnd(activations) + 1; Federation newFederation = new Federation( diff --git a/rskj-core/src/test/java/co/rsk/peg/FederationSupportTest.java b/rskj-core/src/test/java/co/rsk/peg/FederationSupportTest.java index 00bbb3b88f6..6a5d2b4c291 100644 --- a/rskj-core/src/test/java/co/rsk/peg/FederationSupportTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/FederationSupportTest.java @@ -20,18 +20,26 @@ import co.rsk.bitcoinj.core.BtcECKey; import co.rsk.bitcoinj.core.NetworkParameters; import co.rsk.config.BridgeConstants; +import co.rsk.config.BridgeMainNetConstants; +import co.rsk.config.BridgeRegTestConstants; +import co.rsk.config.BridgeTestNetConstants; import org.bouncycastle.util.encoders.Hex; import org.ethereum.config.blockchain.upgrades.ActivationConfig; +import org.ethereum.config.blockchain.upgrades.ActivationConfigsForTest; import org.ethereum.core.Block; import org.ethereum.crypto.ECKey; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import java.time.Instant; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.stream.Stream; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; @@ -96,17 +104,58 @@ void whenOldAndNewFederationArePresentReturnOldFederationByActivationAge() { assertThat(federationSupport.getActiveFederation(), is(oldFederation)); } - @Test - void whenOldAndNewFederationArePresentReturnNewFederationByActivationAge() { - Federation newFederation = getNewFakeFederation(65); - Federation oldFederation = getNewFakeFederation(0); + private static Stream provideTestArguments() { + BridgeConstants bridgeConstants = BridgeTestNetConstants.getInstance(); + ActivationConfig.ForBlock hop = ActivationConfigsForTest.hop400().forBlock(0); + ActivationConfig.ForBlock fingerroot = ActivationConfigsForTest.fingerroot500().forBlock(0); + + return Stream.of( + Arguments.of(hop, 65, 0, 80L, false), + Arguments.of(hop, 65, 0, 65+bridgeConstants.getFederationActivationAge(hop), true), + Arguments.of(fingerroot, 65, 0, 80L, false), + Arguments.of(fingerroot, 65, 0, 65+bridgeConstants.getFederationActivationAge(fingerroot), true) + ); + } + + @ParameterizedTest + @MethodSource("provideTestArguments") + void whenOldAndNewFederationArePresentReturnNewFederationByActivationAge( + ActivationConfig.ForBlock activations, + int newFedCreationBlockNumber, + int oldFedCreationBlockNumber, + long currentBlockNumber, + boolean newFedExpectedToBeActive) + { + // Arrange + Federation newFederation = getNewFakeFederation(newFedCreationBlockNumber); + Federation oldFederation = getNewFakeFederation(oldFedCreationBlockNumber); + + BridgeStorageProvider provider = mock(BridgeStorageProvider.class); when(provider.getNewFederation()).thenReturn(newFederation); when(provider.getOldFederation()).thenReturn(oldFederation); - when(executionBlock.getNumber()).thenReturn(80L); - when(bridgeConstants.getFederationActivationAge(activations)).thenReturn(10L); - assertThat(federationSupport.getActiveFederation(), is(newFederation)); + Block executionBlock = mock(Block.class); + when(executionBlock.getNumber()).thenReturn(currentBlockNumber); + + BridgeConstants bridgeConstants = BridgeTestNetConstants.getInstance(); + + FederationSupport federationSupport = new FederationSupport( + bridgeConstants, + provider, + executionBlock, + activations + ); + + // Act + Federation activeFederation = federationSupport.getActiveFederation(); + + // Assert + if (newFedExpectedToBeActive){ + assertThat(activeFederation, is(newFederation)); + } else { + assertThat(federationSupport.getActiveFederation(), is(oldFederation)); + } } @Test From 863664dc38e465612d0d6d007eed59cb1b25d3af Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Mon, 24 Apr 2023 15:18:22 -0400 Subject: [PATCH 10/24] Added before and after fingerroot tests for logCommitFederation --- .../co/rsk/config/BridgeRegTestConstants.java | 2 +- .../peg/utils/BridgeEventLoggerImplTest.java | 130 ++++++++++++++++++ 2 files changed, 131 insertions(+), 1 deletion(-) diff --git a/rskj-core/src/main/java/co/rsk/config/BridgeRegTestConstants.java b/rskj-core/src/main/java/co/rsk/config/BridgeRegTestConstants.java index 54257fda66a..f7ce7aa7669 100644 --- a/rskj-core/src/main/java/co/rsk/config/BridgeRegTestConstants.java +++ b/rskj-core/src/main/java/co/rsk/config/BridgeRegTestConstants.java @@ -91,7 +91,7 @@ public BridgeRegTestConstants(List federationPublicKeys) { ); federationActivationAgeLegacy = 10L; - federationActivationAge = 10L; + federationActivationAge = 20L; fundsMigrationAgeSinceActivationBegin = 15L; fundsMigrationAgeSinceActivationEnd = 150L; diff --git a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java index bb4a35a2623..952a6019271 100644 --- a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java @@ -25,6 +25,7 @@ import co.rsk.peg.*; import org.bouncycastle.util.encoders.Hex; import org.ethereum.config.blockchain.upgrades.ActivationConfig; +import org.ethereum.config.blockchain.upgrades.ActivationConfigsForTest; import org.ethereum.config.blockchain.upgrades.ConsensusRule; import org.ethereum.core.*; import org.ethereum.crypto.ECKey; @@ -260,6 +261,135 @@ void logCommitFederation() { assertEvent(eventLogs, 0, BridgeEvents.COMMIT_FEDERATION.getEvent(), new Object[]{}, data); } + @Test + void logCommitFederation_hop() { + ActivationConfig.ForBlock activations = ActivationConfigsForTest.hop400().forBlock(0); + // Setup parameters for test method call + Block executionBlock = mock(Block.class); + when(executionBlock.getTimestamp()).thenReturn(15005L); + when(executionBlock.getNumber()).thenReturn(15L); + + List oldFederationKeys = Arrays.asList( + BtcECKey.fromPublicOnly(Hex.decode("036bb9eab797eadc8b697f0e82a01d01cabbfaaca37e5bafc06fdc6fdd38af894a")), + BtcECKey.fromPublicOnly(Hex.decode("031da807c71c2f303b7f409dd2605b297ac494a563be3b9ca5f52d95a43d183cc5")), + BtcECKey.fromPublicOnly(Hex.decode("025eefeeeed5cdc40822880c7db1d0a88b7b986945ed3fc05a0b45fe166fe85e12")), + BtcECKey.fromPublicOnly(Hex.decode("03c67ad63527012fd4776ae892b5dc8c56f80f1be002dc65cd520a2efb64e37b49")) + ); + + List oldFederationMembers = FederationTestUtils.getFederationMembersWithBtcKeys(oldFederationKeys); + + Federation oldFederation = new Federation( + oldFederationMembers, + Instant.ofEpochMilli(15005L), + 15L, + NetworkParameters.fromID(NetworkParameters.ID_REGTEST) + ); + + List newFederationKeys = Arrays.asList( + BtcECKey.fromPublicOnly(Hex.decode("0346cb6b905e4dee49a862eeb2288217d06afcd4ace4b5ca77ebedfbc6afc1c19d")), + BtcECKey.fromPublicOnly(Hex.decode("0269a0dbe7b8f84d1b399103c466fb20531a56b1ad3a7b44fe419e74aad8c46db7")), + BtcECKey.fromPublicOnly(Hex.decode("026192d8ab41bd402eb0431457f6756a3f3ce15c955c534d2b87f1e0372d8ba338")) + ); + + List newFederationMembers = FederationTestUtils.getFederationMembersWithBtcKeys(newFederationKeys); + + Federation newFederation = new Federation( + newFederationMembers, + Instant.ofEpochMilli(5005L), + 0L, + NetworkParameters.fromID(NetworkParameters.ID_REGTEST) + ); + + // Act + eventLogger.logCommitFederation(executionBlock, oldFederation, newFederation); + + commonAssertLogs(eventLogs); + + assertTopics(1, eventLogs); + // Assert log data + long federationActivationAge = CONSTANTS.getFederationActivationAge(activations); + byte[] oldFederationFlatPubKeys = flatKeysAsByteArray(oldFederation.getBtcPublicKeys()); + String oldFederationBtcAddress = oldFederation.getAddress().toBase58(); + byte[] newFederationFlatPubKeys = flatKeysAsByteArray(newFederation.getBtcPublicKeys()); + String newFederationBtcAddress = newFederation.getAddress().toBase58(); + long newFedActivationBlockNumber = executionBlock.getNumber() + federationActivationAge; + Object[] data = new Object[]{ + oldFederationFlatPubKeys, + oldFederationBtcAddress, + newFederationFlatPubKeys, + newFederationBtcAddress, + newFedActivationBlockNumber + }; + assertEvent(eventLogs, 0, BridgeEvents.COMMIT_FEDERATION.getEvent(), new Object[]{}, data); + Assertions.assertEquals(federationActivationAge, 10L); + } + + @Test + void logCommitFederation_fingerroot() { + ActivationConfig.ForBlock activations = ActivationConfigsForTest.fingerroot500().forBlock(0); + BridgeEventLogger eventLogger = new BridgeEventLoggerImpl(CONSTANTS, activations, eventLogs, signatureCache); + + // Setup parameters for test method call + Block executionBlock = mock(Block.class); + when(executionBlock.getTimestamp()).thenReturn(15005L); + when(executionBlock.getNumber()).thenReturn(15L); + + List oldFederationKeys = Arrays.asList( + BtcECKey.fromPublicOnly(Hex.decode("036bb9eab797eadc8b697f0e82a01d01cabbfaaca37e5bafc06fdc6fdd38af894a")), + BtcECKey.fromPublicOnly(Hex.decode("031da807c71c2f303b7f409dd2605b297ac494a563be3b9ca5f52d95a43d183cc5")), + BtcECKey.fromPublicOnly(Hex.decode("025eefeeeed5cdc40822880c7db1d0a88b7b986945ed3fc05a0b45fe166fe85e12")), + BtcECKey.fromPublicOnly(Hex.decode("03c67ad63527012fd4776ae892b5dc8c56f80f1be002dc65cd520a2efb64e37b49")) + ); + + List oldFederationMembers = FederationTestUtils.getFederationMembersWithBtcKeys(oldFederationKeys); + + Federation oldFederation = new Federation( + oldFederationMembers, + Instant.ofEpochMilli(15005L), + 15L, + NetworkParameters.fromID(NetworkParameters.ID_REGTEST) + ); + + List newFederationKeys = Arrays.asList( + BtcECKey.fromPublicOnly(Hex.decode("0346cb6b905e4dee49a862eeb2288217d06afcd4ace4b5ca77ebedfbc6afc1c19d")), + BtcECKey.fromPublicOnly(Hex.decode("0269a0dbe7b8f84d1b399103c466fb20531a56b1ad3a7b44fe419e74aad8c46db7")), + BtcECKey.fromPublicOnly(Hex.decode("026192d8ab41bd402eb0431457f6756a3f3ce15c955c534d2b87f1e0372d8ba338")) + ); + + List newFederationMembers = FederationTestUtils.getFederationMembersWithBtcKeys(newFederationKeys); + + Federation newFederation = new Federation( + newFederationMembers, + Instant.ofEpochMilli(5005L), + 0L, + NetworkParameters.fromID(NetworkParameters.ID_REGTEST) + ); + + // Act + eventLogger.logCommitFederation(executionBlock, oldFederation, newFederation); + + commonAssertLogs(eventLogs); + + assertTopics(1, eventLogs); + // Assert log data + long federationActivationAge = CONSTANTS.getFederationActivationAge(activations); + + byte[] oldFederationFlatPubKeys = flatKeysAsByteArray(oldFederation.getBtcPublicKeys()); + String oldFederationBtcAddress = oldFederation.getAddress().toBase58(); + byte[] newFederationFlatPubKeys = flatKeysAsByteArray(newFederation.getBtcPublicKeys()); + String newFederationBtcAddress = newFederation.getAddress().toBase58(); + long newFedActivationBlockNumber = executionBlock.getNumber() + federationActivationAge; + Object[] data = new Object[]{ + oldFederationFlatPubKeys, + oldFederationBtcAddress, + newFederationFlatPubKeys, + newFederationBtcAddress, + newFedActivationBlockNumber + }; + assertEvent(eventLogs, 0, BridgeEvents.COMMIT_FEDERATION.getEvent(), new Object[]{}, data); + Assertions.assertEquals(federationActivationAge, 20L); + } + @Test void logReleaseBtcRequested() { Keccak256 rskTxHash = PegTestUtils.createHash3(0); From 624d67b59dae0a9909b51e0fb8badb64e5fb5312 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Mon, 24 Apr 2023 15:46:14 -0400 Subject: [PATCH 11/24] Added tests --- .../BridgeEventLoggerLegacyImplTest.java | 60 +++++++++++++++++-- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java index e30d761e6e4..2bf27446e19 100644 --- a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java @@ -26,6 +26,7 @@ import co.rsk.peg.*; import org.bouncycastle.util.encoders.Hex; import org.ethereum.config.blockchain.upgrades.ActivationConfig; +import org.ethereum.config.blockchain.upgrades.ActivationConfigsForTest; import org.ethereum.config.blockchain.upgrades.ConsensusRule; import org.ethereum.core.*; import org.ethereum.util.RLP; @@ -45,6 +46,8 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -105,7 +108,7 @@ void testLogUpdateCollectionsBeforeRskip146() { void testLogUpdateCollectionsAfterRskip146() { when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); Transaction anyTx = any(); - Assertions.assertThrows(DeprecatedMethodCallException.class, () -> eventLogger.logUpdateCollections(anyTx)); + assertThrows(DeprecatedMethodCallException.class, () -> eventLogger.logUpdateCollections(anyTx)); } @Test @@ -147,7 +150,7 @@ void testLogAddSignatureAfterRskip146() { when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); BtcECKey federatorPublicKey = new BtcECKey(); byte[] bytes = rskTxHash.getBytes(); - Assertions.assertThrows(DeprecatedMethodCallException.class, () -> eventLogger.logAddSignature(federatorPublicKey, btcTxMock, bytes)); + assertThrows(DeprecatedMethodCallException.class, () -> eventLogger.logAddSignature(federatorPublicKey, btcTxMock, bytes)); } @Test @@ -185,7 +188,7 @@ void testLogReleaseBtcAfterRskip146() { // Act byte[] bytes = rskTxHash.getBytes(); - Assertions.assertThrows(DeprecatedMethodCallException.class, () -> eventLogger.logReleaseBtc(btcTxMock, bytes)); + assertThrows(DeprecatedMethodCallException.class, () -> eventLogger.logReleaseBtc(btcTxMock, bytes)); } @Test @@ -272,13 +275,62 @@ void testLogCommitFederationBeforeRskip146() { Assertions.assertEquals(15L + constantsMock.getFederationActivationAge(activations), Long.valueOf(new String(dataList.get(2).getRLPData(), StandardCharsets.UTF_8)).longValue()); } + @Test + void testLogCommitFederationAfterFingerroot() { + BridgeConstants constants = BridgeRegTestConstants.getInstance(); + ActivationConfig.ForBlock activations = ActivationConfigsForTest.fingerroot500().forBlock(0); + BridgeEventLogger eventLogger = new BrigeEventLoggerLegacyImpl(constants, activations, eventLogs, new BlockTxSignatureCache(new ReceivedTxSignatureCache())); + + // Setup parameters for test method call + Block executionBlock = mock(Block.class); + when(executionBlock.getTimestamp()).thenReturn(15005L); + when(executionBlock.getNumber()).thenReturn(15L); + + List oldFederationKeys = Arrays.asList( + BtcECKey.fromPublicOnly(Hex.decode("036bb9eab797eadc8b697f0e82a01d01cabbfaaca37e5bafc06fdc6fdd38af894a")), + BtcECKey.fromPublicOnly(Hex.decode("031da807c71c2f303b7f409dd2605b297ac494a563be3b9ca5f52d95a43d183cc5")), + BtcECKey.fromPublicOnly(Hex.decode("025eefeeeed5cdc40822880c7db1d0a88b7b986945ed3fc05a0b45fe166fe85e12")), + BtcECKey.fromPublicOnly(Hex.decode("03c67ad63527012fd4776ae892b5dc8c56f80f1be002dc65cd520a2efb64e37b49")) + ); + + List oldFederationMembers = FederationTestUtils.getFederationMembersWithBtcKeys(oldFederationKeys); + + Federation oldFederation = new Federation(oldFederationMembers, + Instant.ofEpochMilli(15005L), 15L, NetworkParameters.fromID(NetworkParameters.ID_REGTEST)); + + List newFederationKeys = Arrays.asList( + BtcECKey.fromPublicOnly(Hex.decode("0346cb6b905e4dee49a862eeb2288217d06afcd4ace4b5ca77ebedfbc6afc1c19d")), + BtcECKey.fromPublicOnly(Hex.decode("0269a0dbe7b8f84d1b399103c466fb20531a56b1ad3a7b44fe419e74aad8c46db7")), + BtcECKey.fromPublicOnly(Hex.decode("026192d8ab41bd402eb0431457f6756a3f3ce15c955c534d2b87f1e0372d8ba338")) + ); + + List newFederationMembers = FederationTestUtils.getFederationMembersWithBtcKeys(newFederationKeys); + + Federation newFederation = new Federation( + newFederationMembers, + Instant.ofEpochMilli(5005L), + 0L, + NetworkParameters.fromID(NetworkParameters.ID_REGTEST) + ); + + // Act + Exception exception = assertThrows(DeprecatedMethodCallException.class, () -> { + eventLogger.logCommitFederation(executionBlock, oldFederation, newFederation); + }); + + String expectedMessage = "Calling BrigeEventLoggerLegacyImpl.logCommitFederation method after RSKIP146 activation"; + String actualMessage = exception.getMessage(); + + assertTrue(actualMessage.contains(expectedMessage)); + } + @Test void testLogCommitFederationAfterRskip146() { // Setup event logger when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); // Act - Assertions.assertThrows(DeprecatedMethodCallException.class, () -> eventLogger.logCommitFederation(mock(Block.class), mock(Federation.class), mock(Federation.class))); + assertThrows(DeprecatedMethodCallException.class, () -> eventLogger.logCommitFederation(mock(Block.class), mock(Federation.class), mock(Federation.class))); } /********************************** From 4f567dbde678254257099320f2f2eb7334c67f99 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Mon, 24 Apr 2023 16:10:38 -0400 Subject: [PATCH 12/24] Fixed failing tests --- .../src/test/java/co/rsk/config/BridgeConstantsTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/config/BridgeConstantsTest.java b/rskj-core/src/test/java/co/rsk/config/BridgeConstantsTest.java index a6f15c12d60..df45c5aa58f 100644 --- a/rskj-core/src/test/java/co/rsk/config/BridgeConstantsTest.java +++ b/rskj-core/src/test/java/co/rsk/config/BridgeConstantsTest.java @@ -69,8 +69,8 @@ void test_getFundsMigrationAgeSinceActivationEnd_post_RSKIP374(BridgeConstants b private static Stream generatorFederationActivationAge() { return Stream.of( Arguments.of(BridgeMainNetConstants.getInstance(), false), - Arguments.of(BridgeTestNetConstants.getInstance(), true), - Arguments.of(BridgeRegTestConstants.getInstance(), true) + Arguments.of(BridgeTestNetConstants.getInstance(), false), + Arguments.of(BridgeRegTestConstants.getInstance(), false) ); } From 0c158b8f32ed7e01b254037d08589ef9f343f246 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Tue, 25 Apr 2023 14:38:20 -0400 Subject: [PATCH 13/24] - Added new assertions to BridgeEventLoggerImplTest tests - Removed wrong tests --- .../peg/utils/BridgeEventLoggerImplTest.java | 102 +++++++----------- .../BridgeEventLoggerLegacyImplTest.java | 49 --------- 2 files changed, 39 insertions(+), 112 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java index 952a6019271..88f9a4d983d 100644 --- a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java @@ -45,6 +45,7 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -203,6 +204,7 @@ void logReleaseBtc() { @Test void logCommitFederation() { + when(activations.isActive(ConsensusRule.RSKIP383)).thenReturn(false); // Setup parameters for test method call Block executionBlock = mock(Block.class); when(executionBlock.getTimestamp()).thenReturn(15005L); @@ -251,6 +253,7 @@ void logCommitFederation() { byte[] newFederationFlatPubKeys = flatKeysAsByteArray(newFederation.getBtcPublicKeys()); String newFederationBtcAddress = newFederation.getAddress().toBase58(); long newFedActivationBlockNumber = executionBlock.getNumber() + CONSTANTS.getFederationActivationAge(activations); + Object[] data = new Object[]{ oldFederationFlatPubKeys, oldFederationBtcAddress, @@ -258,75 +261,31 @@ void logCommitFederation() { newFederationBtcAddress, newFedActivationBlockNumber }; - assertEvent(eventLogs, 0, BridgeEvents.COMMIT_FEDERATION.getEvent(), new Object[]{}, data); - } - - @Test - void logCommitFederation_hop() { - ActivationConfig.ForBlock activations = ActivationConfigsForTest.hop400().forBlock(0); - // Setup parameters for test method call - Block executionBlock = mock(Block.class); - when(executionBlock.getTimestamp()).thenReturn(15005L); - when(executionBlock.getNumber()).thenReturn(15L); - - List oldFederationKeys = Arrays.asList( - BtcECKey.fromPublicOnly(Hex.decode("036bb9eab797eadc8b697f0e82a01d01cabbfaaca37e5bafc06fdc6fdd38af894a")), - BtcECKey.fromPublicOnly(Hex.decode("031da807c71c2f303b7f409dd2605b297ac494a563be3b9ca5f52d95a43d183cc5")), - BtcECKey.fromPublicOnly(Hex.decode("025eefeeeed5cdc40822880c7db1d0a88b7b986945ed3fc05a0b45fe166fe85e12")), - BtcECKey.fromPublicOnly(Hex.decode("03c67ad63527012fd4776ae892b5dc8c56f80f1be002dc65cd520a2efb64e37b49")) - ); - - List oldFederationMembers = FederationTestUtils.getFederationMembersWithBtcKeys(oldFederationKeys); - - Federation oldFederation = new Federation( - oldFederationMembers, - Instant.ofEpochMilli(15005L), - 15L, - NetworkParameters.fromID(NetworkParameters.ID_REGTEST) - ); - - List newFederationKeys = Arrays.asList( - BtcECKey.fromPublicOnly(Hex.decode("0346cb6b905e4dee49a862eeb2288217d06afcd4ace4b5ca77ebedfbc6afc1c19d")), - BtcECKey.fromPublicOnly(Hex.decode("0269a0dbe7b8f84d1b399103c466fb20531a56b1ad3a7b44fe419e74aad8c46db7")), - BtcECKey.fromPublicOnly(Hex.decode("026192d8ab41bd402eb0431457f6756a3f3ce15c955c534d2b87f1e0372d8ba338")) - ); - List newFederationMembers = FederationTestUtils.getFederationMembersWithBtcKeys(newFederationKeys); + CallTransaction.Function event = BridgeEvents.COMMIT_FEDERATION.getEvent(); + Object[] topics = {}; + assertEvent(eventLogs, 0, event, topics, data); - Federation newFederation = new Federation( - newFederationMembers, - Instant.ofEpochMilli(5005L), - 0L, - NetworkParameters.fromID(NetworkParameters.ID_REGTEST) - ); + final LogInfo log = eventLogs.get(0); + Object[] decodeEventData = event.decodeEventData(log.getData()); + long loggedFedActivationBlockNumber = ((BigInteger) decodeEventData[4]).longValue(); - // Act - eventLogger.logCommitFederation(executionBlock, oldFederation, newFederation); + // assert fed activation has different values before and after RSKIP383 respectively + ActivationConfig.ForBlock activationsAfterRSKIP383 = mock(ActivationConfig.ForBlock.class); + when(activationsAfterRSKIP383.isActive(ConsensusRule.RSKIP383)).thenReturn(true); - commonAssertLogs(eventLogs); + long hopNewFedActivationBlockNumber = executionBlock.getNumber() + CONSTANTS.getFederationActivationAge(activationsAfterRSKIP383); + assertNotEquals(loggedFedActivationBlockNumber, hopNewFedActivationBlockNumber); + assertNotEquals(newFedActivationBlockNumber, hopNewFedActivationBlockNumber); - assertTopics(1, eventLogs); - // Assert log data - long federationActivationAge = CONSTANTS.getFederationActivationAge(activations); - byte[] oldFederationFlatPubKeys = flatKeysAsByteArray(oldFederation.getBtcPublicKeys()); - String oldFederationBtcAddress = oldFederation.getAddress().toBase58(); - byte[] newFederationFlatPubKeys = flatKeysAsByteArray(newFederation.getBtcPublicKeys()); - String newFederationBtcAddress = newFederation.getAddress().toBase58(); - long newFedActivationBlockNumber = executionBlock.getNumber() + federationActivationAge; - Object[] data = new Object[]{ - oldFederationFlatPubKeys, - oldFederationBtcAddress, - newFederationFlatPubKeys, - newFederationBtcAddress, - newFedActivationBlockNumber - }; - assertEvent(eventLogs, 0, BridgeEvents.COMMIT_FEDERATION.getEvent(), new Object[]{}, data); - Assertions.assertEquals(federationActivationAge, 10L); + // assert fed activation value for RSKIP383 is being properly logged + assertEquals(loggedFedActivationBlockNumber, newFedActivationBlockNumber); } @Test - void logCommitFederation_fingerroot() { - ActivationConfig.ForBlock activations = ActivationConfigsForTest.fingerroot500().forBlock(0); + void logCommitFederation_after_RSKIP383() { + ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); + when(activations.isActive(ConsensusRule.RSKIP383)).thenReturn(true); BridgeEventLogger eventLogger = new BridgeEventLoggerImpl(CONSTANTS, activations, eventLogs, signatureCache); // Setup parameters for test method call @@ -379,6 +338,7 @@ void logCommitFederation_fingerroot() { byte[] newFederationFlatPubKeys = flatKeysAsByteArray(newFederation.getBtcPublicKeys()); String newFederationBtcAddress = newFederation.getAddress().toBase58(); long newFedActivationBlockNumber = executionBlock.getNumber() + federationActivationAge; + Object[] data = new Object[]{ oldFederationFlatPubKeys, oldFederationBtcAddress, @@ -386,8 +346,24 @@ void logCommitFederation_fingerroot() { newFederationBtcAddress, newFedActivationBlockNumber }; - assertEvent(eventLogs, 0, BridgeEvents.COMMIT_FEDERATION.getEvent(), new Object[]{}, data); - Assertions.assertEquals(federationActivationAge, 20L); + CallTransaction.Function event = BridgeEvents.COMMIT_FEDERATION.getEvent(); + Object[] topics = {}; + assertEvent(eventLogs, 0, event, topics, data); + + final LogInfo log = eventLogs.get(0); + Object[] decodeEventData = event.decodeEventData(log.getData()); + long loggedFedActivationBlockNumber = ((BigInteger) decodeEventData[4]).longValue(); + + // assert fed activation has different values before and after fingerroot respectively + ActivationConfig.ForBlock activationsBeforeRSKIP383 = mock(ActivationConfig.ForBlock.class); + when(activationsBeforeRSKIP383.isActive(ConsensusRule.RSKIP383)).thenReturn(false); + + long hopNewFedActivationBlockNumber = executionBlock.getNumber() + CONSTANTS.getFederationActivationAge(activationsBeforeRSKIP383); + assertNotEquals(loggedFedActivationBlockNumber, hopNewFedActivationBlockNumber); + assertNotEquals(newFedActivationBlockNumber, hopNewFedActivationBlockNumber); + + // assert fed activation value for fingerroot is being properly logged + assertEquals(loggedFedActivationBlockNumber, newFedActivationBlockNumber); } @Test diff --git a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java index 2bf27446e19..2aa6c6a8786 100644 --- a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java @@ -275,55 +275,6 @@ void testLogCommitFederationBeforeRskip146() { Assertions.assertEquals(15L + constantsMock.getFederationActivationAge(activations), Long.valueOf(new String(dataList.get(2).getRLPData(), StandardCharsets.UTF_8)).longValue()); } - @Test - void testLogCommitFederationAfterFingerroot() { - BridgeConstants constants = BridgeRegTestConstants.getInstance(); - ActivationConfig.ForBlock activations = ActivationConfigsForTest.fingerroot500().forBlock(0); - BridgeEventLogger eventLogger = new BrigeEventLoggerLegacyImpl(constants, activations, eventLogs, new BlockTxSignatureCache(new ReceivedTxSignatureCache())); - - // Setup parameters for test method call - Block executionBlock = mock(Block.class); - when(executionBlock.getTimestamp()).thenReturn(15005L); - when(executionBlock.getNumber()).thenReturn(15L); - - List oldFederationKeys = Arrays.asList( - BtcECKey.fromPublicOnly(Hex.decode("036bb9eab797eadc8b697f0e82a01d01cabbfaaca37e5bafc06fdc6fdd38af894a")), - BtcECKey.fromPublicOnly(Hex.decode("031da807c71c2f303b7f409dd2605b297ac494a563be3b9ca5f52d95a43d183cc5")), - BtcECKey.fromPublicOnly(Hex.decode("025eefeeeed5cdc40822880c7db1d0a88b7b986945ed3fc05a0b45fe166fe85e12")), - BtcECKey.fromPublicOnly(Hex.decode("03c67ad63527012fd4776ae892b5dc8c56f80f1be002dc65cd520a2efb64e37b49")) - ); - - List oldFederationMembers = FederationTestUtils.getFederationMembersWithBtcKeys(oldFederationKeys); - - Federation oldFederation = new Federation(oldFederationMembers, - Instant.ofEpochMilli(15005L), 15L, NetworkParameters.fromID(NetworkParameters.ID_REGTEST)); - - List newFederationKeys = Arrays.asList( - BtcECKey.fromPublicOnly(Hex.decode("0346cb6b905e4dee49a862eeb2288217d06afcd4ace4b5ca77ebedfbc6afc1c19d")), - BtcECKey.fromPublicOnly(Hex.decode("0269a0dbe7b8f84d1b399103c466fb20531a56b1ad3a7b44fe419e74aad8c46db7")), - BtcECKey.fromPublicOnly(Hex.decode("026192d8ab41bd402eb0431457f6756a3f3ce15c955c534d2b87f1e0372d8ba338")) - ); - - List newFederationMembers = FederationTestUtils.getFederationMembersWithBtcKeys(newFederationKeys); - - Federation newFederation = new Federation( - newFederationMembers, - Instant.ofEpochMilli(5005L), - 0L, - NetworkParameters.fromID(NetworkParameters.ID_REGTEST) - ); - - // Act - Exception exception = assertThrows(DeprecatedMethodCallException.class, () -> { - eventLogger.logCommitFederation(executionBlock, oldFederation, newFederation); - }); - - String expectedMessage = "Calling BrigeEventLoggerLegacyImpl.logCommitFederation method after RSKIP146 activation"; - String actualMessage = exception.getMessage(); - - assertTrue(actualMessage.contains(expectedMessage)); - } - @Test void testLogCommitFederationAfterRskip146() { // Setup event logger From b5e74a26bf1e39d34cdeece2f7e3f0a64d549076 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Tue, 25 Apr 2023 15:02:12 -0400 Subject: [PATCH 14/24] - Added new assertions to PowpegMigrationTest tests --- .../java/co/rsk/peg/PowpegMigrationTest.java | 44 ++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java b/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java index af1dfed3517..5ab737f07b3 100644 --- a/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java @@ -30,6 +30,7 @@ import org.ethereum.util.ByteUtil; import org.ethereum.vm.PrecompiledContracts; import org.ethereum.vm.program.InternalTransaction; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.mockito.ArgumentCaptor; @@ -275,13 +276,54 @@ private void testChangePowpeg( /* Activation phase */ - // Move the required blocks ahead for the new powpeg to become active // (overriding block number to ensure we don't move beyond the activation phase) blockNumber = initialBlock.getNumber() + bridgeConstants.getFederationActivationAge(activations); Block activationBlock = mock(Block.class); doReturn(blockNumber).when(activationBlock).getNumber(); + // assuming fed activation age after rskip383 is greater than legacy fed activation age, we can check that new fed + // should not be active at the legacy activation age when RSKIP383 is active + + if (activations.isActive(ConsensusRule.RSKIP383)){ + ActivationConfig.ForBlock activationsBeforeRSKIP383 = mock(ActivationConfig.ForBlock.class); + when(activationsBeforeRSKIP383.isActive(ConsensusRule.RSKIP383)).thenReturn(false); + + long beforeRskip383blockNumber = initialBlock.getNumber() + bridgeConstants.getFederationActivationAge(activationsBeforeRSKIP383); + Assertions.assertTrue(blockNumber > beforeRskip383blockNumber); + + Block beforeRskip383ActivationBlock = mock(Block.class); + doReturn(beforeRskip383blockNumber).when(beforeRskip383ActivationBlock).getNumber(); + + bridgeSupport = new BridgeSupportBuilder() + .withProvider(bridgeStorageProvider) + .withRepository(repository) + .withEventLogger(bridgeEventLogger) + .withExecutionBlock(beforeRskip383ActivationBlock) + .withActivations(activations) + .withBridgeConstants(bridgeConstants) + .withBtcBlockStoreFactory(btcBlockStoreFactory) + .withPeginInstructionsProvider(new PeginInstructionsProvider()) + .build(); + + // New active powpeg and retiring powpeg + assertNotEquals(newPowPegAddress, bridgeSupport.getFederationAddress()); + assertEquals(oldPowPegAddress, bridgeSupport.getFederationAddress()); + assertNotEquals(oldPowPegAddress, bridgeSupport.getRetiringFederationAddress()); + } else { + ActivationConfig.ForBlock activationsAfterRSKIP383 = mock(ActivationConfig.ForBlock.class); + when(activationsAfterRSKIP383.isActive(ConsensusRule.RSKIP383)).thenReturn(true); + + long afterRskip383blockNumber = initialBlock.getNumber() + bridgeConstants.getFederationActivationAge(activationsAfterRSKIP383); + + // assert fed activation age is different before and after RSKIP383 + assertNotEquals(blockNumber, afterRskip383blockNumber); + Assertions.assertTrue(blockNumber < afterRskip383blockNumber); + } + + + + bridgeSupport = new BridgeSupportBuilder() .withProvider(bridgeStorageProvider) .withRepository(repository) From 9ef0e1bf571ae3a0812f9463fa3e689738ad556c Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Thu, 27 Apr 2023 11:25:49 -0400 Subject: [PATCH 15/24] - Refactored tests. - Renamed method. - Removed unused imports. --- .../co/rsk/config/BridgeConstantsTest.java | 31 ++++------ ...ridgeSupportProcessFundsMigrationTest.java | 10 ++-- .../co/rsk/peg/FederationSupportTest.java | 57 +++++++++---------- .../java/co/rsk/peg/PowpegMigrationTest.java | 1 - 4 files changed, 43 insertions(+), 56 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/config/BridgeConstantsTest.java b/rskj-core/src/test/java/co/rsk/config/BridgeConstantsTest.java index df45c5aa58f..60b345e4d81 100644 --- a/rskj-core/src/test/java/co/rsk/config/BridgeConstantsTest.java +++ b/rskj-core/src/test/java/co/rsk/config/BridgeConstantsTest.java @@ -70,36 +70,27 @@ private static Stream generatorFederationActivationAge() { return Stream.of( Arguments.of(BridgeMainNetConstants.getInstance(), false), Arguments.of(BridgeTestNetConstants.getInstance(), false), - Arguments.of(BridgeRegTestConstants.getInstance(), false) + Arguments.of(BridgeRegTestConstants.getInstance(), false), + Arguments.of(BridgeMainNetConstants.getInstance(), true), + Arguments.of(BridgeTestNetConstants.getInstance(), true), + Arguments.of(BridgeRegTestConstants.getInstance(), true) ); } @ParameterizedTest() @MethodSource("generatorFederationActivationAge") - void test_getFederationActivationAge(BridgeConstants bridgeConstants, boolean fieldsHasSameValue) { - // Arrange - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - - // Act - long federationActivationAge = bridgeConstants.getFederationActivationAge(activations); - - // assert - assertEquals(federationActivationAge, bridgeConstants.federationActivationAgeLegacy); - assertEquals(fieldsHasSameValue, federationActivationAge == bridgeConstants.federationActivationAge); - } - - @ParameterizedTest() - @MethodSource("generatorFederationActivationAge") - void test_getFederationActivationAge_post_RSKIP383(BridgeConstants bridgeConstants, boolean fieldsHasSameValue) { + void test_getFederationActivationAge(BridgeConstants bridgeConstants, boolean isRSKIP383Active) { // Arrange ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP383)).thenReturn(true); - + when(activations.isActive(ConsensusRule.RSKIP383)).thenReturn(isRSKIP383Active); // Act long federationActivationAge = bridgeConstants.getFederationActivationAge(activations); // assert - assertEquals(federationActivationAge, bridgeConstants.federationActivationAge); - assertEquals(fieldsHasSameValue, federationActivationAge == bridgeConstants.federationActivationAgeLegacy); + if (isRSKIP383Active){ + assertEquals(bridgeConstants.federationActivationAge, federationActivationAge); + } else { + assertEquals(bridgeConstants.federationActivationAgeLegacy, federationActivationAge); + } } } diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java index 944e0df4d1a..8553c82e2a1 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java @@ -196,11 +196,11 @@ private void test_processFundsMigration( Federation oldFederation = bridgeConstants.getGenesisFederation(); long federationActivationAge = bridgeConstants.getFederationActivationAge(activations); - long expectedFederationActivationAge = activations.isActive(ConsensusRule.RSKIP383)? 120L: 60L; - if (bridgeConstants instanceof BridgeMainNetConstants){ - expectedFederationActivationAge = activations.isActive(ConsensusRule.RSKIP383)? 40320L: 18500L; - } - Assertions.assertEquals(expectedFederationActivationAge, federationActivationAge); +// long expectedFederationActivationAge = activations.isActive(ConsensusRule.RSKIP383)? 120L: 60L; +// if (bridgeConstants instanceof BridgeMainNetConstants){ +// expectedFederationActivationAge = activations.isActive(ConsensusRule.RSKIP383)? 40320L: 18500L; +// } +// Assertions.assertEquals(expectedFederationActivationAge, federationActivationAge); long federationCreationBlockNumber = 5L; long federationInMigrationAgeHeight = federationCreationBlockNumber + federationActivationAge + diff --git a/rskj-core/src/test/java/co/rsk/peg/FederationSupportTest.java b/rskj-core/src/test/java/co/rsk/peg/FederationSupportTest.java index 6a5d2b4c291..ba8415cca7a 100644 --- a/rskj-core/src/test/java/co/rsk/peg/FederationSupportTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/FederationSupportTest.java @@ -21,7 +21,6 @@ import co.rsk.bitcoinj.core.NetworkParameters; import co.rsk.config.BridgeConstants; import co.rsk.config.BridgeMainNetConstants; -import co.rsk.config.BridgeRegTestConstants; import co.rsk.config.BridgeTestNetConstants; import org.bouncycastle.util.encoders.Hex; import org.ethereum.config.blockchain.upgrades.ActivationConfig; @@ -91,42 +90,42 @@ void whenOldFederationIsNullThenActiveFederationIsNewFederation() { assertThat(federationSupport.getActiveFederation(), is(newFederation)); } - @Test - void whenOldAndNewFederationArePresentReturnOldFederationByActivationAge() { - Federation newFederation = getNewFakeFederation(75); - Federation oldFederation = getNewFakeFederation(0); - - when(provider.getNewFederation()).thenReturn(newFederation); - when(provider.getOldFederation()).thenReturn(oldFederation); - when(executionBlock.getNumber()).thenReturn(80L); - when(bridgeConstants.getFederationActivationAge(activations)).thenReturn(10L); - - assertThat(federationSupport.getActiveFederation(), is(oldFederation)); - } - - private static Stream provideTestArguments() { - BridgeConstants bridgeConstants = BridgeTestNetConstants.getInstance(); + private static Stream fedActivationArguments() { + BridgeConstants bridgeTestNetConstants = BridgeTestNetConstants.getInstance(); + BridgeConstants bridgeMainnetConstants = BridgeMainNetConstants.getInstance(); ActivationConfig.ForBlock hop = ActivationConfigsForTest.hop400().forBlock(0); ActivationConfig.ForBlock fingerroot = ActivationConfigsForTest.fingerroot500().forBlock(0); return Stream.of( - Arguments.of(hop, 65, 0, 80L, false), - Arguments.of(hop, 65, 0, 65+bridgeConstants.getFederationActivationAge(hop), true), - Arguments.of(fingerroot, 65, 0, 80L, false), - Arguments.of(fingerroot, 65, 0, 65+bridgeConstants.getFederationActivationAge(fingerroot), true) + Arguments.of(bridgeTestNetConstants, hop, false), + Arguments.of(bridgeTestNetConstants, hop, true), + Arguments.of(bridgeTestNetConstants, fingerroot, false), + Arguments.of(bridgeTestNetConstants, fingerroot, true), + Arguments.of(bridgeMainnetConstants, hop, false), + Arguments.of(bridgeMainnetConstants, hop, true), + Arguments.of(bridgeMainnetConstants, fingerroot, false), + Arguments.of(bridgeMainnetConstants, fingerroot, true) ); } @ParameterizedTest - @MethodSource("provideTestArguments") - void whenOldAndNewFederationArePresentReturnNewFederationByActivationAge( + @MethodSource("fedActivationArguments") + void whenOldAndNewFederationArePresentReturnActiveFederationByActivationAge( + BridgeConstants bridgeConstants, ActivationConfig.ForBlock activations, - int newFedCreationBlockNumber, - int oldFedCreationBlockNumber, - long currentBlockNumber, - boolean newFedExpectedToBeActive) - { + boolean newFedExpectedToBeActive + ) { + int newFedCreationBlockNumber = 65; + int oldFedCreationBlockNumber = 0; + long currentBlockNumber = newFedCreationBlockNumber + bridgeConstants.getFederationActivationAge(activations); + + if (newFedExpectedToBeActive) { + currentBlockNumber++; + } else { + currentBlockNumber--; + } + // Arrange Federation newFederation = getNewFakeFederation(newFedCreationBlockNumber); Federation oldFederation = getNewFakeFederation(oldFedCreationBlockNumber); @@ -138,8 +137,6 @@ void whenOldAndNewFederationArePresentReturnNewFederationByActivationAge( Block executionBlock = mock(Block.class); when(executionBlock.getNumber()).thenReturn(currentBlockNumber); - BridgeConstants bridgeConstants = BridgeTestNetConstants.getInstance(); - FederationSupport federationSupport = new FederationSupport( bridgeConstants, provider, @@ -154,7 +151,7 @@ void whenOldAndNewFederationArePresentReturnNewFederationByActivationAge( if (newFedExpectedToBeActive){ assertThat(activeFederation, is(newFederation)); } else { - assertThat(federationSupport.getActiveFederation(), is(oldFederation)); + assertThat(activeFederation, is(oldFederation)); } } diff --git a/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java b/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java index 5ab737f07b3..4ff848b0a3f 100644 --- a/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java @@ -32,7 +32,6 @@ import org.ethereum.vm.program.InternalTransaction; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; import org.mockito.ArgumentCaptor; import java.io.IOException; From 3df64e2b6d40036c59b8aaaa5e19a90e1b4bb5de Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Thu, 27 Apr 2023 13:28:49 -0400 Subject: [PATCH 16/24] - Refactored tests. - Removed unused imports. --- ...ridgeSupportProcessFundsMigrationTest.java | 33 +++++++++++------ .../java/co/rsk/peg/PowpegMigrationTest.java | 1 - .../peg/utils/BridgeEventLoggerImplTest.java | 37 ++++++++++++------- .../BridgeEventLoggerLegacyImplTest.java | 2 - 4 files changed, 46 insertions(+), 27 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java index 8553c82e2a1..c15f3968284 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java @@ -12,7 +12,6 @@ import org.bouncycastle.util.encoders.Hex; import org.ethereum.config.Constants; import org.ethereum.config.blockchain.upgrades.ActivationConfig; -import org.ethereum.config.blockchain.upgrades.ActivationConfigsForTest; import org.ethereum.config.blockchain.upgrades.ConsensusRule; import org.ethereum.core.Transaction; import org.junit.jupiter.api.Assertions; @@ -102,6 +101,16 @@ void processFundsMigration_past_migration_age_after_rskip_374_activation_testnet test_processFundsMigration(BridgeTestNetConstants.getInstance(), activations, false); } + @Test + void processFundsMigration_past_migration_age_after_rskip383_activation_testnet() throws IOException { + ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); + when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); + when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(true); + when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(true); + when(activations.isActive(ConsensusRule.RSKIP383)).thenReturn(true); + test_processFundsMigration(BridgeTestNetConstants.getInstance(), activations, false); + } + @Test void processFundsMigration_in_migration_age_before_rskip_146_activation_mainnet() throws IOException { ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); @@ -139,8 +148,12 @@ void processFundsMigration_in_migration_age_after_rskip_374_activation_mainnet() } @Test - void processFundsMigration_in_migration_age_after_fingerroot_activation_mainnet() throws IOException { - ActivationConfig.ForBlock activations = ActivationConfigsForTest.fingerroot500().forBlock(0); + void processFundsMigration_in_migration_age_after_rskip383_activation_mainnet() throws IOException { + ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); + when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); + when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(true); + when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(true); + when(activations.isActive(ConsensusRule.RSKIP383)).thenReturn(true); test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, true); } @@ -181,8 +194,12 @@ void processFundsMigration_past_migration_age_after_rskip_374_activation_mainnet } @Test - void processFundsMigration_past_migration_age_after_fingerroot_activation_mainnet() throws IOException { - ActivationConfig.ForBlock activations = ActivationConfigsForTest.fingerroot500().forBlock(0); + void processFundsMigration_past_migration_age_after_rskip383_activation_mainnet() throws IOException { + ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); + when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); + when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(true); + when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(true); + when(activations.isActive(ConsensusRule.RSKIP383)).thenReturn(true); test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, false); } @@ -196,12 +213,6 @@ private void test_processFundsMigration( Federation oldFederation = bridgeConstants.getGenesisFederation(); long federationActivationAge = bridgeConstants.getFederationActivationAge(activations); -// long expectedFederationActivationAge = activations.isActive(ConsensusRule.RSKIP383)? 120L: 60L; -// if (bridgeConstants instanceof BridgeMainNetConstants){ -// expectedFederationActivationAge = activations.isActive(ConsensusRule.RSKIP383)? 40320L: 18500L; -// } -// Assertions.assertEquals(expectedFederationActivationAge, federationActivationAge); - long federationCreationBlockNumber = 5L; long federationInMigrationAgeHeight = federationCreationBlockNumber + federationActivationAge + bridgeConstants.getFundsMigrationAgeSinceActivationBegin() + 1; diff --git a/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java b/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java index 4ff848b0a3f..97286a68ab7 100644 --- a/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java @@ -283,7 +283,6 @@ private void testChangePowpeg( // assuming fed activation age after rskip383 is greater than legacy fed activation age, we can check that new fed // should not be active at the legacy activation age when RSKIP383 is active - if (activations.isActive(ConsensusRule.RSKIP383)){ ActivationConfig.ForBlock activationsBeforeRSKIP383 = mock(ActivationConfig.ForBlock.class); when(activationsBeforeRSKIP383.isActive(ConsensusRule.RSKIP383)).thenReturn(false); diff --git a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java index 88f9a4d983d..a49f6ff058b 100644 --- a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java @@ -25,7 +25,6 @@ import co.rsk.peg.*; import org.bouncycastle.util.encoders.Hex; import org.ethereum.config.blockchain.upgrades.ActivationConfig; -import org.ethereum.config.blockchain.upgrades.ActivationConfigsForTest; import org.ethereum.config.blockchain.upgrades.ConsensusRule; import org.ethereum.core.*; import org.ethereum.crypto.ECKey; @@ -35,6 +34,8 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import java.math.BigInteger; import java.time.Instant; @@ -202,9 +203,10 @@ void logReleaseBtc() { assertEvent(eventLogs, 0, BridgeEvents.RELEASE_BTC.getEvent(), new Object[]{rskTxHash.getBytes()}, new Object[]{btcTx.bitcoinSerialize()}); } - @Test - void logCommitFederation() { - when(activations.isActive(ConsensusRule.RSKIP383)).thenReturn(false); + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void logCommitFederation(boolean isRSKIP383Active) { + when(activations.isActive(ConsensusRule.RSKIP383)).thenReturn(isRSKIP383Active); // Setup parameters for test method call Block executionBlock = mock(Block.class); when(executionBlock.getTimestamp()).thenReturn(15005L); @@ -270,16 +272,25 @@ void logCommitFederation() { Object[] decodeEventData = event.decodeEventData(log.getData()); long loggedFedActivationBlockNumber = ((BigInteger) decodeEventData[4]).longValue(); - // assert fed activation has different values before and after RSKIP383 respectively - ActivationConfig.ForBlock activationsAfterRSKIP383 = mock(ActivationConfig.ForBlock.class); - when(activationsAfterRSKIP383.isActive(ConsensusRule.RSKIP383)).thenReturn(true); - - long hopNewFedActivationBlockNumber = executionBlock.getNumber() + CONSTANTS.getFederationActivationAge(activationsAfterRSKIP383); - assertNotEquals(loggedFedActivationBlockNumber, hopNewFedActivationBlockNumber); - assertNotEquals(newFedActivationBlockNumber, hopNewFedActivationBlockNumber); - - // assert fed activation value for RSKIP383 is being properly logged assertEquals(loggedFedActivationBlockNumber, newFedActivationBlockNumber); + + // assert fed activation has different values before and after RSKIP383 respectively + if (isRSKIP383Active){ + ActivationConfig.ForBlock activationsBeforeRSKIP383 = mock(ActivationConfig.ForBlock.class); + when(activationsBeforeRSKIP383.isActive(ConsensusRule.RSKIP383)).thenReturn(false); + + long beforeRSKIP383fedActivationBlockNumber = executionBlock.getNumber() + CONSTANTS.getFederationActivationAge(activationsBeforeRSKIP383); + assertNotEquals(loggedFedActivationBlockNumber, beforeRSKIP383fedActivationBlockNumber); + assertNotEquals(newFedActivationBlockNumber, beforeRSKIP383fedActivationBlockNumber); + } else { + // assert fed activation has different values before and after RSKIP383 respectively + ActivationConfig.ForBlock activationsAfterRSKIP383 = mock(ActivationConfig.ForBlock.class); + when(activationsAfterRSKIP383.isActive(ConsensusRule.RSKIP383)).thenReturn(true); + + long afterRSKIP383FedActivationBlockNumber = executionBlock.getNumber() + CONSTANTS.getFederationActivationAge(activationsAfterRSKIP383); + assertNotEquals(loggedFedActivationBlockNumber, afterRSKIP383FedActivationBlockNumber); + assertNotEquals(newFedActivationBlockNumber, afterRSKIP383FedActivationBlockNumber); + } } @Test diff --git a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java index 2aa6c6a8786..ea3473dfe26 100644 --- a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerLegacyImplTest.java @@ -26,7 +26,6 @@ import co.rsk.peg.*; import org.bouncycastle.util.encoders.Hex; import org.ethereum.config.blockchain.upgrades.ActivationConfig; -import org.ethereum.config.blockchain.upgrades.ActivationConfigsForTest; import org.ethereum.config.blockchain.upgrades.ConsensusRule; import org.ethereum.core.*; import org.ethereum.util.RLP; @@ -47,7 +46,6 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; From 1f130ee7906be2372b1a913847d10b2a9b15e68f Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Thu, 27 Apr 2023 15:30:44 -0400 Subject: [PATCH 17/24] - Refactored test --- .../java/co/rsk/peg/PowpegMigrationTest.java | 26 +++++-------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java b/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java index 97286a68ab7..dc54e99b84d 100644 --- a/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java @@ -287,41 +287,27 @@ private void testChangePowpeg( ActivationConfig.ForBlock activationsBeforeRSKIP383 = mock(ActivationConfig.ForBlock.class); when(activationsBeforeRSKIP383.isActive(ConsensusRule.RSKIP383)).thenReturn(false); - long beforeRskip383blockNumber = initialBlock.getNumber() + bridgeConstants.getFederationActivationAge(activationsBeforeRSKIP383); - Assertions.assertTrue(blockNumber > beforeRskip383blockNumber); + long fedActivationBlockNumber = initialBlock.getNumber() + bridgeConstants.getFederationActivationAge(activationsBeforeRSKIP383); + Assertions.assertTrue(blockNumber > fedActivationBlockNumber); - Block beforeRskip383ActivationBlock = mock(Block.class); - doReturn(beforeRskip383blockNumber).when(beforeRskip383ActivationBlock).getNumber(); + Block fedActivationBlock = mock(Block.class); + doReturn(fedActivationBlockNumber).when(fedActivationBlock).getNumber(); bridgeSupport = new BridgeSupportBuilder() .withProvider(bridgeStorageProvider) .withRepository(repository) .withEventLogger(bridgeEventLogger) - .withExecutionBlock(beforeRskip383ActivationBlock) + .withExecutionBlock(fedActivationBlock) .withActivations(activations) .withBridgeConstants(bridgeConstants) .withBtcBlockStoreFactory(btcBlockStoreFactory) .withPeginInstructionsProvider(new PeginInstructionsProvider()) .build(); - // New active powpeg and retiring powpeg - assertNotEquals(newPowPegAddress, bridgeSupport.getFederationAddress()); + assertEquals(newPowPegAddress, bridgeSupport.getRetiringFederationAddress()); assertEquals(oldPowPegAddress, bridgeSupport.getFederationAddress()); - assertNotEquals(oldPowPegAddress, bridgeSupport.getRetiringFederationAddress()); - } else { - ActivationConfig.ForBlock activationsAfterRSKIP383 = mock(ActivationConfig.ForBlock.class); - when(activationsAfterRSKIP383.isActive(ConsensusRule.RSKIP383)).thenReturn(true); - - long afterRskip383blockNumber = initialBlock.getNumber() + bridgeConstants.getFederationActivationAge(activationsAfterRSKIP383); - - // assert fed activation age is different before and after RSKIP383 - assertNotEquals(blockNumber, afterRskip383blockNumber); - Assertions.assertTrue(blockNumber < afterRskip383blockNumber); } - - - bridgeSupport = new BridgeSupportBuilder() .withProvider(bridgeStorageProvider) .withRepository(repository) From f7f346b4c82772062d710f853d0ad5dfe3c33a25 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Fri, 28 Apr 2023 11:51:00 -0400 Subject: [PATCH 18/24] Added new assertions --- .../BridgeSupportProcessFundsMigrationTest.java | 17 +++++++++++++++++ .../java/co/rsk/peg/PowpegMigrationTest.java | 12 ++++++------ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java index c15f3968284..bd3c5ac61cc 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java @@ -65,6 +65,16 @@ void processFundsMigration_in_migration_age_after_rskip_374_activation_testnet() test_processFundsMigration(BridgeTestNetConstants.getInstance(), activations, true); } + @Test + void processFundsMigration_in_migration_age_after_rskip383_activation_testnet() throws IOException { + ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); + when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); + when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(true); + when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(true); + when(activations.isActive(ConsensusRule.RSKIP383)).thenReturn(true); + test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, true); + } + @Test void processFundsMigration_past_migration_age_before_rskip_146_activation_testnet() throws IOException { ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); @@ -261,6 +271,13 @@ private void test_processFundsMigration( Assertions.assertEquals(activations.isActive(ConsensusRule.RSKIP146)? 0 : 1, provider.getPegoutsWaitingForConfirmations().getEntriesWithoutHash().size()); Assertions.assertEquals(activations.isActive(ConsensusRule.RSKIP146) ? 1 : 0, provider.getPegoutsWaitingForConfirmations().getEntriesWithHash().size()); + if (!inMigrationAge){ + verify(provider, times(1)).setOldFederation(null); + Assertions.assertTrue(sufficientUTXOsForMigration.isEmpty()); + } else { + verify(provider, never()).setOldFederation(null); + } + if (activations.isActive(ConsensusRule.RSKIP146)) { // Should have been logged with the migrated UTXO PegoutsWaitingForConfirmations.Entry entry = (PegoutsWaitingForConfirmations.Entry) provider.getPegoutsWaitingForConfirmations() diff --git a/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java b/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java index dc54e99b84d..72c3a7c38a9 100644 --- a/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/PowpegMigrationTest.java @@ -287,25 +287,25 @@ private void testChangePowpeg( ActivationConfig.ForBlock activationsBeforeRSKIP383 = mock(ActivationConfig.ForBlock.class); when(activationsBeforeRSKIP383.isActive(ConsensusRule.RSKIP383)).thenReturn(false); - long fedActivationBlockNumber = initialBlock.getNumber() + bridgeConstants.getFederationActivationAge(activationsBeforeRSKIP383); - Assertions.assertTrue(blockNumber > fedActivationBlockNumber); + long legacyFedActivationBlockNumber = initialBlock.getNumber() + bridgeConstants.getFederationActivationAge(activationsBeforeRSKIP383); + Assertions.assertTrue(blockNumber > legacyFedActivationBlockNumber); - Block fedActivationBlock = mock(Block.class); - doReturn(fedActivationBlockNumber).when(fedActivationBlock).getNumber(); + Block legacyFedActivationBlock = mock(Block.class); + doReturn(legacyFedActivationBlockNumber).when(legacyFedActivationBlock).getNumber(); bridgeSupport = new BridgeSupportBuilder() .withProvider(bridgeStorageProvider) .withRepository(repository) .withEventLogger(bridgeEventLogger) - .withExecutionBlock(fedActivationBlock) + .withExecutionBlock(legacyFedActivationBlock) .withActivations(activations) .withBridgeConstants(bridgeConstants) .withBtcBlockStoreFactory(btcBlockStoreFactory) .withPeginInstructionsProvider(new PeginInstructionsProvider()) .build(); - assertEquals(newPowPegAddress, bridgeSupport.getRetiringFederationAddress()); assertEquals(oldPowPegAddress, bridgeSupport.getFederationAddress()); + assertNull(bridgeSupport.getRetiringFederation()); } bridgeSupport = new BridgeSupportBuilder() From b55278edbc0898dda0c7fb923a80fd2e60d17622 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Fri, 28 Apr 2023 14:37:32 -0400 Subject: [PATCH 19/24] Refactored funds migration tests --- ...ridgeSupportProcessFundsMigrationTest.java | 78 ++++++++++++++++++- .../java/co/rsk/peg/BridgeSupportTest.java | 1 + 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java index bd3c5ac61cc..c838544faa1 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java @@ -16,6 +16,9 @@ import org.ethereum.core.Transaction; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import java.io.IOException; import java.math.BigInteger; @@ -23,6 +26,8 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.function.Function; +import java.util.stream.Stream; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; @@ -213,7 +218,76 @@ void processFundsMigration_past_migration_age_after_rskip383_activation_mainnet( test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, false); } - private void test_processFundsMigration( + private static Stream processFundMigrationArgProvider() { + BridgeMainNetConstants bridgeMainNetConstants = BridgeMainNetConstants.getInstance(); + BridgeTestNetConstants bridgeTestNetConstants = BridgeTestNetConstants.getInstance(); + + ActivationConfig.ForBlock activationsBeforeRSKIP146 = mock(ActivationConfig.ForBlock.class); + + Stream beforeRskip146Tests = Stream.of( + Arguments.of(bridgeTestNetConstants, activationsBeforeRSKIP146, false), + Arguments.of(bridgeTestNetConstants, activationsBeforeRSKIP146, true), + Arguments.of(bridgeMainNetConstants, activationsBeforeRSKIP146, false), + Arguments.of(bridgeMainNetConstants, activationsBeforeRSKIP146, true) + ); + + ActivationConfig.ForBlock activationsAfterRSKIP146 = mock(ActivationConfig.ForBlock.class); + when(activationsAfterRSKIP146.isActive(ConsensusRule.RSKIP146)).thenReturn(true); + + Stream afterRskip146Tests = Stream.of( + Arguments.of(bridgeTestNetConstants, activationsAfterRSKIP146, false), + Arguments.of(bridgeTestNetConstants, activationsAfterRSKIP146, true), + Arguments.of(bridgeMainNetConstants, activationsAfterRSKIP146, false), + Arguments.of(bridgeMainNetConstants, activationsAfterRSKIP146, true) + ); + + ActivationConfig.ForBlock activationsAfterRSKIP357 = mock(ActivationConfig.ForBlock.class); + when(activationsAfterRSKIP357.isActive(ConsensusRule.RSKIP146)).thenReturn(true); + when(activationsAfterRSKIP357.isActive(ConsensusRule.RSKIP357)).thenReturn(true); + + Stream afterRskip357Tests = Stream.of( + Arguments.of(bridgeTestNetConstants, activationsAfterRSKIP357, false), + Arguments.of(bridgeTestNetConstants, activationsAfterRSKIP357, true), + Arguments.of(bridgeMainNetConstants, activationsAfterRSKIP357, false), + Arguments.of(bridgeMainNetConstants, activationsAfterRSKIP357, true) + ); + + ActivationConfig.ForBlock activationsAfterRSKIP374 = mock(ActivationConfig.ForBlock.class); + when(activationsAfterRSKIP374.isActive(ConsensusRule.RSKIP146)).thenReturn(true); + when(activationsAfterRSKIP374.isActive(ConsensusRule.RSKIP357)).thenReturn(true); + when(activationsAfterRSKIP374.isActive(ConsensusRule.RSKIP374)).thenReturn(true); + + Stream afterRskip374Tests = Stream.of( + Arguments.of(bridgeTestNetConstants, activationsAfterRSKIP374, false), + Arguments.of(bridgeTestNetConstants, activationsAfterRSKIP374, true), + Arguments.of(bridgeMainNetConstants, activationsAfterRSKIP374, false), + Arguments.of(bridgeMainNetConstants, activationsAfterRSKIP374, true) + ); + + ActivationConfig.ForBlock activationsAfterRSKIP383 = mock(ActivationConfig.ForBlock.class); + when(activationsAfterRSKIP383.isActive(ConsensusRule.RSKIP146)).thenReturn(true); + when(activationsAfterRSKIP383.isActive(ConsensusRule.RSKIP357)).thenReturn(true); + when(activationsAfterRSKIP383.isActive(ConsensusRule.RSKIP374)).thenReturn(true); + when(activationsAfterRSKIP383.isActive(ConsensusRule.RSKIP383)).thenReturn(true); + + Stream afterRskip383Tests = Stream.of( + Arguments.of(bridgeTestNetConstants, activationsAfterRSKIP383, false), + Arguments.of(bridgeTestNetConstants, activationsAfterRSKIP383, true), + Arguments.of(bridgeMainNetConstants, activationsAfterRSKIP383, false), + Arguments.of(bridgeMainNetConstants, activationsAfterRSKIP383, true) + ); + return Stream.of( + beforeRskip146Tests, + afterRskip146Tests, + afterRskip357Tests, + afterRskip374Tests, + afterRskip383Tests + ).flatMap(Function.identity()); + } + + @ParameterizedTest + @MethodSource("processFundMigrationArgProvider") + void test_processFundsMigration( BridgeConstants bridgeConstants, ActivationConfig.ForBlock activations, boolean inMigrationAge @@ -271,9 +345,9 @@ private void test_processFundsMigration( Assertions.assertEquals(activations.isActive(ConsensusRule.RSKIP146)? 0 : 1, provider.getPegoutsWaitingForConfirmations().getEntriesWithoutHash().size()); Assertions.assertEquals(activations.isActive(ConsensusRule.RSKIP146) ? 1 : 0, provider.getPegoutsWaitingForConfirmations().getEntriesWithHash().size()); + Assertions.assertTrue(sufficientUTXOsForMigration.isEmpty()); if (!inMigrationAge){ verify(provider, times(1)).setOldFederation(null); - Assertions.assertTrue(sufficientUTXOsForMigration.isEmpty()); } else { verify(provider, never()).setOldFederation(null); } diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportTest.java index da2efd95fac..58aa3961e41 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportTest.java @@ -2109,6 +2109,7 @@ void rskTxWaitingForSignature_uses_updateCollection_rskTxHash_after_rskip_176_ac private static Stream provideBridgeConstants() { return Stream.of(BridgeRegTestConstants.getInstance(), BridgeTestNetConstants.getInstance(), BridgeMainNetConstants.getInstance()); } + @ParameterizedTest @MethodSource("provideBridgeConstants") void rskTxWaitingForSignature_uses_pegoutCreation_rskTxHash_after_rskip_375_activation(BridgeConstants bridgeConstants) throws IOException { From 571ce77c184f240851cf17fbff8581974321bf41 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Fri, 28 Apr 2023 14:56:21 -0400 Subject: [PATCH 20/24] Renamed test argument method provider. --- .../test/java/co/rsk/config/BridgeConstantsTest.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/config/BridgeConstantsTest.java b/rskj-core/src/test/java/co/rsk/config/BridgeConstantsTest.java index 60b345e4d81..b61385cd019 100644 --- a/rskj-core/src/test/java/co/rsk/config/BridgeConstantsTest.java +++ b/rskj-core/src/test/java/co/rsk/config/BridgeConstantsTest.java @@ -13,7 +13,7 @@ import static org.mockito.Mockito.when; class BridgeConstantsTest { - private static Stream generatorFundsMigrationAgeSinceActivationEnd() { + private static Stream fundsMigrationAgeSinceActivationEndArgsProvider() { return Stream.of( Arguments.of(BridgeMainNetConstants.getInstance(), false), Arguments.of(BridgeTestNetConstants.getInstance(), true), @@ -22,7 +22,7 @@ private static Stream generatorFundsMigrationAgeSinceActivationEnd() } @ParameterizedTest() - @MethodSource("generatorFundsMigrationAgeSinceActivationEnd") + @MethodSource("fundsMigrationAgeSinceActivationEndArgsProvider") void test_getFundsMigrationAgeSinceActivationEnd(BridgeConstants bridgeConstants, boolean hasSameValueForBothFields) { // Arrange ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); @@ -36,7 +36,7 @@ void test_getFundsMigrationAgeSinceActivationEnd(BridgeConstants bridgeConstants } @ParameterizedTest() - @MethodSource("generatorFundsMigrationAgeSinceActivationEnd") + @MethodSource("fundsMigrationAgeSinceActivationEndArgsProvider") void test_getFundsMigrationAgeSinceActivationEnd_post_RSKIP357(BridgeConstants bridgeConstants, boolean hasSameValueForBothMigrationAges) { // Arrange ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); @@ -51,7 +51,7 @@ void test_getFundsMigrationAgeSinceActivationEnd_post_RSKIP357(BridgeConstants b } @ParameterizedTest() - @MethodSource("generatorFundsMigrationAgeSinceActivationEnd") + @MethodSource("fundsMigrationAgeSinceActivationEndArgsProvider") void test_getFundsMigrationAgeSinceActivationEnd_post_RSKIP374(BridgeConstants bridgeConstants, boolean hasSameValueForBothMigrationAges) { // Arrange ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); @@ -66,7 +66,7 @@ void test_getFundsMigrationAgeSinceActivationEnd_post_RSKIP374(BridgeConstants b assertEquals(hasSameValueForBothMigrationAges, fundsMigrationAgeSinceActivationEnd == bridgeConstants.specialCaseFundsMigrationAgeSinceActivationEnd); } - private static Stream generatorFederationActivationAge() { + private static Stream federationActivationAgeArgProvider() { return Stream.of( Arguments.of(BridgeMainNetConstants.getInstance(), false), Arguments.of(BridgeTestNetConstants.getInstance(), false), @@ -78,7 +78,7 @@ private static Stream generatorFederationActivationAge() { } @ParameterizedTest() - @MethodSource("generatorFederationActivationAge") + @MethodSource("federationActivationAgeArgProvider") void test_getFederationActivationAge(BridgeConstants bridgeConstants, boolean isRSKIP383Active) { // Arrange ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); From 5f8adfe718cf5871fee5c7e521ac667b8a8de282 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Fri, 28 Apr 2023 15:04:30 -0400 Subject: [PATCH 21/24] Got rid of redundant tests and assertions --- .../peg/utils/BridgeEventLoggerImplTest.java | 104 ++---------------- 1 file changed, 9 insertions(+), 95 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java index a49f6ff058b..7876cc65a4e 100644 --- a/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/utils/BridgeEventLoggerImplTest.java @@ -276,107 +276,21 @@ void logCommitFederation(boolean isRSKIP383Active) { // assert fed activation has different values before and after RSKIP383 respectively if (isRSKIP383Active){ - ActivationConfig.ForBlock activationsBeforeRSKIP383 = mock(ActivationConfig.ForBlock.class); - when(activationsBeforeRSKIP383.isActive(ConsensusRule.RSKIP383)).thenReturn(false); + ActivationConfig.ForBlock activationsForLegacyFedActivationAge = mock(ActivationConfig.ForBlock.class); + when(activationsForLegacyFedActivationAge.isActive(ConsensusRule.RSKIP383)).thenReturn(false); - long beforeRSKIP383fedActivationBlockNumber = executionBlock.getNumber() + CONSTANTS.getFederationActivationAge(activationsBeforeRSKIP383); - assertNotEquals(loggedFedActivationBlockNumber, beforeRSKIP383fedActivationBlockNumber); - assertNotEquals(newFedActivationBlockNumber, beforeRSKIP383fedActivationBlockNumber); + long legacyFedActivationBlockNumber = executionBlock.getNumber() + CONSTANTS.getFederationActivationAge(activationsForLegacyFedActivationAge); + + assertNotEquals(loggedFedActivationBlockNumber, legacyFedActivationBlockNumber); } else { - // assert fed activation has different values before and after RSKIP383 respectively - ActivationConfig.ForBlock activationsAfterRSKIP383 = mock(ActivationConfig.ForBlock.class); - when(activationsAfterRSKIP383.isActive(ConsensusRule.RSKIP383)).thenReturn(true); + ActivationConfig.ForBlock activationsForDefaultFedActivationAge = mock(ActivationConfig.ForBlock.class); + when(activationsForDefaultFedActivationAge.isActive(ConsensusRule.RSKIP383)).thenReturn(true); - long afterRSKIP383FedActivationBlockNumber = executionBlock.getNumber() + CONSTANTS.getFederationActivationAge(activationsAfterRSKIP383); - assertNotEquals(loggedFedActivationBlockNumber, afterRSKIP383FedActivationBlockNumber); - assertNotEquals(newFedActivationBlockNumber, afterRSKIP383FedActivationBlockNumber); + long defaultFedActivationBlockNumber = executionBlock.getNumber() + CONSTANTS.getFederationActivationAge(activationsForDefaultFedActivationAge); + assertNotEquals(loggedFedActivationBlockNumber, defaultFedActivationBlockNumber); } } - @Test - void logCommitFederation_after_RSKIP383() { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP383)).thenReturn(true); - BridgeEventLogger eventLogger = new BridgeEventLoggerImpl(CONSTANTS, activations, eventLogs, signatureCache); - - // Setup parameters for test method call - Block executionBlock = mock(Block.class); - when(executionBlock.getTimestamp()).thenReturn(15005L); - when(executionBlock.getNumber()).thenReturn(15L); - - List oldFederationKeys = Arrays.asList( - BtcECKey.fromPublicOnly(Hex.decode("036bb9eab797eadc8b697f0e82a01d01cabbfaaca37e5bafc06fdc6fdd38af894a")), - BtcECKey.fromPublicOnly(Hex.decode("031da807c71c2f303b7f409dd2605b297ac494a563be3b9ca5f52d95a43d183cc5")), - BtcECKey.fromPublicOnly(Hex.decode("025eefeeeed5cdc40822880c7db1d0a88b7b986945ed3fc05a0b45fe166fe85e12")), - BtcECKey.fromPublicOnly(Hex.decode("03c67ad63527012fd4776ae892b5dc8c56f80f1be002dc65cd520a2efb64e37b49")) - ); - - List oldFederationMembers = FederationTestUtils.getFederationMembersWithBtcKeys(oldFederationKeys); - - Federation oldFederation = new Federation( - oldFederationMembers, - Instant.ofEpochMilli(15005L), - 15L, - NetworkParameters.fromID(NetworkParameters.ID_REGTEST) - ); - - List newFederationKeys = Arrays.asList( - BtcECKey.fromPublicOnly(Hex.decode("0346cb6b905e4dee49a862eeb2288217d06afcd4ace4b5ca77ebedfbc6afc1c19d")), - BtcECKey.fromPublicOnly(Hex.decode("0269a0dbe7b8f84d1b399103c466fb20531a56b1ad3a7b44fe419e74aad8c46db7")), - BtcECKey.fromPublicOnly(Hex.decode("026192d8ab41bd402eb0431457f6756a3f3ce15c955c534d2b87f1e0372d8ba338")) - ); - - List newFederationMembers = FederationTestUtils.getFederationMembersWithBtcKeys(newFederationKeys); - - Federation newFederation = new Federation( - newFederationMembers, - Instant.ofEpochMilli(5005L), - 0L, - NetworkParameters.fromID(NetworkParameters.ID_REGTEST) - ); - - // Act - eventLogger.logCommitFederation(executionBlock, oldFederation, newFederation); - - commonAssertLogs(eventLogs); - - assertTopics(1, eventLogs); - // Assert log data - long federationActivationAge = CONSTANTS.getFederationActivationAge(activations); - - byte[] oldFederationFlatPubKeys = flatKeysAsByteArray(oldFederation.getBtcPublicKeys()); - String oldFederationBtcAddress = oldFederation.getAddress().toBase58(); - byte[] newFederationFlatPubKeys = flatKeysAsByteArray(newFederation.getBtcPublicKeys()); - String newFederationBtcAddress = newFederation.getAddress().toBase58(); - long newFedActivationBlockNumber = executionBlock.getNumber() + federationActivationAge; - - Object[] data = new Object[]{ - oldFederationFlatPubKeys, - oldFederationBtcAddress, - newFederationFlatPubKeys, - newFederationBtcAddress, - newFedActivationBlockNumber - }; - CallTransaction.Function event = BridgeEvents.COMMIT_FEDERATION.getEvent(); - Object[] topics = {}; - assertEvent(eventLogs, 0, event, topics, data); - - final LogInfo log = eventLogs.get(0); - Object[] decodeEventData = event.decodeEventData(log.getData()); - long loggedFedActivationBlockNumber = ((BigInteger) decodeEventData[4]).longValue(); - - // assert fed activation has different values before and after fingerroot respectively - ActivationConfig.ForBlock activationsBeforeRSKIP383 = mock(ActivationConfig.ForBlock.class); - when(activationsBeforeRSKIP383.isActive(ConsensusRule.RSKIP383)).thenReturn(false); - - long hopNewFedActivationBlockNumber = executionBlock.getNumber() + CONSTANTS.getFederationActivationAge(activationsBeforeRSKIP383); - assertNotEquals(loggedFedActivationBlockNumber, hopNewFedActivationBlockNumber); - assertNotEquals(newFedActivationBlockNumber, hopNewFedActivationBlockNumber); - - // assert fed activation value for fingerroot is being properly logged - assertEquals(loggedFedActivationBlockNumber, newFedActivationBlockNumber); - } - @Test void logReleaseBtcRequested() { Keccak256 rskTxHash = PegTestUtils.createHash3(0); From ea6e74ab1ba5d391d4efae86d7a771aa133c5b3f Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Fri, 28 Apr 2023 15:27:55 -0400 Subject: [PATCH 22/24] Renamed methods --- ...ridgeSupportProcessFundsMigrationTest.java | 4 ++-- .../co/rsk/peg/FederationSupportTest.java | 24 +++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java index c838544faa1..e6c89960677 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java @@ -218,7 +218,7 @@ void processFundsMigration_past_migration_age_after_rskip383_activation_mainnet( test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, false); } - private static Stream processFundMigrationArgProvider() { + private static Stream processFundMigrationArgsProvider() { BridgeMainNetConstants bridgeMainNetConstants = BridgeMainNetConstants.getInstance(); BridgeTestNetConstants bridgeTestNetConstants = BridgeTestNetConstants.getInstance(); @@ -286,7 +286,7 @@ private static Stream processFundMigrationArgProvider() { } @ParameterizedTest - @MethodSource("processFundMigrationArgProvider") + @MethodSource("processFundMigrationArgsProvider") void test_processFundsMigration( BridgeConstants bridgeConstants, ActivationConfig.ForBlock activations, diff --git a/rskj-core/src/test/java/co/rsk/peg/FederationSupportTest.java b/rskj-core/src/test/java/co/rsk/peg/FederationSupportTest.java index ba8415cca7a..42d49ae32b4 100644 --- a/rskj-core/src/test/java/co/rsk/peg/FederationSupportTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/FederationSupportTest.java @@ -90,27 +90,27 @@ void whenOldFederationIsNullThenActiveFederationIsNewFederation() { assertThat(federationSupport.getActiveFederation(), is(newFederation)); } - private static Stream fedActivationArguments() { + private static Stream fedActivationAgeTestArgs() { BridgeConstants bridgeTestNetConstants = BridgeTestNetConstants.getInstance(); BridgeConstants bridgeMainnetConstants = BridgeMainNetConstants.getInstance(); - ActivationConfig.ForBlock hop = ActivationConfigsForTest.hop400().forBlock(0); - ActivationConfig.ForBlock fingerroot = ActivationConfigsForTest.fingerroot500().forBlock(0); + ActivationConfig.ForBlock hopActivations = ActivationConfigsForTest.hop400().forBlock(0); + ActivationConfig.ForBlock fingerrootActivations = ActivationConfigsForTest.fingerroot500().forBlock(0); return Stream.of( - Arguments.of(bridgeTestNetConstants, hop, false), - Arguments.of(bridgeTestNetConstants, hop, true), - Arguments.of(bridgeTestNetConstants, fingerroot, false), - Arguments.of(bridgeTestNetConstants, fingerroot, true), - Arguments.of(bridgeMainnetConstants, hop, false), - Arguments.of(bridgeMainnetConstants, hop, true), - Arguments.of(bridgeMainnetConstants, fingerroot, false), - Arguments.of(bridgeMainnetConstants, fingerroot, true) + Arguments.of(bridgeTestNetConstants, hopActivations, false), + Arguments.of(bridgeTestNetConstants, hopActivations, true), + Arguments.of(bridgeTestNetConstants, fingerrootActivations, false), + Arguments.of(bridgeTestNetConstants, fingerrootActivations, true), + Arguments.of(bridgeMainnetConstants, hopActivations, false), + Arguments.of(bridgeMainnetConstants, hopActivations, true), + Arguments.of(bridgeMainnetConstants, fingerrootActivations, false), + Arguments.of(bridgeMainnetConstants, fingerrootActivations, true) ); } @ParameterizedTest - @MethodSource("fedActivationArguments") + @MethodSource("fedActivationAgeTestArgs") void whenOldAndNewFederationArePresentReturnActiveFederationByActivationAge( BridgeConstants bridgeConstants, ActivationConfig.ForBlock activations, From 76306b40f75b59a3bc386563c81aaf7b417b46a1 Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Fri, 28 Apr 2023 15:56:20 -0400 Subject: [PATCH 23/24] Removed redundant tests after making the base test parameterized. --- ...ridgeSupportProcessFundsMigrationTest.java | 184 ------------------ 1 file changed, 184 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java index e6c89960677..d47f11177ca 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java @@ -34,190 +34,6 @@ class BridgeSupportProcessFundsMigrationTest { - @Test - void processFundsMigration_in_migration_age_before_rskip_146_activation_testnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(false); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(false); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(false); - test_processFundsMigration(BridgeTestNetConstants.getInstance(), activations, true); - } - - @Test - void processFundsMigration_in_migration_age_after_rskip_146_activation_testnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(false); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(false); - test_processFundsMigration(BridgeTestNetConstants.getInstance(), activations, true); - } - - @Test - void processFundsMigration_in_migration_age_after_rskip_357_activation_testnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(false); - test_processFundsMigration(BridgeTestNetConstants.getInstance(), activations, true); - } - - @Test - void processFundsMigration_in_migration_age_after_rskip_374_activation_testnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(true); - test_processFundsMigration(BridgeTestNetConstants.getInstance(), activations, true); - } - - @Test - void processFundsMigration_in_migration_age_after_rskip383_activation_testnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP383)).thenReturn(true); - test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, true); - } - - @Test - void processFundsMigration_past_migration_age_before_rskip_146_activation_testnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(false); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(false); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(false); - test_processFundsMigration(BridgeTestNetConstants.getInstance(), activations, false); - } - - @Test - void processFundsMigration_past_migration_age_after_rskip_146_activation_testnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(false); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(false); - test_processFundsMigration(BridgeTestNetConstants.getInstance(), activations, false); - } - - @Test - void processFundsMigration_past_migration_age_after_rskip_357_activation_testnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(false); - test_processFundsMigration(BridgeTestNetConstants.getInstance(), activations, false); - } - - @Test - void processFundsMigration_past_migration_age_after_rskip_374_activation_testnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(true); - test_processFundsMigration(BridgeTestNetConstants.getInstance(), activations, false); - } - - @Test - void processFundsMigration_past_migration_age_after_rskip383_activation_testnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP383)).thenReturn(true); - test_processFundsMigration(BridgeTestNetConstants.getInstance(), activations, false); - } - - @Test - void processFundsMigration_in_migration_age_before_rskip_146_activation_mainnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(false); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(false); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(false); - test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, true); - } - - @Test - void processFundsMigration_in_migration_age_after_rskip_146_activation_mainnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(false); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(false); - test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, true); - } - - @Test - void processFundsMigration_in_migration_age_after_rskip_357_activation_mainnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(false); - test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, true); - } - - @Test - void processFundsMigration_in_migration_age_after_rskip_374_activation_mainnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(true); - test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, true); - } - - @Test - void processFundsMigration_in_migration_age_after_rskip383_activation_mainnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP383)).thenReturn(true); - test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, true); - } - - @Test - void processFundsMigration_past_migration_age_before_rskip_146_activation_mainnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(false); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(false); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(false); - test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, false); - } - - @Test - void processFundsMigration_past_migration_age_after_rskip_146_activation_mainnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(false); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(false); - test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, false); - } - - @Test - void processFundsMigration_past_migration_age_after_rskip_357_activation_mainnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(false); - test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, false); - } - - @Test - void processFundsMigration_past_migration_age_after_rskip_374_activation_mainnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(true); - test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, false); - } - - @Test - void processFundsMigration_past_migration_age_after_rskip383_activation_mainnet() throws IOException { - ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class); - when(activations.isActive(ConsensusRule.RSKIP146)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP357)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP374)).thenReturn(true); - when(activations.isActive(ConsensusRule.RSKIP383)).thenReturn(true); - test_processFundsMigration(BridgeMainNetConstants.getInstance(), activations, false); - } - private static Stream processFundMigrationArgsProvider() { BridgeMainNetConstants bridgeMainNetConstants = BridgeMainNetConstants.getInstance(); BridgeTestNetConstants bridgeTestNetConstants = BridgeTestNetConstants.getInstance(); From cccfc25ae4303b4ec4dcbfa2fe3b1330c352dc9e Mon Sep 17 00:00:00 2001 From: nathanieliov Date: Tue, 2 May 2023 14:57:03 -0400 Subject: [PATCH 24/24] Changed "if" to checks for the positive case. --- .../rsk/peg/BridgeSupportProcessFundsMigrationTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java index d47f11177ca..17df5e2533f 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportProcessFundsMigrationTest.java @@ -159,13 +159,13 @@ void test_processFundsMigration( bridgeSupport.updateCollections(updateCollectionsTx); Assertions.assertEquals(activations.isActive(ConsensusRule.RSKIP146)? 0 : 1, provider.getPegoutsWaitingForConfirmations().getEntriesWithoutHash().size()); - Assertions.assertEquals(activations.isActive(ConsensusRule.RSKIP146) ? 1 : 0, provider.getPegoutsWaitingForConfirmations().getEntriesWithHash().size()); + Assertions.assertEquals(activations.isActive(ConsensusRule.RSKIP146)? 1 : 0, provider.getPegoutsWaitingForConfirmations().getEntriesWithHash().size()); Assertions.assertTrue(sufficientUTXOsForMigration.isEmpty()); - if (!inMigrationAge){ - verify(provider, times(1)).setOldFederation(null); - } else { + if (inMigrationAge){ verify(provider, never()).setOldFederation(null); + } else { + verify(provider, times(1)).setOldFederation(null); } if (activations.isActive(ConsensusRule.RSKIP146)) {