Merge pull request #2842 from rsksmart/remove_last_retired_p2sh_scrip…

…t_setting Do not set last retired fed p2sh script in new commitFederation impl
rsksmart · Nov 19, 2024 · 3b02692 · 3b02692
2 parents d357dc4 + 80b0d63
commit 3b02692
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 13 deletions.
diff --git a/rskj-core/src/main/java/co/rsk/peg/federation/FederationSupportImpl.java b/rskj-core/src/main/java/co/rsk/peg/federation/FederationSupportImpl.java
@@ -702,7 +702,9 @@ private FederationChangeResponseCode legacyCommitPendingFederation(PendingFedera
         clearPendingFederationVoting();
 
         if (activations.isActive(RSKIP186)) {
-            preserveFederationChangeInfo(activeFederation);
+            // since we are creating the to-be-active-fed in this block,
+            // its creation block height is this block number
+            saveFederationChangeInfo(rskExecutionBlock.getNumber());
         }
 
         Federation currentOldFederation = provider.getOldFederation(constants, activations);
@@ -736,9 +738,7 @@ private FederationChangeResponseCode commitPendingFederation(PendingFederation c
 
         clearPendingFederationVoting();
 
-        Federation activeFederation = getActiveFederation();
-        preserveFederationChangeInfo(activeFederation);
-        logCommitmentWithVotedFederation(eventLogger, activeFederation, proposedFederation);
+        logCommitmentWithVotedFederation(eventLogger, getActiveFederation(), proposedFederation);
 
         return FederationChangeResponseCode.SUCCESSFUL;
     }
@@ -756,9 +756,13 @@ private void clearPendingFederationVoting() {
         provider.getFederationElection(constants.getFederationChangeAuthorizer()).clear();
     }
 
-    private void preserveFederationChangeInfo(Federation activeFederation) {
-        provider.setNextFederationCreationBlockHeight(rskExecutionBlock.getNumber());
+    private void saveFederationChangeInfo(long newActiveFederationCreationBlockHeight) {
+        saveLastRetiredFederationScript();
+        provider.setNextFederationCreationBlockHeight(newActiveFederationCreationBlockHeight);
+    }
 
+    private void saveLastRetiredFederationScript() {
+        Federation activeFederation = getActiveFederation();
         Script activeFederationMembersP2SHScript = getFederationMembersP2SHScript(activeFederation);
         provider.setLastRetiredFederationP2SHScript(activeFederationMembersP2SHScript);
     }

diff --git a/rskj-core/src/test/java/co/rsk/peg/federation/VoteFederationChangeTest.java b/rskj-core/src/test/java/co/rsk/peg/federation/VoteFederationChangeTest.java
@@ -415,7 +415,8 @@ void voteCommitFederation_postRSKIP186_preRSKIP419_whenPendingFederationIsSet_sh
 
         assertPendingFederationVotingWasCleaned();
 
-        assertFederationChangeInfoWasSet();
+        assertNewActiveFederationCreationBlockHeightWasSet();
+        assertLastRetiredFederationScriptWasSet();
 
         Federation oldFederation = storageProvider.getOldFederation(federationMainnetConstants, activations);
         Federation newFederation = storageProvider.getNewFederation(federationMainnetConstants, activations);
@@ -443,11 +444,11 @@ void voteCommitFederation_postRSKIP419_whenPendingFederationIsSet_shouldPerformC
 
         assertPendingFederationVotingWasCleaned();
 
-        assertFederationChangeInfoWasSet();
-
         assertLogCommitFederation(activeFederation, proposedFederation.get());
 
-        // assert new and old federation were not set and utxos were not moved
+        // assert some values were not set
+        assertNewActiveFederationCreationBlockHeightWasNotSet();
+        assertLastRetiredFederationScriptWasNotSet();
         assertNewAndOldFederationsWereNotSet();
         assertUTXOsWereNotMovedFromNewToOldFederation();
     }
@@ -509,19 +510,29 @@ private void assertPendingFederationVotingWasCleaned() {
         assertTrue(federationElectionVotes.isEmpty());
     }
 
-    private void assertFederationChangeInfoWasSet() {
-        // assert federation creation block height was set correctly
+    private void assertNewActiveFederationCreationBlockHeightWasSet() {
         Optional<Long> nextFederationCreationBlockHeight = storageProvider.getNextFederationCreationBlockHeight(activations);
         assertTrue(nextFederationCreationBlockHeight.isPresent());
         assertEquals(RSK_EXECUTION_BLOCK_NUMBER, nextFederationCreationBlockHeight.get());
+    }
+
+    private void assertNewActiveFederationCreationBlockHeightWasNotSet() {
+        Optional<Long> nextFederationCreationBlockHeight = storageProvider.getNextFederationCreationBlockHeight(activations);
+        assertFalse(nextFederationCreationBlockHeight.isPresent());
+    }
 
-        // assert last retired federation p2sh script was set correctly
+    private void assertLastRetiredFederationScriptWasSet() {
         Script activeFederationMembersP2SHScript = getFederationMembersP2SHScript(activations, federationSupport.getActiveFederation());
         Optional<Script> lastRetiredFederationP2SHScript = storageProvider.getLastRetiredFederationP2SHScript(activations);
         assertTrue(lastRetiredFederationP2SHScript.isPresent());
         assertEquals(activeFederationMembersP2SHScript, lastRetiredFederationP2SHScript.get());
     }
 
+    private void assertLastRetiredFederationScriptWasNotSet() {
+        Optional<Script> lastRetiredFederationP2SHScript = storageProvider.getLastRetiredFederationP2SHScript(activations);
+        assertFalse(lastRetiredFederationP2SHScript.isPresent());
+    }
+
     private Script getFederationMembersP2SHScript(ActivationConfig.ForBlock activations, Federation federation) {
         // when the federation is a standard multisig,
         // the members p2sh script is the p2sh script