Skip to content

Refactor: Fullnode facade to Typescript #972

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.

Sign up for GitHub

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

Merged
tuliomir merged 12 commits into from
Nov 26, 2025
Merged

Refactor: Fullnode facade to Typescript #972

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ce08173
refactor: new/wallet.js to typescript
tuliomir Nov 24, 2025
f5e1f28
refactor: (2) new/wallet.js to typescript
tuliomir Nov 24, 2025
74b746e
refactor: (3) new/wallet.js to typescript
tuliomir Nov 24, 2025
7502c46
refactor: (4) new/wallet.js to typescript
tuliomir Nov 24, 2025
45810b8
fix: mandatory properties
tuliomir Nov 24, 2025
caa69b4
fix: builder typing
tuliomir Nov 24, 2025
4f1b19f
fix: sendTransaction typing
tuliomir Nov 24, 2025
63f7b9b
fix: walletService typing
tuliomir Nov 24, 2025
9673109
fix: inconsistent types between facades
tuliomir Nov 24, 2025
427ef03
fix: organize imports
tuliomir Nov 24, 2025
9c5f997
docs: improve wording
tuliomir Nov 24, 2025
eeb9655
fix: avoid circular dependency
tuliomir Nov 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion jest-integration.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ module.exports = {
lines: 42,
},
// We need a high coverage for the HathorWallet class
'./src/new/wallet.js': {
'./src/new/wallet.ts': {
statements: 92,
branches: 85,
functions: 90,
Expand Down
17 changes: 17 additions & 0 deletions src/nano_contracts/builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,16 @@
return this;
}

/**
* Guard that asserts `this.wallet` is not null and narrows its type for the caller.
* Throws a TypeError if wallet is not set.
*/
private assertWallet(): asserts this is { wallet: HathorWallet } {
if (!this.wallet) {
throw new TypeError('Wallet is required to build nano contract transactions.');

Check warning on line 196 in src/nano_contracts/builder.ts

View check run for this annotation

Codecov / codecov/patch

src/nano_contracts/builder.ts#L196 

Added line #L196 was not covered by tests
Copy link
Copy Markdown
Member

@r4mmer r4mmer Nov 24, 2025

Choose a reason for hiding this comment

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

TypeError?

Copy link
Copy Markdown
Contributor Author

@tuliomir tuliomir Nov 24, 2025

Choose a reason for hiding this comment

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

It was the closest I could get to the type of the error. Since this can be only HathorWallet or null , I decided for a type error.

Do you think a simple Error would be more adequate here?

Copy link
Copy Markdown
Member

@r4mmer r4mmer Nov 25, 2025

Choose a reason for hiding this comment

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

We could create a custom error or use an existing one.
WalletTypeError or WalletError for instance.

}
}

/**
* Set vertex type
*
Expand Down Expand Up @@ -217,6 +227,7 @@
async executeDeposit(
action: NanoContractAction
): Promise<{ inputs: IDataInput[]; outputs: IDataOutput[] }> {
this.assertWallet();
Copy link
Copy Markdown
Contributor Author

@tuliomir tuliomir Nov 24, 2025

Choose a reason for hiding this comment

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

Most methods in this file assume this.wallet is present, but never validates it, so it was breaking the build with errors.

The use of this assert method protects and validates the type, however the test adjusments are outside the scope of this PR.

if (action.type !== NanoContractActionType.DEPOSIT) {
throw new NanoContractTransactionError(
"Can't execute a deposit with an action which type is different than deposit."
Expand Down Expand Up @@ -313,6 +324,7 @@
* @inner
*/
executeWithdrawal(action: NanoContractAction): IDataOutput | null {
this.assertWallet();
if (action.type !== NanoContractActionType.WITHDRAWAL) {
throw new NanoContractTransactionError(
"Can't execute a withdrawal with an action which type is different than withdrawal."
Expand Down Expand Up @@ -381,6 +393,7 @@
async executeGrantAuthority(
action: NanoContractAction
): Promise<{ inputs: IDataInput[]; outputs: IDataOutput[] }> {
this.assertWallet();
if (action.type !== NanoContractActionType.GRANT_AUTHORITY) {
throw new NanoContractTransactionError(
"Can't execute a grant authority with an action which type is different than grant authority."
Expand Down Expand Up @@ -449,6 +462,7 @@
* @inner
*/
executeAcquireAuthority(action: NanoContractAction): IDataOutput | null {
this.assertWallet();
if (action.type !== NanoContractActionType.ACQUIRE_AUTHORITY) {
throw new NanoContractTransactionError(
"Can't execute an acquire authority with an action which type is different than acquire authority."
Expand Down Expand Up @@ -481,6 +495,7 @@
* @inner
*/
async verify() {
this.assertWallet();
if (this.method === NANO_CONTRACTS_INITIALIZE_METHOD && !this.blueprintId) {
// Initialize needs the blueprint ID
throw new NanoContractTransactionError('Missing blueprint id. Parameter blueprintId in data');
Expand Down Expand Up @@ -622,6 +637,7 @@
outputs: IDataOutput[],
tokens: string[]
): Promise<Transaction | CreateTokenTransaction> {
this.assertWallet();
if (this.vertexType === NanoContractVertexType.TRANSACTION) {
return transactionUtils.createTransactionFromData(
{
Expand Down Expand Up @@ -682,6 +698,7 @@
* @inner
*/
async build(): Promise<Transaction> {
this.assertWallet();
let inputs;
let outputs;
let tokens;
Expand Down
2 changes: 1 addition & 1 deletion src/new/sendTransaction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export type ISendOutput = ISendDataOutput | ISendTokenOutput;
* 'unexpected-error': if an unexpected error happens;
* */
export default class SendTransaction extends EventEmitter {
wallet: HathorWallet;
wallet: HathorWallet | null;
Copy link

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

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

Changing wallet: HathorWallet to wallet: HathorWallet | null alters the existing API contract. This could introduce potential null reference errors in code that depends on SendTransaction. Since this class is used by both wallet facades, consider whether this change is necessary or if there are other locations in the code that need to be updated to handle the null case safely.

If the wallet can truly be null, ensure all usages of this.wallet in the SendTransaction class properly handle the null case with guards.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@tuliomir tuliomir Nov 24, 2025

Choose a reason for hiding this comment

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

This is already ensured, the only change was to make the type declaration comply with the code.

There are code branches that expect this.wallet not to be present.


storage: IStorage | null;

Expand Down
Loading
Loading