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 570ca8c815..9660b59b41 100644 --- a/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java +++ b/rskj-core/src/main/java/co/rsk/peg/BridgeSupport.java @@ -1022,7 +1022,7 @@ private void logUpdateCollections(Transaction rskTx) { private void updateSvpState(Transaction rskTx) { Optional proposedFederationOpt = federationSupport.getProposedFederation(); if (proposedFederationOpt.isEmpty()) { - logger.debug("[updateSvpState] Proposed federation does not exist, so there's no svp going on."); + logger.trace("[updateSvpState] Proposed federation does not exist, so there's no svp going on."); return; } @@ -1034,11 +1034,19 @@ private void updateSvpState(Transaction rskTx) { "[updateSvpState] Proposed federation validation failed at block {}. SVP failure will be processed", rskExecutionBlock.getNumber() ); - processSVPFailure(proposedFederation); + processSvpFailure(proposedFederation); return; } Keccak256 rskTxHash = rskTx.getHash(); + + if (shouldCreateAndProcessSvpFundTransaction()) { + logger.info( + "[updateSvpState] No svp values were found, so fund tx creation will be processed." + ); + processSvpFundTransactionUnsigned(rskTxHash, proposedFederation); + } + // if the fund tx signed is present, then the fund transaction change was registered, // meaning we can create the spend tx. Optional svpFundTxSigned = provider.getSvpFundTxSigned(); @@ -1047,20 +1055,22 @@ private void updateSvpState(Transaction rskTx) { "[updateSvpState] Fund tx signed was found, so spend tx creation will be processed." ); processSvpSpendTransactionUnsigned(rskTxHash, proposedFederation, svpFundTxSigned.get()); - return; } + } - // if the unsigned fund tx hash is not present, meaning we can proceed with the fund tx creation + private boolean shouldCreateAndProcessSvpFundTransaction() { + // the fund tx will be created when the svp starts, + // so we must ensure all svp values are clear to proceed with its creation Optional svpFundTxHashUnsigned = provider.getSvpFundTxHashUnsigned(); - if (svpFundTxHashUnsigned.isEmpty()) { - logger.info( - "[updateSvpState] Fund tx hash unsigned was not found, so fund tx creation will be processed." - ); - processSvpFundTransactionUnsigned(rskTxHash, proposedFederation); - } + Optional svpFundTxSigned = provider.getSvpFundTxSigned(); + Optional svpSpendTxHashUnsigned = provider.getSvpSpendTxHashUnsigned(); // spendTxHash will be removed the last, after spendTxWFS, so is enough checking just this value + + return svpFundTxHashUnsigned.isEmpty() + && svpFundTxSigned.isEmpty() + && svpSpendTxHashUnsigned.isEmpty(); } - private void processSVPFailure(Federation proposedFederation) { + private void processSvpFailure(Federation proposedFederation) { eventLogger.logCommitFederationFailure(rskExecutionBlock, proposedFederation); logger.info("[processSVPFailure] Federation election will be allowed again."); diff --git a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportSvpTest.java b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportSvpTest.java index 40efac33e9..c2414dc678 100644 --- a/rskj-core/src/test/java/co/rsk/peg/BridgeSupportSvpTest.java +++ b/rskj-core/src/test/java/co/rsk/peg/BridgeSupportSvpTest.java @@ -141,8 +141,8 @@ void setUp() { @Nested @TestInstance(TestInstance.Lifecycle.PER_CLASS) - @Tag("SVP failure tests") - class SVPFailureTests { + @Tag("Failure tests") + class FailureTests { @BeforeEach void setUp() { @@ -259,6 +259,7 @@ private void assertNoSVPValues() { @TestInstance(TestInstance.Lifecycle.PER_CLASS) @Tag("Fund transaction creation and processing tests") class FundTxCreationAndProcessingTests { + @Test void updateCollections_whenProposedFederationDoesNotExist_shouldNotCreateFundTransaction() throws IOException { // arrange @@ -288,10 +289,10 @@ void updateCollections_whenThereAreNoEnoughUTXOs_shouldNotCreateFundTransaction( void updateCollections_whenFundTxCanBeCreated_createsExpectedFundTxAndSavesTheHashInStorageEntryAndPerformsPegoutActions() throws Exception { // act bridgeSupport.updateCollections(rskTx); - bridgeStorageProvider.save(); // to save the tx sig hash + bridgeStorageProvider.save(); // assert - assertSvpFundTxHashUnsignedWasSavedInStorage(); + assertSvpFundTxHashUnsignedIsSavedInStorage(); assertSvpFundTxReleaseWasSettled(); assertSvpFundTransactionHasExpectedInputsAndOutputs(); } @@ -335,6 +336,59 @@ private void assertOneOutputIsToProposedFederationWithFlyoverPrefixWithExpectedA assertOutputWasSentToExpectedScriptWithExpectedAmount(svpFundTransactionUnsignedOutputs, proposedFederationWithFlyoverPrefixScriptPubKey, spendableValueFromProposedFederation); } + + @Test + void updateCollections_whenWaitingForFundTxToBeRegistered_shouldNotCreateFundTransactionAgain() throws Exception { + // arrange + arrangeSvpFundTransactionUnsigned(); + assertSvpFundTxHashUnsignedIsSavedInStorage(); + + // act + bridgeSupport.updateCollections(rskTx); + bridgeStorageProvider.save(); + + // assert + assertJustUpdateCollectionsWasLogged(); + } + + @Test + void updateCollections_whenFundTxSignedIsSaved_shouldNotCreateFundTransaction() throws Exception { + // arrange + arrangeSvpFundTransactionSigned(); + + // act + bridgeSupport.updateCollections(rskTx); + bridgeStorageProvider.save(); + + // assert + assertNoSvpFundTxHashUnsigned(); + } + + @Test + void updateCollections_whenWaitingForSpendTxToBeRegistered_shouldNotCreateFundTransaction() throws Exception { + // arrange + arrangeSvpSpendTransaction(); + + // act + bridgeSupport.updateCollections(rskTx); + bridgeStorageProvider.save(); + + // assert + assertNoSvpFundTxHashUnsigned(); + assertJustUpdateCollectionsWasLogged(); + } + + private void assertJustUpdateCollectionsWasLogged() { + assertEquals(1, logs.size()); + + CallTransaction.Function updateCollectionsEvent = BridgeEvents.UPDATE_COLLECTIONS.getEvent(); + List encodedTopics = getEncodedTopics(updateCollectionsEvent); + assertEventWasEmittedWithExpectedTopics(encodedTopics); + + String senderData = rskTx.getSender(mock(SignatureCache.class)).toHexString(); + byte[] encodedData = getEncodedData(updateCollectionsEvent, senderData); + assertEventWasEmittedWithExpectedData(encodedData); + } } private void arrangeSvpFundTransactionUnsigned() { @@ -358,7 +412,7 @@ private void saveSvpFundTransactionHashUnsigned(Sha256Hash svpFundTransactionHas bridgeSupport.save(); } - private void assertSvpFundTxHashUnsignedWasSavedInStorage() { + private void assertSvpFundTxHashUnsignedIsSavedInStorage() { Optional svpFundTransactionHashUnsignedOpt = bridgeStorageProvider.getSvpFundTxHashUnsigned(); assertTrue(svpFundTransactionHashUnsignedOpt.isPresent()); @@ -937,13 +991,17 @@ private void arrangeExecutionBlockIsAfterValidationPeriodEnded() { } private void arrangeSvpFundTransactionSigned() { - recreateSvpFundTransactionUnsigned(); - signInputs(svpFundTransaction); + recreateSvpFundTransactionSigned(); bridgeStorageProvider.setSvpFundTxSigned(svpFundTransaction); bridgeStorageProvider.save(); } + private void recreateSvpFundTransactionSigned() { + recreateSvpFundTransactionUnsigned(); + signInputs(svpFundTransaction); + } + private void recreateSvpFundTransactionUnsigned() { svpFundTransaction = new BtcTransaction(btcMainnetParams); @@ -991,14 +1049,14 @@ private void signInputs(BtcTransaction transaction) { private void arrangeSvpSpendTransaction() { recreateSvpSpendTransactionUnsigned(); - saveSvpSpendTransactionWFSValues(); + saveSvpSpendTransactionValues(); } private void recreateSvpSpendTransactionUnsigned() { svpSpendTransaction = new BtcTransaction(btcMainnetParams); svpSpendTransaction.setVersion(BTC_TX_VERSION_2); - arrangeSvpFundTransactionSigned(); + recreateSvpFundTransactionSigned(); // add inputs addInputFromMatchingOutputScript(svpSpendTransaction, svpFundTransaction, proposedFederation.getP2SHScript()); Script proposedFederationRedeemScript = proposedFederation.getRedeemScript(); @@ -1014,9 +1072,12 @@ private void recreateSvpSpendTransactionUnsigned() { svpSpendTransaction.addOutput(Coin.valueOf(1762), federationSupport.getActiveFederationAddress()); } - private void saveSvpSpendTransactionWFSValues() { + private void saveSvpSpendTransactionValues() { + bridgeStorageProvider.setSvpSpendTxHashUnsigned(svpSpendTransaction.getHash()); + Map.Entry svpSpendTxWaitingForSignatures = new AbstractMap.SimpleEntry<>(svpSpendTxCreationHash, svpSpendTransaction); bridgeStorageProvider.setSvpSpendTxWaitingForSignatures(svpSpendTxWaitingForSignatures); + bridgeStorageProvider.save(); }