Skip to content

chore: upgrade wallet-lib to v1.14.1#216

Merged
r4mmer merged 18 commits into
masterfrom
feat/upgrade-wallet-lib-v1.14.1
Mar 25, 2025
Merged

chore: upgrade wallet-lib to v1.14.1#216
r4mmer merged 18 commits into
masterfrom
feat/upgrade-wallet-lib-v1.14.1

Conversation

@r4mmer
Copy link
Copy Markdown
Member

@r4mmer r4mmer commented Feb 26, 2025

Motivation

Wallet-lib is a core dependency but is currently 2 major versions outdated.
This PR is the first part upgrading and refactoring the codebase to work with @hathor/wallet-lib@1.14.1

Acceptance Criteria

  • Upgrade the wallet-lib version to v1.14.1
  • Refactor codebase to work after breaking changes

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged
  • Make sure either the unit tests and/or the QA tests are capable of testing the new features
  • Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

Decisions

### Decision 1

Code: DEC-0001

Some constants are network configured and come from the /v1a/version response of the fullnode.

These constants were previously configured on the config module and updated by the hathorLib.version.checkApiVersion.
This was replaced by having an ApiConfig instance on the storage to avoid having 1 config for all wallets connected.

Currently we need to have a way to get these configured constants, we could go with one of the following:
1. Use pre-configured hardcoded constants.
2. Get from connected fullnode.
~~ - The method maybeRefreshWalletConstants in packages/wallet-service/src/commons.ts can be used to refresh constants based on a request to the fullnode.~~
3. Get from wallet-service config.

Current changes that may change after a decision is made:
- Uses of hathorLib.transaction.getMaxOutputsConstant()
~~ - Temporarily fixed by using constants.MAX_OUTPUTS = 255.~~
- Call to hathorLib.version.checkApiVersion() to update variables does not exist
~~ - Will use the version api directly so the data can be saved on the database.~~
~~ - Will not save the values until a decision is made.~~

### Decision 2

Code: DEC-0002

Some constants of the wallet-lib were deprecated, they are being used as sensible defaults but we need to check if we are still going to use these values.
An example would be the DEFAULT_SERVERS and DEFAULT_SERVER constants.

We can either:
- hardcode the values
~~ - includes creating a constants file to keep these values.~~
- create a way to ensure some settings are configured
~~ - This way we do not depend on sensible defaults~~
~~ - The deploy should fail if these are not configured~~

Currently the values are being hardcoded to avoid changing any behavior.

Files

List of files that were changed:

  • packages/daemon/src/config.ts
    • Use String(process.env...) only in requiredEnvs to ensure typing as string and not string | undefined.

Files that contain @hathor/wallet-lib

  • packages/common/src/types.ts
    • No changes, only uses common constants that have not changed.
  • packages/common/src/utils/nft.utils.ts
    • No changes, uses utils that exist on most recent version.
  • packages/common/src/utils/wallet.utils.ts
    • No changes, only uses common constants that have not changed.
  • packages/daemon/src/services/index.ts
    • No changes, but to ensure typing we need to make getConfig().NETWORK to be strictly a string.
  • packages/daemon/src/types/token.ts
    • Uses HATHOR_TOKEN_CONFIG which was deprecated, need to use a combination of DEFAULT_NATIVE_TOKEN_CONFIG and NATIVE_TOKEN_UID.
  • packages/daemon/src/utils/wallet.ts
    • Merged split imports of wallet-lib
    • XXX: Sets properties of Output that do NOT exist.
    • Some ts-ignore may be introducing bugs
  • packages/wallet-service/src/api/tokens.ts
    • Changed HATHOR_TOKEN_CONFIG.uid for NATIVE_TOKEN_UID
  • packages/wallet-service/src/api/totalSupply.ts
    • Changed HATHOR_TOKEN_CONFIG.uid for NATIVE_TOKEN_UID
  • packages/wallet-service/src/api/txOutputs.ts
  • packages/wallet-service/src/api/txProposalCreate.ts
  • packages/wallet-service/src/api/txProposalSend.ts
  • packages/wallet-service/src/api/txhistory.ts
    • Uses HATHOR_TOKEN_CONFIG which was deprecated, changed to NATIVE_TOKEN_UID.
  • packages/wallet-service/src/commons.ts
  • packages/wallet-service/src/db/index.ts
  • packages/wallet-service/src/types.ts
    • Uses HATHOR_TOKEN_CONFIG which was deprecated, need to use a combination of DEFAULT_NATIVE_TOKEN_CONFIG and NATIVE_TOKEN_UID.
  • packages/wallet-service/src/utils.ts
    • Uses constants that do not exist, like DEFAULT_SERVER.

Tests files will be changed last since we need to change the methods first.
Package manager files are deferred since they should be updated automatically when upgrading.

  • yarn.lock
  • package.json
  • packages/common/tests/utils/nft.utils.test.ts
  • packages/common/package.json
  • packages/daemon/tests/db/index.test.ts
  • packages/daemon/tests/services/services.test.ts
  • packages/daemon/package.json
  • packages/wallet-service/AUTHENTICATION.md
  • packages/wallet-service/package.json
  • packages/wallet-service/tests/api.test.ts
  • packages/wallet-service/tests/commons.test.ts
  • packages/wallet-service/tests/db.test.ts
  • packages/wallet-service/tests/txProposal.test.ts
  • packages/wallet-service/tests/utils.test.ts
  • packages/wallet-service/tests/utils.ts

