feat: wrapped the variable shouldSave.. and federation into a tracker…

… class
rsksmart · Jan 2, 2025 · 0bfc14b · 0bfc14b
1 parent 87bde23
commit 0bfc14b
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 25 deletions.
diff --git a/rskj-core/src/main/java/co/rsk/peg/federation/FederationStorageProviderImpl.java b/rskj-core/src/main/java/co/rsk/peg/federation/FederationStorageProviderImpl.java
@@ -23,21 +23,14 @@
 public class FederationStorageProviderImpl implements FederationStorageProvider {
     private final StorageAccessor bridgeStorageAccessor;
     private final HashMap<DataWord, Optional<Integer>> storageVersionEntries;
-
     private List<UTXO> newFederationBtcUTXOs;
     private List<UTXO> oldFederationBtcUTXOs;
     private Federation newFederation;
-    private Federation oldFederation;
-    private boolean shouldSaveOldFederation = false;
-
-    private PendingFederation pendingFederation;
-    private boolean shouldSavePendingFederation = false;
-
+    private final FederationTracker<Federation> oldFederationTracker = new FederationTrackerImpl();
+    private final FederationTracker<PendingFederation> pendingFederationTracker = new PendingFederationTrackerImpl();
     private ABICallElection federationElection;
-
     private Long activeFederationCreationBlockHeight;
     private Long nextFederationCreationBlockHeight; // if -1, then clear value
-
     private Script lastRetiredFederationP2SHScript;
 
     public FederationStorageProviderImpl(StorageAccessor bridgeStorageAccessor) {
@@ -146,13 +139,13 @@ public void setNewFederation(Federation federation) {
 
     @Override
     public Federation getOldFederation(FederationConstants federationConstants, ActivationConfig.ForBlock activations) {
-        if (oldFederation != null || shouldSaveOldFederation) {
-            return oldFederation;
+        if (oldFederationTracker.isFederationPresent() || oldFederationTracker.isModified()) {
+            return oldFederationTracker.getFederation();
         }
 
         Optional<Integer> storageVersion = getStorageVersion(OLD_FEDERATION_FORMAT_VERSION.getKey());
 
-        oldFederation = bridgeStorageAccessor.getFromRepository(
+        Federation oldFederation = bridgeStorageAccessor.getFromRepository(
             OLD_FEDERATION_KEY.getKey(),
             data -> {
                 if (data == null) {
@@ -166,24 +159,24 @@ public Federation getOldFederation(FederationConstants federationConstants, Acti
             }
         );
 
+        oldFederationTracker.replaceFederation(oldFederation);
         return oldFederation;
     }
 
     @Override
     public void setOldFederation(Federation federation) {
-        shouldSaveOldFederation = true;
-        oldFederation = federation;
+        oldFederationTracker.setFederationWithTracking(federation);
     }
 
     @Override
     public PendingFederation getPendingFederation() {
-        if (pendingFederation != null || shouldSavePendingFederation) {
-            return pendingFederation;
+        if (pendingFederationTracker.isFederationPresent() || pendingFederationTracker.isModified()) {
+            return pendingFederationTracker.getFederation();
         }
 
         Optional<Integer> storageVersion = getStorageVersion(PENDING_FEDERATION_FORMAT_VERSION.getKey());
 
-        pendingFederation =
+        PendingFederation pendingFederation =
             bridgeStorageAccessor.getFromRepository(PENDING_FEDERATION_KEY.getKey(),
                 data -> {
                     if (data == null) {
@@ -197,13 +190,13 @@ public PendingFederation getPendingFederation() {
                 }
             );
 
+        pendingFederationTracker.replaceFederation(pendingFederation);
         return pendingFederation;
     }
 
     @Override
     public void setPendingFederation(PendingFederation federation) {
-        shouldSavePendingFederation = true;
-        pendingFederation = federation;
+        pendingFederationTracker.setFederationWithTracking(federation);
     }
 
     @Override
@@ -329,10 +322,11 @@ private void saveNewFederation(ActivationConfig.ForBlock activations) {
     }
 
     private void saveOldFederation(ActivationConfig.ForBlock activations) {
-        if (!shouldSaveOldFederation) {
+        if (!oldFederationTracker.isModified()) {
             return;
         }
 
+        Federation oldFederation = oldFederationTracker.getFederation();
         if (!activations.isActive(RSKIP123)) {
             bridgeStorageAccessor.saveToRepository(OLD_FEDERATION_KEY.getKey(), oldFederation, BridgeSerializationUtils::serializeFederationOnlyBtcKeys);
             return;
@@ -344,16 +338,16 @@ private void saveOldFederation(ActivationConfig.ForBlock activations) {
     }
 
     private int getOldFederationFormatVersion() {
-        if (oldFederation == null) {
+        if (!oldFederationTracker.isFederationPresent()) {
             // assume it is a standard federation to keep backwards compatibility
             return STANDARD_MULTISIG_FEDERATION.getFormatVersion();
         }
 
-        return oldFederation.getFormatVersion();
+        return oldFederationTracker.getFederation().getFormatVersion();
     }
 
     private void savePendingFederation(ActivationConfig.ForBlock activations) {
-        if (!shouldSavePendingFederation) {
+        if (!pendingFederationTracker.isModified()) {
             return;
         }
 
@@ -369,11 +363,11 @@ private void savePendingFederation(ActivationConfig.ForBlock activations) {
 
     @Nullable
     private byte[] serializePendingFederation(ActivationConfig.ForBlock activations) {
-        if (pendingFederation == null) {
+        if (!pendingFederationTracker.isFederationPresent()) {
             return null;
         }
 
-        return pendingFederation.serialize(activations);
+        return pendingFederationTracker.getFederation().serialize(activations);
     }
 
     private void saveFederationFormatVersion(DataWord versionKey, Integer version) {

diff --git a/rskj-core/src/main/java/co/rsk/peg/federation/FederationTracker.java b/rskj-core/src/main/java/co/rsk/peg/federation/FederationTracker.java
@@ -0,0 +1,25 @@
+package co.rsk.peg.federation;
+
+public abstract class FederationTracker<T> {
+    private T federation;
+    private boolean modified = false;
+    public boolean isModified() {
+        return this.modified;
+    }
+
+    public T getFederation() {
+        return this.federation;
+    }
+    public boolean isFederationPresent() {
+        return this.federation != null;
+    }
+
+    public void setFederationWithTracking(T aFederation) {
+        this.federation = aFederation;
+        this.modified = true;
+    }
+
+    public void replaceFederation(T aFederation) {
+        this.federation = aFederation;
+    }
+}
diff --git a/rskj-core/src/main/java/co/rsk/peg/federation/FederationTrackerImpl.java b/rskj-core/src/main/java/co/rsk/peg/federation/FederationTrackerImpl.java
@@ -0,0 +1,4 @@
+package co.rsk.peg.federation;
+
+public class FederationTrackerImpl extends FederationTracker<Federation> {
+}
diff --git a/rskj-core/src/main/java/co/rsk/peg/federation/PendingFederationTrackerImpl.java b/rskj-core/src/main/java/co/rsk/peg/federation/PendingFederationTrackerImpl.java
@@ -0,0 +1,4 @@
+package co.rsk.peg.federation;
+
+public class PendingFederationTrackerImpl extends FederationTracker<PendingFederation> {
+}