hiero-ledger / hiero-ledger/hiero-sdk-cpp
[Beginner]: Compare `mKey` and `mPublicKeyAlias` in `AccountInfo::operator==`
- Dominant language
- C++
- Stars
- 42
- Forks
- 108
- Avg merge
- 11h 45m
- Merged PRs (30d)
- 2
Description
#
### 🐥 Beginner Friendly
This issue is a great fit for contributors who are ready to explore the Hiero C++ codebase a little more and take on slightly more independent work.
Beginner Issues often involve reading existing C++ code, understanding how different parts of the SDK fit together, and making small, thoughtful updates that follow established patterns.
The goal is to support skill growth while keeping the experience approachable, well-scoped, and enjoyable.
> [!IMPORTANT]
> ### 🐥 About Beginner Issues
>
> Beginner Issues are a great next step for contributors who feel comfortable with the basic project workflow and want to explore the codebase a little more.
>
> These issues often involve:
> - Reading existing C++ code
> - Understanding how different parts of the SDK fit together
> - Making small, thoughtful updates that follow established patterns
>
> You'll usually see Beginner Issues focused on things like:
> - Small, well-scoped improvements to existing tests
> - Narrow updates to `src` functionality (e.g. refining helpers or improving readability)
> - Documentation or comment clarity
> - Enhancements to existing examples
>
> Other types of contributions — such as brand-new features, broader system changes, or deeper technical work — are just as valuable and may use different labels.
### 👾 Description of the Task
PR #1488 added `AccountInfo::operator==`, but two fields — `mKey` and `mPublicKeyAlias` — are silently excluded from the comparison. The merged code carries this comment:
```cpp
// Note: mKey and mPublicKeyAlias (std::shared_ptr) are intentionally excluded
// to avoid pointer identity comparison. Comparing the pointed-to values would
// require knowing the concrete Key type, which is not available here.
```
The result is a real correctness gap: two `AccountInfo` instances that differ only in their key (or public-key alias) currently compare equal. That violates the equality contract a caller would reasonably expect.
The SDK already uses a byte-comparison approach for similar fields in this same `operator==` — `mEvmAddressAlias`, `mStakingInfo`, and `mTokenRelationships` are all compared by serialized bytes rather than by pointer. `Key` exposes `toBytes()`, so the same approach is implementable today without needing a polymorphic `Key::operator==`.
Relevant files:
```
src/sdk/main/include/AccountInfo.h
src/sdk/main/src/AccountInfo.cc
src/sdk/tests/unit/AccountInfoUnitTests.cc
```
### 💡 Proposed Solution
Inside `AccountInfo::operator==`, compare `mKey` and `mPublicKeyAlias` by their serialized byte representation, with a `nullptr` guard:
```cpp
auto sameKeyPtr = [](const std::shared_ptr& a, const std::shared_ptr& b) {
if (!a && !b) return true;
if (!a || !b) return false;
return a->toBytes() == b->toBytes();
};
return ... &&
sameKeyPtr(mKey, rhs.mKey) &&
sameKeyPtr(mPublicKeyAlias, rhs.mPublicKeyAlias) &&
...;
```
Pick whichever helper shape best matches the existing local style of the function (a small static lambda, a free helper, or inlined `if (...) return false;` blocks).
### 👩💻 Implementation Steps
- [ ] Open [src/sdk/main/src/AccountInfo.cc](../../src/sdk/main/src/AccountInfo.cc) and locate the existing `operator==` implementation and the comment that excludes `mKey` / `mPublicKeyAlias`.
- [ ] Replace the exclusion with a byte-comparison check that handles the `nullptr` cases (both null → equal; one null → not equal; both non-null → compare `toBytes()`).
- [ ] Remove the now-stale comment about pointer identity.
- [ ] Open [src/sdk/tests/unit/AccountInfoUnitTests.cc](../../src/sdk/tests/unit/AccountInfoUnitTests.cc) and add or extend tests covering the four nullable cases for each of `mKey` and `mPublicKeyAlias`:
- both null → equal
- one null, one set → not equal (both directions)
- both set, same bytes → equal
- both set, different bytes → not equal
- [ ] Run the unit suite: `ctest -C Debug --test-dir build/linux-x64-debug -R AccountInfoUnitTests`.
### ✅ Acceptance Criteria
- [ ] **Scope:** Changes are limited to `AccountInfo.cc`, optionally `AccountInfo.h`, and `AccountInfoUnitTests.cc`.
- [ ] **Behavior:** `AccountInfo::operator==` now reflects the values of `mKey` and `mPublicKeyAlias`. No other public API changes.
- [ ] **Tests:** New equality tests cover the four `nullptr`/byte-equality combinations for each field; all existing tests still pass.
- [ ] **Review:** All code review feedback addressed.
---
### 📋 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
- The byte-comparison pattern already used in this same function for `mEvmAddressAlias`, `mStakingInfo`, and `mTokenRelationships` is the right reference — match its style.
- A broader, polymorphic `Key::operator==` would also solve this, but is materially more involved (it has to handle every concrete `Key` subtype). This issue intentionally takes the lighter byte-comparison path so the gap is closed today; a `Key::operator==` can be filed separately if there is appetite.
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.