Skip to content

[Beginner]: Promote toEvmAddress() to a virtual method on PublicKey returning std::optional<EvmAddress> #1675

Description

@Dosik13

🐥 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):

[[nodiscard]] EvmAddress toEvmAddress() const;

Any code holding a generic std::shared_ptr<PublicKey> (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:

// src/tck/src/key/KeyService.cc:142-145
return std::dynamic_pointer_cast<ECDSAsecp256k1PublicKey>(privateKey->getPublicKey())
  ->toEvmAddress()
  .toString();

// src/sdk/tests/integration/TransferTransactionIntegrationTests.cc:104-105
const EvmAddress evmAddress =
  std::dynamic_pointer_cast<ECDSAsecp256k1PublicKey>(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:

// 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<EvmAddress> toEvmAddress() const;
// ECDSAsecp256k1PublicKey.h — change the existing declaration to an override
[[nodiscard]] std::optional<EvmAddress> toEvmAddress() const override;

The body of the existing override in src/sdk/main/src/ECDSAsecp256k1PublicKey.cc:404 stays exactly as it is — the EvmAddress it returns converts implicitly to std::optional<EvmAddress>.

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<PublicKey>. However, the return type of the existing ECDSAsecp256k1PublicKey::toEvmAddress() changes from EvmAddress to std::optional<EvmAddress> (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, add #include <optional>, 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, define the default implementation: return std::nullopt; (include EvmAddress.h there).
  • In src/sdk/main/include/ECDSAsecp256k1PublicKey.h (line 188) and src/sdk/main/src/ECDSAsecp256k1PublicKey.cc (line 404), change the return type to std::optional<EvmAddress> and mark the declaration override.
  • Update the typed call sites (mechanical):
    • src/sdk/main/src/AccountCreateTransaction.cc:67 and :88no change needed; mAlias is already a std::optional<EvmAddress>, 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:
  • Build and run the affected unit suites:
    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 for step-by-step workflow guidance.
Read README.md for setup instructions.

❗ Pull requests cannot be merged without S and s signed commits.
See the Signing Guide.

🤔 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

Whether you need help finding the right file, understanding the existing key hierarchy, or confirming your approach — we're happy to help.

Metadata

Metadata

Assignees

No one assigned

    Labels

    skill: beginnerSuitable for contributors who have completed a good first issue and want to build skills

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions