Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: account create transaction with key derived alias #2834

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

venilinvasilev
Copy link

@venilinvasilev venilinvasilev commented Jan 31, 2025

Description:
This PR extends the AccountCreateTransaction with the following APIs:

  • setECDSAKeyWithAlias(ECDSAKey) - Sets ECDSA private key, derives and sets it's EVM address in the background.
  • setKeyWithAlias(Key, ECDSAKey) - Allows for setting the account key and a separate ECDSA private key that the EVM address should be derived from. A user must sign the transaction with both keys for this flow to be successful.
  • setKeyWithoutAlias(Key) - Explicitly calls out that the alias is not set.

🚨 Deprecation Notice:

  • setKey(Key) is now deprecated in favor of setKeyWithoutAlias.

Related issue(s):
#2795

✅ Checklist:

  • Extended the APIs of AccountCreateTransaction
  • Deprecated setKey(Key) API of AccountCreateTransaction
  • Tested (unit, integration, etc.)
  • Replaced deprecated API (setKey(Key)) occurences with the new relevant one (setKeyWithoutAlias(Key))
  • Add examples

@venilinvasilev venilinvasilev requested a review from a team as a code owner January 31, 2025 13:53
@venilinvasilev venilinvasilev force-pushed the feat/account-create-transaction-with-key-derived-alias branch 2 times, most recently from db5f836 to eae742b Compare January 31, 2025 13:58
@venilinvasilev venilinvasilev marked this pull request as draft January 31, 2025 14:26
Copy link

codecov bot commented Jan 31, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
src/account/AccountCreateTransaction.js 92.30% <100.00%> (ø)

@venilinvasilev venilinvasilev force-pushed the feat/account-create-transaction-with-key-derived-alias branch 3 times, most recently from a1f8f3e to 2e7d9d6 Compare February 3, 2025 09:17
src/account/AccountCreateTransaction.js Outdated Show resolved Hide resolved
src/account/AccountCreateTransaction.js Show resolved Hide resolved
examples/create-account-without-alias.js Outdated Show resolved Hide resolved
@venilinvasilev venilinvasilev force-pushed the feat/account-create-transaction-with-key-derived-alias branch 5 times, most recently from 30f3dd4 to da465eb Compare February 3, 2025 10:01
test/unit/AccountCreateTransaction.js Outdated Show resolved Hide resolved
test/unit/AccountCreateTransaction.js Outdated Show resolved Hide resolved
src/account/AccountCreateTransaction.js Show resolved Hide resolved
@venilinvasilev venilinvasilev force-pushed the feat/account-create-transaction-with-key-derived-alias branch from da465eb to 7a66df4 Compare February 3, 2025 13:13
Added new APIs to AccountCreateTransaction:
- `setECDSAKeyWithAlias(ECDSAPrivateKey)`: Sets ECDSA private key and alias derived from it.
- `setKeyWithAlias(Key, ECDSAPrivateKey)`: Allows setting account key and a separate ECDSA private key for EVM address derivation.
- `setKeyWithoutAlias(Key)`: Explicitly sets key without alias setting.

Deprecated:
- `setKey(Key)`: Replaced with `setKeyWithoutAlias` method.

Includes unit tests for all new functionality.

Signed-off-by: venilinvasilev <[email protected]>
Added integration tests for `AccountCreateTransaction` to validate new key alias APIs

Replaced deprecated `setKey(Key)` references  with `setKeyWithoutAlias(Key)`, aligning with the new API changes.

Signed-off-by: venilinvasilev <[email protected]>
Updated all test suites to replace deprecated `setKey(Key)` calls with `setKeyWithoutAlias(Key)`, ensuring compatibility with the latest API changes.

Signed-off-by: venilinvasilev <[email protected]>
Replaced all occurrences of `setKey(Key)` with `setKeyWithoutAlias(Key)` in example implementations, ensuring consistency with the latest API updates.

Signed-off-by: venilinvasilev <[email protected]>
Updated a remaining deprecated reference to `setKey(Key)` in tck

Signed-off-by: venilinvasilev <[email protected]>
@venilinvasilev venilinvasilev force-pushed the feat/account-create-transaction-with-key-derived-alias branch from 7a66df4 to 293f83e Compare February 3, 2025 14:52
@venilinvasilev venilinvasilev marked this pull request as ready for review February 3, 2025 14:54
@venilinvasilev venilinvasilev force-pushed the feat/account-create-transaction-with-key-derived-alias branch from 293f83e to 1eddac8 Compare February 3, 2025 14:57
Added code examples demonstrating how to use:
- `setECDSAKeyWithAlias(ECDSAKey)`
- `setKeyWithAlias(Key, ECDSAKey)`
- `setKeyWithoutAlias(Key)`

Signed-off-by: venilinvasilev <[email protected]>
@venilinvasilev venilinvasilev force-pushed the feat/account-create-transaction-with-key-derived-alias branch from 1eddac8 to 718149b Compare February 3, 2025 17:00
Copy link
Contributor

@ivaylogarnev-limechain ivaylogarnev-limechain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants