Skip to content

feat: export all types and utils from public API#1034

Merged
pedroferreira1 merged 8 commits intomasterfrom
feat/export-all-types-and-utils
Mar 24, 2026
Merged

feat: export all types and utils from public API#1034
pedroferreira1 merged 8 commits intomasterfrom
feat/export-all-types-and-utils

Conversation

@andreabadesso
Copy link
Copy Markdown
Contributor

@andreabadesso andreabadesso commented Mar 15, 2026

Motivation

All TypeScript types, interfaces, enums, and utility functions from every types.ts file are now re-exported through the library's public API (lib.ts), making them available to consumers via import { ... } from '@hathor/wallet-lib'.

Instead, we resolved the naming conflicts at their source files so lib.ts can use clean export * re-exports. New types added to any types.ts file are now automatically part of the public API with zero lib.ts changes.

Breaking Changes

The following types have been renamed at their source. External consumers importing the old names will need to update:

Old Name New Name Source File
Balance WalletServiceBalance src/wallet/types.ts
Authority WalletServiceAuthority src/wallet/types.ts
DelegateAuthorityOptions WalletServiceDelegateAuthorityOptions src/wallet/types.ts
DestroyAuthorityOptions WalletServiceDestroyAuthorityOptions src/wallet/types.ts
CreateNanoTxData FullnodeCreateNanoTxData src/new/types.ts

Note: The models/types.ts versions (Balance, Authority) and nano_contracts/types.ts version (CreateNanoTxData) keep their original names. Only the duplicates in wallet/types.ts and new/types.ts were renamed.

Acceptance Criteria

  • All TypeScript types, interfaces, enums, and utility functions from every types.ts file are re-exported through lib.ts
  • lib.ts uses only export * for type re-exports (no manual type listings)
  • No naming conflicts between any types.ts modules
  • TypeScript compiles clean, all export tests pass

Security Checklist

  • 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.

Re-export all type definitions from types.ts, wallet/types.ts,
new/types.ts, template/transaction/types.ts, headers/types.ts,
nano_contracts/types.ts, and models/types.ts through the library's
public API entry point (lib.ts).

Renamed conflicting types to avoid ambiguity:
- wallet/types.ts: Balance -> WalletBalance, Authority -> WalletAuthority
- new/types.ts: DelegateAuthorityOptions -> FullnodeDelegateAuthorityOptions,
  DestroyAuthorityOptions -> FullnodeDestroyAuthorityOptions,
  CreateNanoTxData -> FullnodeCreateNanoTxData
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 15, 2026

📝 Walkthrough

Walkthrough

Broadens the library's public TypeScript type surface via wider export * re-exports in src/lib.ts, renames and refactors several wallet/nano-contract types and method signatures, and adds an exhaustive runtime and compile-time export validation test at __tests__/exports.test.ts.

Changes

Cohort / File(s) Summary
Export Validation Tests
__tests__/exports.test.ts
Adds a large Jest test that imports the library public surface and asserts enum values, type-guard behavior, presence of utility namespaces and functions, major public classes, nano-contract schema parsing, and several type-only compile-time shapes.
Top-level export surface
src/lib.ts
Replaced curated named exports with broader re-exports: added export * from ./types, ./template/transaction/types, ./headers/types, ./wallet/types, and ./new/types; removed named exports WalletType, HistorySyncMode, and TokenInfo.
Fullnode nano-tx types
src/new/types.ts
Renamed interface CreateNanoTxDataFullnodeCreateNanoTxData (same fields).
Fullnode wallet API
src/new/wallet.ts
Updated method signatures for nano-contract transaction helpers to accept FullnodeCreateNanoTxData instead of CreateNanoTxData (affects create/send and create-token variants).
Wallet service types
src/wallet/types.ts
Renamed service-facing types: BalanceWalletServiceBalance, AuthorityWalletServiceAuthority; renamed option interfaces to WalletServiceDelegateAuthorityOptions and WalletServiceDestroyAuthorityOptions; updated interfaces and IHathorWallet method parameter types accordingly.
Wallet implementation parameter types
src/wallet/wallet.ts
Replaced import and parameter annotations to use WalletServiceDelegateAuthorityOptions and WalletServiceDestroyAuthorityOptions in prepareDelegateAuthorityData, delegateAuthority, prepareDestroyAuthorityData, and destroyAuthority.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • pedroferreira1
  • raul-oliveira
  • tuliomir

Poem

🐇 I hopped through exports, clean and neat,
Types aligned in tidy rows,
Tests sniffed each name with tiny feet,
And now our public garden grows. 🌱✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: export all types and utils from public API' directly describes the main objective: re-exporting all types and utility functions through the public API entry point (lib.ts).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/export-all-types-and-utils

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@__tests__/exports.test.ts`:
- Around line 33-45: Remove the unused type-import lint warnings by creating a
single type-only alias that references the unused exported types so they remain
checked at compile time but no runtime code is emitted; for example, add a type
alias like "type _ExportedTypesCheck = EcdsaTxSign | UtxoSelectionAlgorithm |
AddressScanPolicy | AddressScanPolicyData | ILogger | ITxSignatureData |
IInputSignature | IAddressInfo | IAddressMetadata;" (ensure this uses the exact
symbols from the import list) instead of leaving them as standalone unused
imports or disabling ESLint.

In `@src/new/wallet.ts`:
- Around line 3017-3018: FullnodeCreateNanoTxData marks blueprintId, ncId, and
args optional but the code calls createNanoContractTransaction and
createNanoContractCreateTokenTransaction with non-null assertions; add runtime
validation before those calls: check blueprintId and ncId are non-empty strings
and args is present/has expected shape (array/object) and if not throw a clear
Error (or return a rejected Promise) indicating which field is missing/invalid;
remove the non-null assertions when calling createNanoContractTransaction and
createNanoContractCreateTokenTransaction so the builders only receive validated
values and apply this same guard to the other similar invocation sites that pass
blueprintId, ncId, and args.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 42c84ca8-e1eb-4924-838d-725ccfc30f11

📥 Commits

Reviewing files that changed from the base of the PR and between 088ec8e and e6d0d93.

📒 Files selected for processing (5)
  • __tests__/exports.test.ts
  • src/lib.ts
  • src/new/types.ts
  • src/new/wallet.ts
  • src/wallet/types.ts

Revert source type renames (Balance, Authority, DelegateAuthorityOptions,
DestroyAuthorityOptions, CreateNanoTxData) to preserve backward
compatibility for consumers importing from deep paths.

Instead, handle naming conflicts at the lib.ts re-export level by:
- Using `export *` for modules without conflicts
- Using explicit named exports for wallet/types.ts and new/types.ts
- Providing disambiguated aliases (WalletBalance, WalletAuthority,
  FullnodeDelegateAuthorityOptions, etc.) for conflicting names
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 15, 2026

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 87.76%. Comparing base (5ec21f6) to head (643eec9).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/wallet/wallet.ts 0.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #1034       +/-   ##
===========================================
+ Coverage   66.44%   87.76%   +21.31%     
===========================================
  Files         114      114               
  Lines        8828     8828               
  Branches     2006     1996       -10     
===========================================
+ Hits         5866     7748     +1882     
+ Misses       2934     1052     -1882     
  Partials       28       28               

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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.

@andreabadesso andreabadesso self-assigned this Mar 15, 2026
@andreabadesso andreabadesso added the enhancement New feature or request label Mar 15, 2026
@andreabadesso andreabadesso moved this to In Progress (Done) in Hathor Network Mar 15, 2026
@andreabadesso
Copy link
Copy Markdown
Contributor Author

Re: CodeRabbit comment on src/new/wallet.ts non-null assertions:

This file is no longer modified in this PR (the rename was reverted). The non-null assertion issue on blueprintId!, ncId!, args! is pre-existing and out of scope for this change.

The imports are intentionally exhaustive to validate the public API
surface at compile time, not to be used at runtime.
@andreabadesso andreabadesso moved this from In Progress (Done) to In Progress (WIP) in Hathor Network Mar 16, 2026
@andreabadesso andreabadesso moved this from In Progress (WIP) to In Progress (Done) in Hathor Network Mar 20, 2026
andreabadesso and others added 2 commits March 20, 2026 15:04
Resolve naming conflicts between wallet/types, new/types, models/types,
and nano_contracts/types at their source files instead of aliasing in
lib.ts. This allows lib.ts to use clean `export *` re-exports, removing
~120 lines of manual type listings.

Renames:
- wallet/types: Balance → WalletServiceBalance
- wallet/types: Authority → WalletServiceAuthority
- wallet/types: DelegateAuthorityOptions → WalletServiceDelegateAuthorityOptions
- wallet/types: DestroyAuthorityOptions → WalletServiceDestroyAuthorityOptions
- new/types: CreateNanoTxData → FullnodeCreateNanoTxData

BREAKING CHANGE: External consumers importing the old names will need
to update their imports.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/wallet/types.ts (1)

384-395: ⚠️ Potential issue | 🟠 Major

IHathorWallet now diverges between the two wallet facades.

These four signatures were switched to wallet-service-specific option types, but the provided snippets show src/new/wallet.ts still implements the same methods with DelegateAuthorityOptions / DestroyAuthorityOptions. That leaves the shared IHathorWallet contract inconsistent across implementations and breaks assignability for callers expecting one common interface.

Also applies to: 396-407

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/wallet/types.ts` around lines 384 - 395, The IHathorWallet method
signatures for prepareDelegateAuthorityData, delegateAuthority,
prepareDestroyAuthorityData, and destroyAuthority were changed to use
WalletServiceDelegateAuthorityOptions/WalletServiceDestroyAuthorityOptions but
some implementations (e.g. in src/new/wallet.ts) still accept
DelegateAuthorityOptions/DestroyAuthorityOptions, causing an inconsistent
contract; update the implementations of prepareDelegateAuthorityData,
delegateAuthority, prepareDestroyAuthorityData, and destroyAuthority in the new
wallet facade to accept the WalletServiceDelegateAuthorityOptions and
WalletServiceDestroyAuthorityOptions types (and update any related
imports/usages) so the IHathorWallet interface and all implementations use the
same option types.
src/wallet/wallet.ts (1)

2831-2838: ⚠️ Potential issue | 🟠 Major

Validate nano-contract IDs before feeding them into the builder.

Both methods still coerce data.blueprintId / data.ncId with as string and then pass them straight into the builder. If the caller omits the required field for the selected method, the builder receives undefined and we silently assemble a malformed nano-contract transaction instead of failing fast.

Based on learnings: In src/new/wallet.ts, the nano-contract creation methods use non-null assertions on optional CreateNanoTxData fields without prior runtime validation, and missing blueprintId, ncId, or args can silently produce malformed transactions.

Also applies to: 2994-3019

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/wallet/wallet.ts` around lines 2831 - 2838, The
NanoContractTransactionBuilder is being given possibly undefined
blueprintId/ncId via setBlueprintId/setNcId (in the builder chain that starts
with new NanoContractTransactionBuilder().setMethod(method).setWallet(...)),
which can silently produce malformed transactions; add runtime validation before
constructing the builder: check that data.blueprintId and data.ncId are present
(and non-empty strings) when the selected method requires them (e.g., methods
that create/operate on nano-contracts), and if missing throw or return an error
immediately (or reject the promise) instead of coercing with `as string`; update
the code paths around the builder creation to use the validated values and
ensure args are validated similarly when required.
♻️ Duplicate comments (1)
src/new/wallet.ts (1)

3038-3069: ⚠️ Potential issue | 🟠 Major

Add runtime guards before using blueprintId, ncId, and args.

FullnodeCreateNanoTxData is accepted at the API boundary, but Line 3064/3065/3068 and Line 3198/3199/3202 still force non-null assertions. Missing fields can still produce malformed nano-contract transactions at runtime.

Suggested fix
+    if (method === 'initialize' && !data.blueprintId) {
+      throw new NanoContractTransactionError('blueprintId is required for initialize method.');
+    }
+    if (method !== 'initialize' && !data.ncId) {
+      throw new NanoContractTransactionError('ncId is required for non-initialize methods.');
+    }
+    if (data.args == null) {
+      throw new NanoContractTransactionError('args is required (use [] if there are no args).');
+    }
+
     const builder = new NanoContractTransactionBuilder()
       .setMethod(method)
       .setWallet(this)
-      .setBlueprintId(data.blueprintId!)
-      .setNcId(data.ncId!)
+      .setBlueprintId(data.blueprintId)
+      .setNcId(data.ncId)
       .setCaller(new Address(address, { network: this.getNetworkObject() }))
       .setActions(data.actions)
-      .setArgs(data.args!)
+      .setArgs(data.args)

Based on learnings: in src/new/wallet.ts, createNanoContractTransaction and createNanoContractCreateTokenTransaction must validate blueprintId, ncId, and args explicitly before builder usage.

Also applies to: 3130-3203

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/new/wallet.ts` around lines 3038 - 3069, In createNanoContractTransaction
(and similarly in createNanoContractCreateTokenTransaction) add runtime guards
that explicitly check FullnodeCreateNanoTxData.blueprintId, .ncId, and .args
before calling NanoContractTransactionBuilder methods; if any are missing or
invalid, throw a descriptive NanoContractTransactionError (or appropriate error)
instead of using non-null assertions, then pass the validated values into
setBlueprintId, setNcId and setArgs to avoid malformed transactions at runtime.
🧹 Nitpick comments (2)
src/new/types.ts (1)

