hiero-ledger / hiero-ledger/hiero-sdk-cpp
[Intermediate]: Expand Dynamic Address Book integration test coverage to match Swift SDK
- Dominant language
- C++
- Stars
- 42
- Forks
- 108
- Avg merge
- 11h 45m
- Merged PRs (30d)
- 2
Description
### 🧩 Intermediate Friendly
This issue is a good fit for contributors who are already familiar with the Hiero C++ SDK and feel comfortable navigating the codebase.
Intermediate Issues often involve:
- Exploring existing implementations
- Understanding how different components work together
- Making thoughtful changes that follow established patterns
The goal is to support deeper problem-solving while keeping the task clear, focused, and enjoyable to work on.
> [!IMPORTANT]
> ### 🧭 About Intermediate Issues
>
> Intermediate Issues are a great next step for contributors who enjoy digging into the codebase and reasoning about how things work.
>
> These issues often:
> - Involve multiple related files or components
> - Encourage investigation and understanding of existing behavior
> - Leave room for thoughtful implementation choices
> - Stay focused on a clearly defined goal
>
> Other kinds of contributions — from beginner-friendly tasks to large system-level changes — are just as valuable and use different labels.
### 👾 Description of the Task
PR #1507 re-enabled three Dynamic Address Book (DAB) integration tests:
- `CanExecuteNodeCreateTransaction`
- `CanExecuteNodeDeleteTransaction`
- `CanExecuteNodeUpdateTransaction`
The Hiero Swift SDK has a materially more complete DAB integration suite that exercises multiple endpoint variants, key-rotation paths, and error-path scenarios. The maintainer flagged on PR #1507 that bringing the C++ DAB suite up to Swift parity should be a separate, follow-up issue rather than scope-creep on the re-enable.
> Maintainer comment on PR #1507: *"the Hiero Swift SDK has a more complete set of DAB integration tests that are already enabled in CI and can serve as a guide… You're not required to add all the test cases that SDK has here (that can be a follow-up issue)…"*
This issue also folds in a small cleanup the same review surfaced: in `NodeUpdateTransactionIntegrationTests`, after the merge, `mNodeId` (returned by `getNodeId()`) and `nodeIDToUpdate` both equal `1` and represent the same thing. Tidying that redundancy belongs naturally inside this broader test-expansion effort.
Relevant files:
```
src/sdk/tests/integration/NodeCreateTransactionIntegrationTests.cc
src/sdk/tests/integration/NodeUpdateTransactionIntegrationTests.cc
src/sdk/tests/integration/NodeDeleteTransactionIntegrationTests.cc
config/local_node.json
```
Reference (Swift SDK DAB tests): https://github.com/hiero-ledger/hiero-sdk-swift — see the `NodeCreate`/`NodeUpdate`/`NodeDelete` integration test files.
### 💡 Proposed Approach
1. Read through the Swift SDK's DAB integration tests and produce a short list of scenarios that are present there but missing in the C++ suite. Typical gaps to look for:
- **Endpoint-variant coverage:** IPv4 vs. IPv6, multiple service/gossip endpoints, very-long endpoint lists.
- **Key-rotation paths:** updating only the admin key, only the gossip CA cert hash, or rotating both atomically.
- **Error-path coverage:** invalid endpoints, oversized cert hashes, empty admin key, unauthorized signer.
- **Description and metadata edge cases:** empty description, max-length description, non-ASCII bytes.
2. Implement the missing scenarios as new `TEST_F` cases in the existing C++ integration test files, following the Given/When/Then style already used.
3. While in `NodeUpdateTransactionIntegrationTests.cc`, collapse the redundant `mNodeId` / `nodeIDToUpdate` pair into a single source of truth (use whichever name reads better in context — likely `mNodeId` since it is the existing fixture member).
4. Run the suite locally against Solo and confirm all new and existing DAB tests pass.
A reasonable scope cap for one PR is the highest-value subset (probably endpoint variants + key rotation + the redundant-ID cleanup). Error-path coverage can ship in a second PR if the diff grows too large — call this out in the PR description if you choose to split.
### 👩💻 Implementation Steps
- [ ] Skim the Swift SDK DAB integration tests and produce a written list (in the PR description) of scenarios you are adding and why.
- [ ] For each scenario, add a `TEST_F` to the appropriate `Node*TransactionIntegrationTests.cc` file, mirroring the Given/When/Then comment style of the three already-enabled tests.
- [ ] Collapse the redundant `mNodeId` / `nodeIDToUpdate` in `NodeUpdateTransactionIntegrationTests.cc` to a single member.
- [ ] Confirm the integration tests still respect the same opt-out pattern that excludes them in CI by default (CI skips `NodeUpdateTransactionIntegrationTests` per `CLAUDE.md`).
- [ ] Bring up Solo locally and run `ctest -j 6 -C Debug --test-dir build/linux-x64-debug -R NodeCreateTransactionIntegrationTests` (and the equivalent for Update/Delete). Confirm all pass.
- [ ] If you split coverage across two PRs (e.g. happy-path + error-path), file the split in the PR description so a maintainer can confirm the cut.
### ✅ Acceptance Criteria
- [ ] DAB integration test coverage in C++ approaches Swift-SDK parity for at least the endpoint-variant and key-rotation scenarios.
- [ ] The redundant `mNodeId` / `nodeIDToUpdate` pair in `NodeUpdateTransactionIntegrationTests.cc` is collapsed into a single member.
- [ ] All new and existing DAB integration tests pass against a local Solo instance.
- [ ] No production-source SDK changes are introduced (this is a tests-only PR; if a real SDK gap is uncovered, file a separate issue).
- [ ] The PR description lists the Swift-SDK scenarios audited and which were added vs. deferred.
---
### 📋 Step-by-Step Contribution Guide
To help keep contributions consistent and easy to review, we recommend following these steps:
- [ ] Comment `/assign` to request the issue
- [ ] Wait for assignment
- [ ] Fork the repository and create a branch
- [ ] Set up the project using the instructions in `README.md`
- [ ] Make the requested changes
- [ ] Sign each commit using `-s -S`
- [ ] Push your branch and open a pull request
Read [Workflow Guide](https://github.com/hiero-ledger/hiero-sdk-cpp/blob/main/docs/training/workflow.md) for step-by-step workflow guidance.
Read [README.md](https://github.com/hiero-ledger/hiero-sdk-cpp/blob/main/README.md) for setup instructions.
❗ Pull requests **cannot be merged** without `S` and `s` signed commits.
See the [Signing Guide](https://github.com/hiero-ledger/hiero-sdk-cpp/blob/main/docs/training/signing.md).
### 🤔 Additional Information
- Original DAB feature issue: [#1004](https://github.com/hiero-ledger/hiero-sdk-cpp/issues/1004) (closed by PR #1507).
- Related but distinct: [#718](https://github.com/hiero-ledger/hiero-sdk-cpp/issues/718) — *Add HIP-869 Dynamic Address Book Example*. That issue is about an example program; this issue is strictly about integration test coverage.
- Solo setup is described in `README.md` and `config/local_node.json`. CI excludes `NodeUpdateTransactionIntegrationTests` because it is slow against Solo (see `CLAUDE.md`); local runs are the expected validation path for this work.
If you have questions while working on this issue, feel free to ask! [Hiero-SDK-C++ Discord](https://discord.com/channels/905194001349627914/1337424839761465364)
Contributor guide
Assessment
This issue has not been assessed yet.