chore: remove nextNodeId from config.txt (#15791)

Signed-off-by: Edward Wertz <[email protected]> Signed-off-by: Edward Wertz <[email protected]> Co-authored-by: anthony-swirldslabs <[email protected]>
hashgraph · Oct 8, 2024 · 3119e6c · 3119e6c
1 parent 14625b6
commit 3119e6c
Show file tree

Hide file tree

Showing 23 changed files with 64 additions and 391 deletions.
diff --git a/hedera-node/config.txt b/hedera-node/config.txt
@@ -37,8 +37,6 @@ app,		HederaNode.jar
 # address,   10, K, Kent,     1, 127.0.0.1, 50214, 127.0.0.1, 50214
 # address,   11, L, Lucy,     1, 127.0.0.1, 50215, 127.0.0.1, 50215
 
-nextNodeId, 1
-
 # The above addresses assume all are running on the same computer.
 # If multiple computers are being used, then the listed IP addresses should be changed.
 

diff --git a/hedera-node/configuration/compose/config.txt b/hedera-node/configuration/compose/config.txt
@@ -8,8 +8,6 @@ address, 1, B, Bob, 1, node_1, 50204, node_1, 50204, 0.0.4
 address, 2, C, Carol, 1, node_2, 50204, node_2, 50204, 0.0.5
 #address, 3, D, Dave, 1, 172.20.0.0, 50207, 70.240.247.13, 50207, 0.0.6
 
-nextNodeId, 3
-
 ###############################################################################################
 # The first line can be “swirld, “ and then a name for this swirld (shared world / ledger),
 # where the name is any string without commas, line breaks, or leading/trailing whitespace.

diff --git a/.../com/hedera/node/app/service/networkadmin/impl/handlers/ReadableFreezeUpgradeActions.java b/.../com/hedera/node/app/service/networkadmin/impl/handlers/ReadableFreezeUpgradeActions.java
@@ -17,7 +17,6 @@
 package com.hedera.node.app.service.networkadmin.impl.handlers;
 
 import static com.hedera.node.app.hapi.utils.CommonUtils.noThrowSha384HashOf;
-import static com.hedera.node.app.service.addressbook.AddressBookHelper.getNextNodeID;
 import static com.hedera.node.app.service.addressbook.AddressBookHelper.writeCertificatePemFile;
 import static com.swirlds.common.io.utility.FileUtils.getAbsolutePath;
 import static com.swirlds.common.utility.CommonUtils.nameToAlias;
@@ -231,10 +230,9 @@ private CompletableFuture<Void> extractNow(
         // we spin off a separate thread to avoid blocking handleTransaction
         // if we block handle, there could be a dramatic spike in E2E latency at the time of PREPARE_UPGRADE
         final var activeNodes = desc.equals(PREPARE_UPGRADE_DESC) ? allActiveNodes() : null;
-        final var nextNodeId = getNextNodeID(nodeStore);
         return runAsync(
                 () -> extractAndReplaceArtifacts(
-                        artifactsLoc, keysLoc, archiveData, size, desc, marker, now, activeNodes, nextNodeId),
+                        artifactsLoc, keysLoc, archiveData, size, desc, marker, now, activeNodes),
                 executor);
     }
 