247-252: Export a safer fullnode nano-contract payload shape.

Now that this type is part of the public API, keeping blueprintId, ncId, and args as nullable optionals bakes the old “null means omitted” pattern into consumers and still doesn't encode the real invariant that one of blueprintId or ncId is required. This would be a good place to switch to undefined-based optionals and, ideally, split initialize vs execute payloads.

Based on learnings: HathorNetwork/hathor-wallet-lib uses undefined/? instead of null for optional TypeScript parameters, and nano-contract payloads need explicit validation for missing blueprintId, ncId, and args.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/new/types.ts` around lines 247 - 252, The exported
FullnodeCreateNanoTxData interface currently uses nullable optionals
(blueprintId?: string | null, ncId?: string | null, args?: unknown[] | null)
which encourages a "null means omitted" pattern and fails to encode that either
blueprintId or ncId must be present; change the shape to use undefined-based
optionals (remove | null) and enforce the invariant by splitting into two
discriminated types (e.g., InitializeNanoTxData with blueprintId: string and
optional args, and ExecuteNanoTxData with ncId: string and optional args) or by
making a union type that requires one of blueprintId or ncId, and update any
validation logic that consumes FullnodeCreateNanoTxData (places referencing
FullnodeCreateNanoTxData, blueprintId, ncId, args) to validate presence of the
required field and to treat absence as undefined rather than null.
src/wallet/types.ts (1)

323-327: Make the wallet-service authority option fields truly optional.

These renamed interfaces are now public, but anotherAuthorityAddress, createAnother, and pinCode are still modeled as required fields and use null to mean “not provided”. That doesn't match the runtime defaults in src/wallet/wallet.ts, and it forces consumers to pass placeholder nulls just to satisfy the type.

♻️ Suggested type-only cleanup
 export interface WalletServiceDelegateAuthorityOptions {
-  anotherAuthorityAddress: string | null;
-  createAnother: boolean;
-  pinCode: string | null;
+  anotherAuthorityAddress?: string;
+  createAnother?: boolean;
+  pinCode?: string;
 }
 
 export interface WalletServiceDestroyAuthorityOptions {
-  pinCode: string | null;
+  pinCode?: string;
 }

Based on learnings: In HathorNetwork/hathor-wallet-lib, avoid using null to indicate an optional/unprovided parameter in TypeScript. Use undefined via ? optional syntax instead.

Also applies to: 329-331

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/wallet/types.ts` around lines 323 - 327, The public interface
WalletServiceDelegateAuthorityOptions currently forces fields
anotherAuthorityAddress, createAnother, and pinCode to be present and use null
to indicate "not provided"; update WalletServiceDelegateAuthorityOptions to make
those fields optional (use the ? optional syntax and remove | null) so callers
can omit them (i.e., anotherAuthorityAddress?: string; createAnother?: boolean;
pinCode?: string), and apply the same optional/undefined pattern to the other
related interfaces around the same area (the interfaces referenced at lines
329-331) so no API surface uses null as a placeholder for "absent".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@__tests__/exports.test.ts`:
- Around line 193-194: The test imports non-existent types
FullnodeDelegateAuthorityOptions and FullnodeDestroyAuthorityOptions; either
update the test to import the actual exported names DelegateAuthorityOptions and
DestroyAuthorityOptions (replace FullnodeDelegateAuthorityOptions ->
DelegateAuthorityOptions and FullnodeDestroyAuthorityOptions ->
DestroyAuthorityOptions wherever used in __tests__/exports.test.ts) or add
simple type aliases in the source (in the module that exports
DelegateAuthorityOptions/DestroyAuthorityOptions, export type
FullnodeDelegateAuthorityOptions = DelegateAuthorityOptions and export type
FullnodeDestroyAuthorityOptions = DestroyAuthorityOptions) so the test compiles;
adjust all occurrences mentioned in the review (including the other affected
block around the 646-651 reference).
- Around line 300-309: The test asserts enum members that don't exist: update
the assertions in the failing test to use the actual exported enum members from
src/types.ts—replace HistorySyncMode.POLLING_HTTP_API / MANUAL_STREAM_WS /
XPUB_STREAM_WS with HistorySyncMode.DISABLED, HistorySyncMode.LIGHT,
HistorySyncMode.FULL (or whatever the current three string values are) and
replace WalletType.P2PKH with WalletType.STANDARD while keeping
WalletType.MULTISIG; ensure the expected string values match the enum
definitions (e.g., 'disabled','light','full','standard','multisig') so the test
compiles and verifies the real API.

---

Outside diff comments:
In `@src/wallet/types.ts`:
- Around line 384-395: The IHathorWallet method signatures for
prepareDelegateAuthorityData, delegateAuthority, prepareDestroyAuthorityData,
and destroyAuthority were changed to use
WalletServiceDelegateAuthorityOptions/WalletServiceDestroyAuthorityOptions but
some implementations (e.g. in src/new/wallet.ts) still accept
DelegateAuthorityOptions/DestroyAuthorityOptions, causing an inconsistent
contract; update the implementations of prepareDelegateAuthorityData,
delegateAuthority, prepareDestroyAuthorityData, and destroyAuthority in the new
wallet facade to accept the WalletServiceDelegateAuthorityOptions and
WalletServiceDestroyAuthorityOptions types (and update any related
imports/usages) so the IHathorWallet interface and all implementations use the
same option types.

In `@src/wallet/wallet.ts`:
- Around line 2831-2838: The NanoContractTransactionBuilder is being given
possibly undefined blueprintId/ncId via setBlueprintId/setNcId (in the builder
chain that starts with new
NanoContractTransactionBuilder().setMethod(method).setWallet(...)), which can
silently produce malformed transactions; add runtime validation before
constructing the builder: check that data.blueprintId and data.ncId are present
(and non-empty strings) when the selected method requires them (e.g., methods
that create/operate on nano-contracts), and if missing throw or return an error
immediately (or reject the promise) instead of coercing with `as string`; update
the code paths around the builder creation to use the validated values and
ensure args are validated similarly when required.

---

Duplicate comments:
In `@src/new/wallet.ts`:
- Around line 3038-3069: In createNanoContractTransaction (and similarly in
createNanoContractCreateTokenTransaction) add runtime guards that explicitly
check FullnodeCreateNanoTxData.blueprintId, .ncId, and .args before calling
NanoContractTransactionBuilder methods; if any are missing or invalid, throw a
descriptive NanoContractTransactionError (or appropriate error) instead of using
non-null assertions, then pass the validated values into setBlueprintId, setNcId
and setArgs to avoid malformed transactions at runtime.

---

Nitpick comments:
In `@src/new/types.ts`:
- Around line 247-252: The exported FullnodeCreateNanoTxData interface currently
uses nullable optionals (blueprintId?: string | null, ncId?: string | null,
args?: unknown[] | null) which encourages a "null means omitted" pattern and
fails to encode that either blueprintId or ncId must be present; change the
shape to use undefined-based optionals (remove | null) and enforce the invariant
by splitting into two discriminated types (e.g., InitializeNanoTxData with
blueprintId: string and optional args, and ExecuteNanoTxData with ncId: string
and optional args) or by making a union type that requires one of blueprintId or
ncId, and update any validation logic that consumes FullnodeCreateNanoTxData
(places referencing FullnodeCreateNanoTxData, blueprintId, ncId, args) to
validate presence of the required field and to treat absence as undefined rather
than null.

In `@src/wallet/types.ts`:
- Around line 323-327: The public interface
WalletServiceDelegateAuthorityOptions currently forces fields
anotherAuthorityAddress, createAnother, and pinCode to be present and use null
to indicate "not provided"; update WalletServiceDelegateAuthorityOptions to make
those fields optional (use the ? optional syntax and remove | null) so callers
can omit them (i.e., anotherAuthorityAddress?: string; createAnother?: boolean;
pinCode?: string), and apply the same optional/undefined pattern to the other
related interfaces around the same area (the interfaces referenced at lines
329-331) so no API surface uses null as a placeholder for "absent".

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6ac00338-18c5-4349-9274-03f45c5f2513

📥 Commits

Reviewing files that changed from the base of the PR and between a458c82 and c8820a7.

📒 Files selected for processing (6)
  • __tests__/exports.test.ts
  • src/lib.ts
  • src/new/types.ts
  • src/new/wallet.ts
  • src/wallet/types.ts
  • src/wallet/wallet.ts

Comment on lines +300 to +309
it('should export HistorySyncMode with correct values', () => {
expect(HistorySyncMode.POLLING_HTTP_API).toBe('polling-http-api');
expect(HistorySyncMode.MANUAL_STREAM_WS).toBe('manual-stream-ws');
expect(HistorySyncMode.XPUB_STREAM_WS).toBe('xpub-stream-ws');
});

it('should export WalletType with correct values', () => {
expect(WalletType.P2PKH).toBe('p2pkh');
expect(WalletType.MULTISIG).toBe('multisig');
});
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 20, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

These enum assertions target members that don't exist on the exported enums.

src/types.ts defines HistorySyncMode as DISABLED | LIGHT | FULL and WalletType as STANDARD | MULTISIG. The members used here (POLLING_HTTP_API, MANUAL_STREAM_WS, XPUB_STREAM_WS, P2PKH) are not part of the current API, so this test can't compile as written.

🧪 Minimal expectation fix
   it('should export HistorySyncMode with correct values', () => {
-    expect(HistorySyncMode.POLLING_HTTP_API).toBe('polling-http-api');
-    expect(HistorySyncMode.MANUAL_STREAM_WS).toBe('manual-stream-ws');
-    expect(HistorySyncMode.XPUB_STREAM_WS).toBe('xpub-stream-ws');
+    expect(HistorySyncMode.DISABLED).toBe('disabled');
+    expect(HistorySyncMode.LIGHT).toBe('light');
+    expect(HistorySyncMode.FULL).toBe('full');
   });
 
   it('should export WalletType with correct values', () => {
-    expect(WalletType.P2PKH).toBe('p2pkh');
+    expect(WalletType.STANDARD).toBe('standard');
     expect(WalletType.MULTISIG).toBe('multisig');
   });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@__tests__/exports.test.ts` around lines 300 - 309, The test asserts enum
