Skip to content

[Intermediate]: Add setECDSAKeyWithAlias overload accepting an ECDSAsecp256k1PublicKey to unblock HSM / external-signer flows #1676

Description

@Dosik13

🧩 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

AccountCreateTransaction::setECDSAKeyWithAlias — the recommended one-call way to set an account key and its derived EVM-address alias together — only accepts a private key (src/sdk/main/include/AccountCreateTransaction.h:102):

AccountCreateTransaction& setECDSAKeyWithAlias(const std::shared_ptr<ECDSAsecp256k1PrivateKey>& ecdsaKey);

The implementation (src/sdk/main/src/AccountCreateTransaction.cc:52-70) only ever uses the private key to reach its public key:

mKey = ecdsaKey;
const std::shared_ptr<ECDSAsecp256k1PublicKey> ecdsaPublicKey =
  std::dynamic_pointer_cast<ECDSAsecp256k1PublicKey>(ecdsaKey->getPublicKey());
mAlias = ecdsaPublicKey->toEvmAddress();

This shape blocks HSM and key-separation flows, where private key material never enters the application process — the caller holds only an ECDSAsecp256k1PublicKey and signing happens externally (HSM, KMS, hardware wallet, remote signer). Such callers today must fall back to composing two calls themselves:

transaction.setKeyWithoutAlias(publicKey).setAlias(publicKey->toEvmAddress());

That workaround works, but it bypasses the documented combined setter, is easy to get wrong (e.g. forgetting the alias entirely), and leaves the C++ SDK without parity for the public-key path being added across the sibling SDKs.

Note: the companion key-separation method setKeyWithAlias(key, ecdsaPrivateKey) (AccountCreateTransaction.h:114) has the same limitation. Whether to extend it in the same PR is left to contributor judgment (see Proposed Approach).

Relevant files:

src/sdk/main/include/AccountCreateTransaction.h
src/sdk/main/src/AccountCreateTransaction.cc
src/sdk/tests/unit/AccountCreateTransactionUnitTests.cc
src/sdk/tests/integration/AccountCreateTransactionIntegrationTests.cc

💡 Proposed Approach

Add a new overload, mirroring the existing one (the existing private-key overload is retained unchanged):

/**
 * Set an ECDSA public key, derive its EVM address in the background, and set both the key and alias.
 * Intended for flows where the private key is held externally (e.g. an HSM) and only the public key
 * is available in-process. The corresponding private key must still sign the transaction.
 *
 * @param ecdsaPublicKey The ECDSA public key to be set.
 * @return A reference to this AccountCreateTransaction object with the newly-set key and alias.
 * @throws IllegalStateException If this AccountCreateTransaction is frozen.
 * @throws std::invalid_argument If the key is null.
 */
AccountCreateTransaction& setECDSAKeyWithAlias(const std::shared_ptr<ECDSAsecp256k1PublicKey>& ecdsaPublicKey);

The implementation follows the existing pattern: requireNotFrozen(), null check throwing std::invalid_argument, then mKey = ecdsaPublicKey; and mAlias = ecdsaPublicKey->toEvmAddress(); (no dynamic_pointer_cast needed — the caller already hands us the typed public key).

Points that are left to contributor judgment — state your choices in the PR description:

  1. Overload-resolution caveat. Once the overload exists, a literal setECDSAKeyWithAlias(nullptr) (or {}) becomes ambiguous. No in-repo call site does this (the only caller is AccountCreateTransactionIntegrationTests.cc:689, which passes a typed shared_ptr), but your null-check unit test must use a typed null, e.g. std::shared_ptr<ECDSAsecp256k1PublicKey>{}. Worth a sentence in the PR description.
  2. Scope of the companion method. Decide whether to also add the analogous setKeyWithAlias(const std::shared_ptr<Key>&, const std::shared_ptr<ECDSAsecp256k1PublicKey>&) overload now or file it as a follow-up. Either is acceptable; keep the PR focused.
  3. Related work. There is a companion issue proposing PublicKey::toEvmAddress() become a base-class virtual returning std::optional<EvmAddress>. There is no hard dependency in either direction (mAlias is already a std::optional<EvmAddress>, so mAlias = ecdsaPublicKey->toEvmAddress(); compiles before and after that change), but expect a trivial merge interaction if both land.

👩‍💻 Implementation Steps

  • Add the overload declaration directly after the existing private-key overload in src/sdk/main/include/AccountCreateTransaction.h (line 102), with Doxygen documentation matching the surrounding style.
  • Implement it in src/sdk/main/src/AccountCreateTransaction.cc, mirroring the style of setECDSAKeyWithAlias / setKeyWithoutAlias at lines 52-108.
  • Add unit tests in src/sdk/tests/unit/AccountCreateTransactionUnitTests.cc — mirror the SetKeyWithAlias / SetKeyWithAliasFrozen pair at lines 138-167:
    • key is set to the public key and getAlias() equals publicKey->toEvmAddress() (compare via toBytes()),
    • calling on a frozen transaction throws,
    • typed-null argument throws std::invalid_argument.
    • There are currently no unit tests at all for the existing private-key setECDSAKeyWithAlias overload — backfilling them in the same PR is welcome but optional.
  • Add an integration test in src/sdk/tests/integration/AccountCreateTransactionIntegrationTests.cc — mirror CreateTransactionWithAliasAndKeyECDSACanExecute (line 680). Simulate the HSM split: generate an ECDSAsecp256k1PrivateKey in the test, pass only privateKey->getPublicKey() (cast to ECDSAsecp256k1PublicKey) to the new overload, then freezeWith(&getTestClient()).sign(privateKey) to play the role of the external signer, execute, and assert the receipt contains an account ID.
  • Build and run the tests:
    cmake --preset linux-x64-debug -DBUILD_TESTS=ON
    cmake --build --preset linux-x64-debug -j 6
    
    # unit tests (no network needed)
    ctest -C Debug --test-dir build/linux-x64-debug -R AccountCreateTransactionUnitTests
    
    # integration tests (requires a running Solo network — see README.md)
    ctest -C Debug --test-dir build/linux-x64-debug -R AccountCreateTransactionIntegrationTests
  • Run clang-format-17 against any file you touched.

✅ Acceptance Criteria

  • The new setECDSAKeyWithAlias(const std::shared_ptr<ECDSAsecp256k1PublicKey>&) overload exists with Doxygen documentation matching the surrounding style, sets mKey to the public key, and sets mAlias to the derived EVM address.
  • The existing private-key overload is unchanged (signature and behavior).
  • Unit tests cover: successful set + alias derivation, frozen-transaction throw, and typed-null std::invalid_argument throw.
  • An integration test exercises the external-signer flow (public key set on the transaction, signature provided separately) and verifies the account is created.
  • The PR description states the choices made on the judgment points above (null-ambiguity note, setKeyWithAlias scope decision).
  • No unrelated behavior or public API changes; all existing tests still pass.

📋 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

  • The integration test requires a local Solo network; see README.md for setup. CI also runs the integration suite.
  • Equivalent public-key alias setters are being introduced across the sibling SDKs (Java/JS/Go) as part of the HSM / key-separation work.

If you have questions while working on this issue, feel free to ask! Hiero-SDK-C++ Discord

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions