feat: 13472 implement node delete handler (#13708)

Signed-off-by: Lev Povolotsky <[email protected]>

hedera-node/hedera-addressbook-service-impl/src/main/java/com/hedera/node/app/service/addressbook/impl/handlers/NodeDeleteHandler.java

@@ -16,10 +16,18 @@
 
 package com.hedera.node.app.service.addressbook.impl.handlers;
 
+import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_NODE_ID;
+import static com.hedera.hapi.node.base.ResponseCodeEnum.NODE_DELETED;
+import static com.hedera.node.app.spi.workflows.HandleException.validateFalse;
+import static com.hedera.node.app.spi.workflows.PreCheckException.validateFalsePreCheck;
 import static java.util.Objects.requireNonNull;
 
+import com.hedera.hapi.node.addressbook.NodeDeleteTransactionBody;
 import com.hedera.hapi.node.base.HederaFunctionality;
+import com.hedera.hapi.node.base.SubType;
+import com.hedera.hapi.node.state.addressbook.Node;
 import com.hedera.hapi.node.transaction.TransactionBody;
+import com.hedera.node.app.service.addressbook.impl.WritableNodeStore;
 import com.hedera.node.app.spi.fees.FeeContext;
 import com.hedera.node.app.spi.fees.Fees;
 import com.hedera.node.app.spi.workflows.HandleContext;
@@ -42,37 +50,47 @@ public NodeDeleteHandler() {
 
     @Override
     public void pureChecks(@NonNull final TransactionBody txn) throws PreCheckException {
-        // implement
+        final NodeDeleteTransactionBody transactionBody = txn.nodeDeleteOrThrow();
+        final long nodeId = transactionBody.nodeId();
+
+        validateFalsePreCheck(nodeId < 0, INVALID_NODE_ID);
     }
 
     @Override
-    public void preHandle(@NonNull final PreHandleContext context) throws PreCheckException {
-        requireNonNull(context);
-
-        final var op = context.body().nodeDelete();
-        throw new UnsupportedOperationException("need implementation");
-    }
+    public void preHandle(@NonNull final PreHandleContext context) throws PreCheckException {}
 
     /**
-     * Given the appropriate context, deletes a topic.
+     * Given the appropriate context, deletes a node.
      *
      * @param context the {@link HandleContext} of the active transaction
      * @throws NullPointerException if one of the arguments is {@code null}
      */
     @Override
     public void handle(@NonNull final HandleContext context) {
-        requireNonNull(context, "The argument 'context' must not be null");
+        requireNonNull(context);
+
+        final NodeDeleteTransactionBody transactionBody = context.body().nodeDeleteOrThrow();
+        var nodeId = transactionBody.nodeId();
 
-        final var op = context.body().nodeDelete();
-        throw new UnsupportedOperationException("need implementation");
+        final var nodeStore = context.writableStore(WritableNodeStore.class);
+
+        Node node = nodeStore.get(nodeId);
+
+        validateFalse(node == null, INVALID_NODE_ID);
+
+        validateFalse(node.deleted(), NODE_DELETED);
+
+        /* Copy all the fields from existing, and mark deleted flag  */
+        final var nodeBuilder = node.copyBuilder().deleted(true);
+
+        /* --- Put the modified node. It will be in underlying state's modifications map.
+        It will not be committed to state until commit is called on the state.--- */
+        nodeStore.put(nodeBuilder.build());
     }
 
     @NonNull
     @Override
     public Fees calculateFees(@NonNull final FeeContext feeContext) {
-        requireNonNull(feeContext);
-        final var op = feeContext.body();
-
-        throw new UnsupportedOperationException("need implementation");
+        return feeContext.feeCalculator(SubType.DEFAULT).calculate();
     }
 }

hedera-node/hedera-addressbook-service-impl/src/test/java/com/hedera/node/app/service/addressbook/impl/test/handlers/AddressBookTestBase.java

@@ -51,11 +51,11 @@ public class AddressBookTestBase {
     protected final AccountID accountId = AccountID.newBuilder().accountNum(3).build();
 
     protected final AccountID payerId = AccountID.newBuilder().accountNum(2).build();
-    protected final AccountID invalidId =
-            AccountID.newBuilder().accountNum(Long.MAX_VALUE).build();
     protected final byte[] grpcCertificateHash = "grpcCertificateHash".getBytes();
     protected final byte[] gossipCaCertificate = "gossipCaCertificate".getBytes();
-    protected final EntityNumber nodeId = EntityNumber.newBuilder().number(1L).build();
+    protected final long WELL_KNOWN_NODE_ID = 1L;
+    protected final EntityNumber nodeId =
+            EntityNumber.newBuilder().number(WELL_KNOWN_NODE_ID).build();
     protected final EntityNumber nodeId2 = EntityNumber.newBuilder().number(3L).build();
     protected final Timestamp consensusTimestamp =
             Timestamp.newBuilder().seconds(1_234_567L).build();
@@ -113,6 +113,16 @@ protected void refreshStoresWithCurrentNodeInReadable() {
         writableStore = new WritableNodeStore(writableStates, configuration, storeMetricsService);
     }
 
+    protected void refreshStoresWithCurrentNodeInBothReadableAndWritable() {
+        readableNodeState = readableNodeState();
+        writableNodeState = writableNodeStateWithOneKey();
+        given(readableStates.<EntityNumber, Node>get(NODES_KEY)).willReturn(readableNodeState);
+        given(writableStates.<EntityNumber, Node>get(NODES_KEY)).willReturn(writableNodeState);
+        readableStore = new ReadableNodeStoreImpl(readableStates);
+        final var configuration = HederaTestConfigBuilder.createConfig();
+        writableStore = new WritableNodeStore(writableStates, configuration, storeMetricsService);
+    }
+
     protected void refreshStoresWithCurrentNodeInWritable() {
         writableNodeState = writableNodeStateWithOneKey();
         given(writableStates.<EntityNumber, Node>get(NODES_KEY)).willReturn(writableNodeState);
@@ -160,6 +170,10 @@ protected MapReadableKVState<EntityNumber, Node> emptyReadableNodeState() {
     }
 
     protected void givenValidNode() {
+        givenValidNode(false);
+    }
+
+    protected void givenValidNode(boolean deleted) {
         node = new Node(
                 nodeId.number(),
                 accountId,
@@ -169,7 +183,7 @@ protected void givenValidNode() {
                 Bytes.wrap(gossipCaCertificate),
                 Bytes.wrap(grpcCertificateHash),
                 0,
-                false);
+                deleted);
     }
 
     protected Node createNode() {

hedera-node/hedera-addressbook-service-impl/src/test/java/com/hedera/node/app/service/addressbook/impl/test/handlers/NodeDeleteHandlerTest.java

@@ -0,0 +1,215 @@
+/*
+ * Copyright (C) 2023-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.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 org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
+import static org.assertj.core.api.AssertionsForClassTypes.catchThrowable;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.notNull;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.lenient;
+import static org.mockito.Mockito.mock;
+
+import com.hedera.hapi.node.addressbook.NodeDeleteTransactionBody;
+import com.hedera.hapi.node.base.ResponseCodeEnum;
+import com.hedera.hapi.node.base.TransactionID;
+import com.hedera.hapi.node.state.addressbook.Node;
+import com.hedera.hapi.node.state.common.EntityNumber;
+import com.hedera.hapi.node.transaction.TransactionBody;
+import com.hedera.node.app.service.addressbook.impl.ReadableNodeStoreImpl;
+import com.hedera.node.app.service.addressbook.impl.WritableNodeStore;
+import com.hedera.node.app.service.addressbook.impl.handlers.NodeDeleteHandler;
+import com.hedera.node.app.service.token.ReadableAccountStore;
+import com.hedera.node.app.spi.fees.FeeCalculator;
+import com.hedera.node.app.spi.fees.FeeContext;
+import com.hedera.node.app.spi.fees.Fees;
+import com.hedera.node.app.spi.metrics.StoreMetricsService;
+import com.hedera.node.app.spi.workflows.HandleContext;
+import com.hedera.node.app.spi.workflows.HandleException;
+import com.hedera.node.app.spi.workflows.PreCheckException;
+import com.hedera.node.config.testfixtures.HederaTestConfigBuilder;
+import com.swirlds.config.api.Configuration;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.Mock.Strictness;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class NodeDeleteHandlerTest extends AddressBookTestBase {
+
+    @Mock
+    private ReadableAccountStore accountStore;
+
+    @Mock
+    private ReadableNodeStoreImpl mockStore;
+
+    @Mock(strictness = Strictness.LENIENT)
+    private HandleContext handleContext;
+
+    @Mock
+    private NodeDeleteHandler subject;
+
+    @Mock
+    private StoreMetricsService storeMetricsService;
+
+    protected Configuration testConfig;
+
+    @BeforeEach
+    void setUp() {
+        mockStore = mock(ReadableNodeStoreImpl.class);
+        subject = new NodeDeleteHandler();
+
+        writableNodeState = writableNodeStateWithOneKey();
+        given(writableStates.<EntityNumber, Node>get(NODES_KEY)).willReturn(writableNodeState);
+        testConfig = HederaTestConfigBuilder.createConfig();
+        writableStore = new WritableNodeStore(writableStates, testConfig, storeMetricsService);
+        lenient().when(handleContext.configuration()).thenReturn(testConfig);
+    }
+
+    @Test
+    @DisplayName("pureChecks throws exception when node id is negative or zero")
+    public void testPureChecksThrowsExceptionWhenFileIdIsNull() {
+        NodeDeleteTransactionBody transactionBody = mock(NodeDeleteTransactionBody.class);
+        TransactionBody transaction = mock(TransactionBody.class);
+        given(handleContext.body()).willReturn(transaction);
+        given(transaction.nodeDeleteOrThrow()).willReturn(transactionBody);
+        given(transactionBody.nodeId()).willReturn(-1L);
+
+        assertThatThrownBy(() -> subject.pureChecks(handleContext.body())).isInstanceOf(PreCheckException.class);
+        var msg = assertThrows(PreCheckException.class, () -> subject.pureChecks(handleContext.body()));
+        assertThat(msg.responseCode()).isEqualTo(INVALID_NODE_ID);
+    }
+
+    @Test
+    @DisplayName("pureChecks does not throw exception when node id is not null")
+    public void testPureChecksDoesNotThrowExceptionWhenNodeIdIsNotNull() {
+        given(handleContext.body()).willReturn(newDeleteTxn());
+
+        assertThatCode(() -> subject.pureChecks(handleContext.body())).doesNotThrowAnyException();
+    }
+
+    @Test
+    @DisplayName("check that fees are free for delete node trx")
+    public void testCalculateFeesInvocations() {
+        final var feeCtx = mock(FeeContext.class);
+        final var feeCalc = mock(FeeCalculator.class);
+        given(feeCtx.feeCalculator(notNull())).willReturn(feeCalc);
+        given(feeCalc.calculate()).willReturn(Fees.FREE);
+
+        assertThat(subject.calculateFees(feeCtx)).isEqualTo(Fees.FREE);
+    }
+
+    @Test
+    @DisplayName("Fails handle if node doesn't exist")
+    void fileDoesntExist() {
+        final var txn = newDeleteTxn().nodeDeleteOrThrow();
+
+        writableNodeState = emptyWritableNodeState();
+        given(writableStates.<EntityNumber, Node>get(NODES_KEY)).willReturn(writableNodeState);
+        writableStore = new WritableNodeStore(writableStates, testConfig, storeMetricsService);
+        given(handleContext.writableStore(WritableNodeStore.class)).willReturn(writableStore);
+
+        given(handleContext.body())
+                .willReturn(TransactionBody.newBuilder().nodeDelete(txn).build());
+        given(handleContext.writableStore(WritableNodeStore.class)).willReturn(writableStore);
+
+        HandleException thrown = (HandleException) catchThrowable(() -> subject.handle(handleContext));
+        assertThat(thrown.getStatus()).isEqualTo(INVALID_NODE_ID);
+    }
+
+    @Test
+    @DisplayName("Node is null")
+    void NodeIsNull() {
+        final var txn = newDeleteTxn().nodeDeleteOrThrow();
+
+        node = null;
+
+        writableNodeState = writableNodeStateWithOneKey();
+        given(writableStates.<EntityNumber, Node>get(NODES_KEY)).willReturn(writableNodeState);
+        writableStore = new WritableNodeStore(writableStates, testConfig, storeMetricsService);
+        given(handleContext.writableStore(WritableNodeStore.class)).willReturn(writableStore);
+
+        given(handleContext.body())
+                .willReturn(TransactionBody.newBuilder().nodeDelete(txn).build());
+        given(handleContext.writableStore(WritableNodeStore.class)).willReturn(writableStore);
+        HandleException thrown = (HandleException) catchThrowable(() -> subject.handle(handleContext));
+        assertThat(thrown.getStatus()).isEqualTo(INVALID_NODE_ID);
+    }
+
+    @Test
+    @DisplayName("Handle works as expected")
+    void handleWorksAsExpected() {
+        final var txn = newDeleteTxn().nodeDeleteOrThrow();
+
+        final var existingNode = writableStore.get(WELL_KNOWN_NODE_ID);
+        assertThat(existingNode).isNotNull();
+        assertThat(existingNode.deleted()).isFalse();
+
+        given(handleContext.body())
+                .willReturn(TransactionBody.newBuilder().nodeDelete(txn).build());
+        given(handleContext.writableStore(WritableNodeStore.class)).willReturn(writableStore);
+
+        subject.handle(handleContext);
+
+        final var changedFile = writableStore.get(WELL_KNOWN_NODE_ID);
+
+        assertThat(changedFile).isNotNull();
+        assertThat(changedFile.deleted()).isTrue();
+    }
+
+    @Test
+    @DisplayName("Node already deleted returns error")
+    void noFileKeys() {
+        givenValidNode(true);
+        refreshStoresWithCurrentNodeInBothReadableAndWritable();
+
+        final var txn = newDeleteTxn().nodeDeleteOrThrow();
+
+        final var existingNode = writableStore.get(WELL_KNOWN_NODE_ID);
+        assertThat(existingNode).isNotNull();
+        assertThat(existingNode.deleted()).isTrue();
+
+        given(handleContext.body())
+                .willReturn(TransactionBody.newBuilder().nodeDelete(txn).build());
+        given(handleContext.writableStore(WritableNodeStore.class)).willReturn(writableStore);
+        // expect:
+        assertFailsWith(() -> subject.handle(handleContext), ResponseCodeEnum.NODE_DELETED);
+    }
+
+    private TransactionBody newDeleteTxn() {
+        final var txnId = TransactionID.newBuilder().accountID(payerId).build();
+        final var deleteFileBuilder = NodeDeleteTransactionBody.newBuilder().nodeId(WELL_KNOWN_NODE_ID);
+        return TransactionBody.newBuilder()
+                .transactionID(txnId)
+                .nodeDelete(deleteFileBuilder.build())
+                .build();
+    }
+
+    private static void assertFailsWith(final Runnable something, final ResponseCodeEnum status) {
+        assertThatThrownBy(something::run)
+                .isInstanceOf(HandleException.class)
+                .extracting(ex -> ((HandleException) ex).getStatus())
+                .isEqualTo(status);
+    }
+}