members that don't exist: update the assertions in the failing test to use the
actual exported enum members from src/types.ts—replace
HistorySyncMode.POLLING_HTTP_API / MANUAL_STREAM_WS / XPUB_STREAM_WS with
HistorySyncMode.DISABLED, HistorySyncMode.LIGHT, HistorySyncMode.FULL (or
whatever the current three string values are) and replace WalletType.P2PKH with
WalletType.STANDARD while keeping WalletType.MULTISIG; ensure the expected
string values match the enum definitions (e.g.,
'disabled','light','full','standard','multisig') so the test compiles and
verifies the real API.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Could you verify this against the source? The actual enum definitions are:

src/types.ts:56-60:

export enum HistorySyncMode {
  POLLING_HTTP_API = 'polling-http-api',
  MANUAL_STREAM_WS = 'manual-stream-ws',
  XPUB_STREAM_WS = 'xpub-stream-ws',
}

src/types.ts:363-366:

export enum WalletType {
  P2PKH = 'p2pkh',
  MULTISIG = 'multisig',
}

The test assertions match these definitions. All 46 tests pass with a clean TypeScript compilation.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the actual HistorySyncMode and WalletType enum definitions in src/types.ts
rg -n "HistorySyncMode|WalletType" --type=ts -A 5 src/types.ts

