[wallet/common] feat: create core project #1473

OlegMakarenko · 2025-07-07T13:08:51Z

Problem:

We didn't yet have a shared, reusable, protocol-agnostic core library for managing wallets, accounts, signing, storage, and network connections across different wallet apps (mobile, desktop, browser extension).

Each app would have to implement its own controller, storage abstraction, keystore logic, and protocol integration, leading to duplicated effort, inconsistent APIs, and maintenance overhead.

Solution:

This PR creates the new Wallet Common Core project, providing:

Project structure and setup.
A reusable, modular orchestration layer (WalletController)
Pluggable keystore system (MnemonicKeystore, ExternalAccountKeystore)
- Abstracted storage interface
Protocol API and SDK wrappers
Entry point exports
Added README.md
Added tests

This project/package can be consumed by any wallet app <NEM|Symbol><Desktop|Mobile|Extension>Wallet to handle core wallet functionality.

codecov · 2025-07-07T13:10:06Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.22%. Comparing base (be149f3) to head (aa84495).
Report is 6 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1473      +/-   ##
==========================================
- Coverage   98.26%   98.22%   -0.05%     
==========================================
  Files         163      157       -6     
  Lines        6691     6477     -214     
  Branches      143      143              
==========================================
- Hits         6575     6362     -213     
+ Misses        116      115       -1

Flag	Coverage Δ
explorer-rest	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

see 6 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

wallet/common/core/.eslintrc.yaml

wallet/common/core/README.md

wallet/common/core/scripts/ci/setup_build.sh

wallet/common/core/scripts/ci/build.sh

wallet/common/core/src/constants/index.js

wallet/common/core/tests/test-utils/keysore.js

wallet/common/core/src/lib/controller/WalletController.js

wallet/common/core/src/utils/storage.js

wallet/common/core/src/lib/ProtocolSdk.js

… fix incorrect types, add missing method to ProtocolApi class

OlegMakarenko · 2025-07-10T11:57:28Z

@cryptoBeliever thanks, new changes pushed!

Wayonb · 2025-07-20T13:47:17Z

wallet/common/core/package.json