@r4mmer r4mmer self-assigned this Feb 26, 2025
@r4mmer r4mmer moved this from Todo to In Progress (WIP) in Hathor Network Feb 26, 2025
@r4mmer r4mmer moved this from In Progress (WIP) to In Progress (Done) in Hathor Network Mar 13, 2025
@r4mmer r4mmer marked this pull request as ready for review March 13, 2025 01:44

const hathorConfig = hathorLib.constants.HATHOR_TOKEN_CONFIG;
// XXX: get config from settings?
const hathorConfig = constants.DEFAULT_NATIVE_TOKEN_CONFIG;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This means that the daemon only supports Hathor as native token.
If we require support for side-dags we should add a nodeConfig module on the daemon (or common).

expect(returnBody.details[0].message).toBe('Signatures are not valid');
});

test('PUT /wallet/auth should fail if we cannot confirm the firstAddress', async () => {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This test was created just to increase coverage to pass the CI

expect(wallet.status).toStrictEqual(WalletStatus.ERROR);
}, 30000);

test('loadWalletFailed should create alert if xpubkey is missing', async () => {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This test was created just to increase coverage to pass the CI

});

// reload module
jest.resetModules();
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

reset modules is required so that the import('@src/utils/pushnotification.utils') reimports the config and use the most recent values

Comment thread packages/daemon/src/config.ts
Comment thread packages/wallet-service/package.json Outdated
Comment thread db/migrations/20250306170811-node_version_proxy.js Outdated
Comment thread packages/common/__tests__/utils/nft.utils.test.ts
Comment thread packages/daemon/src/utils/wallet.ts
Comment thread packages/wallet-service/src/ws/utils.ts
// get the first address
const xpubChangeDerivation = walletUtils.xpubDeriveChild(XPUBKEY, 0);
const firstAddress = walletUtils.getAddressAtIndex(xpubChangeDerivation, 0, process.env.NETWORK);
const firstAddressData = addressUtils.deriveAddressFromXPubP2PKH(xpubChangeDerivation, 0, process.env.NETWORK);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Use the config to get the network?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Did not seem necessary to refactor the tests to also use the config.
What do you think?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe to guarantee that the config is being filled correctly?

Comment thread packages/wallet-service/tests/api.test.ts
Comment thread packages/wallet-service/tests/db.test.ts
Comment thread packages/wallet-service/tests/txProposal.test.ts Outdated
@github-project-automation github-project-automation Bot moved this from In Progress (Done) to In Review (WIP) in Hathor Network Mar 18, 2025
Comment thread db/migrations/20250306170811-node_version_proxy.js Outdated
Comment thread packages/wallet-service/src/schemas.ts
Comment thread packages/common/src/utils/nft.utils.ts
@pedroferreira1 pedroferreira1 moved this from In Review (WIP) to In Review (Done) in Hathor Network Mar 19, 2025
@r4mmer r4mmer moved this from In Review (Done) to In Progress (Done) in Hathor Network Mar 19, 2025
pedroferreira1
pedroferreira1 previously approved these changes Mar 20, 2025
// get the first address
const xpubChangeDerivation = walletUtils.xpubDeriveChild(XPUBKEY, 0);
const firstAddress = walletUtils.getAddressAtIndex(xpubChangeDerivation, 0, process.env.NETWORK);
const firstAddressData = addressUtils.deriveAddressFromXPubP2PKH(xpubChangeDerivation, 0, process.env.NETWORK);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe to guarantee that the config is being filled correctly?

@pedroferreira1 pedroferreira1 moved this from In Progress (Done) to In Review (WIP) in Hathor Network Mar 20, 2025
andreabadesso
andreabadesso previously approved these changes Mar 21, 2025
Comment thread packages/wallet-service/tests/env.test.ts Outdated
@r4mmer r4mmer moved this from In Review (WIP) to In Review (Done) in Hathor Network Mar 24, 2025
@r4mmer r4mmer dismissed stale reviews from andreabadesso and pedroferreira1 via dfd52bb March 24, 2025 23:31
@andreabadesso andreabadesso self-requested a review March 25, 2025 13:47
@pedroferreira1 pedroferreira1 self-requested a review March 25, 2025 15:36
@github-project-automation github-project-automation Bot moved this from In Review (Done) to In Review (WIP) in Hathor Network Mar 25, 2025
@pedroferreira1 pedroferreira1 moved this from In Review (WIP) to In Review (Done) in Hathor Network Mar 25, 2025
@r4mmer r4mmer merged commit d252f45 into master Mar 25, 2025
@github-project-automation github-project-automation Bot moved this from In Review (Done) to Waiting to be deployed in Hathor Network Mar 25, 2025
@r4mmer r4mmer deleted the feat/upgrade-wallet-lib-v1.14.1 branch March 25, 2025 16:00
@r4mmer r4mmer mentioned this pull request Apr 1, 2025
2 tasks
@r4mmer r4mmer moved this from Waiting to be deployed to Done in Hathor Network Apr 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants