hiero-ledger / hiero-ledger/hiero-sdk-cpp
[Beginner]: Promote `toEvmAddress()` to a virtual method on `PublicKey` returning `std::optional<EvmAddress>`
- 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
`toEvmAddress()` is currently declared only on the concrete subclass `ECDSAsecp256k1PublicKey` ([src/sdk/main/include/ECDSAsecp256k1PublicKey.h:188](../../src/sdk/main/include/ECDSAsecp256k1PublicKey.h)):
```cpp
[[nodiscard]] EvmAddress toEvmAddress() const;
```
Any code holding a generic `std::shared_ptr` (which is what `PrivateKey::getPublicKey()` returns) must downcast before it can derive an EVM address. This pattern is scattered across the SDK and tests today:
```cpp
// src/tck/src/key/KeyService.cc:142-145
return std::dynamic_pointer_cast(privateKey->getPublicKey())
->toEvmAddress()
.toString();
// src/sdk/tests/integration/TransferTransactionIntegrationTests.cc:104-105
const EvmAddress evmAddress =
std::dynamic_pointer_cast(privateKey->getPublicKey())->toEvmAddress();
```
The sibling SDKs already expose this on the base public-key type: the JS SDK returns `undefined` and the Go SDK returns `""` for non-ECDSA keys. The C++ SDK should follow the same pattern with `std::optional`.
Relevant files:
```
src/sdk/main/include/PublicKey.h
src/sdk/main/src/PublicKey.cc
src/sdk/main/include/ECDSAsecp256k1PublicKey.h
src/sdk/main/src/ECDSAsecp256k1PublicKey.cc
src/tck/src/key/KeyService.cc
src/sdk/tests/unit/ECDSAsecp256k1PublicKeyUnitTests.cc
src/sdk/tests/unit/ED25519PublicKeyUnitTests.cc
src/sdk/tests/unit/AccountCreateTransactionUnitTests.cc
src/sdk/tests/integration/AccountCreateTransactionIntegrationTests.cc
src/sdk/tests/integration/TransferTransactionIntegrationTests.cc
```
### 💡 Proposed Solution
Add a virtual method to `PublicKey` with a default implementation returning `std::nullopt`, and override it in `ECDSAsecp256k1PublicKey` with the existing Keccak-256 derivation:
```cpp
// PublicKey.h (declare; define in PublicKey.cc to keep EvmAddress a forward declaration)
/**
* Get the EVM address derived from this PublicKey.
*
* @return The EvmAddress derived from this PublicKey, or std::nullopt if this key is not an
* ECDSAsecp256k1PublicKey.
*/
[[nodiscard]] virtual std::optional toEvmAddress() const;
```
```cpp
// ECDSAsecp256k1PublicKey.h — change the existing declaration to an override
[[nodiscard]] std::optional toEvmAddress() const override;
```
The body of the existing override in [src/sdk/main/src/ECDSAsecp256k1PublicKey.cc:404](../../src/sdk/main/src/ECDSAsecp256k1PublicKey.cc) stays exactly as it is — the `EvmAddress` it returns converts implicitly to `std::optional`.
**Compatibility note (please preserve this in the PR description):** adding the base-class virtual with a default implementation is backward compatible for code holding `PublicKey*`/`std::shared_ptr`. However, the return type of the existing `ECDSAsecp256k1PublicKey::toEvmAddress()` changes from `EvmAddress` to `std::optional` (C++ does not allow non-covariant return-type overloading), so call sites on the typed subclass need a trivial `.value()` / `->` adjustment. All in-repo call sites are enumerated below.
### 👩💻 Implementation Steps
- [ ] In [src/sdk/main/include/PublicKey.h](../../src/sdk/main/include/PublicKey.h), add `#include `, forward-declare `class EvmAddress;` next to the existing `class AccountId;` forward declaration, and declare the new virtual method shown above (a good spot is next to `toAccountId` around line 138).
- [ ] In [src/sdk/main/src/PublicKey.cc](../../src/sdk/main/src/PublicKey.cc), define the default implementation: `return std::nullopt;` (include `EvmAddress.h` there).
- [ ] In [src/sdk/main/include/ECDSAsecp256k1PublicKey.h](../../src/sdk/main/include/ECDSAsecp256k1PublicKey.h) (line 188) and [src/sdk/main/src/ECDSAsecp256k1PublicKey.cc](../../src/sdk/main/src/ECDSAsecp256k1PublicKey.cc) (line 404), change the return type to `std::optional` and mark the declaration `override`.
- [ ] Update the typed call sites (mechanical):
- `src/sdk/main/src/AccountCreateTransaction.cc:67` and `:88` — **no change needed**; `mAlias` is already a `std::optional`, so the assignment compiles as-is.
- `src/tck/src/key/KeyService.cc:143`, `:151`, `:160` — change `.toString()` on the result to `->toString()`. Optional cleanup: with the new virtual, the public-key branch no longer needs `std::dynamic_pointer_cast` at all — it can call `key->toEvmAddress()` directly and treat `std::nullopt` as the "not ECDSA" error case.
- `src/sdk/tests/unit/ECDSAsecp256k1PublicKeyUnitTests.cc:415` and `src/sdk/tests/unit/AccountCreateTransactionUnitTests.cc:144` — append `.value()`.
- `src/sdk/tests/integration/AccountCreateTransactionIntegrationTests.cc` lines 94, 241, 287, 338, 379, 432, 562, 652 and `src/sdk/tests/integration/TransferTransactionIntegrationTests.cc:105` — append `.value()`. (At lines 379, 432, and 105 the surrounding `std::dynamic_pointer_cast` may also be dropped, since the base class now exposes the method.)
- [ ] Add unit tests:
- In [src/sdk/tests/unit/ED25519PublicKeyUnitTests.cc](../../src/sdk/tests/unit/ED25519PublicKeyUnitTests.cc): calling `toEvmAddress()` on an `ED25519PublicKey` (through a `std::shared_ptr`) returns `std::nullopt`.
- In [src/sdk/tests/unit/ECDSAsecp256k1PublicKeyUnitTests.cc](../../src/sdk/tests/unit/ECDSAsecp256k1PublicKeyUnitTests.cc): calling `toEvmAddress()` through a base-class `std::shared_ptr` returns an engaged optional equal to the address returned by the typed subclass call.
- [ ] Build and run the affected unit suites:
```bash
cmake --preset linux-x64-debug -DBUILD_TESTS=ON
cmake --build --preset linux-x64-debug -j 6
ctest -C Debug --test-dir build/linux-x64-debug \
-R "ECDSAsecp256k1PublicKeyUnitTests|ED25519PublicKeyUnitTests|AccountCreateTransactionUnitTests"
```
(The integration-test edits are compile-only fixes; CI runs them against Solo.)
- [ ] Run `clang-format-17` against any file you touched.
### ✅ Acceptance Criteria
- [ ] **Scope:** Changes are limited to the files listed above.
- [ ] **Behavior:** `PublicKey::toEvmAddress()` returns `std::nullopt` for ED25519 keys and the Keccak-256-derived address for ECDSAsecp256k1 keys. No other behavior changes.
- [ ] **API:** The subclass return-type change and its migration (`.value()` / `->`) are called out in the PR description.
- [ ] **Tests:** New unit tests cover both the `std::nullopt` path (ED25519) and the engaged path (ECDSA) through a base-class pointer; 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
If you have questions while working on this issue, feel free to ask!
You can reach the community and maintainers here: [Hiero-SDK-C++ Discord](https://discord.com/channels/905194001349627914/1337424839761465364)
Whether you need help finding the right file, understanding the existing key hierarchy, or confirming your approach — we're happy to help.
Contributor guide
Assessment
This issue has not been assessed yet.