Length of output: 757


@andreabadesso, you're absolutely right — I've verified against the source and the enum definitions are exactly as you showed:

  • HistorySyncMode (lines 56–60): POLLING_HTTP_API, MANUAL_STREAM_WS, XPUB_STREAM_WS
  • WalletType (lines 363–366): P2PKH, MULTISIG

The test assertions are correct and match the actual definitions. My original review comment was a false positive based on incorrect assumptions about the enum members. I apologize for the noise.


🧠 Learnings used
Learnt from: tuliomir
Repo: HathorNetwork/hathor-wallet-lib PR: 1015
File: src/new/wallet.ts:3076-3081
Timestamp: 2026-03-03T23:42:09.591Z
Learning: In `src/new/wallet.ts` (HathorNetwork/hathor-wallet-lib), the methods `createNanoContractTransaction` and `createNanoContractCreateTokenTransaction` use non-null assertions (`data.blueprintId!`, `data.ncId!`, `data.args!`) on optional fields of `CreateNanoTxData` (defined in `src/new/types.ts`) without prior runtime validation. If these fields are `undefined` or `null`, malformed transactions will be created silently. A fix was deliberately deferred (not in scope of typings-only PRs). Always flag this missing validation in future reviews of these methods until it is resolved.

Learnt from: tuliomir
Repo: HathorNetwork/hathor-wallet-lib PR: 1026
File: src/new/wallet.ts:3429-3432
Timestamp: 2026-03-05T16:59:01.672Z
Learning: In `src/new/wallet.ts`, the method `getNanoHeaderSeqnum(address: string)` uses non-null assertions (`addressInfo!.seqnum!`) on the result of `this.storage.getAddressInfo(address)` without runtime validation. `getAddressInfo` can return null when the address is not found, and `seqnum` is an optional field on `IAddressMetadata`. Explicit runtime validation should be added: throw an `AddressError` if `addressInfo` is null/undefined, and handle the case where `seqnum` is undefined (e.g., return 1 as the first seqnum). This guidance applies to HathorNetwork/hathor-wallet-lib and should be considered in reviews of this method until resolved.

