diff --git a/projects/hathor-wallet-lib/0001-shared-facades/0001-shared-facades.md b/projects/hathor-wallet-lib/0001-shared-facades/0001-shared-facades.md new file mode 100644 index 0000000..5ba5155 --- /dev/null +++ b/projects/hathor-wallet-lib/0001-shared-facades/0001-shared-facades.md @@ -0,0 +1,135 @@ +- Feature Name: wallet-lib-shared-facades +- Start Date: 2026-01-21 +- RFC PR: (leave this empty) +- Hathor Issue: (leave this empty) +- Author: tulio@hathor.network + +# Summary +[summary]: #summary + +The Wallet Lib facades for the Fullnode and the Wallet Service should share a +subset of the most important methods, with the same parameters and output data +format. + +This will cause breaking changes that will require a major version update +on the lib. + +# Motivation +[motivation]: #motivation + +Why are we doing this? What use cases does it support? What is the expected +outcome? + +At the moment, both facades are similar, but not equal. This causes duplicated +test suites, requires additional time and attention from the developers and +ultimately introduces bugs on the Wallet Lib clients when they need to switch +between facades. + +# Guide-level explanation +[guide-level-explanation]: #guide-level-explanation + +To achieve this state it's necessary to change the contracts for some methods, +both in the wallets and in their shared interface `IHathorWallet`, which is now +outdated. An [initial investigation](./shared-contract-investigation.md) was made +to understand the necessary changes, and they will be summarized below. + +- Change methods from `sync` to `async` on the Wallet Service facade +- Modify `IHathorWallet` return types that currently do not match the fullnode facade +- Create types for the parameters of both facades, facilitating visual inspection from devs +- Decide on `UTXO` and `Tx History` objects that currently differ, and are used by most of the applications +- Decide which subset of methods will consist of the `IHathorWallet`, and leave each facade to have its own complementary methods +- Add methods that are present in both facades but not in the `IHathorWallet`, such as `getAuthorityUtxo()` +- Create a `shared_types.ts` on the root folder, instead of using types from each facade folder + +At the end of this project, it should be possible for a hypothetical client +implementation to call methods from a Fullnode or Wallet Service facades +interchangeably during its operation, with no adaptation to their inner workings. + +# Reference-level explanation +[reference-level-explanation]: #reference-level-explanation + +This is the technical portion of the RFC. Explain the design in sufficient +detail that: + +- Its interaction with other features is clear. +- It is reasonably clear how the feature would be implemented. +- Corner cases are dissected by example. + +The section should return to the examples given in the previous section, and +explain more fully how the detailed proposal makes those examples work. + +# Drawbacks +[drawbacks]: #drawbacks + +The only reasons not to do this would be to preserve backwards compatibility with +existing clients, or to dedicate development priority elsewhere. + +# Rationale and alternatives +[rationale-and-alternatives]: #rationale-and-alternatives + +The facades could be kept separate and a wrapper `WalletWrapper` could be +developed to abstract away their differences. This would allow for shared +integration tests and a simpler developer experience, while the internal work +is postponed for a better time. + +The downside of this alternative is having additional work and complexity in +order to avoid a synchronization work that will have to be done eventually. + +By not implementing this shared facade, we keep the responsibility on the client +to know what facade is being used and the details of each one separately in the +client code. + +# Prior art +[prior-art]: #prior-art + +The objetive of the `IHathorWallet` interface was to create a shared contract for +both facades, but without automated testing it was natural to drift away from +a compliance strict enough for a full Typescript validation. + +The main proposal of this RFC is to bring this interface up-to-date with the +current state of the wallets, improving it albeit with breaking changes. + +# Unresolved questions +[unresolved-questions]: #unresolved-questions + +The main questions to resolve through this RFC are the actual method signatures, +deciding which properties are mandatory, which will be optional. + +A special attention should be paid to the `null` parameter, which is used in +many places as a valid replace for `undefined`. This is + +We should consider keeping a `v2` version with minor and patch updates for +critical issues, but going forward we would fully migrate to `v3`. + +- What parts of the design do you expect to resolve through the RFC process + before this gets merged? +- What parts of the design do you expect to resolve through the implementation + of this feature before stabilization? +- What related issues do you consider out of scope for this RFC that could be + addressed in the future independently of the solution that comes out of this + RFC? + +# Future possibilities +[future-possibilities]: #future-possibilities + +Think about breaking the facade files into multiple pieces with common +group responsabilities, that could be shared between the two facades. This would +make it easier for LLMs to interact with those files, instead of having +two facades with over 3k lines that do not fit their context windows. + +Think about what the natural extension and evolution of your proposal would be +and how it would affect the network and project as a whole in a holistic way. +Try to use this section as a tool to more fully consider all possible +interactions with the project and network in your proposal. Also consider how +this all fits into the roadmap for the project and of the relevant sub-team. + +This is also a good place to "dump ideas", if they are out of scope for the +RFC you are writing but otherwise related. + +If you have tried and cannot think of any future possibilities, +you may simply state that you cannot think of anything. + +Note that having something written down in the future-possibilities section +is not a reason to accept the current or a future RFC; such notes should be +in the section on motivation or rationale in this or subsequent RFCs. +The section merely provides additional information. diff --git a/projects/hathor-wallet-lib/0001-shared-facades/shared-contract-investigation.md b/projects/hathor-wallet-lib/0001-shared-facades/shared-contract-investigation.md new file mode 100644 index 0000000..92cf8c0 --- /dev/null +++ b/projects/hathor-wallet-lib/0001-shared-facades/shared-contract-investigation.md @@ -0,0 +1,388 @@ +# Shared Contract Investigation Report + +## Executive Summary + +This report analyzes the differences between `HathorWallet` (fullnode facade), `HathorWalletServiceWallet` (wallet service facade), and the `IHathorWallet` interface to identify contract misalignments that need resolution for unified testing. + +**Key Findings:** +- 3 methods have inconsistent async/sync signatures (critical) +- 4 methods throw "Not implemented" in WalletServiceWallet +- ~20 methods exist only in HathorWallet +- Return types differ significantly for several shared methods +- The `IHathorWallet` interface has incomplete type annotations + +--- + +## Part 1: Critical Contract Violations + +### 1.1 Async/Sync Signature Mismatches + +These methods have different async behaviors between facades: + +| Method | HathorWallet | WalletServiceWallet | IHathorWallet | Impact | +|--------|-------------|---------------------|---------------|--------| +| `getCurrentAddress()` | `async` → `Promise` | `sync` → `AddressInfoObject` | `AddressInfoObject \| Promise` (FIXME) | **CRITICAL** | +| `getNextAddress()` | `async` → `Promise` | `sync` → `AddressInfoObject` | `AddressInfoObject \| Promise` (FIXME) | **CRITICAL** | +| `getFullHistory()` | `async` → `Promise>` | `sync` (throws "Not implemented") | `TransactionFullObject[] \| Promise` (FIXME) | **CRITICAL** | +| `stop()` | `async` → `Promise` | `async` → `Promise` | `sync` → `void` | **MEDIUM** | + +**Impact:** Tests cannot use a simple `await` pattern because the WalletServiceWallet returns sync values. + +**Current Workaround (from shared tests):** +```typescript +// Wrapping with Promise.resolve() handles both cases +const address = await Promise.resolve(wallet.getCurrentAddress()); +``` + +### 1.2 Return Type Mismatches + +| Method | HathorWallet Return | WalletServiceWallet Return | Interface Declares | +|--------|--------------------|-----------------------------|-------------------| +| `getCurrentAddress()` | `{ address, index, addressPath }` | `{ address, index, addressPath, info? }` | Mixed | +| `getMintAuthority()` | `IUtxo[]` | `AuthorityTxOutput[]` | **NOT DEFINED** | +| `getMeltAuthority()` | `IUtxo[]` | `AuthorityTxOutput[]` | **NOT DEFINED** | +| `getAuthorityUtxo()` | `IUtxo[]` | `AuthorityTxOutput[]` | **NOT DEFINED** | +| `sendManyOutputsTransaction()` | `Transaction \| null` | `Transaction` | `Transaction` | +| `sendTransaction()` | `Transaction \| null` | `Transaction` | `Transaction` | +| `getFullHistory()` | `Record` | N/A (throws) | `TransactionFullObject[]` | +| `getAddressPrivKey()` | `Promise` | `Promise` | `Promise` | + +#### 1.2.1 Detailed Type Property Comparison + +**Address Return Types:** + +| Property | `GetCurrentAddressFullnodeFacadeReturnType` (HathorWallet) | `AddressInfoObject` (WalletServiceWallet) | +|----------|-----------------------------------------------------------|-------------------------------------------| +| `address` | `string` | `string` | +| `index` | `number \| null` | `number` | +| `addressPath` | `string` | `string` | +| `info` | ❌ Not present | `string \| undefined` (optional) | + +**Authority UTXO Return Types:** + +| Property | `IUtxo` (HathorWallet) | `AuthorityTxOutput` (WalletServiceWallet) | +|----------|------------------------|-------------------------------------------| +| `txId` | ✅ `string` | ✅ `string` | +| `index` | ✅ `number` | ✅ `number` | +| `address` | ✅ `string` | ✅ `string` | +| `authorities` | ✅ `OutputValueType` | ✅ `OutputValueType` | +| `token` | ✅ `string` | ❌ Not present | +| `value` | ✅ `OutputValueType` | ❌ Not present | +| `timelock` | ✅ `number \| null` | ❌ Not present | +| `type` | ✅ `number` (tx version byte) | ❌ Not present | +| `height` | ✅ `number \| null` (block outputs) | ❌ Not present | + +**History Return Types:** + +| Property | `IHistoryTx` (HathorWallet) | `TransactionFullObject` (Interface declares) | +|----------|-----------------------------|--------------------------------------------| +| `tx_id` | ✅ `string` | ✅ `string` | +| `version` | ✅ `number` | ✅ `number` | +| `timestamp` | ✅ `number` | ✅ `number` | +| `is_voided` | ✅ `boolean` | ✅ `boolean` | +| `inputs` | ✅ `IHistoryInput[]` | ✅ `Input[]` | +| `outputs` | ✅ `IHistoryOutput[]` | ✅ `Output[]` | +| `parents` | ✅ `string[]` | ✅ `string[]` | +| `weight` | ✅ `number` | ❌ Not present | +| `signalBits` | ✅ `number` (optional) | ❌ Not present | +| `nonce` | ✅ `number` (optional) | ❌ Not present | +| `token_name` | ✅ `string` (optional, create token) | ❌ Not present | +| `token_symbol` | ✅ `string` (optional, create token) | ❌ Not present | +| `token_version` | ✅ `TokenVersion` (optional) | ❌ Not present | +| `tokens` | ✅ `string[]` (optional) | ❌ Not present | +| `height` | ✅ `number` (optional) | ❌ Not present | +| `processingStatus` | ✅ `TxHistoryProcessingStatus` (optional) | ❌ Not present | +| `nc_id` | ✅ `string` (optional, nano contract) | ❌ Not present | +| `nc_blueprint_id` | ✅ `string` (optional, nano contract) | ❌ Not present | +| `nc_method` | ✅ `string` (optional, nano contract) | ❌ Not present | +| `nc_args` | ✅ `string` (optional, nano contract) | ❌ Not present | + +**Note:** HathorWallet's `getFullHistory()` returns `Record` (keyed by tx_id), not an array. The interface declares `TransactionFullObject[]` which is neither format. + +--- + +## Part 2: Methods Not Implemented + +### 2.1 WalletServiceWallet "Not Implemented" Methods + +These methods exist but throw `WalletError('Not implemented.')`: + +| Method | Parameters | HathorWallet Has | Notes | +|--------|------------|------------------|-------| +| `getTx()` | `id: string` | ✅ Yes | Returns `IHistoryTx \| null` | +| `getAddressInfo()` | `address: string, options?: {}` | ✅ Yes | Returns address analytics | +| `consolidateUtxos()` | `destinationAddress: string, options?: {}` | ✅ Yes | UTXO consolidation | +| `getFullHistory()` | None | ✅ Yes | Full tx history | + +### 2.2 Methods Missing from WalletServiceWallet + +These methods exist in HathorWallet but not in WalletServiceWallet: + +**Transaction Template Methods:** +- `buildTxTemplate()` +- `runTxTemplate()` + +**On-Chain Blueprint Methods:** +- `createOnChainBlueprintTransaction()` +- `createAndSendOnChainBlueprintTransaction()` + +**Nano Contract Token Methods:** +- `createNanoContractMintTokensTransaction()` (if exists) +- `createNanoContractMeltTokensTransaction()` (if exists) + +**UTXO & Address Methods:** +- `getAvailableUtxos()` (generator) +- `prepareConsolidateUtxosData()` +- `consolidateUtxosSendTransaction()` +- `getAuthorityUtxos()` + +**Multisig Methods:** +- `getAllSignatures()` +- `assemblePartialTransaction()` +- `getMultisigData()` + +**Configuration Methods:** +- `setGapLimit()` +- `getGapLimit()` +- `indexLimitLoadMore()` +- `indexLimitSetEndIndex()` +- `setExternalTxSigningMethod()` +- `setHistorySyncMode()` + +**Internal/Lifecycle Methods:** +- `syncHistory()` +- `reloadStorage()` +- `scanAddressesToLoad()` +- `processTxQueue()` +- `onEnterStateProcessing()` +- `handleWebsocketMsg()` +- `enqueueOnNewTx()` +- `onNewTx()` + +--- + +## Part 3: Methods Missing from IHathorWallet Interface + +The following methods exist in BOTH facades but are NOT in the interface: + +| Method | HathorWallet | WalletServiceWallet | Should Add to Interface? | +|--------|-------------|---------------------|--------------------------| +| `getMintAuthority()` | ✅ | ✅ | **YES** | +| `getMeltAuthority()` | ✅ | ✅ | **YES** | +| `getAuthorityUtxo()` | ✅ | ✅ | **YES** | +| `markUtxoSelected()` | ✅ | ✅ (no-op) | Consider | +| `handleSendPreparedTransaction()` | ✅ | ✅ | Consider | +| `isReady()` | ✅ | ✅ | **YES** | +| `getTokenData()` | ✅ | ❌ | No | +| `clearSensitiveData()` | ✅ | ✅ | Consider | +| `isHardwareWallet()` | ✅ | ✅ | Consider | +| `setState()` | ✅ | ✅ | No (internal) | + +--- + +## Part 4: Interface Type Issues + +### 4.1 Missing Return Types in IHathorWallet + +```typescript +// These methods lack proper return type annotations: +getNetworkObject(); // Return type missing +getPrivateKeyFromAddress(address: string, options: { pinCode?: string }); // Return type missing +``` + +### 4.2 Loose `any`/`options` Types + +Many methods use untyped `options` parameter: +```typescript +prepareCreateNewToken(name: string, symbol: string, amount: OutputValueType, options): Promise; +createNewToken(name: string, symbol: string, amount: OutputValueType, options): Promise; +createNFT(name: string, symbol: string, amount: OutputValueType, data: string, options): Promise; +prepareMintTokensData(token: string, amount: OutputValueType, options): Promise; +mintTokens(token: string, amount: OutputValueType, options): Promise; +prepareMeltTokensData(token: string, amount: OutputValueType, options): Promise; +meltTokens(token: string, amount: OutputValueType, options): Promise; +getTxBalance(tx: IHistoryTx, optionsParams): Promise<{ [tokenId: string]: OutputValueType }>; +``` + +### 4.3 FIXME Comments in Interface + +The interface has explicit FIXME comments acknowledging inconsistencies: +```typescript +getCurrentAddress(options?: { markAsUsed: boolean }): AddressInfoObject | Promise; // FIXME: Should have a single return type +getNextAddress(): AddressInfoObject | Promise; // FIXME: Should have a single return type; +getFullHistory(): TransactionFullObject[] | Promise; // FIXME: Should have a single return type; +``` + +--- + +## Part 5: Options Parameter Differences + +### 5.1 `sendTransaction()` Options + +| Option | HathorWallet | WalletServiceWallet | Interface | +|--------|-------------|---------------------|-----------| +| `token` | ✅ | ✅ | ✅ | +| `changeAddress` | ✅ (`null` allowed) | ✅ | ✅ | +| `pinCode` | ✅ (`null` allowed) | ✅ | ❌ Missing | + +### 5.2 `sendManyOutputsTransaction()` Options + +| Option | HathorWallet | WalletServiceWallet | Interface | +|--------|-------------|---------------------|-----------| +| `inputs` | ✅ | ✅ | ✅ | +| `changeAddress` | ✅ (`null` allowed) | ✅ | ✅ | +| `pinCode` | ✅ (`null` allowed) | ✅ | ❌ Missing | +| `startMiningTx` | ✅ | ❌ | ❌ | + +### 5.3 Token Creation Options + +HathorWallet uses `CreateTokenOptions`: +- `address`, `changeAddress`, `startMiningTx`, `pinCode` +- `createMint`, `mintAuthorityAddress`, `allowExternalMintAuthorityAddress` +- `createMelt`, `meltAuthorityAddress`, `allowExternalMeltAuthorityAddress` +- `data`, `isCreateNFT`, `signTx`, `tokenVersion` + +WalletServiceWallet uses inline options object with similar but not identical fields. + +--- + +## Part 6: Authority Methods Deep Dive + +Both facades have authority methods but with different return types: + +### HathorWallet +```typescript +getMintAuthority(tokenUid: string, options?: GetAuthorityOptions): Promise +getMeltAuthority(tokenUid: string, options?: GetAuthorityOptions): Promise +getAuthorityUtxo(tokenUid: string, authority: 'mint' | 'melt', options?: GetAuthorityOptions): Promise + +// GetAuthorityOptions: +{ + many?: boolean; + only_available_utxos?: boolean; + filter_address?: string; +} +``` + +### WalletServiceWallet +```typescript +getMintAuthority(tokenId: string, options?: { many?: boolean; skipSpent?: boolean }): Promise +getMeltAuthority(tokenId: string, options?: { many?: boolean; skipSpent?: boolean }): Promise +getAuthorityUtxo(tokenUid: string, authority: string, options?: {...}): Promise + +// AuthorityTxOutput: +{ + txId: string; + index: number; + address: string; + authorities: OutputValueType; +} +``` + +### IUtxo vs AuthorityTxOutput + +| Field | IUtxo | AuthorityTxOutput | +|-------|-------|-------------------| +| txId | ✅ | ✅ | +| index | ✅ | ✅ | +| address | ✅ | ✅ | +| authorities | ✅ | ✅ | +| tokenId | ✅ | ❌ | +| value | ✅ | ❌ | +| timelock | ✅ | ❌ | +| heightlock | ✅ | ❌ | +| locked | ✅ | ❌ | +| addressPath | ✅ | ❌ | + +--- + +## Part 7: Prioritized Recommendations + +### Priority 1: Fix Critical Async/Sync Mismatches + +1. **Make `getCurrentAddress()` async in both facades** + - WalletServiceWallet needs to return `Promise` + - Update interface to `Promise` + +2. **Make `getNextAddress()` async in both facades** + - Same approach as above + +3. **Update `stop()` in interface to be async** + - Change interface from `void` to `Promise` + - Both facades already return Promise + +### Priority 2: Unify Return Types + +1. **Standardize authority method returns** + - Define a common `AuthorityUtxo` type + - Update both facades to return the same structure + - Add methods to interface + +2. **Standardize `sendTransaction()` return type** + - Decide: `Transaction` or `Transaction | null` + - HathorWallet returns `null` on certain conditions + +### Priority 3: Add Missing Interface Methods + +```typescript +// Add to IHathorWallet: +getMintAuthority(tokenUid: string, options?: AuthorityOptions): Promise; +getMeltAuthority(tokenUid: string, options?: AuthorityOptions): Promise; +isReady(): boolean; +``` + +### Priority 4: Type the Options Parameters + +Create explicit types for all options objects and use them in the interface. + +### Priority 5: Implement Missing Methods in WalletServiceWallet + +- `getTx()` - Consider implementing via API +- `getAddressInfo()` - Consider implementing via API +- `consolidateUtxos()` - May require backend support +- `getFullHistory()` - Consider implementing via API + +--- + +## Part 8: Shared Test Compatibility Matrix + +Based on the current state, here's what can be tested with the shared test factory: + +| Test Category | Compatible | Notes | +|---------------|------------|-------| +| Lifecycle (`start`/`stop`/`isReady`) | ✅ | Need to handle async stop | +| Balance Operations | ✅ | Compatible | +| Address Operations | ⚠️ | Need Promise.resolve() wrapper | +| Simple Transactions | ✅ | Return type differs but compatible | +| Multi-output Transactions | ✅ | Return type differs but compatible | +| UTXO Operations | ✅ | Compatible | +| Token Creation | ✅ | Compatible | +| Token Details | ✅ | Compatible | +| Mint Tokens | ⚠️ | WalletService has sync issues | +| Melt Tokens | ⚠️ | WalletService has sync issues | +| Authority Operations | ⚠️ | Different return types | +| UTXO Consolidation | ❌ | Not implemented in WalletService | +| Full History | ❌ | Not implemented in WalletService | +| Address Info | ❌ | Not implemented in WalletService | +| Nano Contracts | ✅ | Both implement | + +--- + +## Appendix A: Method Count Summary + +| Category | HathorWallet | WalletServiceWallet | IHathorWallet | +|----------|-------------|---------------------|---------------| +| Total Methods | ~113 | ~75 | ~52 | +| Async Methods | ~97 | ~62 | ~45 | +| Sync Methods | ~16 | ~13 | ~4 | +| Not Implemented | 0 | 4 | N/A | + +--- + +## Appendix B: Files Referenced + +- `src/new/wallet.ts` - HathorWallet (Fullnode Facade) - ~3,372 lines +- `src/wallet/wallet.ts` - HathorWalletServiceWallet - ~3,002 lines +- `src/wallet/types.ts` - IHathorWallet interface and related types +- `src/new/types.ts` - HathorWallet-specific types +- `src/types.ts` - Shared types (IUtxo, OutputValueType, etc.) \ No newline at end of file