@@ -259,8 +257,7 @@ private void extractAndReplaceArtifacts(
             @NonNull final String desc,
             @NonNull final String marker,
             @Nullable final Timestamp now,
-            @Nullable List<ActiveNode> nodes,
-            long nextNodeId) {
+            @Nullable final List<ActiveNode> nodes) {
         try {
             final var artifactsDir = artifactsLoc.toFile();
             final var keysDir = keysLoc.toFile();
@@ -275,7 +272,7 @@ private void extractAndReplaceArtifacts(
             UnzipUtility.unzip(archiveData.toByteArray(), artifactsLoc);
             log.info("Finished unzipping {} bytes for {} update into {}", size, desc, artifactsLoc);
             if (nodes != null && nodesConfig.enableDAB()) {
-                generateConfigPem(artifactsLoc, keysLoc, nodes, nextNodeId);
+                generateConfigPem(artifactsLoc, keysLoc, nodes);
                 log.info("Finished generating config.txt and pem files into {}", artifactsLoc);
             }
             writeSecondMarker(marker, now);
@@ -291,8 +288,7 @@ private void extractAndReplaceArtifacts(
     private void generateConfigPem(
             @NonNull final Path artifactsLoc,
             @NonNull final Path keysLoc,
-            @NonNull final List<ActiveNode> activeNodes,
-            long nextNodeId) {
+            @NonNull final List<ActiveNode> activeNodes) {
         requireNonNull(artifactsLoc, "Cannot generate config.txt without a valid artifacts location");
         requireNonNull(keysLoc, "Cannot generate pem files without a valid keys location");
         requireNonNull(activeNodes, "Cannot generate config.txt without a valid list of active nodes");
@@ -306,7 +302,6 @@ private void generateConfigPem(
         try (final var fw = new FileWriter(configTxt.toFile());
                 final var bw = new BufferedWriter(fw)) {
             activeNodes.forEach(node -> writeConfigLineAndPem(node, bw, keysLoc));
-            bw.write("nextNodeId, " + nextNodeId);
             bw.flush();
         } catch (final IOException e) {
             log.error("Failed to generate {} with exception : {}", configTxt, e);

diff --git a/...t/java/com/hedera/node/app/service/networkadmin/impl/test/handlers/FreezeHandlerTest.java b/...t/java/com/hedera/node/app/service/networkadmin/impl/test/handlers/FreezeHandlerTest.java
@@ -92,7 +92,7 @@ class FreezeHandlerTest {
     @Mock(strictness = LENIENT)
     private Account account;
 
-    @Mock
+    @Mock(strictness = LENIENT)
     private ReadableNodeStore nodeStore;
 
     @Mock

diff --git a/...ra/node/app/service/networkadmin/impl/test/handlers/ReadableFreezeUpgradeActionsTest.java b/...ra/node/app/service/networkadmin/impl/test/handlers/ReadableFreezeUpgradeActionsTest.java
@@ -512,7 +512,6 @@ private void assertDABFilesCreated(final String file, final Path baseDir, final
                 .append("address, 1, 1, node2, 5, 127.0.0.1, 1234, 35.186.191.247, 50211, 0.0.3\n")
                 .append("address, 2, 2, node3, 10, 127.0.0.2, 1245, 35.186.191.245, 50221, 0.0.4\n")
                 .append("address, 4, 4, node5, 20, 127.0.0.4, 1445, test.domain.com, 50225, 0.0.8\n")
-                .append("nextNodeId, 5")
                 .toString();
         final byte[] pemFile1Bytes = pemFile1.getEncoded();
         final byte[] pemFile2Bytes = pemFile2.getEncoded();
@@ -637,7 +636,6 @@ private void assertDABFilesCreated2(final String file, final Path baseDir, final
                 .append("address, 0, 0, node1, 5, 127.0.0.1, 1234, 35.186.191.247, 50211, 0.0.3\n")
                 .append("address, 1, 1, node2, 10, 127.0.0.2, 1245, 35.186.191.245, 50221, 0.0.4\n")
                 .append("address, 2, 2, node3, 20, 127.0.0.3, 1245, 35.186.191.235, 50221, 0.0.6\n")
-                .append("nextNodeId, 4")
                 .toString();
         final byte[] pemFile1Bytes = pemFile1.getEncoded();
         final byte[] pemFile2Bytes = pemFile2.getEncoded();

diff --git a/...st-clients/src/main/java/com/hedera/services/bdd/junit/hedera/utils/AddressBookUtils.java b/...st-clients/src/main/java/com/hedera/services/bdd/junit/hedera/utils/AddressBookUtils.java
@@ -82,10 +82,10 @@ public static String configTxtForLocal(
                     .append(", ")
                     .append("0.0.")
                     .append(node.getAccountId().accountNumOrThrow())
-                    .append("\n");
+                    .append('\n');
             maxNodeId = Math.max(node.getNodeId(), maxNodeId);
         }
-        sb.append("\nnextNodeId, ").append(maxNodeId + 1).append("\n");
+        sb.append('\n');
         return sb.toString();
     }
 

diff --git a/hedera-node/test-clients/src/main/resources/testfiles/config.txt b/hedera-node/test-clients/src/main/resources/testfiles/config.txt
@@ -38,8 +38,6 @@ app,		HederaNode.jar
 # address,  K, Kent,     1, 10.0.0.10, 50210, 10.0.0.10, 50210
 # address,  L, Lucy,     1, 10.0.1.37, 50211, 10.0.1.37, 50211
 
-nextNodeId, 1
-
 # The above addresses assume all are running on the same computer.
 # If multiple computers are being used, then the listed IP addresses should be changed.
 

diff --git a/platform-sdk/platform-apps/tests/AddressBookTestingTool/README.md b/platform-sdk/platform-apps/tests/AddressBookTestingTool/README.md
@@ -374,7 +374,6 @@ address,  1, B, Bob,      10, 127.0.0.1, 15302, 127.0.0.1, 15302
 address,  2, C, Carol,    10, 127.0.0.1, 15303, 127.0.0.1, 15303
 address,  3, D, Dave,     10, 127.0.0.1, 15304, 127.0.0.1, 15304
 # address,  4, E, Eric,     10, 127.0.0.1, 15305, 127.0.0.1, 15305
-nextNodeId, 4
 ```
 
 4. Run the app for 60 seconds
@@ -399,7 +398,6 @@ address,  1, B, Bob,      10, 127.0.0.1, 15302, 127.0.0.1, 15302
 address,  2, C, Carol,    10, 127.0.0.1, 15303, 127.0.0.1, 15303
 address,  3, D, Dave,     10, 127.0.0.1, 15304, 127.0.0.1, 15304
 address,  4, E, Eric,     10, 127.0.0.1, 15305, 127.0.0.1, 15305
-nextNodeId, 5
 ```
 
 8. Run the app for 60 seconds.
@@ -447,7 +445,6 @@ address,  1, B, Bob,      10, 127.0.0.1, 15302, 127.0.0.1, 15302
 address,  2, C, Carol,    10, 127.0.0.1, 15303, 127.0.0.1, 15303
 address,  3, D, Dave,     10, 127.0.0.1, 15304, 127.0.0.1, 15304
 address,  4, E, Eric,     10, 127.0.0.1, 15305, 127.0.0.1, 15305
-nextNodeId, 5
 ```
 
 4. Run the app for 60 seconds
@@ -472,7 +469,6 @@ address,  1, B, Bob,      10, 127.0.0.1, 15302, 127.0.0.1, 15302
 address,  2, C, Carol,    10, 127.0.0.1, 15303, 127.0.0.1, 15303
 address,  3, D, Dave,     10, 127.0.0.1, 15304, 127.0.0.1, 15304
 # address,  4, E, Eric,     10, 127.0.0.1, 15305, 127.0.0.1, 15305
-nextNodeId, 5
 ```
 
 8. Run the app for 60 seconds.

diff --git a/...okTestingTool/src/main/java/com/swirlds/demo/addressbook/AddressBookTestingToolState.java b/...okTestingTool/src/main/java/com/swirlds/demo/addressbook/AddressBookTestingToolState.java
@@ -674,14 +674,7 @@ private boolean removedNodeFromAddressBook(
                     "The new address book contains new nodes instead of only having nodes removed. {}",
                     StackTrace.getStackTrace());
         }
-        final boolean nextNodeIdEqual = newAddressBook.getNextNodeId().compareTo(oldAddressBook.getNextNodeId()) == 0;
-        if (!nextNodeIdEqual) {
-            logger.error(
-                    EXCEPTION.getMarker(),
-                    "The new address book has a different nextNodeId value than the old address book. {}",
-                    StackTrace.getStackTrace());
-        }
-        return sizesCorrespond && atLeastOneNodeRemoved && nextNodeIdEqual;
+        return sizesCorrespond && atLeastOneNodeRemoved;
     }
 
     /**
@@ -719,14 +712,7 @@ private boolean addedNodeToAddressBook(
                     "The new address book has nodes removed instead of only having nodes added. {}",
                     StackTrace.getStackTrace());
         }
-        final boolean nextNodeIdGreater = newAddressBook.getNextNodeId().compareTo(oldAddressBook.getNextNodeId()) > 0;
-        if (!nextNodeIdGreater) {
-            logger.error(
-                    EXCEPTION.getMarker(),
-                    "The new address book has a nextNodeId value that is not greater than the old address book. {}",
-                    StackTrace.getStackTrace());
-        }
-        return sizesCorrespond && atLeastOneNodeAdded && nextNodeIdGreater;
+        return sizesCorrespond && atLeastOneNodeAdded;
     }
 
     /**

diff --git a/platform-sdk/sdk/config.txt b/platform-sdk/sdk/config.txt
@@ -62,8 +62,6 @@ swirld, 123
 # address,  37, ], node],     1, 127.0.0.1, 15338, 127.0.0.1, 15338
 # address,  38, <, node<,     1, 127.0.0.1, 15339, 127.0.0.1, 15339
 
-nextNodeId, 4
-
 # The above addresses assume all are running on the same computer.
 # If multiple computers are being used, then the listed IP addresses should be changed.
 

diff --git a/...m-core/src/main/java/com/swirlds/platform/config/legacy/LegacyConfigPropertiesLoader.java b/...m-core/src/main/java/com/swirlds/platform/config/legacy/LegacyConfigPropertiesLoader.java
@@ -18,7 +18,6 @@
 
 import static com.swirlds.logging.legacy.LogMarker.EXCEPTION;
 
-import com.swirlds.common.platform.NodeId;
 import com.swirlds.common.utility.CommonUtils;
 import com.swirlds.platform.system.address.Address;
 import com.swirlds.platform.system.address.AddressBook;
@@ -50,8 +49,6 @@ public final class LegacyConfigPropertiesLoader {
 
     private static final String APP_PROPERTY_NAME = "app";
     private static final String ADDRESS_PROPERTY_NAME = "address";
-    private static final String NEXT_NODE_ID_PROPERTY_NAME = "nextNodeId";
-    private static final String NEXT_NODE_ID_PROPERTY_NAME_LOWERCASE = "nextnodeid";
     private static final String SWIRLD_PROPERTY_NAME = "swirld";
 
     public static final String ERROR_CONFIG_TXT_NOT_FOUND_BUT_EXISTS =
@@ -85,7 +82,6 @@ public static LegacyConfigProperties loadConfigFile(@NonNull final Path configPa
 
         try (final Scanner scanner = new Scanner(configPath, StandardCharsets.UTF_8)) {
             final AddressBook addressBook = new AddressBook();
-            boolean nextNodeIdParsed = false;
             long lineNumber = 0;
             while (scanner.hasNextLine()) {
                 final String line = readNextLine(scanner);
@@ -129,30 +125,10 @@ public static LegacyConfigProperties loadConfigFile(@NonNull final Path configPa
                                 onError(ERROR_ADDRESS_NOT_ENOUGH_PARAMETERS);
                             }
                         }
-                        case NEXT_NODE_ID_PROPERTY_NAME_LOWERCASE -> {
-                            try {
-                                if (!parsOriginalCase[0].equals(AddressBookUtils.NEXT_NODE_ID_KEYWORD)) {
-                                    onError(ERROR_PROPERTY_NOT_KNOWN.formatted(pars[0]));
-                                } else {
-                                    final NodeId nextNodeId = new NodeId(Long.parseLong(pars[1]));
-                                    if (nextNodeId.compareTo(addressBook.getNextNodeId()) < 0) {
-                                        onError(ERROR_NEXT_NODE_NOT_GREATER_THAN_HIGHEST_ADDRESS);
-                                    }
-                                    addressBook.setNextNodeId(nextNodeId);
-                                    nextNodeIdParsed = true;
-                                }
-                            } catch (final NumberFormatException ex) {
-                                onError(ERROR_NO_PARAMETER.formatted(NEXT_NODE_ID_PROPERTY_NAME));
-                            }
-                        }
                         default -> onError(ERROR_PROPERTY_NOT_KNOWN.formatted(pars[0]));
                     }
                 }
             }
-            if (!nextNodeIdParsed) {
-                onError(ERROR_NO_PARAMETER.formatted(NEXT_NODE_ID_PROPERTY_NAME));
-                throw new ConfigurationException("config.txt did not have a `nextNodeId` property. (Case Sensitive)");
-            }
             if (addressBook.getSize() > 0) {
                 configurationProperties.setAddressBook(addressBook);
             }

diff --git a/.../swirlds-platform-core/src/main/java/com/swirlds/platform/system/address/AddressBook.java b/.../swirlds-platform-core/src/main/java/com/swirlds/platform/system/address/AddressBook.java
@@ -92,6 +92,10 @@ private static class ClassVersion {
     private long round = UNKNOWN_ROUND;
 
     /**
+     * DEPRECATED FIELD as of v0.56.0 It remains for compatibility with protobuf serialization.  Its value will always
+     * be > the highest node ID of address in the address book, but constraints will no longer be checked when changing
+     * address books.
+     * <p>
      * The node ID of the next address that can be added must be greater than or equal to this value.
      * <p>
      * INVARIANT: that nextNodeId is greater than the node ids of all addresses in the address book.
@@ -153,6 +157,7 @@ private AddressBook(@NonNull final AddressBook that) {
         }
 
         this.round = that.round;
+        // the next node id is deprecated, but the value is copied here for compatibility with protobuf serialization
         this.nextNodeId = that.nextNodeId;
     }
 
@@ -282,16 +287,35 @@ public int getIndexOfNodeId(@NonNull final NodeId id) {
     }
 
     /**
-     * Get the next available node ID. When adding a new address, it must always have a node ID equal to this value.
+     *  @return the next available node id for use in the address book.
+     */
+    public NodeId getNextAvailableNodeId() {
+        return getSize() == 0 ? NodeId.FIRST_NODE_ID : getNodeId(getSize() - 1).getOffset(1);
+    }
+
+    /**
+     * This method and its internal field are deprecated and no longer supported. The existence of this method and its
+     * internal field remains for compatibility with protobuf serialization.  The returned value is either the last
+     * value set by {@link #setNextNodeId(NodeId)} or the result of increasing the value to 1+ the node id of a new
+     * address when adding a new address to the address book.  The comments below reflect the old usage
+     * of the method.
+     * <p>
+     * Get the next available node ID.
      *
      * @return the next available node ID
      */
+    @Deprecated(forRemoval = true, since = "0.56.0")
     @NonNull
     public NodeId getNextNodeId() {
         return nextNodeId;
     }
 
     /**
+     * This method and its internal field are deprecated and no longer supported. The set value updates the internal
+     * field, but is no longer checked against other data in the address book.  The existence of this method and its
+     * internal field remains for compatibility with protobuf serialization.  The comments below reflect the old usage
+     * of the method and are no longer accurate.
+     *
      * <p>
      * Set the expected next node ID to be added to this address book.
      * </p>
@@ -306,16 +330,9 @@ public NodeId getNextNodeId() {
      * @param newNextNodeId the next node ID for the address book
      * @return this object
      */
+    @Deprecated(forRemoval = true, since = "0.56.0")
     @NonNull
     public AddressBook setNextNodeId(@NonNull final NodeId newNextNodeId) {
-        Objects.requireNonNull(newNextNodeId);
-        if (newNextNodeId.compareTo(this.nextNodeId) < 0) {
-            throw new IllegalArgumentException("The provided nextNodeId %s is less than the current nextNodeId %s"
-                    .formatted(newNextNodeId, this.nextNodeId));
-        }
-        // the newNextNodeId is greater than all address node ids in the address book is true because it is
-        // greater than or equal to the current nextNodeId which is greater than all address node ids.
-
         this.nextNodeId = newNextNodeId;
         return this;
     }
@@ -423,24 +440,26 @@ private void updateAddress(@NonNull final Address address) {
     }
 
     /**
-     * Add a new address.
+     * Add a new address at the end of the address book.
      *
      * @param address the address to add
      */
     private void addNewAddress(@NonNull final Address address) {
         final NodeId addressNodeId = address.getNodeId();
-        final int nodeIdComparison = addressNodeId.compareTo(nextNodeId);
+        final int addressBookSize = getSize();
+        final NodeId nextAvailable = addressBookSize == 0
+                ? NodeId.FIRST_NODE_ID
+                : getNodeId(getSize() - 1).getOffset(1);
+        final int nodeIdComparison = addressNodeId.compareTo(nextAvailable);
         if (nodeIdComparison < 0) {
             throw new IllegalArgumentException("Can not add address with node ID " + address.getNodeId()
                     + ", the next address to be added is required have a node ID greater or equal to "
-                    + nextNodeId);
+                    + nextAvailable);
         }
         if (addresses.size() >= MAX_ADDRESSES) {
             throw new IllegalStateException("Address book is only permitted to hold " + MAX_ADDRESSES + " entries");
         }
 
-        nextNodeId = addressNodeId.getOffset(1);
-
         addresses.put(address.getNodeId(), address);
         publicKeyToId.put(address.getNickname(), address.getNodeId());
         addToOrderedList(address.getNodeId());
@@ -454,10 +473,8 @@ private void addNewAddress(@NonNull final Address address) {
     /**
      * Add an address to the address book, replacing the existing address with the same ID if one is present.
      *
-     * @param address the address for that member, may not be null, must have a node ID greater or equal to
-     *                {@link #nextNodeId} if the address is not currently in the address book
+     * @param address the address for that member, may not be null
      * @return this object
-     * @throws IllegalStateException if a new address is added that has a node ID that is less than {@link #nextNodeId}
      */
     @NonNull
     public AddressBook add(@NonNull final Address address) {
@@ -603,9 +620,8 @@ public boolean equals(final Object o) {
         }
 
         final AddressBook that = (AddressBook) o;
-        return Objects.equals(addresses, that.addresses)
-                && getRound() == that.getRound()
-                && Objects.equals(getNextNodeId(), that.getNextNodeId());
+        return Objects.equals(addresses, that.addresses) && getRound() == that.getRound();
+        // The nextNodeId field has been removed from the equality check
     }
 
     /**