Learnt from: tuliomir
Repo: HathorNetwork/hathor-wallet-lib PR: 1036
File: src/utils/tokens.ts:960-960
Timestamp: 2026-03-18T20:29:28.330Z
Learning: In HathorNetwork/hathor-wallet-lib, avoid using null to indicate optional/unprovided parameters in TypeScript. Use undefined and the ? optional syntax (e.g., data?: string[]). Null should be reserved for explicit semantic meaning. This applies repo-wide as part of the TS migration; flag any occurrence of a parameter type like T | null for optional parameters in reviews and prefer data?: T or data: T | undefined with appropriate guards when needed.

@andreabadesso andreabadesso requested a review from tuliomir March 20, 2026 16:13
@andreabadesso andreabadesso moved this from In Progress (Done) to In Review (WIP) in Hathor Network Mar 23, 2026
@tuliomir tuliomir moved this from In Review (WIP) to In Review (Done) in Hathor Network Mar 23, 2026
@pedroferreira1 pedroferreira1 merged commit c663620 into master Mar 24, 2026
5 of 6 checks passed
@github-project-automation github-project-automation bot moved this from In Review (Done) to Waiting to be deployed in Hathor Network Mar 24, 2026
@pedroferreira1 pedroferreira1 deleted the feat/export-all-types-and-utils branch March 24, 2026 20:33
r4mmer added a commit that referenced this pull request Mar 26, 2026
…-policy-2

* origin/master:
  Test: Shared tests for `start` (#1032)
  feat: export all types and utils from public API (#1034)
raul-oliveira added a commit that referenced this pull request Apr 10, 2026
* feat: export all types and utils from public API (#1034)

* Test: Shared tests for `start` (#1032)

* Merge pull request #1046 from HathorNetwork/refact/fee-test-suite

refact: move fee test suite to the right folder

* test: add new fee template integration tests (#1048)

* refact: move fee test suite to the right folder

* review changes

* refact: keep only moved tests, remove new template tests

* test: add new fee template integration tests

* fix: native token version defaulting to deposit (#1054)

* test: Shared tests for internal wallet methods (#1047)

BREAKING CHANGE: HathorWallet.changeServer now returns
Promise<void> instead of void, matching the service
facade signature. Added changeServer to IHathorWallet.

* fix: precalculated wallet address derivation bug (#1056)

* feat: single address policy (#1038)

* feat: single address policy

* chore: improve CI integration test logs (#1049)

---------

Co-authored-by: André Abadesso <andre.abadesso@gmail.com>
Co-authored-by: Tulio Miranda <tulio.mir@gmail.com>
Co-authored-by: André Carneiro <andreluizmrcarneiro@gmail.com>
This was referenced Apr 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: Waiting to be deployed

Development

Successfully merging this pull request may close these issues.

3 participants