@@ -0,0 +1,19 @@
+{
+	"name": "@wallet/common-core",
+	"main": "./src/index.js",


Does the package need a version? How do the other components comsume this module?

I'm thinking of using it as a local package, e.g.:

{ "dependencies": { "@wallet/common-core": "file:../../common/core" } }

Does the package need a version?

I will add the version.

Wayonb · 2025-07-20T13:53:37Z

wallet/common/core/package.json

+	"devDependencies": {
+		"c8": "~10.1.2",
+		"eslint": "~8.57.0",
+		"eslint-config-airbnb": "~19.0.4",


eslint-config-airbnb has not been updated to support ESLint 9. We may need to replace it if it is not updated. Don't need to change now, but just making a note.

Wayonb · 2025-07-20T14:00:28Z

wallet/common/core/src/error/AppError.js

+	constructor(code, message) {
+		super(message);
+		this.name = this.constructor.name;
+		this.code = code;


Should the code be included in the toString()?

Wayonb · 2025-07-20T14:04:22Z

wallet/common/core/src/lib/ProtocolApi.js

+/** @typedef {import('../types/Transaction').SignedTransactionDTO} SignedTransactionDTO */
+
+/**
+ * @callback FetchAccountInfoFn


should this be FetchAccountInfoCallback?

This function does not truly represent a callback function. It wasn't clear until I saw the code. For JavaScript, any function parameter is considered a callback. 😅
This is the Strategy pattern, where you allow different implementations? Is this support for NEM and Symbol only?

Wayonb · 2025-07-20T14:17:40Z

README.md

+[wallet-common-core-lint]: https://jenkins.symbolsyndicate.us/buildStatus/icon?job=Symbol%2Fgenerated%2Fproduct%2Fwallet-common-core%2Fdev%2F&config=wallet-common-core-lint
+[wallet-common-core-test]: https://jenkins.symbolsyndicate.us/buildStatus/icon?job=Symbol%2Fgenerated%2Fproduct%2Fwallet-common-core%2Fdev%2F&config=wallet-common-core-test
+[wallet-common-core-cov]: https://codecov.io/gh/symbol/product/branch/dev/graph/badge.svg?token=SSYYBMK0M7&flag=wallet-common-core
+[wallet-common-core-cov-link]: https://codecov.io/gh/symbol/product/tree/dev/tools/vanity


tools/vanity => wallet/common/core?

wallet/common/core/src/lib/storage/PersistentStorageRepository.js

Wayonb · 2025-07-20T19:06:20Z

wallet/common/core/src/lib/storage/PersistentStorageRepository.js

+	 */
+	clear = async () => {
+		await Promise.all(Object.values(PersistentStorageRepository.STORAGE_KEYS).map(key => this.storage.removeItem(key)));
+	};


Would this.storage.clear()work better instead of removing each item, since I am assuming localStorage is used?

If not, you might want to check if the storage is empty

We only want to remove the keys that are managed by this repository. Since the same storage may be shared across multiple repositories, it's important to ensure that only data related to the current repository is cleared.

localStorage is used by the browser extension wallet and mobile wallets use an API for device storage and encrypted storage.

If not, you might want to check if the storage is empty

Removing a non-existent value from localStorage doesn’t throw an error, so checking whether a value exists beforehand is unnecessary in this case. If other storage implementations behave differently and throw an error, such checks should be handled inside the StorageInterface, IMO.

Ok. That makes sense.

across multiple repositories

What do you mean by multiple repositories? Is it when multiple users use the wallet on the same device?

We have two separate repositories: PersistentStorageRepository for storing WalletController-related data (like public account info), and SecureStorageRepository for storing sensitive encrypted data (such as private keys). Both repositories rely on a shared StorageInterface, which provides methods like set, get, and remove.

In the mobile wallet, these repositories use different StorageInterface implementations (AsyncStorage and EncryptedStorage). However, in the browser extension wallet, both repositories use the same underlying storage - LocalStorage. As a result, calling localStorage.clear() from PersistentStorageRepository would clear data managed by SecureStorageRepository as well. This is why I avoid using .clear() and instead remove only the relevant keys tied to each repository.

Wayonb · 2025-07-21T01:25:43Z

wallet/common/core/Jenkinsfile

+
+	packageId = 'wallet-common-core'
+
+	codeCoverageTool = 'c8'


Did you want to set the minimumCodeCoverage?

Wayonb · 2025-07-21T02:25:02Z

wallet/common/core/src/lib/ProtocolSdk.js

+
+/**
+ * @callback SignTransactionFn
+ * @param {NetworkProperties} networkProperties - The network properties.


Is the networkProperties needed to get the generationHashSeed for tx signing?

In Symbol implementation we use networkIdentifier to create SymbolFasade('testnet' | 'mainnet'), networkCurrency.divisibility for fee and epochAdjustment for deadline.

Looks like generationHashSeed is not used.

Wayonb · 2025-07-21T02:25:47Z

wallet/common/core/src/lib/ProtocolSdk.js

+
+/**
+ * @callback CosignTransactionFn
+ * @param {NetworkProperties} networkProperties - The network properties.


Why is the networkProperties needed here?

You are right, networkProperties doesn't needed here. We don't need to create instance of SymbolFacade since cosignTransactionHash is a static method.

[wallet/common] feat: create core project

4060a51

[wallet/common] feat: add CI configuration for core project

3720484

OlegMakarenko force-pushed the wallet/common branch from 2aa31ee to 3720484 Compare July 7, 2025 13:44

OlegMakarenko added 4 commits July 7, 2025 15:54

[wallet/common] fix: timestamp issue in test

fd40179

[wallet/common] fix: example in readme

85695b4

[wallet/common] fix: jsdoc

d41fa0f

[wallet/common] fix: readme

4ac7db2

OlegMakarenko requested a review from Wayonb July 8, 2025 08:00

cryptoBeliever reviewed Jul 9, 2025

View reviewed changes

wallet/common/core/.eslintrc.yaml Outdated Show resolved Hide resolved

cryptoBeliever reviewed Jul 9, 2025

View reviewed changes

OlegMakarenko added 7 commits July 9, 2025 20:20

[wallet/common] fix: remove unused CI scripts

b27e4d1

[wallet/common] fix: update path to default config in eslintrc

237bc24

[wallet/common] fix: typo in filename

f5f59ef

[wallet/common] fix: reove commented code

87ab8d7

[wallet/common] fix: improve null handling in decodeJson helper

1d49746

[wallet/common] feat: add more js-doc comments, improve type imports,…

8abcd29

… fix incorrect types, add missing method to ProtocolApi class

[monorepo] feat: add wallet-common-core project to readme

48e714d

cryptoBeliever approved these changes Jul 10, 2025

View reviewed changes

[wallet/common] fix: comment in transaction message js-doc

aa84495

Wayonb requested changes Jul 21, 2025

View reviewed changes

[wallet/common] feat: create core project #1473

Are you sure you want to change the base?

[wallet/common] feat: create core project #1473

Uh oh!

Conversation

OlegMakarenko commented Jul 7, 2025

Problem:

Solution:

Uh oh!

codecov bot commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

OlegMakarenko commented Jul 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Jul 7, 2025 •

edited

Loading