From 0fede296592a28e116c52cd5d78bf248f09d7c5a Mon Sep 17 00:00:00 2001 From: Julian Len Date: Thu, 2 Jan 2025 13:21:42 -0300 Subject: [PATCH 1/4] feat: wrapped the variable shouldSave.. and federation into a tracker class --- .../FederationStorageProviderImpl.java | 48 ++++++++----------- .../rsk/peg/federation/FederationTracker.java | 27 +++++++++++ .../peg/federation/FederationTrackerImpl.java | 4 ++ .../PendingFederationTrackerImpl.java | 4 ++ 4 files changed, 56 insertions(+), 27 deletions(-) create mode 100644 rskj-core/src/main/java/co/rsk/peg/federation/FederationTracker.java create mode 100644 rskj-core/src/main/java/co/rsk/peg/federation/FederationTrackerImpl.java create mode 100644 rskj-core/src/main/java/co/rsk/peg/federation/PendingFederationTrackerImpl.java 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 index a4c44058834..5c8b6e3e0ba 100644 --- 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> storageVersionEntries; - private List newFederationBtcUTXOs; private List oldFederationBtcUTXOs; private Federation newFederation; - private Federation oldFederation; - private boolean shouldSaveOldFederation = false; - - private PendingFederation pendingFederation; - private boolean shouldSavePendingFederation = false; - + private final FederationTracker oldFederationTracker = new FederationTrackerImpl(); + private final FederationTracker 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.isPresent() || oldFederationTracker.isModified()) { + return oldFederationTracker.get(); } Optional 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 } ); - return oldFederation; + oldFederationTracker.replace(oldFederation); + return oldFederationTracker.get(); } @Override public void setOldFederation(Federation federation) { - shouldSaveOldFederation = true; - oldFederation = federation; + oldFederationTracker.setNew(federation); } @Override public PendingFederation getPendingFederation() { - if (pendingFederation != null || shouldSavePendingFederation) { - return pendingFederation; + if (pendingFederationTracker.isPresent() || pendingFederationTracker.isModified()) { + return pendingFederationTracker.get(); } Optional 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() { } ); - return pendingFederation; + pendingFederationTracker.replace(pendingFederation); + return pendingFederationTracker.get(); } @Override public void setPendingFederation(PendingFederation federation) { - shouldSavePendingFederation = true; - pendingFederation = federation; + pendingFederationTracker.setNew(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.get(); 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.isPresent()) { // assume it is a standard federation to keep backwards compatibility return STANDARD_MULTISIG_FEDERATION.getFormatVersion(); } - return oldFederation.getFormatVersion(); + return oldFederationTracker.get().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.isPresent()) { return null; } - return pendingFederation.serialize(activations); + return pendingFederationTracker.get().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 new file mode 100644 index 00000000000..3eb4c536dbc --- /dev/null +++ b/rskj-core/src/main/java/co/rsk/peg/federation/FederationTracker.java @@ -0,0 +1,27 @@ +package co.rsk.peg.federation; + +public abstract class FederationTracker { + private T federation; + private boolean modified = false; + + public boolean isModified() { + return this.modified; + } + + public T get() { + return this.federation; + } + + public boolean isPresent() { + return this.federation != null; + } + + public void setNew(T aFederation) { + this.federation = aFederation; + this.modified = true; + } + + public void replace(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 new file mode 100644 index 00000000000..121f396b67e --- /dev/null +++ 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 { +} 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 new file mode 100644 index 00000000000..9061a885b61 --- /dev/null +++ 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 { +} From d96f79110c87fa719cd12a5532c9efe09759d4c6 Mon Sep 17 00:00:00 2001 From: Julian Len Date: Fri, 3 Jan 2025 12:14:31 -0300 Subject: [PATCH 2/4] fix: improved the changes in FederationStorageProvider --- .../co/rsk/peg/federation/FederationStorageProviderImpl.java | 4 ++-- .../main/java/co/rsk/peg/federation/FederationTracker.java | 2 +- .../java/co/rsk/peg/federation/FederationTrackerImpl.java | 4 ---- .../co/rsk/peg/federation/PendingFederationTrackerImpl.java | 4 ---- 4 files changed, 3 insertions(+), 11 deletions(-) delete mode 100644 rskj-core/src/main/java/co/rsk/peg/federation/FederationTrackerImpl.java delete mode 100644 rskj-core/src/main/java/co/rsk/peg/federation/PendingFederationTrackerImpl.java 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 index 5c8b6e3e0ba..8b5b498ef97 100644 --- 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,11 +23,11 @@ public class FederationStorageProviderImpl implements FederationStorageProvider { private final StorageAccessor bridgeStorageAccessor; private final HashMap> storageVersionEntries; + private final FederationTracker oldFederationTracker = new FederationTracker<>(); + private final FederationTracker pendingFederationTracker = new FederationTracker<>(); private List newFederationBtcUTXOs; private List oldFederationBtcUTXOs; private Federation newFederation; - private final FederationTracker oldFederationTracker = new FederationTrackerImpl(); - private final FederationTracker pendingFederationTracker = new PendingFederationTrackerImpl(); private ABICallElection federationElection; private Long activeFederationCreationBlockHeight; private Long nextFederationCreationBlockHeight; // if -1, then clear value 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 index 3eb4c536dbc..cb6a7392b8d 100644 --- a/rskj-core/src/main/java/co/rsk/peg/federation/FederationTracker.java +++ b/rskj-core/src/main/java/co/rsk/peg/federation/FederationTracker.java @@ -1,6 +1,6 @@ package co.rsk.peg.federation; -public abstract class FederationTracker { +public class FederationTracker { private T federation; private boolean modified = false; 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 deleted file mode 100644 index 121f396b67e..00000000000 --- a/rskj-core/src/main/java/co/rsk/peg/federation/FederationTrackerImpl.java +++ /dev/null @@ -1,4 +0,0 @@ -package co.rsk.peg.federation; - -public class FederationTrackerImpl extends FederationTracker { -} 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 deleted file mode 100644 index 9061a885b61..00000000000 --- a/rskj-core/src/main/java/co/rsk/peg/federation/PendingFederationTrackerImpl.java +++ /dev/null @@ -1,4 +0,0 @@ -package co.rsk.peg.federation; - -public class PendingFederationTrackerImpl extends FederationTracker { -} From 511db0238b37958f4b3668e5087591c32accf347 Mon Sep 17 00:00:00 2001 From: Julian Len Date: Fri, 3 Jan 2025 12:34:38 -0300 Subject: [PATCH 3/4] feat: move the logic to determine if there is a federation present to the tracker, also merged setNew and replace into the same method using a parameter to clarify if it requires being saved or not --- .../federation/FederationStorageProviderImpl.java | 12 ++++++------ .../co/rsk/peg/federation/FederationTracker.java | 8 ++++---- 2 files changed, 10 insertions(+), 10 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 index 8b5b498ef97..86dad0d5bc0 100644 --- a/rskj-core/src/main/java/co/rsk/peg/federation/FederationStorageProviderImpl.java +++ b/rskj-core/src/main/java/co/rsk/peg/federation/FederationStorageProviderImpl.java @@ -139,7 +139,7 @@ public void setNewFederation(Federation federation) { @Override public Federation getOldFederation(FederationConstants federationConstants, ActivationConfig.ForBlock activations) { - if (oldFederationTracker.isPresent() || oldFederationTracker.isModified()) { + if (oldFederationTracker.hasBeenSet()) { return oldFederationTracker.get(); } @@ -159,18 +159,18 @@ public Federation getOldFederation(FederationConstants federationConstants, Acti } ); - oldFederationTracker.replace(oldFederation); + oldFederationTracker.set(oldFederation, false); return oldFederationTracker.get(); } @Override public void setOldFederation(Federation federation) { - oldFederationTracker.setNew(federation); + oldFederationTracker.set(federation, true); } @Override public PendingFederation getPendingFederation() { - if (pendingFederationTracker.isPresent() || pendingFederationTracker.isModified()) { + if (pendingFederationTracker.hasBeenSet()) { return pendingFederationTracker.get(); } @@ -190,13 +190,13 @@ public PendingFederation getPendingFederation() { } ); - pendingFederationTracker.replace(pendingFederation); + pendingFederationTracker.set(pendingFederation, false); return pendingFederationTracker.get(); } @Override public void setPendingFederation(PendingFederation federation) { - pendingFederationTracker.setNew(federation); + pendingFederationTracker.set(federation, true); } @Override 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 index cb6a7392b8d..d1ab1daa9e0 100644 --- a/rskj-core/src/main/java/co/rsk/peg/federation/FederationTracker.java +++ b/rskj-core/src/main/java/co/rsk/peg/federation/FederationTracker.java @@ -16,12 +16,12 @@ public boolean isPresent() { return this.federation != null; } - public void setNew(T aFederation) { - this.federation = aFederation; - this.modified = true; + public boolean hasBeenSet() { + return this.isPresent() || this.isModified(); } - public void replace(T aFederation) { + public void set(T aFederation, boolean shouldSave) { this.federation = aFederation; + this.modified = shouldSave; } } From 1599b5c8bf2f3b02ada54593eee0090ecac1785f Mon Sep 17 00:00:00 2001 From: Julian Len Date: Thu, 9 Jan 2025 18:24:51 -0300 Subject: [PATCH 4/4] feat: changed the tracker for a more generic class --- .../FederationStorageProviderImpl.java | 20 ++++++------ .../rsk/peg/federation/FederationTracker.java | 27 ---------------- .../co/rsk/peg/federation/ValueTracker.java | 31 +++++++++++++++++++ 3 files changed, 41 insertions(+), 37 deletions(-) delete mode 100644 rskj-core/src/main/java/co/rsk/peg/federation/FederationTracker.java create mode 100644 rskj-core/src/main/java/co/rsk/peg/federation/ValueTracker.java 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 index 86dad0d5bc0..ad9356a7833 100644 --- 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,8 +23,8 @@ public class FederationStorageProviderImpl implements FederationStorageProvider { private final StorageAccessor bridgeStorageAccessor; private final HashMap> storageVersionEntries; - private final FederationTracker oldFederationTracker = new FederationTracker<>(); - private final FederationTracker pendingFederationTracker = new FederationTracker<>(); + private final ValueTracker oldFederationTracker = new ValueTracker<>(); + private final ValueTracker pendingFederationTracker = new ValueTracker<>(); private List newFederationBtcUTXOs; private List oldFederationBtcUTXOs; private Federation newFederation; @@ -139,7 +139,7 @@ public void setNewFederation(Federation federation) { @Override public Federation getOldFederation(FederationConstants federationConstants, ActivationConfig.ForBlock activations) { - if (oldFederationTracker.hasBeenSet()) { + if (oldFederationTracker.isPresent()) { return oldFederationTracker.get(); } @@ -159,18 +159,18 @@ public Federation getOldFederation(FederationConstants federationConstants, Acti } ); - oldFederationTracker.set(oldFederation, false); + oldFederationTracker.set(oldFederation); return oldFederationTracker.get(); } @Override public void setOldFederation(Federation federation) { - oldFederationTracker.set(federation, true); + oldFederationTracker.setNew(federation); } @Override public PendingFederation getPendingFederation() { - if (pendingFederationTracker.hasBeenSet()) { + if (pendingFederationTracker.isPresent()) { return pendingFederationTracker.get(); } @@ -190,13 +190,13 @@ public PendingFederation getPendingFederation() { } ); - pendingFederationTracker.set(pendingFederation, false); + pendingFederationTracker.set(pendingFederation); return pendingFederationTracker.get(); } @Override public void setPendingFederation(PendingFederation federation) { - pendingFederationTracker.set(federation, true); + pendingFederationTracker.setNew(federation); } @Override @@ -338,7 +338,7 @@ private void saveOldFederation(ActivationConfig.ForBlock activations) { } private int getOldFederationFormatVersion() { - if (!oldFederationTracker.isPresent()) { + if (oldFederationTracker.isNull()) { // assume it is a standard federation to keep backwards compatibility return STANDARD_MULTISIG_FEDERATION.getFormatVersion(); } @@ -363,7 +363,7 @@ private void savePendingFederation(ActivationConfig.ForBlock activations) { @Nullable private byte[] serializePendingFederation(ActivationConfig.ForBlock activations) { - if (!pendingFederationTracker.isPresent()) { + if (pendingFederationTracker.isNull()) { return null; } 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 deleted file mode 100644 index d1ab1daa9e0..00000000000 --- a/rskj-core/src/main/java/co/rsk/peg/federation/FederationTracker.java +++ /dev/null @@ -1,27 +0,0 @@ -package co.rsk.peg.federation; - -public class FederationTracker { - private T federation; - private boolean modified = false; - - public boolean isModified() { - return this.modified; - } - - public T get() { - return this.federation; - } - - public boolean isPresent() { - return this.federation != null; - } - - public boolean hasBeenSet() { - return this.isPresent() || this.isModified(); - } - - public void set(T aFederation, boolean shouldSave) { - this.federation = aFederation; - this.modified = shouldSave; - } -} diff --git a/rskj-core/src/main/java/co/rsk/peg/federation/ValueTracker.java b/rskj-core/src/main/java/co/rsk/peg/federation/ValueTracker.java new file mode 100644 index 00000000000..6403c8dd2c7 --- /dev/null +++ b/rskj-core/src/main/java/co/rsk/peg/federation/ValueTracker.java @@ -0,0 +1,31 @@ +package co.rsk.peg.federation; + +public class ValueTracker { + private T value; + private boolean modified = false; + + public boolean isModified() { + return this.modified; + } + + public T get() { + return this.value; + } + + public boolean isPresent() { + return !isNull() || this.isModified(); + } + + public boolean isNull() { + return this.value == null; + } + + public void setNew(T aValue) { + this.value = aValue; + this.modified = true; + } + + public void set(T aValue) { + this.value = aValue; + } +}