fix: fixed a few issues on Dynamic Address Book (#14316)

Signed-off-by: Iris Simon <[email protected]>
hashgraph · Jul 22, 2024 · ebd210e · ebd210e
1 parent 7bcc875
commit ebd210e
Show file tree

Hide file tree

Showing 23 changed files with 282 additions and 128 deletions.
diff --git a/...pl/src/main/java/com/hedera/node/app/service/addressbook/impl/AddressBookServiceImpl.java b/...pl/src/main/java/com/hedera/node/app/service/addressbook/impl/AddressBookServiceImpl.java
@@ -26,7 +26,6 @@
  * Standard implementation of the {@link AddressBookService} {@link RpcService}.
  */
 public final class AddressBookServiceImpl implements AddressBookService {
-    public static final String NODES_KEY = "NODES";
 
     @Override
     public void registerSchemas(@NonNull SchemaRegistry registry) {

diff --git a/...mpl/src/main/java/com/hedera/node/app/service/addressbook/impl/ReadableNodeStoreImpl.java b/...mpl/src/main/java/com/hedera/node/app/service/addressbook/impl/ReadableNodeStoreImpl.java
@@ -16,7 +16,7 @@
 
 package com.hedera.node.app.service.addressbook.impl;
 
-import static com.hedera.node.app.service.addressbook.impl.AddressBookServiceImpl.NODES_KEY;
+import static com.hedera.node.app.service.addressbook.AddressBookHelper.NODES_KEY;
 import static java.util.Objects.requireNonNull;
 
 import com.hedera.hapi.node.state.addressbook.Node;

diff --git a/...ce-impl/src/main/java/com/hedera/node/app/service/addressbook/impl/WritableNodeStore.java b/...ce-impl/src/main/java/com/hedera/node/app/service/addressbook/impl/WritableNodeStore.java
@@ -27,7 +27,6 @@
 import com.swirlds.state.spi.WritableKVState;
 import com.swirlds.state.spi.WritableStates;
 import edu.umd.cs.findbugs.annotations.NonNull;
-import java.util.Iterator;
 import java.util.Set;
 
 /**
@@ -98,14 +97,4 @@ public long sizeOfState() {
     public Set<EntityNumber> modifiedNodes() {
         return nodesState().modifiedKeys();
     }
-
-    /**
-     * Returns an iterator over the keys in the state.
-     * @return
-     */
-    @Override
-    @NonNull
-    public Iterator<EntityNumber> keys() {
-        return nodesState().keys();
-    }
 }
diff --git a/...rc/main/java/com/hedera/node/app/service/addressbook/impl/handlers/NodeCreateHandler.java b/...rc/main/java/com/hedera/node/app/service/addressbook/impl/handlers/NodeCreateHandler.java
@@ -22,6 +22,7 @@
 import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_NODE_ACCOUNT_ID;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_SERVICE_ENDPOINT;
 import static com.hedera.hapi.node.base.ResponseCodeEnum.MAX_NODES_CREATED;
+import static com.hedera.node.app.service.addressbook.AddressBookHelper.getNextNodeID;
 import static com.hedera.node.app.spi.workflows.HandleException.validateFalse;
 import static com.hedera.node.app.spi.workflows.HandleException.validateTrue;
 import static com.hedera.node.app.spi.workflows.PreCheckException.validateFalsePreCheck;
@@ -45,7 +46,6 @@
 import com.hedera.node.config.data.NodesConfig;
 import com.hedera.pbj.runtime.io.buffer.Bytes;
 import edu.umd.cs.findbugs.annotations.NonNull;
-import java.util.concurrent.atomic.AtomicLong;
 import javax.inject.Inject;
 import javax.inject.Singleton;
 
@@ -114,8 +114,6 @@ public void handle(@NonNull final HandleContext handleContext) {
                 .description(op.description())
                 .gossipEndpoint(op.gossipEndpoint())
                 .serviceEndpoint(op.serviceEndpoint())
-                .gossipEndpoint(op.gossipEndpoint())
-                .serviceEndpoint(op.serviceEndpoint())
                 .gossipCaCertificate(op.gossipCaCertificate())
                 .grpcCertificateHash(op.grpcCertificateHash())
                 .adminKey(op.adminKey());
@@ -139,12 +137,4 @@ public Fees calculateFees(@NonNull final FeeContext feeContext) {
         calculator.addVerificationsPerTransaction(Math.max(0, feeContext.numTxnSignatures() - 1));
         return calculator.calculate();
     }
-
-    private long getNextNodeID(@NonNull final WritableNodeStore nodeStore) {
-        requireNonNull(nodeStore);
-        final var nodeIds = nodeStore.keys();
-        AtomicLong max = new AtomicLong(-1);
-        nodeIds.forEachRemaining(nodeId -> max.set(Math.max(max.get(), nodeId.number())));
-        return max.get() + 1;
-    }
 }
diff --git a/...main/java/com/hedera/node/app/service/addressbook/impl/schemas/V053AddressBookSchema.java b/...main/java/com/hedera/node/app/service/addressbook/impl/schemas/V053AddressBookSchema.java
@@ -16,17 +16,21 @@
 
 package com.hedera.node.app.service.addressbook.impl.schemas;
 
-import static com.hedera.node.app.service.addressbook.impl.AddressBookServiceImpl.NODES_KEY;
+import static com.hedera.node.app.service.addressbook.AddressBookHelper.NODES_KEY;
 import static java.util.Objects.requireNonNull;
 
+import com.hedera.hapi.node.base.AccountID;
 import com.hedera.hapi.node.base.Key;
 import com.hedera.hapi.node.base.SemanticVersion;
 import com.hedera.hapi.node.base.ServiceEndpoint;
 import com.hedera.hapi.node.state.addressbook.Node;
 import com.hedera.hapi.node.state.common.EntityNumber;
+import com.hedera.hapi.node.state.token.Account;
+import com.hedera.node.config.data.AccountsConfig;
 import com.hedera.node.config.data.BootstrapConfig;
 import com.hedera.pbj.runtime.io.buffer.Bytes;
 import com.swirlds.state.spi.MigrationContext;
+import com.swirlds.state.spi.ReadableKVState;
 import com.swirlds.state.spi.Schema;
 import com.swirlds.state.spi.StateDefinition;
 import com.swirlds.state.spi.WritableKVState;
@@ -49,6 +53,7 @@ public class V053AddressBookSchema extends Schema {
     private static final long MAX_NODES = 100L;
     private static final SemanticVersion VERSION =
             SemanticVersion.newBuilder().major(0).minor(53).patch(0).build();
+    public static final String ACCOUNTS_KEY = "ACCOUNTS";
 
     public V053AddressBookSchema() {
         super(VERSION);
@@ -65,13 +70,30 @@ public void migrate(@NonNull final MigrationContext ctx) {
         requireNonNull(ctx);
         final WritableKVState<EntityNumber, Node> writableNodes =
                 ctx.newStates().get(NODES_KEY);
+        ReadableKVState<AccountID, Account> readableAccounts = null;
+        try {
+            readableAccounts = ctx.newStates().get(ACCOUNTS_KEY);
+        } catch (IllegalArgumentException e) {
+            log.info("AccountStore is not found, can be ignored.");
+        }
         final var networkInfo = ctx.networkInfo();
         final var addressBook = networkInfo.addressBook();
         final var bootstrapConfig = ctx.configuration().getConfigData(BootstrapConfig.class);
-        final var adminKey =
-                Key.newBuilder().ed25519(bootstrapConfig.genesisPublicKey()).build();
+        final var accountConfig = ctx.configuration().getConfigData(AccountsConfig.class);
+        var adminKey = Key.DEFAULT;
+        if (readableAccounts != null) {
+            final var adminAccount = readableAccounts.get(AccountID.newBuilder()
+                    .accountNum(accountConfig.addressBookAdmin())
+                    .build());
+            if (adminAccount != null) {
+                adminKey = adminAccount.keyOrElse(Key.DEFAULT);
+            }
+        }
         log.info("Started migrating nodes from address book");
 
+        Key finalAdminKey = adminKey == null || adminKey.equals(Key.DEFAULT)
+                ? Key.newBuilder().ed25519(bootstrapConfig.genesisPublicKey()).build()
+                : adminKey;
         addressBook.forEach(nodeInfo -> {
             final var node = Node.newBuilder()
                     .nodeId(nodeInfo.nodeId())
@@ -82,7 +104,7 @@ public void migrate(@NonNull final MigrationContext ctx) {
                             endpointFor(nodeInfo.externalHostName(), nodeInfo.externalPort())))
                     .gossipCaCertificate(nodeInfo.sigCertBytes())
                     .weight(nodeInfo.stake())
-                    .adminKey(adminKey)
+                    .adminKey(finalAdminKey)
                     .build();
             writableNodes.put(
                     EntityNumber.newBuilder().number(nodeInfo.nodeId()).build(), node);

diff --git a/...est/java/com/hedera/node/app/service/addressbook/impl/test/ReadableNodeStoreImplTest.java b/...est/java/com/hedera/node/app/service/addressbook/impl/test/ReadableNodeStoreImplTest.java
@@ -17,7 +17,7 @@
 package com.hedera.node.app.service.addressbook.impl.test;
 
 import static com.hedera.node.app.hapi.utils.CommonPbjConverters.asBytes;
-import static com.hedera.node.app.service.addressbook.impl.AddressBookServiceImpl.NODES_KEY;
+import static com.hedera.node.app.service.addressbook.AddressBookHelper.NODES_KEY;
 import static java.util.stream.Collectors.toSet;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;

diff --git a/.../java/com/hedera/node/app/service/addressbook/impl/test/handlers/AddressBookTestBase.java b/.../java/com/hedera/node/app/service/addressbook/impl/test/handlers/AddressBookTestBase.java
@@ -17,7 +17,7 @@
 package com.hedera.node.app.service.addressbook.impl.test.handlers;
 
 import static com.hedera.node.app.hapi.utils.CommonPbjConverters.asBytes;
-import static com.hedera.node.app.service.addressbook.impl.AddressBookServiceImpl.NODES_KEY;
+import static com.hedera.node.app.service.addressbook.AddressBookHelper.NODES_KEY;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
 

diff --git a/...ava/com/hedera/node/app/service/addressbook/impl/test/handlers/NodeDeleteHandlerTest.java b/...ava/com/hedera/node/app/service/addressbook/impl/test/handlers/NodeDeleteHandlerTest.java
@@ -17,7 +17,7 @@
 package com.hedera.node.app.service.addressbook.impl.test.handlers;
 
 import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_NODE_ID;
-import static com.hedera.node.app.service.addressbook.impl.AddressBookServiceImpl.NODES_KEY;
+import static com.hedera.node.app.service.addressbook.AddressBookHelper.NODES_KEY;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;

diff --git a/.../com/hedera/node/app/service/addressbook/impl/test/schemas/V053AddressBookSchemaTest.java b/.../com/hedera/node/app/service/addressbook/impl/test/schemas/V053AddressBookSchemaTest.java
@@ -16,15 +16,22 @@
 
 package com.hedera.node.app.service.addressbook.impl.test.schemas;
 
-import static com.hedera.node.app.service.addressbook.impl.AddressBookServiceImpl.NODES_KEY;
+import static com.hedera.node.app.service.addressbook.AddressBookHelper.NODES_KEY;
+import static com.hedera.node.app.service.addressbook.impl.schemas.V053AddressBookSchema.ACCOUNTS_KEY;
+import static com.hedera.node.app.service.addressbook.impl.schemas.V053AddressBookSchema.endpointFor;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.AssertionsForClassTypes.assertThatCode;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.mockito.BDDMockito.given;
 
+import com.hedera.hapi.node.base.AccountID;
+import com.hedera.hapi.node.state.addressbook.Node;
+import com.hedera.hapi.node.state.common.EntityNumber;
+import com.hedera.hapi.node.state.token.Account;
 import com.hedera.node.app.info.NodeInfoImpl;
 import com.hedera.node.app.service.addressbook.impl.schemas.V053AddressBookSchema;
 import com.hedera.node.app.service.addressbook.impl.test.handlers.AddressBookTestBase;
+import com.hedera.node.app.spi.fixtures.state.MapWritableStates;
 import com.hedera.node.app.spi.fixtures.util.LogCaptor;
 import com.hedera.node.app.spi.fixtures.util.LogCaptureExtension;
 import com.hedera.node.app.spi.fixtures.util.LoggingSubject;
@@ -33,10 +40,11 @@
 import com.hedera.pbj.runtime.io.buffer.Bytes;
 import com.swirlds.state.spi.MigrationContext;
 import com.swirlds.state.spi.StateDefinition;
-import com.swirlds.state.spi.WritableKVStateBase;
-import com.swirlds.state.spi.WritableStates;
 import com.swirlds.state.spi.info.NetworkInfo;
+import com.swirlds.state.test.fixtures.MapWritableKVState;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -51,18 +59,21 @@ class V053AddressBookSchemaTest extends AddressBookTestBase {
     @Mock
     private MigrationContext migrationContext;
 
-    @Mock
-    private WritableStates writableStates;
-
     @Mock
     private NetworkInfo networkInfo;
 
-    @Mock
-    private WritableKVStateBase writableKVState;
-
     @LoggingSubject
     private V053AddressBookSchema subject;
 
+    private final Map<AccountID, Account> accounts = new HashMap<>();
+    private final MapWritableKVState<AccountID, Account> writableAccounts =
+            new MapWritableKVState<>(ACCOUNTS_KEY, accounts);
+
+    private final Map<EntityNumber, Node> nodes = new HashMap<>();
+    private final MapWritableKVState<EntityNumber, Node> writableNodes = new MapWritableKVState<>(NODES_KEY, nodes);
+
+    private MapWritableStates writableStates = null;
+
     @BeforeEach
     void setUp() {
         subject = new V053AddressBookSchema();
@@ -81,14 +92,46 @@ void registersExpectedSchema() {
     void migrateAsExpected() {
         setupMigrationContext();
 
+        assertThatCode(() -> subject.migrate(migrationContext)).doesNotThrowAnyException();
+        assertThat(logCaptor.infoLogs()).contains("Started migrating nodes from address book");
+        assertThat(logCaptor.infoLogs()).contains("AccountStore is not found, can be ignored.");
+        assertThat(logCaptor.infoLogs()).contains("Migrated 2 nodes from address book");
+    }
+
+    @Test
+    void migrateAsExpected2() {
+        setupMigrationContext2();
+
         assertThatCode(() -> subject.migrate(migrationContext)).doesNotThrowAnyException();
         assertThat(logCaptor.infoLogs()).contains("Started migrating nodes from address book");
         assertThat(logCaptor.infoLogs()).contains("Migrated 2 nodes from address book");
+        assertEquals(
+                Node.newBuilder()
+                        .nodeId(1)
+                        .accountId(payerId)
+                        .description("memo1")
+                        .gossipEndpoint(List.of(endpointFor("127.0.0.1", 123), endpointFor("23.45.34.245", 22)))
+                        .gossipCaCertificate(Bytes.wrap(gossipCaCertificate))
+                        .weight(0)
+                        .adminKey(anotherKey)
+                        .build(),
+                writableNodes.get(EntityNumber.newBuilder().number(1).build()));
+        assertEquals(
+                Node.newBuilder()
+                        .nodeId(2)
+                        .accountId(accountId)
+                        .description("memo2")
+                        .gossipEndpoint(List.of(endpointFor("127.0.0.2", 123), endpointFor("23.45.34.240", 23)))
+                        .gossipCaCertificate(Bytes.wrap(grpcCertificateHash))
+                        .weight(1)
+                        .adminKey(anotherKey)
+                        .build(),
+                writableNodes.get(EntityNumber.newBuilder().number(2).build()));
     }
 
     private void setupMigrationContext() {
+        writableStates = MapWritableStates.builder().state(writableNodes).build();
         given(migrationContext.newStates()).willReturn(writableStates);
-        given(writableStates.get(NODES_KEY)).willReturn(writableKVState);
 
         final var nodeInfo1 = new NodeInfoImpl(
                 1,
@@ -101,7 +144,7 @@ private void setupMigrationContext() {
                 "pubKey1",
                 "memo1",
                 Bytes.wrap(gossipCaCertificate),
-                "memo2");
+                "memo1");
         final var nodeInfo2 = new NodeInfoImpl(
                 2,
                 accountId,
@@ -121,4 +164,22 @@ private void setupMigrationContext() {
                 .getOrCreateConfig();
         given(migrationContext.configuration()).willReturn(config);
     }
+
+    private void setupMigrationContext2() {
+        setupMigrationContext();
+        accounts.put(
+                AccountID.newBuilder().accountNum(55).build(),
+                Account.newBuilder().key(anotherKey).build());
+        writableStates = MapWritableStates.builder()
+                .state(writableAccounts)
+                .state(writableNodes)
+                .build();
+        given(migrationContext.newStates()).willReturn(writableStates);
+
+        final var config = HederaTestConfigBuilder.create()
+                .withValue("bootstrap.genesisPublicKey", defauleAdminKeyBytes)
+                .withValue("accounts.addressBookAdmin", "55")
+                .getOrCreateConfig();
+        given(migrationContext.configuration()).willReturn(config);
+    }
 }
diff --git a/...book-service/src/main/java/com/hedera/node/app/service/addressbook/AddressBookHelper.java b/...book-service/src/main/java/com/hedera/node/app/service/addressbook/AddressBookHelper.java
@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2024 Hedera Hashgraph, LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.hedera.node.app.service.addressbook;
+
+import static java.util.Objects.requireNonNull;
+import static java.util.Spliterator.DISTINCT;
+
+import com.hedera.hapi.node.state.common.EntityNumber;
+import edu.umd.cs.findbugs.annotations.NonNull;
+import java.util.Spliterators;
+import java.util.stream.StreamSupport;
+
+/**
+ * Utility class that provides static methods and constants to facilitate the Address Book Services functions.
+ */
+public class AddressBookHelper {
+    public static final String NODES_KEY = "NODES";
+
+    /**
+     * Get the next Node ID number from the reaReadableNodeStore
+     * @param nodeStore
+     * @return nextNodeId
+     */
+    public static long getNextNodeID(@NonNull final ReadableNodeStore nodeStore) {
+        requireNonNull(nodeStore);
+        final long maxNodeId = StreamSupport.stream(
+                        Spliterators.spliterator(nodeStore.keys(), nodeStore.sizeOfState(), DISTINCT), false)
+                .mapToLong(EntityNumber::number)
+                .max()
+                .orElse(-1L);
+        return maxNodeId + 1;
+    }
+}