Do not set active federation creation block height before validation

rsksmart · Nov 19, 2024 · 80b0d63 · 80b0d63
1 parent 6983bb2
commit 80b0d63
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 14 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,8 +702,9 @@ private FederationChangeResponseCode legacyCommitPendingFederation(PendingFedera
         clearPendingFederationVoting();
 
         if (activations.isActive(RSKIP186)) {
-            setNewActiveFederationCreationBlockHeight();
-            preserveLastRetiredFederationScript();
+            // 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);
@@ -734,7 +735,6 @@ private FederationChangeResponseCode commitPendingFederation(PendingFederation c
         // set proposed federation
         Federation proposedFederation = buildFederationFromPendingFederation(currentPendingFederation);
         provider.setProposedFederation(proposedFederation);
-        setNewActiveFederationCreationBlockHeight();
 
         clearPendingFederationVoting();
 
@@ -756,19 +756,17 @@ private void clearPendingFederationVoting() {
         provider.getFederationElection(constants.getFederationChangeAuthorizer()).clear();
     }
 
-    private void preserveLastRetiredFederationScript() {
+    private void saveFederationChangeInfo(long newActiveFederationCreationBlockHeight) {
+        saveLastRetiredFederationScript();
+        provider.setNextFederationCreationBlockHeight(newActiveFederationCreationBlockHeight);
+    }
+
+    private void saveLastRetiredFederationScript() {
         Federation activeFederation = getActiveFederation();
         Script activeFederationMembersP2SHScript = getFederationMembersP2SHScript(activeFederation);
         provider.setLastRetiredFederationP2SHScript(activeFederationMembersP2SHScript);
     }
 
-    private void setNewActiveFederationCreationBlockHeight() {
-        // since we are creating the to-be-active-fed in this block,
-        // its creation block height is this block number
-        long newActiveFederationCreationBlockHeight = rskExecutionBlock.getNumber();
-        provider.setNextFederationCreationBlockHeight(newActiveFederationCreationBlockHeight);
-    }
-
     private Script getFederationMembersP2SHScript(Federation federation) {
         // when the federation is a standard multisig,
         // the members p2sh script is the p2sh script

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
@@ -444,11 +444,11 @@ void voteCommitFederation_postRSKIP419_whenPendingFederationIsSet_shouldPerformC
 
         assertPendingFederationVotingWasCleaned();
 
-        assertNewActiveFederationCreationBlockHeightWasSet();
-
         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();
     }
@@ -516,13 +516,23 @@ private void assertNewActiveFederationCreationBlockHeightWasSet() {
         assertEquals(RSK_EXECUTION_BLOCK_NUMBER, nextFederationCreationBlockHeight.get());
     }
 
+    private void assertNewActiveFederationCreationBlockHeightWasNotSet() {
+        Optional<Long> nextFederationCreationBlockHeight = storageProvider.getNextFederationCreationBlockHeight(activations);
+        assertFalse(nextFederationCreationBlockHeight.isPresent());
+    }
+
     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