diff --git a/src/sdk/main/include/NodeUpdateTransaction.h b/src/sdk/main/include/NodeUpdateTransaction.h index 36d8fdd2..a7a58686 100644 --- a/src/sdk/main/include/NodeUpdateTransaction.h +++ b/src/sdk/main/include/NodeUpdateTransaction.h @@ -274,7 +274,14 @@ class NodeUpdateTransaction : public Transaction * The node identified MUST NOT be deleted. * This value is REQUIRED. */ - uint64_t mNodeId; + uint64_t mNodeId = 0; + + /** + * Flag to track whether nodeId has been explicitly set. + * This is necessary because protobuf uint64 defaults to 0, which cannot distinguish + * between an explicit 0 and an unset value. + */ + bool mNodeIdSet = false; /** * A Node account identifier. diff --git a/src/sdk/main/src/NodeUpdateTransaction.cc b/src/sdk/main/src/NodeUpdateTransaction.cc index 35536494..13003474 100644 --- a/src/sdk/main/src/NodeUpdateTransaction.cc +++ b/src/sdk/main/src/NodeUpdateTransaction.cc @@ -1,6 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 #include "NodeUpdateTransaction.h" #include "TransactionId.h" +#include "exceptions/IllegalStateException.h" #include "impl/Node.h" #include "impl/Utilities.h" @@ -31,6 +32,7 @@ NodeUpdateTransaction& NodeUpdateTransaction::setNodeId(uint64_t nodeId) { requireNotFrozen(); mNodeId = nodeId; + mNodeIdSet = true; return *this; } @@ -148,6 +150,7 @@ void NodeUpdateTransaction::initFromSourceTransactionBody() const aproto::NodeUpdateTransactionBody& body = transactionBody.nodeupdate(); mNodeId = body.node_id(); + mNodeIdSet = true; mAccountId = AccountId::fromProtobuf(body.account_id()); if (body.has_description()) @@ -191,6 +194,12 @@ void NodeUpdateTransaction::initFromSourceTransactionBody() //----- aproto::NodeUpdateTransactionBody* NodeUpdateTransaction::build() const { + // Validate that nodeId has been explicitly set + if (!mNodeIdSet) + { + throw IllegalStateException("NodeUpdateTransaction requires nodeId to be explicitly set before execution"); + } + auto body = std::make_unique(); body->set_node_id(mNodeId); diff --git a/src/sdk/tests/unit/NodeUpdateTransactionUnitTests.cc b/src/sdk/tests/unit/NodeUpdateTransactionUnitTests.cc index 56daa66f..0375e199 100644 --- a/src/sdk/tests/unit/NodeUpdateTransactionUnitTests.cc +++ b/src/sdk/tests/unit/NodeUpdateTransactionUnitTests.cc @@ -1,6 +1,9 @@ // SPDX-License-Identifier: Apache-2.0 +#include "AccountId.h" #include "ED25519PrivateKey.h" #include "NodeUpdateTransaction.h" +#include "TransactionId.h" +#include "exceptions/IllegalStateException.h" #include "impl/Utilities.h" #include @@ -226,3 +229,60 @@ TEST_F(NodeUpdateTransactionUnitTests, DeleteGrpcWebProxyEndpoint) // Then ASSERT_FALSE(transaction.getGrpcWebProxyEndpoint().has_value()); } + +//----- +TEST_F(NodeUpdateTransactionUnitTests, SetAndGetNodeId) +{ + // Given + uint64_t nodeId = 5; + + // When + transaction.setNodeId(nodeId); + + // Then + ASSERT_EQ(transaction.getNodeId(), nodeId); +} + +//----- +TEST_F(NodeUpdateTransactionUnitTests, ThrowsWhenNodeIdNotSet) +{ + // Given + NodeUpdateTransaction tx; + tx.setTransactionId(TransactionId::generate(AccountId(2))); + tx.setNodeAccountIds({ AccountId(3) }); + tx.setAccountId(AccountId(100)); + tx.setDescription("Test node"); + + // When / Then + EXPECT_THROW(tx.freeze(),IllegalStateException); +} + +//----- +TEST_F(NodeUpdateTransactionUnitTests, SucceedsWhenNodeIdIsSet) +{ + // Given + NodeUpdateTransaction tx; + tx.setTransactionId(TransactionId::generate(AccountId(2))); + tx.setNodeAccountIds({ AccountId(3) }); + tx.setNodeId(5); + tx.setAccountId(AccountId(100)); + + // When / Then - should not throw + EXPECT_NO_THROW(tx.freeze()); +} + +//----- +TEST_F(NodeUpdateTransactionUnitTests, NodeIdSetToZeroIsValid) +{ + // Given - nodeId can legitimately be 0 if explicitly set + NodeUpdateTransaction tx; + tx.setTransactionId(TransactionId::generate(AccountId(2))); + tx.setNodeAccountIds({ AccountId(3) }); + tx.setNodeId(0); + tx.setAccountId(AccountId(100)); + + // When / Then - should not throw even with nodeId = 0 + EXPECT_NO_THROW(tx.freeze()); + EXPECT_EQ(tx.getNodeId(), 0); +} +