hiero-ledger / hiero-ledger/hiero-sdk-cpp

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

Open
#1,676 0 comments 0 reactions 1 assignee Claimed by @Adityarya11 View on GitHub
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

`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](../../src/sdk/main/include/AccountCreateTransaction.h)):

```cpp
AccountCreateTransaction& setECDSAKeyWithAlias(const std::shared_ptr& ecdsaKey);
```

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

```cpp
mKey = ecdsaKey;
const std::shared_ptr ecdsaPublicKey =
std::dynamic_pointer_cast(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:

```cpp
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](../../src/sdk/main/include/AccountCreateTransaction.h)) 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):

```cpp
/**
* 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& 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{}`. 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&, const std::shared_ptr&)` 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`. There is no hard dependency in either direction (`mAlias` is already a `std::optional`, 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](../../src/sdk/main/include/AccountCreateTransaction.h) (line 102), with Doxygen documentation matching the surrounding style.
- [ ] Implement it in [src/sdk/main/src/AccountCreateTransaction.cc](../../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](../../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](../../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:
```bash
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&)` 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](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 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](https://discord.com/channels/905194001349627914/1337424839761465364)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.