diff --git a/AGENTS.md b/AGENTS.md index 4d59066fa..f39581700 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,7 +7,7 @@ Angor is a Bitcoin investment platform with two frontends: - **Blazor WASM web app** (legacy, .NET 8) in `src/webapp/` - **Shared library** (.NET 8) in `src/shared/` - **SDK** (.NET 9) in `src/sdk/` -- **App** (future fresh Avalonia rewrite) in `src/app/` +- **App** (new Avalonia rewrite) in `src/design/` ## Repository Structure @@ -16,10 +16,10 @@ src/ shared/ Angor.Shared (net8.0), Angor.Shared.Tests (net8.0) sdk/ Angor.Sdk (net9.0), Angor.Sdk.Tests, Angor.Data.Documents, Angor.Data.Documents.LiteDb avalonia/ AngorApp, AngorApp.Model, AngorApp.Tests, AngorApp.Desktop, AngorApp.Android, AngorApp.iOS, AngorApp.Browser - app/ (empty - future fresh Avalonia app) + design/ App, App.Desktop, App.Test.Integration (new Avalonia rewrite) webapp/ Angor.Client (Blazor WASM, net8.0) Avalonia.sln Main solution: avalonia + sdk + shared - App.sln Future app solution: app + sdk + shared + App.sln New app solution: design + sdk + shared WebApp.sln Web solution: webapp + shared Directory.Build.props Directory.Packages.props Central Package Management (CPM) @@ -40,6 +40,7 @@ dotnet build src/App.sln # Single project build dotnet build src/avalonia/AngorApp.Desktop/AngorApp.Desktop.csproj +dotnet build src/design/App.Desktop/App.Desktop.csproj ``` ## Test Commands diff --git a/docs/TEST_COVERAGE_ANALYSIS.md b/docs/TEST_COVERAGE_ANALYSIS.md new file mode 100644 index 000000000..d02da6a80 --- /dev/null +++ b/docs/TEST_COVERAGE_ANALYSIS.md @@ -0,0 +1,184 @@ +# Test Coverage Analysis + +> **Last updated:** April 2026 -- re-analyzed after significant test additions. + +## Overview + +This document provides a comprehensive analysis of the current test suite across all four test projects, identifies gaps in coverage, and recommends improvements to existing tests and new tests to create. + +| Test Project | Tests (Original) | Tests (Current) | Nature | +|---|---|---|---| +| `App.Test.Integration` (src/design/) | 7 | **9** | E2E headless UI tests (real testnet) | +| `Angor.Sdk.Tests` (src/sdk/) | ~121 | **~310+** | SDK unit tests + skipped integration tests | +| `Angor.Shared.Tests` (src/shared/) | ~114 | ~105+ | Bitcoin protocol & domain model unit tests | +| `AngorApp.Tests` (src/avalonia/) | 52 | ~50+ | ViewModel & UI logic unit tests | +| **Total** | **~294** | **~475+** | | + +### What Changed + +The SDK test project **nearly tripled** in size since the original analysis. Two new E2E integration tests were added. Key additions: + +- **~190 new SDK unit tests** covering nearly every MediatR handler (founder, investor, project, lightning operations) +- **`WalletImportAndProjectScanTest`** (E2E) -- validates wallet import from mnemonic and project scanning +- **`InvestmentCancellationTest`** (E2E) -- validates cancel before/after approval and re-investment +- **`ScanFounderProjectsTests`** (7 unit tests) -- covers storage errors, null keys, discovery, graceful degradation +- **`CancelInvestmentRequestTests`** (5 unit tests) -- covers cancel when not on-chain, already published, hash mismatch +- **`LiteDbGenericDocumentCollectionTests`** (3 unit tests) -- validates `??=` expression caching bug fix +- **Lightning/Boltz tests** (17+ unit tests) -- `ClaimLightningSwapTests`, `CreateLightningSwapTests`, `MonitorLightningSwapTests` + +## Related Documents + +- [Improvements to Existing Tests](TEST_IMPROVEMENTS.md) +- [New Test Proposals](TEST_NEW_PROPOSALS.md) +- [Edge Cases and Known Issues](TEST_EDGE_CASES.md) +- [SDK-Level Test Proposals](TEST_SDK_PROPOSALS.md) +- [SDK Call Parity Analysis](TEST_SDK_PARITY_ANALYSIS.md) + +--- + +## Current Coverage by Area + +### E2E Integration Tests (App.Test.Integration) + +| Test File | What It Covers | Status | +|---|---|---| +| `SmokeTest.cs` | Headless platform boot, AutomationId lookup | Existing | +| `SendToSelfTest.cs` | Wallet create, faucet fund, send to self, verify balance | Existing | +| `CreateProjectTest.cs` | 6-step Invest-type project wizard, deploy, SDK validation | Existing | +| `FundAndRecoverTest.cs` | Fund-type project: invest above threshold, approve, spend stage, recover | Existing | +| `MultiFundClaimAndRecoverTest.cs` | 3 profiles: Fund project, below/above threshold, claim, penalty recovery | Existing | +| `MultiInvestClaimAndRecoverTest.cs` | 3 profiles: Invest project, stages in past, claim, release, investor claim | Existing | +| `WalletImportAndProjectScanTest.cs` | Import wallet from seed, verify same WalletId, scan projects, verify balance | **NEW** | +| `InvestmentCancellationTest.cs` | 8-phase: cancel before/after approval, re-invest, confirm | **NEW** | + +### SDK Unit Tests (Angor.Sdk.Tests) + +| Area | Files | Coverage | Status | +|---|---|---|---| +| Founder operations | `FounderAppServiceTests`, `ApproveInvestmentTests`, `CreateProjectTests`, `CreateProjectProfileTests`, `CreateProjectKeysTests`, `SpendStageFundsTests`, `ScanFounderProjectsTests`, `ReleaseFundsTests`, `PublishFounderTransactionTests`, `GetReleasableTransactionsTests`, `GetMoonshotProjectTests`, `GetFounderProjectsTests`, `GetClaimableTransactionsTests` | ~80 tests covering all founder MediatR handlers | **MOSTLY NEW** | +| Project operations | `ProjectAppServiceTests`, `GetProjectRelaysTests`, `ProjectInvestmentsServiceTests` | ~31 tests | **MOSTLY NEW** | +| Investment operations | `InvestmentAppServiceTests`, `CreateInvestmentTests`, `CancelInvestmentRequestTests`, `RequestInvestmentSignaturesTests`, `PublishInvestmentTests`, `PublishAndStoreInvestorTransactionTests`, `NotifyFounderOfInvestmentTests`, `NotifyFounderOfCancellationTests`, `GetTotalInvestedTests`, `GetInvestorNsecTests`, `CheckPenaltyThresholdTests`, `CheckForReleaseSignaturesTests`, `BuildUnfundedReleaseTransactionTests`, `BuildRecoveryTransactionTests`, `BuildPenaltyReleaseTransactionTests`, `BuildEndOfProjectClaimTests` | ~100+ tests covering all investor MediatR handlers | **MOSTLY NEW** | +| Address monitoring | `MonitorAddressForFundsTests`, `AddressPollingServiceTests` | ~26 tests: funds detection, retry, timeout, cancellation | **NEW** | +| Lightning/Boltz | `ClaimLightningSwapTests`, `CreateLightningSwapTests`, `MonitorLightningSwapTests`, `SwapStateExtensionTests`, `BoltzMusig2Tests` | ~37 tests: swap lifecycle, claim, monitoring, BIP-327 key aggregation | **MOSTLY NEW** | +| Database | `LiteDbGenericDocumentCollectionTests` | 3 tests: expression caching bug fix verification | **NEW** | +| Balance | `WalletAccountBalanceServiceTests` | 1 test: stale pending UTXO removal | Existing | + +### What Is NOT Covered (Updated) + +Areas with zero test coverage have been significantly reduced: + +| Area | Gap | Status | +|---|---|---| +| ~~Wallet import (from mnemonic)~~ | ~~Only "Generate" path tested in E2E~~ | **RESOLVED** -- `WalletImportAndProjectScanTest` | +| **Wallet delete** | No test exercises the delete flow | Still missing | +| **Subscribe project type** | Only Invest and Fund types tested E2E | Still missing (unit tests exist for dynamic stages) | +| ~~Investment cancellation E2E~~ | ~~Unit tests only, no end-to-end flow~~ | **RESOLVED** -- `InvestmentCancellationTest` | +| ~~Project scanning after import~~ | ~~ScanFounderProjects never tested E2E~~ | **RESOLVED** -- `WalletImportAndProjectScanTest` | +| **Database state validation** | No test checks LiteDB entries against real LiteDB | Partially addressed (E2E tests now check some DB state; mocks only at SDK level) | +| **Non-zero penalty duration** | MultiFundClaimAndRecoverTest uses PenaltyDays=0 | Still missing | +| **Multiple wallets per profile** | All tests create a single wallet | Still missing | +| **Project discovery/browse** | FindProjects only used as step in invest flows | Still missing | +| ~~Lightning/Boltz swap~~ | ~~Only isolated skipped integration tests~~ | **RESOLVED** -- 17+ unit tests for full swap lifecycle | +| **Database wipe and rebuild** | DeleteAllDataAsync and RebuildAllWalletBalancesAsync untested | Still missing | +| **Network switch** | RebuildAllWalletBalancesAsync re-derivation untested | Still missing | + +--- + +## Database Layer Coverage + +### Document Types Stored in LiteDB + +There are 8 document collections managed by `IGenericDocumentCollection`: + +| Document Type | Key | Description | Tested? | +|---|---|---|---| +| `Project` | `ProjectId` | Local cache for projects fetched from indexer/Nostr | No direct tests | +| `DerivedProjectKeys` | `WalletId` | 15 founder key slots per wallet | Checked in `WalletImportAndProjectScanTest` (E2E) | +| `FounderProjectsDocument` | `WalletId` | Tracks which projects each wallet created | Checked in `WalletImportAndProjectScanTest` (E2E) | +| `InvestmentRecordsDocument` | `WalletId` | Local cache of investment records | Checked in `InvestmentCancellationTest` (E2E) | +| `InvestmentHandshake` | MD5 composite | Investment request/approval handshakes | Checked in `InvestmentCancellationTest` (E2E) | +| `BoltzSwapDocument` | `SwapId` | Lightning submarine swap state | Mocked in swap unit tests | +| `WalletAccountBalanceInfo` | `WalletId` | Cached wallet balance data | Partial (1 unit test + E2E balance checks) | +| `QueryTransaction` | `TransactionId` | Cached transaction data from indexer | No | +| `TransactionHexDocument` | `TransactionId` | Cached raw transaction hex | No | + +**Improvement:** The new E2E tests (`WalletImportAndProjectScanTest`, `InvestmentCancellationTest`) now validate some DB state indirectly by checking that data survives through SDK operations. However, no test exercises the `IGenericDocumentCollection` CRUD operations against **real LiteDB** at the SDK unit test level. + +### IGenericDocumentCollection Operations Used by Services + +| Operation | Used By | Tested? | +|---|---|---| +| `FindByIdAsync(id)` | PortfolioService, DocumentProjectService, WalletAccountBalanceService | Partial (mocks + E2E indirect) | +| `FindByIdsAsync(ids)` | DocumentProjectService | No | +| `InsertAsync(keySelector, items)` | DocumentProjectService | No | +| `UpsertAsync(keySelector, item)` | WalletFactory, PortfolioService, InvestmentHandshakeService | 3 tests (caching bug fix, mock-level) | +| `DeleteAsync(id)` | WalletAccountBalanceService | No | +| `DeleteAllAsync()` | DatabaseManagementService | No | +| `FindAsync(predicate)` | Various | No | +| `CountAsync(predicate)` | Various | No | +| `ExistsAsync(id)` | Various | No | + +--- + +## Data Flow: What Gets Persisted When + +### Wallet Creation / Import + +| Step | Data Persisted | Storage | +|---|---|---| +| 1. Encrypt wallet | `EncryptedWallet` (AES-256) | `wallets.json` via IStore | +| 2. Init balance | `WalletAccountBalanceInfo` | LiteDB | +| 3. Derive keys | `DerivedProjectKeys` (15 slots) | LiteDB | + +### Wallet Deletion + +| Step | Data Cleaned | Storage | Gap? | +|---|---|---|---| +| 1. Remove balance | `WalletAccountBalanceInfo` | LiteDB | - | +| 2. Remove from store | `EncryptedWallet` | `wallets.json` | - | +| 3. Clear memory cache | Sensitive data | In-memory | - | +| - | `DerivedProjectKeys` | LiteDB | **NOT CLEANED** (Bug #1 -- still exists) | +| - | `FounderProjectsDocument` | LiteDB | **NOT CLEANED** (Bug #1 -- still exists) | +| - | `InvestmentRecordsDocument` | LiteDB | **NOT CLEANED** (Bug #1 -- still exists) | +| - | `InvestmentHandshake` | LiteDB | **NOT CLEANED** (Bug #1 -- still exists) | + +### Project Creation / Deploy + +| Step | Data Persisted | Storage | +|---|---|---| +| 1. Nostr profile | Project metadata | Nostr relay | +| 2. Nostr event | ProjectInfo (kind 3030) | Nostr relay | +| 3. On-chain tx | Creation transaction | Bitcoin blockchain | +| 4. Local record | `FounderProjectsDocument` | LiteDB | +| 5. Cache | `Project` | LiteDB | + +### Investment Flow + +| Step | Data Persisted | Storage | +|---|---|---| +| 1. Request | `InvestmentHandshake` (Status=Pending) | LiteDB + Nostr DM | +| 2. Approval | `InvestmentHandshake` (Status=Approved) | LiteDB + Nostr DM | +| 3. Confirm | `InvestmentHandshake` (Status=Invested) | LiteDB | +| 4. Record | `InvestmentRecordsDocument` | LiteDB + Nostr relay | +| 5. On-chain | Investment transaction | Bitcoin blockchain | + +--- + +## Recommended Priority (Revised) + +Given the massive SDK unit test improvements, the priority ranking has shifted. Database-layer and lifecycle tests are now the highest-value additions. + +| Priority | Area | Effort | Value | Notes | +|---|---|---|---|---| +| 1 | **Fix Bug #1:** Orphaned DB data on wallet delete | Low | High | Code fix in `WalletAppService.DeleteWallet()` | +| 2 | **Fix Bug #3:** Duplicate wallet guard | Low | High | Code fix in `WalletFactory.CreateWallet()` | +| 3 | LiteDB round-trip tests against real LiteDB (SDK-level, CI-friendly) | Low | High | Still the #1 test gap | +| 4 | DatabaseManagementService tests (SDK-level, CI-friendly) | Low | High | | +| 5 | WalletFactory + WalletAppServiceDelete tests (SDK-level) | Medium | High | Prevents regression on bugs above | +| 6 | Wallet Delete + Reimport (E2E) | Medium | High | Validates full lifecycle | +| 7 | Database Integrity test (E2E) | Medium | High | | +| 8 | Subscribe project type (E2E) | Medium | Medium | | +| 9 | Non-zero PenaltyDays recovery (E2E) | Medium | Medium | | +| 10 | Many Investors scenario (E2E) | High | Medium | | +| 11 | Fix/remove dead `ConfigureMappings()` code | Low | Low | Bug #2 | +| 12 | Expose fee rate selection in investment flow | Low | Medium | Hardcoded to 2 sat/vB | diff --git a/docs/TEST_EDGE_CASES.md b/docs/TEST_EDGE_CASES.md new file mode 100644 index 000000000..56cb4cdf5 --- /dev/null +++ b/docs/TEST_EDGE_CASES.md @@ -0,0 +1,168 @@ +# Edge Cases and Known Issues + +> **Last updated:** April 2026 -- re-analyzed after significant test additions. + +This document catalogs edge cases that should be tested and issues discovered during the test coverage analysis. + +--- + +## Known Issues (Discovered During Analysis) + +### Issue 1: DerivedProjectKeys Not Cleaned on Wallet Delete -- STILL EXISTS + +**Location:** `WalletAppService.DeleteWallet()` in `src/sdk/Angor.Sdk/Wallet/Infrastructure/Impl/WalletAppService.cs` (lines 233-264) + +**Description:** When a wallet is deleted, the `EncryptedWallet` and `WalletAccountBalanceInfo` are removed, but `DerivedProjectKeys`, `FounderProjectsDocument`, `InvestmentRecordsDocument`, and `InvestmentHandshake` are left orphaned in LiteDB. + +**Impact:** Disk space leak and potential data confusion if a different wallet is later created and happens to collide (unlikely but possible). + +**What gets cleaned:** +| Data | Cleaned? | +|---|---| +| `EncryptedWallet` in `wallets.json` | Yes | +| `WalletAccountBalanceInfo` in LiteDB | Yes | +| In-memory sensitive data cache | Yes | +| `DerivedProjectKeys` in LiteDB | **No** | +| `FounderProjectsDocument` in LiteDB | **No** | +| `InvestmentRecordsDocument` in LiteDB | **No** | +| `InvestmentHandshake` records in LiteDB | **No** | + +**Recommended fix:** Inject the 4 missing `IGenericDocumentCollection` instances into `WalletAppService` and add deletion by `WalletId` in the delete flow. The `WalletAppService` constructor (lines 13-21) currently does not have these collections as dependencies. + +**Test coverage:** None. Proposed in `WalletAppServiceDeleteTests` (see TEST_SDK_PROPOSALS.md section 6). + +--- + +### Issue 2: LiteDbDocumentMapping.ConfigureMappings() is Dead Code -- STILL EXISTS + +**Location:** `src/sdk/Angor.Data.Documents.LiteDb/LiteDbDocumentMapping.cs` (line 6) + +**Description:** `ConfigureMappings()` is defined but never called anywhere in the codebase (zero call sites found by grep). LiteDB uses its default conventions (which happen to map `Id` to `_id` automatically), so things work by accident. + +**Impact:** Low risk currently, but fragile. If a document type is added with non-standard field names, the explicit mapping won't apply. + +**Recommended fix:** Either call `ConfigureMappings()` during DI setup or remove the dead code. + +--- + +### Issue 3: No Duplicate Wallet Guard -- STILL EXISTS + +**Location:** `WalletFactory.CreateWallet()` in `src/sdk/Angor.Sdk/Wallet/Infrastructure/Impl/WalletFactory.cs` (lines 23-57, `SaveEncryptedWalletToStoreAsync` at lines 59-68) + +**Description:** There is no check for whether a wallet with the same `WalletId` (derived from xpub hash) already exists. Importing the same seed words twice will: +1. Append a second `EncryptedWallet` entry to `wallets.json` with the same `Id`. +2. Upsert (overwrite) `WalletAccountBalanceInfo` in LiteDB. +3. Upsert (overwrite) `DerivedProjectKeys` in LiteDB. + +**Impact:** The duplicate entry in `wallets.json` could cause issues when loading wallets (two entries with the same ID). The LiteDB upserts are harmless but wasteful. + +**Recommended fix:** Check `walletStore.GetAll()` for an existing wallet with the same `Id` before creating. Return `Result.Failure("Wallet already exists")` if found. + +**Test coverage:** None. Proposed in `WalletFactoryIntegrationTests` (see TEST_SDK_PROPOSALS.md section 2) and `DuplicateWalletImportTest` E2E (see TEST_NEW_PROPOSALS.md section 8). + +--- + +### Issue 4: FounderProjectsDocument Not Cleaned on Wallet Delete -- STILL EXISTS + +Same root cause as Issue 1. `FounderProjectsDocument` tracks which projects a wallet created but is not cleaned up on wallet deletion. + +--- + +## Edge Cases to Test + +### Wallet Operations + +| Edge Case | Expected Behavior | Risk | Test Coverage | +|---|---|---|---| +| Import wallet with 24-word mnemonic | Should work (only 12-word tested currently) | Medium - different derivation | None | +| Import wallet with BIP-39 passphrase | Different `WalletId` from same seed (passphrase changes master key) | High - totally untested path | None | +| Import same seed words twice | Should reject duplicate or handle gracefully | High - known Issue 3 | None | +| Delete wallet with active investments | Should warn or block (currently doesn't) | High - investor loses recovery ability | None | +| Create wallet with empty encryption key | Should fail validation | Medium | None | +| Wrong encryption key during decrypt | Should return `Result.Failure`, not crash | Medium | None | +| Import with invalid/misspelled seed words | UI validates BIP-39 words, SDK throws `DomainException` | Low - validation exists | None | + +### Project Operations + +| Edge Case | Expected Behavior | Risk | Test Coverage | +|---|---|---|---| +| Create project with all 15 key slots used | Should fail gracefully when no slots available | High - no guard exists | None | +| Create project with 0 stages | Should be rejected by validation | Medium | None | +| Create project with stages summing to != 100% | SDK validation exists (`CreateProjectInfo` checks), needs E2E confirmation | Low | None | +| Invest in own project (founder invests in own project) | Should be blocked or warned | Medium - unclear if guarded | None | +| Invest with exactly threshold amount | Off-by-one: is threshold "above" or "at-or-above"? | Medium | Partially covered: `InvestmentCancellationTest` tests above-threshold, `MultiFundClaimAndRecoverTest` tests below-threshold | +| Invest with zero amount | Should be rejected | Low | None | +| Invest when project has expired | Should be rejected | Medium | None | +| Invest when project hasn't started yet | Should be rejected | Medium | None | + +### Database Operations + +| Edge Case | Expected Behavior | Risk | Test Coverage | +|---|---|---|---| +| `DeleteAllDataAsync` during active wallet operations | Should not corrupt LiteDB (thread safety) | Medium - LiteDB is single-writer | None | +| `FindByIdAsync` with null/empty ID | Should return `Result.Failure` | Low | None | +| `UpsertAsync` with null entity | Should return `Result.Failure` | Low | None | +| `InsertAsync` with duplicate key | LiteDB throws; should be caught and returned as `Result.Failure` | Medium | None | +| `FindAsync` with complex predicate | Expression rewriting in `LiteDbGenericDocumentCollection` might fail for complex lambdas | Medium | Partially: `LiteDbGenericDocumentCollectionTests` validates expression compilation | +| LiteDB file corruption recovery | What happens if `.db` file is corrupted? | High - no recovery mechanism | None | +| Concurrent writes to same document | LiteDB is single-writer but SDK services might queue writes | Low | None | + +### Network / Infrastructure Edge Cases + +| Edge Case | Expected Behavior | Risk | Test Coverage | +|---|---|---|---| +| Indexer down during project scan | `ScanFounderProjects` should fail gracefully with `Result.Failure` | Medium | **Covered** -- `ScanFounderProjectsTests.Handle_WhenScanFailsButLocalProjectsExist_StillReturnsLocalProjects` | +| Nostr relay timeout during handshake | Partial `InvestmentHandshake` state in DB (Pending but never Approved) | High | None | +| Indexer stale data after broadcast | Optimistic local updates should prevent stale reads | Medium - documented in AGENTS.md | None | +| Network switch mid-investment | Derived keys change, handshake breaks, funds at risk | High | None | +| Multiple concurrent investments to same project | Race conditions in handshake sync on Nostr | Medium | None | +| Relay returns duplicate events | `DocumentProjectService` deduplicates by identifier, but handshake service might not | Medium | None | + +### Multi-Profile / Multi-Wallet Edge Cases + +| Edge Case | Expected Behavior | Risk | Test Coverage | +|---|---|---|---| +| Two wallets in same profile | Balance and project isolation between wallets | Medium - untested | None | +| Two profiles with same wallet (imported separately) | Should work independently (separate LiteDB files) | Low | **Covered** -- `WalletImportAndProjectScanTest` validates same wallet in two profiles | +| Profile name with special characters | `SanitizeProfileName` replaces invalid chars with `-` | Low - sanitization exists | None | +| Very long profile name | Should be handled by filesystem limits | Low | None | + +--- + +## Edge Cases by Priority (Revised) + +### Must Test (High Risk, No Coverage) + +1. **Import same seed words twice** (duplicate wallet guard missing -- Bug #3) +2. **Delete wallet with active investments** (no warning, data orphaned -- Bug #1) +3. **Create project when all 15 key slots used** (no guard) +4. **Network switch mid-investment** (derived keys change) +5. **Nostr relay timeout during handshake** (partial state) + +### Should Test (Medium Risk) + +6. **Import with 24-word mnemonic** (different from tested 12-word) +7. **Import with BIP-39 passphrase** (different WalletId) +8. **Invest at exactly threshold amount** (off-by-one) +9. **Invest in own project** (unclear if blocked) +10. ~~Indexer down during project scan~~ **COVERED** by `ScanFounderProjectsTests` +11. **LiteDB InsertAsync with duplicate key** (error propagation) +12. **`DeleteAllDataAsync` during active operations** (thread safety) + +### Nice to Test (Low Risk) + +13. **Invalid seed words at UI level** (validation exists) +14. **Empty/null IDs in DB operations** (basic error handling) +15. **Profile name sanitization** (exists, just needs confirmation) + +--- + +## Progress Since Original Analysis + +| Category | Original Edge Cases Without Coverage | Current | +|---|---|---| +| Network/Infrastructure | 6 | **5** (indexer down now covered) | +| Multi-Profile | 4 | **3** (same wallet in two profiles now covered) | +| Wallet Operations | 7 | 7 (no change) | +| Project Operations | 8 | 8 (no change, threshold partially covered) | +| Database Operations | 7 | 7 (expression rewriting partially covered) | diff --git a/docs/TEST_IMPROVEMENTS.md b/docs/TEST_IMPROVEMENTS.md new file mode 100644 index 000000000..c10d19a55 --- /dev/null +++ b/docs/TEST_IMPROVEMENTS.md @@ -0,0 +1,341 @@ +# Improvements to Existing Integration Tests + +> **Last updated:** April 2026 -- re-analyzed after new E2E tests added. + +This document describes specific improvements to the existing E2E integration tests in `src/design/App.Test.Integration/`. + +**Current E2E test count: 9** (was 7). + +--- + +## 1. SendToSelfTest + +**Current coverage:** Wallet create (generate), faucet fund, send to self, verify balance change. + +### Missing Assertions + +#### 1.1 Database entries after wallet creation + +After `CreateWallet` completes, resolve services from DI and assert: + +```csharp +// Verify DerivedProjectKeys persisted with 15 founder key slots +var derivedKeysCollection = serviceProvider.GetRequiredService>(); +var keysResult = await derivedKeysCollection.FindByIdAsync(walletId); +keysResult.IsSuccess.Should().BeTrue(); +keysResult.Value.Should().NotBeNull(); +keysResult.Value.Keys.Should().HaveCount(15); + +// Verify WalletAccountBalanceInfo initialized in DB +var balanceCollection = serviceProvider.GetRequiredService>(); +var balanceResult = await balanceCollection.FindByIdAsync(walletId); +balanceResult.IsSuccess.Should().BeTrue(); +balanceResult.Value.Should().NotBeNull(); +``` + +> **Note:** The new `WalletImportAndProjectScanTest` validates some of these assertions (WalletId determinism, balance after import), but `SendToSelfTest` would benefit from explicit DB assertions at each step of its own flow. + +#### 1.2 Transaction list after send + +Navigate to the transaction history section and verify the send transaction: +- Appears in the list +- Shows the correct amount (0.0001 BTC) +- Shows the correct direction (outgoing) +- Has a valid TxId + +#### 1.3 Wallet metadata persistence + +```csharp +// Verify EncryptedWallet saved to store +var walletStore = serviceProvider.GetRequiredService(); +var walletsResult = await walletStore.GetAll(); +walletsResult.Value.Should().Contain(w => w.Id == walletId); +``` + +#### 1.4 Stronger UTXO reservation assertion + +While the send transaction is pending, assert specific values: + +```csharp +// While pending: +availableSats.Should().BeLessThan(totalBalanceSats, "spent UTXO should be reserved"); +// After confirmation: +availableSats.Should().Be(totalBalanceSats, "reservation should be released after confirmation"); +``` + +--- + +## 2. CreateProjectTest + +**Current coverage:** Creates an Invest-type project through the 6-step wizard, deploys, validates `ProjectDto` fields via SDK. + +### Missing Assertions + +#### 2.1 FounderProjectsDocument in DB + +```csharp +// After deploy, verify the founder project record was persisted +var founderProjectsCollection = serviceProvider.GetRequiredService>(); +var docResult = await founderProjectsCollection.FindByIdAsync(walletId); +docResult.IsSuccess.Should().BeTrue(); +docResult.Value.Projects.Should().Contain(p => p.ProjectIdentifier == projectIdentifier); +docResult.Value.Projects.First().CreationTransactionId.Should().NotBeNullOrEmpty(); +``` + +> **Note:** The new `WalletImportAndProjectScanTest` validates that `FounderProjectsDocument` is populated after project scan, but `CreateProjectTest` should validate it is populated immediately after deploy. + +#### 2.2 DerivedProjectKeys slot consumed + +```csharp +// Verify one of the 15 derived key slots matches the project +var keysResult = await derivedKeysCollection.FindByIdAsync(walletId); +keysResult.Value.Keys.Should().Contain(k => k.ProjectIdentifier == projectIdentifier); +``` + +#### 2.3 Project appears in browse/discovery + +After deploy, navigate to "Find Projects" and verify the project appears. This validates the full Nostr publish -> indexer round-trip -> DB cache pipeline. + +#### 2.4 Create a second project + +After the first project is created, create a second one and verify: +- It uses a different derived key slot (different index) +- Both projects appear in "My Projects" +- Both `FounderProjectRecord` entries exist in `FounderProjectsDocument` + +--- + +## 3. FundAndRecoverTest + +**Current coverage:** Fund-type project, invest above threshold, founder approves, investor confirms, founder spends stage 1, investor recovers remaining. + +### Missing Assertions + +#### 3.1 InvestmentRecordsDocument after investing + +```csharp +// After investor confirms, verify the investment record in DB +var investmentRecordsCollection = serviceProvider.GetRequiredService>(); +var recordResult = await investmentRecordsCollection.FindByIdAsync(investorWalletId); +recordResult.IsSuccess.Should().BeTrue(); +var record = recordResult.Value.Investments.First(); +record.ProjectIdentifier.Should().Be(projectIdentifier); +record.InvestmentTransactionHash.Should().NotBeNullOrEmpty(); +record.InvestedAmountSats.Should().Be(investedAmount); +``` + +> **Note:** The new `InvestmentCancellationTest` validates portfolio/investment record state during cancel/re-invest cycles, but `FundAndRecoverTest` would benefit from explicit DB assertions at the investment and recovery stages. + +#### 3.2 InvestmentHandshake status progression + +At each stage of the handshake, verify the `InvestmentHandshake` document in the DB: + +```csharp +// After request: Status = Pending +// After approval: Status = Approved, ApprovalEventId not null +// After confirm: Status = Invested, InvestmentTransactionId not null +``` + +> **Note:** The new `InvestmentCancellationTest` validates the handshake status becomes "Cancelled" and that re-invested handshakes progress through "Pending Approval" -> "Approved" -> confirmed, but explicit handshake status assertions at each intermediate step are still missing from the simpler `FundAndRecoverTest`. + +#### 3.3 Exact balance deltas + +Instead of just checking "balance changed", compute and assert exact satoshi deltas: + +```csharp +// Founder after stage 1 spend: +founderBalanceAfter.Should().BeApproximately(founderBalanceBefore + stage1Amount - fees, tolerance); + +// Investor after recovery: +investorBalanceAfter.Should().BeApproximately(investorBalanceBefore + investedAmount - stage1Amount - penaltyFee - txFees, tolerance); +``` + +#### 3.4 Recovery amount verification + +When the investor recovers with penalty, assert the recovered amount against the expected calculation: + +```csharp +recoveredAmount.Should().Be(investedAmount - stage1Claimed - penaltyDeduction); +``` + +--- + +## 4. MultiFundClaimAndRecoverTest + +**Current coverage:** 3 profiles (Founder + below-threshold + above-threshold investor), PenaltyDays=0. + +### Key Issue: PenaltyDays=0 Bypasses Penalty Logic + +The test uses `PenaltyDays = 0`, which means the penalty time-lock is effectively zero. The penalty recovery path executes but the actual penalty *calculation* (proportional to remaining lock time) is never tested. + +### Missing Assertions + +#### 4.1 Cross-profile DB validation + +After each profile performs an action, switch to the other profile and verify: + +```csharp +// After Investor1 invests: switch to Founder, verify InvestmentHandshake appears +using (new TestProfileScope("founder")) +{ + var handshakes = await handshakeCollection.FindAsync(h => h.ProjectId == projectId); + handshakes.Value.Should().Contain(h => h.InvestorNostrPubKey == investor1NostrPubKey); +} +``` + +#### 4.2 Threshold boundary validation + +Assert that below-threshold investments are auto-approved (no founder action needed) vs above-threshold requiring explicit approval: + +```csharp +// Below threshold: handshake goes directly to Approved/Invested without founder action +belowThresholdHandshake.IsDirectInvestment.Should().BeTrue(); + +// Above threshold: handshake requires founder approval +aboveThresholdHandshake.IsDirectInvestment.Should().BeFalse(); +aboveThresholdHandshake.ApprovalEventId.Should().NotBeNullOrEmpty(); +``` + +> **Note:** The new `InvestmentCancellationTest` validates `IsAutoApproved` is false for above-threshold investments, but below-threshold auto-approval assertion is still only partially covered. + +--- + +## 5. MultiInvestClaimAndRecoverTest + +**Current coverage:** 3 profiles, Invest-type project, stages in past, founder claims stage 1, releases remaining, investors claim. + +### Missing Assertions + +#### 5.1 Per-investor balance accounting + +After the founder releases remaining stages and both investors claim: + +```csharp +// Each investor's claim should be proportional to their investment +investor1Claimed.Should().BeApproximately(investor1Invested * releasedRatio, tolerance); +investor2Claimed.Should().BeApproximately(investor2Invested * releasedRatio, tolerance); +``` + +#### 5.2 Final DB state + +At the end of the test, verify the complete state across all three profiles: + +```csharp +// Founder: FounderProjectsDocument with project, all handshakes completed +// Investor1: InvestmentRecordsDocument with EndOfProjectTransactionId set +// Investor2: InvestmentRecordsDocument with EndOfProjectTransactionId set +``` + +--- + +## 6. WalletImportAndProjectScanTest (NEW) + +**Current coverage:** Import wallet from seed, verify same WalletId, scan projects, verify balance. ~20+ assertions including DB and balance checks. + +### Already Covered +- WalletId determinism (imported == created) +- Balance discovery after import (UTXO refresh) +- FounderProjectsDocument populated after ScanFounderProjects +- Project name and identifier match across profiles + +### Missing Assertions + +#### 6.1 DerivedProjectKeys comparison + +```csharp +// Verify the imported wallet's DerivedProjectKeys match the creator's exactly +var importedKeys = await derivedKeysCollection.FindByIdAsync(importedWalletId); +importedKeys.Value.Keys.Should().BeEquivalentTo(creatorKeys.Value.Keys); +``` + +#### 6.2 InvestmentRecordsDocument after import + +If the creator had investments, verify they are discoverable after import and scan. + +--- + +## 7. InvestmentCancellationTest (NEW) + +**Current coverage:** 8-phase E2E with ~40+ assertions. Validates cancel before approval, cancel after approval, re-invest, and confirm. + +### Already Covered +- Cancel before founder approval (status becomes "Cancelled", balance released) +- Cancel after founder approval (status becomes "Cancelled", balance released) +- Re-invest after cancellation succeeds +- Confirm after approval reaches Step 3 (active) +- Portfolio investment records updated at each stage + +### Missing Assertions + +#### 7.1 Founder-side cancellation visibility + +After investor cancels, verify the founder's view: + +```csharp +// Switch to founder profile, verify cancelled investment no longer appears as pending +var pendingRequests = await investmentRequestsCollection.FindAsync(r => r.Status != "Cancelled"); +pendingRequests.Should().NotContain(r => r.InvestorId == cancelledInvestorId); +``` + +#### 7.2 Nostr DM cancellation notification + +Verify the `NotifyFounderOfCancellation` MediatR request was dispatched (this is covered by `CancelInvestmentRequestTests` at the unit level but not verified E2E). + +--- + +## 8. General Improvements for All Tests + +### 8.1 Add DB assertion helper methods + +Create shared helper methods in `TestHelpers.cs`: + +```csharp +public static async Task GetDocumentFromDb(IServiceProvider sp, string id) where T : class +{ + var collection = sp.GetRequiredService>(); + var result = await collection.FindByIdAsync(id); + result.IsSuccess.Should().BeTrue($"Expected document {typeof(T).Name} with id {id} to exist"); + return result.Value!; +} + +public static async Task AssertDocumentExists(IServiceProvider sp, string id) where T : class +{ + var collection = sp.GetRequiredService>(); + var result = await collection.ExistsAsync(id); + result.IsSuccess.Should().BeTrue(); + result.Value.Should().BeTrue($"Expected document {typeof(T).Name} with id {id} to exist"); +} + +public static async Task AssertDocumentNotExists(IServiceProvider sp, string id) where T : class +{ + var collection = sp.GetRequiredService>(); + var result = await collection.ExistsAsync(id); + result.IsSuccess.Should().BeTrue(); + result.Value.Should().BeFalse($"Expected document {typeof(T).Name} with id {id} to NOT exist"); +} +``` + +### 8.2 Add balance snapshot helpers + +```csharp +public static async Task GetBalanceSats(IServiceProvider sp, string walletId) +{ + var collection = sp.GetRequiredService>(); + var result = await collection.FindByIdAsync(walletId); + return result.Value?.AccountBalanceInfo?.TotalBalance ?? 0; +} +``` + +### 8.3 Log DB state at end of each test + +For debugging flaky tests, log the full DB state at the end: + +```csharp +[AfterTest] +private async Task DumpDbState() +{ + var projects = await projectsCollection.FindAllAsync(); + output.WriteLine($"Projects in DB: {projects.Value.Count()}"); + // ... for each collection +} +``` diff --git a/docs/TEST_NEW_PROPOSALS.md b/docs/TEST_NEW_PROPOSALS.md new file mode 100644 index 000000000..0c8f35cfd --- /dev/null +++ b/docs/TEST_NEW_PROPOSALS.md @@ -0,0 +1,304 @@ +# New Integration Test Proposals + +> **Last updated:** April 2026 -- re-analyzed after new E2E tests added. + +This document describes new end-to-end integration tests to create in `src/design/App.Test.Integration/`. + +**Status: 2 of 10 proposals have been implemented.** The remaining 8 are still recommended. + +--- + +## 1. ~~Wallet Import and Project Scan~~ -- IMPLEMENTED + +**Status:** **DONE** -- `WalletImportAndProjectScanTest.cs` added. + +**What it covers:** +- Profile A: create wallet, fund, create project, deploy +- Profile B: import same seed words +- Asserts: same `WalletId` (deterministic derivation), non-zero balance (UTXO discovery), `ScanFounderProjects` finds the project, project name and identifier match + +**Remaining gaps in the implementation:** +- Does not compare `DerivedProjectKeys` between profiles (proposed in TEST_IMPROVEMENTS.md section 6.1) +- Does not verify `InvestmentRecordsDocument` discoverable after import + +--- + +## 2. Wallet Delete and Reimport (HIGH PRIORITY) + +**Status:** NOT DONE + +**Why:** Delete has a known gap (Bug #1): `DerivedProjectKeys`, `FounderProjectsDocument`, `InvestmentRecordsDocument`, and `InvestmentHandshake` are NOT cleaned up. This test documents and exposes that behavior. + +**File:** `WalletDeleteAndReimportTest.cs` + +### Steps + +1. Create wallet, fund it, create a project. +2. Record seed words and wallet ID. +3. Delete the wallet via the UI. +4. Assert: `EncryptedWallet` removed from `wallets.json`. +5. Assert: `WalletAccountBalanceInfo` removed from LiteDB. +6. Assert (document the gap): `DerivedProjectKeys` still exists in DB. +7. Assert (document the gap): `FounderProjectsDocument` still exists in DB. +8. Assert: UI shows no wallets. +9. Import wallet with the same seed words. +10. Assert: Wallet recreated with the same `WalletId`. +11. Assert: Balance is recoverable from on-chain data. +12. Trigger project scan. +13. Assert: Project is rediscovered and visible. + +### What This Validates + +- The delete flow and exactly what gets cleaned up. +- Documents the orphaned data issue for future fix. +- Reimport after delete restores full functionality. + +--- + +## 3. Many Investors Scenario (HIGH PRIORITY) + +**Status:** NOT DONE + +**Why:** Current tests use 2-3 investors max. Real projects may have many investors, and the threshold/approval logic becomes complex at scale. + +**File:** `ManyInvestorsTest.cs` + +### Steps + +1. Founder creates a project with `PenaltyThreshold = X satoshis`. +2. **Investors 1-3:** Invest below threshold (should be auto-approved). +3. **Investors 4-5:** Invest above threshold (require founder approval). +4. Assert: Founder sees all 5 handshake requests in the UI. +5. Assert: `InvestmentHandshake` documents for investors 1-3 have `IsDirectInvestment = true`. +6. Founder approves investors 4 and 5. +7. All 5 investors confirm their investments. +8. Assert: All 5 `InvestmentHandshake` records have `Status = Invested`. +9. Founder claims stage 1. +10. Assert: Each investor can independently recover their remaining funds. +11. Assert: Total invested across all records equals the sum of individual investments. +12. Assert: All 5 `InvestmentRecordsDocument` entries exist with correct amounts. + +### What This Validates + +- Threshold logic works correctly at boundaries. +- Auto-approval vs manual approval paths. +- Multiple concurrent investments don't interfere. +- Founder can manage many handshakes. +- Independent recovery for each investor. + +--- + +## 4. Database Integrity After Operations (HIGH PRIORITY) + +**Status:** NOT DONE + +**Why:** No test currently validates the full database lifecycle including wipe and rebuild. + +**File:** `DatabaseIntegrityTest.cs` + +### Steps + +1. Create wallet. + - Assert: `WalletAccountBalanceInfo` exists in LiteDB. + - Assert: `DerivedProjectKeys` exists with 15 key slots. + - Assert: `EncryptedWallet` exists in `wallets.json`. +2. Fund wallet. + - Assert: `WalletAccountBalanceInfo.TotalBalance > 0` in DB. +3. Create and deploy project. + - Assert: `FounderProjectsDocument` exists with project record. + - Assert: `Project` exists in cache collection. +4. Invest in the project (using a second profile). + - Assert: `InvestmentHandshake` document created. + - Assert: `InvestmentRecordsDocument` created for investor. +5. Call `DatabaseManagementService.DeleteAllDataAsync()`. + - Assert: All 8 collections are empty. + - Assert: `wallets.json` is NOT affected (separate storage). +6. Trigger `RebuildAllWalletBalancesAsync`. + - Assert: `WalletAccountBalanceInfo` rebuilt from on-chain data. + - Assert: `DerivedProjectKeys` re-derived. +7. Trigger `ScanFounderProjects`. + - Assert: `FounderProjectsDocument` rebuilt from indexer. + +### What This Validates + +- Every database write operation during the core flows. +- `DeleteAllDataAsync` correctly wipes all collections. +- `RebuildAllWalletBalancesAsync` correctly reconstructs wallet state. +- The system can fully recover from a database wipe. + +--- + +## 5. Subscribe Project Type (MEDIUM PRIORITY) + +**Status:** NOT DONE (unit tests exist for dynamic stage patterns but no E2E) + +**Why:** Only `Invest` and `Fund` types are tested. `Subscribe` is completely untested E2E. + +**File:** `SubscribeProjectTest.cs` + +### Steps + +1. Create a Subscribe-type project with `DynamicStagePatterns`. +2. Deploy. +3. Assert: `ProjectDto.ProjectType == Subscribe`. +4. Assert: `DynamicStagePatterns` correctly persisted in both Nostr and DB. +5. Investor invests in the project. +6. Founder claims first dynamic stage. +7. Assert: Stage release dates calculated correctly per the dynamic pattern. +8. Assert: Claimed amount matches the stage percentage. + +### What This Validates + +- Subscribe project creation wizard works E2E. +- `DynamicStagePattern` data round-trips through Nostr and DB correctly. +- Dynamic stage release date calculations work in practice. + +--- + +## 6. ~~Investment Cancellation Flow~~ -- IMPLEMENTED + +**Status:** **DONE** -- `InvestmentCancellationTest.cs` added. + +**What it covers (8 phases, ~40+ assertions):** +1. Founder creates project, investor invests +2. Investor cancels **before** founder approval -- status becomes "Cancelled", balance released +3. Investor re-invests after cancel -- new handshake created +4. Founder approves the re-investment +5. Investor cancels **after** founder approval -- status becomes "Cancelled", balance released +6. Investor re-invests again +7. Founder approves again +8. Investor confirms approved investment -- reaches Step 3 (active) + +**Remaining gaps in the implementation:** +- Does not verify founder-side cancellation visibility (proposed in TEST_IMPROVEMENTS.md section 7.1) +- Does not verify Nostr DM cancellation notification E2E + +--- + +## 7. Project Discovery and Cache Behavior (MEDIUM PRIORITY) + +**Status:** NOT DONE + +**Why:** The `DocumentProjectService` caching layer (LiteDB cache-aside) is untested end-to-end. + +**File:** `ProjectDiscoveryCacheTest.cs` + +### Steps + +1. Founder creates and deploys a project. +2. Switch to investor profile. +3. Browse "Find Projects" -- project appears. +4. Assert: `Project` document cached in LiteDB (check DB directly). +5. Call `DatabaseManagementService.DeleteAllDataAsync()` (wipe cache). +6. Assert: `Project` collection is empty. +7. Browse "Find Projects" again. +8. Assert: Project re-fetched from indexer/Nostr and visible. +9. Assert: `Project` document re-cached in LiteDB with same data. + +### What This Validates + +- Cache-aside pattern: first access fetches from network, subsequent from DB. +- Cache wipe forces re-fetch. +- Re-fetched data matches original. +- Nostr -> indexer -> DB pipeline works correctly. + +--- + +## 8. Duplicate Wallet Import (EDGE CASE) + +**Status:** NOT DONE (Bug #3 still exists) + +**Why:** There is NO duplicate wallet check in `WalletFactory`. Importing the same seed twice produces the same `WalletId`, and `wallets.json` gets a duplicate entry. This edge case is currently unhandled. + +**File:** `DuplicateWalletImportTest.cs` + +### Steps + +1. Create wallet with seed words using "Generate." +2. Record the seed words and wallet ID. +3. Import wallet using the exact same seed words. +4. Assert: What happens? + - Does `wallets.json` now have two entries with the same ID? + - Does the UI show one wallet or two? + - Do balance lookups still work? +5. Document the actual behavior (this test serves as a spec). + +### Expected Outcome + +This test should document whether the system: +- Silently creates a duplicate (bug -- this is the current behavior per analysis). +- Detects the duplicate and rejects it (ideal). +- Crashes or corrupts data (critical bug). + +The test result will drive a fix: adding a duplicate `WalletId` check to `WalletFactory.CreateWallet`. + +--- + +## 9. Non-Zero Penalty Recovery (EDGE CASE) + +**Status:** NOT DONE + +**Why:** `MultiFundClaimAndRecoverTest` uses `PenaltyDays=0`. The actual penalty calculation with time-locked funds is untested. + +**File:** `PenaltyRecoveryWithTimeLockTest.cs` + +### Steps + +1. Founder creates a Fund project with `PenaltyDays = 30`. +2. Stages set with release dates in the future. +3. Investor invests above threshold. +4. Founder approves, investor confirms. +5. Founder claims stage 1. +6. Investor recovers remaining funds (with penalty). +7. Assert: Recovered amount is LESS than `invested - stage1` (penalty was deducted). +8. Assert: Penalty amount correlates to remaining lock time. +9. Assert: `RecoveryTransactionId` stored in `InvestmentRecordsDocument`. + +### What This Validates + +- Penalty calculation produces a non-trivial deduction. +- Time-lock penalty is proportional to remaining duration. +- Recovery transaction includes the penalty output. + +--- + +## 10. Wallet Network Switch (EDGE CASE) + +**Status:** NOT DONE + +**Why:** `RebuildAllWalletBalancesAsync` re-derives keys for the new network but this is never tested. + +**File:** `NetworkSwitchTest.cs` + +### Steps + +1. Create wallet on Testnet, derive keys, create a project. +2. Switch network to Mainnet (or signet). +3. Assert: `DerivedProjectKeys` re-derived with the new network's `AngorKey`. +4. Assert: Old project NOT visible in "My Projects" (different network name filter). +5. Switch back to the original network. +6. Assert: Project visible again in "My Projects." +7. Assert: Balance restored. + +### What This Validates + +- Key re-derivation happens on network switch. +- Projects are filtered by network name. +- Network switch is reversible without data loss. + +--- + +## Summary + +| # | Proposal | Priority | Status | +|---|---|---|---| +| 1 | Wallet Import and Project Scan | HIGH | **DONE** | +| 2 | Wallet Delete and Reimport | HIGH | Not done | +| 3 | Many Investors | HIGH | Not done | +| 4 | Database Integrity | HIGH | Not done | +| 5 | Subscribe Project Type | MEDIUM | Not done | +| 6 | Investment Cancellation | MEDIUM | **DONE** | +| 7 | Project Discovery Cache | MEDIUM | Not done | +| 8 | Duplicate Wallet Import | EDGE | Not done | +| 9 | Non-Zero Penalty Recovery | EDGE | Not done | +| 10 | Network Switch | EDGE | Not done | diff --git a/docs/TEST_SDK_PARITY_ANALYSIS.md b/docs/TEST_SDK_PARITY_ANALYSIS.md new file mode 100644 index 000000000..31849f59c --- /dev/null +++ b/docs/TEST_SDK_PARITY_ANALYSIS.md @@ -0,0 +1,280 @@ +# SDK Call Parity: Analysis of What Was Done and What Remains + +> **Last updated:** April 2026 -- re-analyzed after significant code and test additions. + +This document analyzes the 9 critical SDK call parity gaps identified in the +[SDK Call Comparison](../sdk-call-comparison-app-vs-avalonia.md) between the design app +(`src/design/App/`) and the Avalonia reference app (`src/avalonia/AngorApp/`). For each gap, +we assess whether it has been resolved, what integration test coverage exists, and what +work remains. + +--- + +## Status Summary + +| # | Gap | Original Status | Current Status | Integration Test Coverage | +|---|-----|-----------------|----------------|--------------------------| +| 1 | ConfirmInvestment | **DONE** | **DONE** | Covered (3 E2E + 1 in cancellation test) | +| 2 | CancelInvestmentRequest | **DONE** | **DONE** | **Covered** (5 unit + 1 E2E) | +| 3 | INetworkConfiguration.SetNetwork() | **DONE** | **DONE** | Not covered | +| 4 | Lightning payments | NOT DONE | **RESOLVED** | 17+ unit tests | +| 5 | Recovery state machine | **DONE** | **DONE** | Partial (3/4 paths) | +| 6 | Transaction draft preview | NOT DONE | **RESOLVED** | Not covered | +| 7 | BuildInvestmentDraft FundingAddress | **DONE** | **DONE** | Implicit | +| 8 | Fee rates hardcoded to 20 | PARTIAL | **PARTIALLY RESOLVED** | Not covered | +| 9 | DeleteAllDataAsync on wipe/switch | **DONE** | **DONE** | Not covered | + +**All 9 code gaps resolved (was 7/9). 3 gaps now have test coverage (was 1). Fee rates partially resolved.** + +--- + +## Detailed Analysis + +### Gap 1: ConfirmInvestment -- DONE + +**What was missing:** No way to publish an investment after the founder signs it. + +**What was implemented:** +- `PortfolioViewModel.ConfirmInvestmentAsync()` at `src/design/App/UI/Sections/Portfolio/PortfolioViewModel.cs:941` +- Calls `IInvestmentAppService.ConfirmInvestment(PublishInvestmentRequest)` with the investment transaction hex, project identifier, and wallet ID. +- UI button "Confirm Investment" in `InvestmentDetailView.axaml:462`. +- Code-behind handler in `InvestmentDetailView.axaml.cs:37`. + +**Integration test coverage:** Covered by 4 tests: +- `FundAndRecoverTest` (line 504) +- `MultiFundClaimAndRecoverTest` (line 493) +- `MultiInvestClaimAndRecoverTest` (line 497) +- `InvestmentCancellationTest` Phase 8 (confirms approved investment, reaches Step 3) **NEW** + +**Remaining work:** None. This gap is fully resolved and tested. + +--- + +### Gap 2: CancelInvestmentRequest -- DONE + TESTED + +**What was missing:** No way to cancel a pending investment. + +**What was implemented:** +- `PortfolioViewModel.CancelInvestmentAsync()` at `PortfolioViewModel.cs:993` +- Calls `IInvestmentAppService.CancelInvestmentRequest(CancelInvestmentRequestRequest)`. +- UI buttons in `InvestmentDetailView.axaml:384` (step 1) and `:488` (general). +- Code-behind handler in `InvestmentDetailView.axaml.cs:41-43`. + +**Integration test coverage:** **Now covered.** (Was: None) +- **SDK unit tests:** `CancelInvestmentRequestTests` (5 tests) -- cancel when not on-chain, already published, hash mismatch, no record, Nostr notification +- **SDK unit tests:** `NotifyFounderOfCancellationTests` (6 tests) -- founder notification on cancel +- **E2E integration:** `InvestmentCancellationTest` (8-phase, ~40+ assertions) -- cancel before approval, cancel after approval, re-invest, confirm + +**Remaining work:** None for core functionality. Minor gap: founder-side cancellation visibility not verified in E2E. + +--- + +### Gap 3: INetworkConfiguration.SetNetwork() -- DONE + +**What was missing:** Network switching was cosmetic only -- the runtime network object was not updated. + +**What was implemented:** +- `SettingsViewModel.cs:236` calls `_networkStorage.SetNetwork(newNetwork)`. +- `SettingsViewModel.cs:242` calls `_networkConfig.SetNetwork(...)` mapping string to `Network` object. +- This means the runtime `INetworkConfiguration` is now updated along with the persisted setting. + +**Integration test coverage:** None. No test switches networks. + +**Remaining work:** +- Create `NetworkSwitchTest` (see [TEST_NEW_PROPOSALS.md](TEST_NEW_PROPOSALS.md) section 10). +- Test should verify: DerivedProjectKeys re-derived, projects filtered by network, switch is reversible. + +--- + +### Gap 4: Lightning Payments -- RESOLVED + +**What was missing:** `CreateLightningSwap` and `MonitorLightningSwap` entirely absent. + +**Previous state:** Only UI placeholders existed (`"Lightning invoices coming soon"`). + +**Current state:** Full end-to-end implementation: + +| Layer | Implementation | +|-------|---------------| +| **SDK Operations** | `CreateLightningSwapForInvestment` (calculate invoice, call Boltz API, derive claim key, persist swap), `MonitorLightningSwap` (WebSocket monitoring, auto-claim), `ClaimLightningSwap` (retrieve swap, derive key, claim on-chain) | +| **SDK Storage** | `BoltzSwapStorageService` -- save, get, get-for-wallet, get-pending, update status, mark claimed | +| **SDK DI** | `BoltzConfiguration`, `IBoltzSwapService`, `IBoltzClaimService`, `IBoltzSwapStorageService`, `IBoltzWebSocketClient` registered in `FundingContextServices` | +| **Shared Library** | 12 files under `Angor.Shared/Integration/Lightning/` -- `BoltzSwapService`, `BoltzClaimService`, `BoltzMusig2`, `BoltzWebSocketClient`, interfaces, models, DTOs | +| **Avalonia UI** | `InvoiceViewModel` (571 lines) -- on-chain/Lightning toggle, lazy-loads Lightning invoice from Boltz, monitors swap via WebSocket, falls back to on-chain on error | +| **Tests** | `ClaimLightningSwapTests` (10 tests), `CreateLightningSwapTests` (4 tests), `MonitorLightningSwapTests` (3 tests), `SwapStateExtensionTests` (3 tests), `BoltzMusig2Tests` (~17 tests) | + +**Note:** The design app (`src/design/`) still has the placeholder (`"Lightning invoices coming soon"` in `Constants.cs:18`). This is expected -- the Lightning implementation was done in the primary Avalonia app and SDK. The design app is a separate frontend. + +**Integration test coverage:** 17+ unit tests (was 0). No E2E test yet (requires local Boltz server). + +**Remaining work:** +- Wire up Lightning in the design app's invest flow (if the design app is still maintained) +- E2E integration test (requires Boltz testnet infrastructure) + +--- + +### Gap 5: Recovery State Machine -- DONE + +**What was missing:** App only checked `HasItemsInPenalty` and `HasUnspentItems`. Missing `HasSpendableItemsInPenalty`, `HasReleaseSignatures`, `EndOfProject`, and `IsAboveThreshold`. + +**What was implemented:** +- Full `RecoveryStatus` record at `PortfolioViewModel.cs:26-30`: + ``` + record RecoveryStatus(HasUnspentItems, HasSpendableItemsInPenalty, + HasReleaseSignatures, EndOfProject, IsAboveThreshold) + ``` +- Pattern matching for all recovery paths at lines 39-54: + - `HasReleaseSignatures` -> "Recover without Penalty" (unfunded release) + - `EndOfProject` or `!IsAboveThreshold` -> "Recover" (end of project claim) + - `!HasSpendableItemsInPenalty` -> "Recover to Penalty" + - `HasSpendableItemsInPenalty` -> "Recover from Penalty" (penalty release) +- Dedicated methods for each path (lines 744-940). + +**Integration test coverage:** Partial. +- `FundAndRecoverTest` covers the basic recovery path. +- `MultiFundClaimAndRecoverTest` covers penalty recovery (with PenaltyDays=0). +- `MultiInvestClaimAndRecoverTest` covers end-of-project claim. +- **Not covered:** Unfunded release path (`HasReleaseSignatures`), penalty release with non-zero penalty days. + +**Remaining work:** +- Create `PenaltyRecoveryWithTimeLockTest` (see [TEST_NEW_PROPOSALS.md](TEST_NEW_PROPOSALS.md) section 9). +- Create a test for the unfunded release path (founder releases signatures, investor claims without penalty). + +--- + +### Gap 6: Transaction Draft Preview -- RESOLVED + +**What was missing:** No fee preview before broadcasting transactions anywhere in the app. + +**Previous state:** `IWalletAppService.EstimateFeeAndSize()` and `ITransactionDraftPreviewer` / `PreviewAndCommit` never referenced in the design app. All transactions broadcast directly. + +**Current state:** Full implementation in the primary Avalonia app: + +| Component | Location | What It Does | +|-----------|----------|--------------| +| `IWalletAppService.EstimateFeeAndSize()` | `src/sdk/Angor.Sdk/Wallet/Application/IWalletAppService.cs:16` | SDK method: estimates fee and size for a transaction | +| `DomainFeeRate` | `src/sdk/Angor.Sdk/Wallet/Domain/DomainFeeRate.cs` | Strong domain type: `record DomainFeeRate(long SatsPerVByte)` | +| `ITransactionDraftPreviewer` | `src/avalonia/AngorApp.Model/Funded/Shared/Model/ITransactionDraftPreviewer.cs` | Interface: `PreviewAndCommit(createDraft, commitDraft, title, walletId)` | +| `TransactionDraftPreviewer` | `src/avalonia/AngorApp/UI/TransactionDrafts/TransactionDraftPreviewer.cs` | Implementation: dialog with fee rate selection, draft preview, user confirmation | +| `FeerateSelector` | `src/avalonia/AngorApp/UI/Shared/Controls/Feerate/` | UI control: presets (Priority/Standard/Economy) + custom input, validated 0-1000 | +| `GetFeeratePresetsAsync()` | `src/avalonia/AngorApp/UI/Shared/Services/UIServices.cs:137-179` | Fetches dynamic fee estimates from `walletAppService.GetFeeEstimates()`, falls back to defaults (Economy=2, Standard=12, Priority=20) | + +**Used in:** +- Wallet sends (`TransactionDraftViewModel`) +- All recovery/claim operations via `FundedBase.DoRecoverFunds` -> `draftPreviewer.PreviewAndCommit()` +- DI registered in `UIServicesRegistration.cs:45` + +**Note:** The design app still broadcasts directly without preview. The Avalonia app has the complete two-step flow. + +**Integration test coverage:** None. + +**Remaining work:** +- Add preview step to the design app's broadcast flows (if still maintained) +- Integration test verifying estimated fee approximately matches actual fee + +--- + +### Gap 7: BuildInvestmentDraft FundingAddress -- DONE + +**What was missing:** `FundingAddress` not passed to `BuildInvestmentDraft`, causing wrong UTXO selection. + +**What was implemented:** +- `InvestPageViewModel.cs:761` now passes `FundingAddress: addressResult.Value.Value` to the `BuildInvestmentDraft` request. +- The funding address is obtained from `IWalletAppService.GetNextReceiveAddress()` earlier in the flow. + +**Integration test coverage:** Implicit. The E2E invest tests (`FundAndRecoverTest`, `MultiFundClaimAndRecoverTest`, `InvestmentCancellationTest`) call the invest flow through the UI and investments succeed, implicitly validating correct address propagation. + +**Remaining work:** A specific assertion that the correct address was used would strengthen confidence, but this is low priority given successful E2E validation. + +--- + +### Gap 8: Fee Rates Hardcoded to 20 -- PARTIALLY RESOLVED + +**What was missing:** All fee rates hardcoded to `20` sats/vbyte instead of being user-configurable. + +**Current state:** + +**Avalonia app (primary):** Dynamic fee rate selection with network-fetched presets: +- `GetFeeratePresetsAsync()` fetches from `walletAppService.GetFeeEstimates()`, maps confirmations to named presets (Priority <= 1 block, Standard <= 6 blocks, Economy), falls back to defaults (Economy=2, Standard=12, Priority=20 sat/vB) +- `FeerateSelector` control: user picks from presets or enters custom fee rate +- Used in wallet sends and all recovery/claim operations via `PreviewAndCommit` + +**However, the investment flow hardcodes fee rate to 2 sat/vB:** +- `InvoiceViewModel.cs:25`: `private const int DefaultFeeRateSatsPerVbyte = 2;` +- Used at lines 234 and 325 for Lightning swap creation and investment transaction building +- No user-selectable fee rate picker in the investment flow + +**Design app:** Uses hardcoded presets (50/20/5) without fetching dynamic fee estimates from the network: +- `InvestPageViewModel.cs:113` and `DeployFlowViewModel.cs:46`: `selectedFeeRate = 20` +- `PortfolioViewModel.cs:744,793,842,891`: Recovery/claim methods default to `feeRateSatsPerVByte = 20` +- `FeeSelectionPopup`: Priority=50, Standard=20, Economy=5 (static, not network-fetched) + +**SDK operations:** Some hardcode low fee rates: +- `MonitorLightningSwap.cs:183`: `FeeRate: 2` +- `ClaimLightningSwap.cs:39`: `FeeRate: 2` + +**Integration test coverage:** None. No test verifies fee rate behavior. + +**Remaining work:** +- Expose fee rate selection in the Avalonia investment flow (currently hardcoded to 2) +- Lower design app defaults or add dynamic fee fetching +- Integration test verifying fee rate propagation + +--- + +### Gap 9: DeleteAllDataAsync on Wipe/Network Switch -- DONE + +**What was missing:** Incomplete data cleanup when wiping data or switching networks. + +**What was implemented:** +- `SettingsViewModel.cs:28` injects `IDatabaseManagementService`. +- `SettingsViewModel.cs:227` calls `_databaseManagementService.DeleteAllDataAsync()` during network switch. +- `SettingsViewModel.cs:363` calls `_databaseManagementService.DeleteAllDataAsync()` during data wipe. +- Both paths wipe all 8 LiteDB collections. + +**Integration test coverage:** None. No test exercises the wipe/switch flow. + +**Remaining work:** +- Create `DatabaseIntegrityTest` (see [TEST_NEW_PROPOSALS.md](TEST_NEW_PROPOSALS.md) section 4) that calls `DeleteAllDataAsync` and verifies all collections are empty. +- Create `NetworkSwitchTest` (see [TEST_NEW_PROPOSALS.md](TEST_NEW_PROPOSALS.md) section 10) that verifies `DeleteAllDataAsync` is called during network switch and data is rebuilt. + +--- + +## Integration Test Coverage of Resolved Gaps (Revised) + +Of the 9 resolved gaps, **3 now have test coverage** (was 1): + +| Resolved Gap | Has Integration Test? | Change | Proposed Test | +|---|---|---|---| +| 1. ConfirmInvestment | Yes (4 tests) | +1 (cancellation test) | -- | +| 2. CancelInvestmentRequest | **Yes** (5 unit + 1 E2E) | **NEW** | -- | +| 3. SetNetwork | No | No change | `NetworkSwitchTest` | +| 4. Lightning payments | **Yes** (17+ unit tests) | **NEW** | E2E needs Boltz infra | +| 5. Recovery state machine | Partial (3 paths of 4) | No change | `PenaltyRecoveryWithTimeLockTest` | +| 6. Transaction draft preview | No | No change | Fee estimation test | +| 7. FundingAddress | Implicit (3+ E2E) | +1 (cancellation test) | Strengthen existing | +| 8. Fee rates (partial) | No | No change | Fee rate propagation test | +| 9. DeleteAllDataAsync | No | No change | `DatabaseIntegrityTest` | + +--- + +## Recommended Next Steps (Revised) + +### Immediate (highest value) + +1. **Fix Bug #1:** Orphaned DB data on wallet delete (`WalletAppService.DeleteWallet()`) +2. **Fix Bug #3:** Duplicate wallet guard (`WalletFactory.CreateWallet()`) +3. **LiteDB round-trip tests** against real LiteDB -- still the #1 test gap + +### Short-term (test the remaining untested gaps) + +4. **Create `DatabaseIntegrityTest`** to cover Gap 9 end-to-end. +5. **Create `NetworkSwitchTest`** to cover Gaps 3 and 9 end-to-end. +6. **Create `PenaltyRecoveryWithTimeLockTest`** to cover the remaining Gap 5 path. +7. **Expose fee rate selection** in the investment flow (Gap 8). + +### Medium-term (strengthen existing tests) + +8. Add database assertions to all existing integration tests (see [TEST_IMPROVEMENTS.md](TEST_IMPROVEMENTS.md)). +9. Create SDK-level tests for database round-trips (see [TEST_SDK_PROPOSALS.md](TEST_SDK_PROPOSALS.md)). +10. Wire Lightning into the design app (if still maintained) to close Gap 4 there. diff --git a/docs/TEST_SDK_PROPOSALS.md b/docs/TEST_SDK_PROPOSALS.md new file mode 100644 index 000000000..acf0c6d25 --- /dev/null +++ b/docs/TEST_SDK_PROPOSALS.md @@ -0,0 +1,377 @@ +# SDK-Level Test Proposals + +> **Last updated:** April 2026 -- re-analyzed after significant test additions. + +This document describes new tests to add at the SDK level (`src/sdk/Angor.Sdk.Tests/`). These tests use real LiteDB but mock external dependencies (indexer, Nostr relays), so they can run in CI without any network infrastructure. + +**Status: 1 of 8 proposals fully implemented, 1 partially implemented, 6 not done.** + +--- + +## 1. LiteDB Document Round-Trip Tests (HIGH PRIORITY) + +**Status:** PARTIAL -- `LiteDbGenericDocumentCollectionTests` exists (3 tests) but only validates the `??=` expression caching bug fix using mocks, not round-trip persistence of each document type against real LiteDB. + +**Why:** The `IGenericDocumentCollection` implementation (`LiteDbGenericDocumentCollection`) has complex expression rewriting logic and wraps/unwraps entities in `Document`. The existing 3 tests validate a specific caching bug but don't test CRUD operations against real LiteDB. + +**File:** `Angor.Sdk.Tests/Data/LiteDbDocumentRoundTripTests.cs` + +### Test Setup + +```csharp +public class LiteDbDocumentRoundTripTests : IDisposable +{ + private readonly LiteDatabase _db; + private readonly IGenericDocumentCollection _collection; + // Create a temp LiteDB in-memory or temp file + // Wire up LiteDbDocumentCollection -> LiteDbGenericDocumentCollection +} +``` + +### Tests for Each Document Type + +#### Project + +``` +- Project_InsertAndFindById_RoundTrips +- Project_InsertMultipleAndFindByIds_ReturnsAll +- Project_InsertAndFindAsync_WithPredicate_FiltersCorrectly +- Project_InsertAndDelete_RemovesDocument +- Project_Upsert_InsertsNew_ThenUpdatesExisting +``` + +#### DerivedProjectKeys + +``` +- DerivedProjectKeys_UpsertAndFindById_PreservesAllKeySlots +- DerivedProjectKeys_Upsert_OverwritesExistingKeys +- DerivedProjectKeys_FindById_WhenMissing_ReturnsNull +``` + +#### InvestmentRecordsDocument + +``` +- InvestmentRecords_UpsertWithMultipleRecords_PreservesAll +- InvestmentRecords_AddRecordThenRemove_UpdatesCorrectly +``` + +#### InvestmentHandshake (Composite Key) + +``` +- InvestmentHandshake_UpsertAndFindById_WithCompositeKey +- InvestmentHandshake_FindAsync_ByStatus_FiltersCorrectly +- InvestmentHandshake_FindAsync_ByProjectId_FiltersCorrectly +``` + +#### BoltzSwapDocument + +``` +- BoltzSwap_InsertAndFindById_RoundTrips +- BoltzSwap_UpdateStatus_PersistsChange +- BoltzSwap_Delete_RemovesDocument +``` + +#### WalletAccountBalanceInfo + +``` +- WalletAccountBalance_UpsertAndFindById_RoundTrips +- WalletAccountBalance_Delete_RemovesDocument +``` + +#### QueryTransaction / TransactionHexDocument + +``` +- QueryTransaction_InsertAndFindById_RoundTrips +- TransactionHex_InsertAndFindById_RoundTrips +``` + +### Cross-Cutting Tests + +``` +- DeleteAllAsync_ClearsAllDocuments +- CountAsync_WithNoPredicate_ReturnsTotal +- CountAsync_WithPredicate_ReturnsFilteredCount +- ExistsAsync_WhenExists_ReturnsTrue +- ExistsAsync_WhenMissing_ReturnsFalse +- FindByIdAsync_WithEmptyString_ReturnsFailureOrNull +- InsertAsync_WithDuplicateKey_HandlesGracefully +``` + +--- + +## 2. WalletFactory Integration Tests (HIGH PRIORITY) + +**Status:** NOT DONE + +**Why:** `WalletFactory.CreateWallet` is the core wallet lifecycle method. It persists to both the file store and LiteDB. Currently has zero test coverage. + +**File:** `Angor.Sdk.Tests/Wallet/WalletFactoryIntegrationTests.cs` + +### Test Setup + +Uses real LiteDB (temp file), real `AesWalletEncryption`, an in-memory `IStore`, and real `DerivationOperations` / `WalletOperations` from `TestNetworkFixture`. + +### Tests + +``` +CreateWallet_PersistsEncryptedWallet_ToStore + - Arrange: Configure InMemoryStore, real WalletFactory + - Act: CreateWallet("Test Wallet", seedwords, passphrase, "password123", Testnet) + - Assert: IWalletStore.GetAll() returns 1 wallet with correct Id and Name + +CreateWallet_PersistsDerivedProjectKeys_ToLiteDb + - Act: CreateWallet(...) + - Assert: IGenericDocumentCollection.FindByIdAsync(walletId) returns 15 key slots + +CreateWallet_PersistsAccountBalanceInfo_ToLiteDb + - Act: CreateWallet(...) + - Assert: IGenericDocumentCollection.FindByIdAsync(walletId) returns initialized balance + +CreateWallet_SameSeedTwice_ProducesSameWalletId + - Act: CreateWallet twice with same seed words + - Assert: Both produce the same WalletId + - Assert: wallets.json has TWO entries with same Id (documenting Bug #3) + +CreateWallet_InvalidSeedPhrase_ReturnsFailure + - Act: CreateWallet("", "invalid words here", ...) + - Assert: Result.IsFailure with appropriate error message + +CreateWallet_EmptySeedPhrase_ThrowsDomainException + - Act: CreateWallet("", "", ...) + - Assert: DomainException thrown from WalletDescriptorFactory + +RebuildFounderKeys_UpsertsNewKeys_InLiteDb + - Arrange: CreateWallet first (keys exist) + - Act: RebuildFounderKeysAsync with same wallet words + - Assert: DerivedProjectKeys still has 15 key slots (upserted, not duplicated) + +CreateWallet_WithPassphrase_ProducesDifferentWalletId + - Act: CreateWallet with same seed, no passphrase vs. with passphrase + - Assert: WalletIds are different +``` + +--- + +## 3. DatabaseManagementService Tests (HIGH PRIORITY) + +**Status:** NOT DONE + +**Why:** `DeleteAllDataAsync` is the nuclear reset option. It operates on all 8 collections but is untested. + +**File:** `Angor.Sdk.Tests/Funding/Services/DatabaseManagementServiceTests.cs` + +### Tests + +``` +DeleteAllData_ClearsAllEightCollections + - Arrange: Insert documents into all 8 collections + - Act: DeleteAllDataAsync() + - Assert: All 8 collections are empty (CountAsync == 0) + +DeleteAllData_OnEmptyDatabase_Succeeds + - Arrange: Empty DB + - Act: DeleteAllDataAsync() + - Assert: Result.IsSuccess, no errors + +DeleteAllData_ReturnsCorrectDeleteCount + - Arrange: Insert known counts into each collection + - Act: DeleteAllDataAsync() + - Assert: Log message shows correct total (verify via mock logger) +``` + +--- + +## 4. PortfolioService Integration Tests (MEDIUM PRIORITY) + +**Status:** NOT DONE (PortfolioService is only mocked in `CancelInvestmentRequestTests`, `InvestmentAppServiceTests`, `PublishInvestmentTests`, and `CreateInvestmentTests`) + +**Why:** `PortfolioService` has a two-tier cache (local LiteDB + Nostr relay) but only the Nostr path is covered via mocks. The local cache path and the add/remove operations are untested. + +**File:** `Angor.Sdk.Tests/Funding/Investor/Domain/PortfolioServiceIntegrationTests.cs` + +### Test Setup + +Real LiteDB, mocked `IRelayService`, mocked `ISeedwordsProvider` / `IEncryptionService`. + +### Tests + +``` +GetByWalletId_WhenLocalExists_ReturnsWithoutRelayCall + - Arrange: Pre-insert InvestmentRecordsDocument into LiteDB + - Act: GetByWalletId(walletId) + - Assert: Returns local data, relay NOT called (verify via Mock) + +GetByWalletId_WhenLocalMissing_FetchesFromRelay_CachesLocally + - Arrange: Empty LiteDB, relay returns encrypted investment records + - Act: GetByWalletId(walletId) + - Assert: Returns relay data AND caches to LiteDB + +AddOrUpdate_InsertsNew_RecordToCollection + - Arrange: Empty collection + - Act: AddOrUpdate(walletId, newRecord) + - Assert: LiteDB contains InvestmentRecordsDocument with 1 record + +AddOrUpdate_UpdatesExisting_RecordInCollection + - Arrange: Collection has 1 record for projectX + - Act: AddOrUpdate(walletId, updatedRecordForProjectX) + - Assert: Collection still has 1 record, but with updated data + +AddOrUpdate_WithMultipleProjects_MaintainsList + - Act: AddOrUpdate for project1, then project2, then project3 + - Assert: Collection has 3 records, all preserved + +RemoveInvestmentRecord_RemovesFromLocal + - Arrange: Collection has 2 records + - Act: RemoveInvestmentRecordAsync(walletId, record1) + - Assert: Collection has 1 record (record2 only) + +RemoveInvestmentRecord_WhenNotFound_Succeeds + - Act: RemoveInvestmentRecordAsync with non-existent record + - Assert: Result.IsSuccess (nothing to remove) +``` + +--- + +## 5. DocumentProjectService Integration Tests (MEDIUM PRIORITY) + +**Status:** NOT DONE + +**Why:** The project caching layer in LiteDB is critical for performance and offline-ish behavior. The cache-hit vs cache-miss paths are untested. + +**File:** `Angor.Sdk.Tests/Funding/Services/DocumentProjectServiceIntegrationTests.cs` + +### Test Setup + +Real LiteDB, mocked `IRelayService`, mocked `IAngorIndexerService`. + +### Tests + +``` +GetAllAsync_CacheHit_ReturnsFromLiteDb_WithoutCallingIndexer + - Arrange: Insert Project into LiteDB + - Act: GetAllAsync(projectId) + - Assert: Returns project from DB, indexer NOT called + +GetAllAsync_CacheMiss_FetchesFromIndexerAndNostr_CachesResult + - Arrange: Empty LiteDB, indexer returns ProjectData, relay returns ProjectInfo + Metadata + - Act: GetAllAsync(projectId) + - Assert: Returns project AND caches in LiteDB for next call + +GetAllAsync_PartialCacheHit_FetchesMissingOnly + - Arrange: LiteDB has project1, missing project2 + - Act: GetAllAsync(project1Id, project2Id) + - Assert: Returns both, indexer called only for project2 + +TryGetAsync_WhenNotFound_ReturnsNone + - Arrange: Empty LiteDB, indexer returns null + - Act: TryGetAsync(unknownId) + - Assert: Returns Maybe.None + +GetAllAsync_WithNullIds_ReturnsFailure + - Act: GetAllAsync(null) + - Assert: Result.IsFailure with "ProjectId cannot be null" + +GetAllAsync_CachedProject_PreservesAllFields + - Arrange: Insert project with all fields populated (stages, URIs, dynamic patterns, etc.) + - Act: FindByIdAsync + - Assert: All fields match original (validates LiteDB serialization) +``` + +--- + +## 6. WalletAppService.DeleteWallet Tests (MEDIUM PRIORITY) + +**Status:** NOT DONE + +**Why:** Delete is a destructive operation with the known orphaned-data gap (Bug #1). Tests should document what gets cleaned and what doesn't. + +**File:** `Angor.Sdk.Tests/Wallet/WalletAppServiceDeleteTests.cs` + +### Tests + +``` +DeleteWallet_RemovesEncryptedWallet_FromStore + - Arrange: CreateWallet, then DeleteWallet + - Assert: IWalletStore.GetAll() is empty + +DeleteWallet_RemovesAccountBalanceInfo_FromLiteDb + - Assert: WalletAccountBalanceInfo collection no longer has entry + +DeleteWallet_DoesNotRemoveDerivedProjectKeys_FromLiteDb + - Assert: DerivedProjectKeys STILL exists (documents Bug #1) + +DeleteWallet_DoesNotRemoveFounderProjectsDocument_FromLiteDb + - Assert: FounderProjectsDocument STILL exists (documents Bug #1) + +DeleteWallet_DoesNotRemoveInvestmentHandshake_FromLiteDb + - Assert: InvestmentHandshake records STILL exist (documents Bug #1) + +DeleteWallet_WhenWalletNotFound_ReturnsFailure + - Act: DeleteWallet(unknownId) + - Assert: Result.IsFailure("Wallet not found") + +DeleteWallet_ClearsSensitiveDataFromMemory + - Arrange: CreateWallet (sensitive data cached) + - Act: DeleteWallet + - Assert: ISensitiveWalletDataProvider no longer has cached data +``` + +--- + +## 7. ~~ScanFounderProjects Tests~~ -- IMPLEMENTED + +**Status:** **DONE** -- `ScanFounderProjectsTests.cs` added with 7 unit tests. + +**What it covers:** +- `Handle_WhenDerivedKeysFails_ReturnsFailure` -- storage error handling +- `Handle_WhenDerivedKeysReturnsNull_ReturnsFailure` -- null keys handling +- `Handle_WhenNoUnknownKeysAndNoLocal_ReturnsEmptyList` -- empty state +- `Handle_WhenAllKeysAlreadyKnown_SkipsScanAndReturnsProjects` -- cache hit (no indexer call) +- `Handle_WhenNewProjectsDiscovered_PersistsAndReturnsThem` -- new project discovery + `AddRange` persistence +- `Handle_WhenScanFailsButLocalProjectsExist_StillReturnsLocalProjects` -- graceful degradation +- `Handle_WhenFinalProjectLoadFails_ReturnsFailure` -- final load error + +**All use mocks (Moq), not real LiteDB.** A real-LiteDB variant would further strengthen confidence but is lower priority now. + +--- + +## 8. AesWalletEncryption Round-Trip Tests (LOW PRIORITY) + +**Status:** NOT DONE + +**Why:** Encryption/decryption is critical but untested at the unit level. + +**File:** `Angor.Sdk.Tests/Wallet/AesWalletEncryptionTests.cs` + +### Tests + +``` +EncryptAndDecrypt_RoundTrip_PreservesData + - Act: Encrypt WalletData, then Decrypt with same key + - Assert: Decrypted data matches original + +Decrypt_WithWrongKey_ReturnsFailureOrThrows + - Act: Encrypt with key1, Decrypt with key2 + - Assert: Failure (not silent corruption) + +Encrypt_ProducesUniqueSaltAndIV_EachTime + - Act: Encrypt same data twice with same key + - Assert: Salt and IV differ between the two encrypted results +``` + +--- + +## Implementation Priority (Revised) + +| Priority | Test Class | Effort | Dependencies | Status | +|---|---|---|---|---| +| 1 | `LiteDbDocumentRoundTripTests` | Low | LiteDB only (temp file) | **PARTIAL** (3 mock-based tests exist) | +| 2 | `DatabaseManagementServiceTests` | Low | LiteDB only | Not done | +| 3 | `WalletFactoryIntegrationTests` | Medium | LiteDB + InMemoryStore + TestNetworkFixture | Not done | +| 4 | `WalletAppServiceDeleteTests` | Medium | Same as above | Not done | +| 5 | `PortfolioServiceIntegrationTests` | Medium | LiteDB + Moq (relay, encryption) | Not done | +| 6 | `DocumentProjectServiceIntegrationTests` | Medium | LiteDB + Moq (relay, indexer) | Not done | +| 7 | `ScanFounderProjectsTests` | Medium | LiteDB + Moq (indexer, project service) | **DONE** (7 tests) | +| 8 | `AesWalletEncryptionTests` | Low | No dependencies | Not done | + +All of these can run in CI without any network infrastructure. They use real LiteDB (temp files) and mock all external services.