Reviewing files that changed from the base of the PR and between c829fa8 and c141c8f.

• pkg/boltzrpc/autoswaprpc/autoswaprpc.pb.go is excluded by !**/*.pb.go
• pkg/boltzrpc/autoswaprpc/autoswaprpc_grpc.pb.go is excluded by !**/*.pb.go
• pkg/boltzrpc/boltzrpc.pb.go is excluded by !**/*.pb.go
• pkg/boltzrpc/boltzrpc.pb.gw.go is excluded by !**/*.pb.gw.go
• pkg/boltzrpc/boltzrpc_grpc.pb.go is excluded by !**/*.pb.go

• .github/workflows/test.yml (3 hunks)
• .gitignore (1 hunks)
• .gitmodules (1 hunks)
• .golangci.yml (1 hunks)
• Makefile (3 hunks)
• cmd/boltzcli/commands.go (2 hunks)
• docs/grpc.md (1 hunks)
• internal/cln/cln.go (2 hunks)
• internal/database/database.go (1 hunks)
• internal/database/migration.go (2 hunks)
• internal/database/wallet.go (4 hunks)
• internal/lnd/lnd.go (2 hunks)
• internal/mocks/lightning/LightningNode_mock.go (2 hunks)
• internal/mocks/onchain/Wallet_mock.go (2 hunks)
• internal/nursery/nursery.go (2 hunks)
• internal/nursery/nursery_test.go (2 hunks)
• internal/nursery/refund.go (2 hunks)
• internal/nursery/reverse.go (5 hunks)
• internal/onchain/liquid-wallet/lwk/dynamic.go (1 hunks)
• internal/onchain/liquid-wallet/lwk/static.go (1 hunks)
• internal/onchain/liquid-wallet/wallet.go (1 hunks)
• internal/onchain/liquid-wallet/wallet_test.go (1 hunks)
• internal/onchain/wallet.go (3 hunks)
• internal/onchain/wallet/wallet.go (4 hunks)
• internal/onchain/wallet/wallet_test.go (2 hunks)
• internal/rpcserver/router.go (18 hunks)
• internal/rpcserver/rpcserver_test.go (8 hunks)
• internal/rpcserver/serializer.go (1 hunks)
• internal/rpcserver/server.go (2 hunks)
• internal/test/test.go (3 hunks)
• lwk (1 hunks)
• pkg/boltzrpc/boltzrpc.proto (4 hunks)
• pkg/boltzrpc/client/client.go (1 hunks)
• rust-toolchain.toml (1 hunks)

• GitHub Check: CI (ubuntu-latest, 1.24.x)

rust-toolchain.toml (1)

2-2: Comment LGTM, but keep version drift in mind

Locking to 1.81.0 is sensible for cache-hits; just remember to bump when lwk moves to a newer compiler.
No code changes needed.

internal/database/database.go (1)

221-222: It looks like the migration for legacy and lastIndex is already implemented in internal/database/migration.go:

ALTER TABLE wallets ADD COLUMN legacy BOOLEAN DEFAULT FALSE;
ALTER TABLE wallets ADD COLUMN lastIndex INT;
UPDATE wallets SET legacy = TRUE;

No “version 16” bump is required here—this file uses a self-contained migration sequence rather than discrete numbered versions. All existing wallets will be marked as legacy=true, preserving the intended backward-compatibility.

All set!

docs/grpc.md (1)

1514-1514: LGTM! Proper deprecation marking aligns with subaccount removal.

The documentation correctly marks the subaccount field as deprecated, which aligns with the coordinated deprecation of subaccount functionality across the codebase mentioned in the AI summary.

cmd/boltzcli/commands.go (2)

1832-1832: LGTM - Appropriate use of linter directive for deprecated function.

The staticcheck linter directive is correctly applied to suppress warnings for the printSubaccount function, which aligns with the deprecation of subaccount functionality mentioned in the PR objectives.

---

1912-1912: LGTM - Simplified wallet import flow.

The removal of selectSubaccount call and always printing the total balance aligns with the deprecation of subaccount functionality. The change provides consistent user feedback regardless of wallet type.

internal/onchain/liquid-wallet/lwk/static.go (1)

1-8: LGTM - Correct CGO static linking implementation.

The build constraints and linker flags are properly configured for static linking:

• Build tag static && !dynamic correctly excludes dynamic builds
• Static library path ${SRCDIR}/liblwk.a is appropriate for build-time linking
• Linker flags -Wl,--no-as-needed -ldl ensure proper static linking with dynamic loading support

.github/workflows/test.yml (3)

23-24: LGTM - Required for lwk submodule integration.

The addition of submodules: recursive is necessary to properly checkout the new lwk Git submodule introduced in this PR.

---

34-36: LGTM - Appropriate cache expansion for new Rust workspace.

The addition of ./lwk to the cache-workspaces list is correct for the new lwk Rust library. This will improve CI performance by caching Rust dependencies for both the existing bolt12 and new lwk workspaces.

---

47-47: LGTM - Simplified regtest startup aligns with Makefile refactoring.

The removal of make download-regtest is consistent with the Makefile refactoring that replaced it with setup-regtest and integrated the functionality into the startup process.

internal/onchain/liquid-wallet/lwk/dynamic.go (1)

1-9: LGTM - Correct CGO dynamic linking implementation.

The build constraints and linker flags are properly configured for dynamic linking:

• Build tag dynamic || !static correctly includes dynamic builds and default builds
• Dynamic library linking -llwk with search path -L${SRCDIR} is appropriate
• Runtime path -Wl,-rpath=${SRCDIR} ensures library can be found at runtime

internal/lnd/lnd.go (2)

77-79: LGTM - Appropriate no-op implementation for interface compliance.

The Sync() method returning nil is correct for LND since synchronization is handled internally by the LND node. This aligns with the wallet interface standardization across implementations.

---

404-406: LGTM - Consistent unsupported method implementation.

The GetOutputs() method correctly returns lightning.ErrUnsupported, which is consistent with other unsupported methods in the LND implementation like GetTransactions() and BumpTransactionFee().

internal/onchain/wallet.go (3)

11-11: LGTM: Well-defined constant for transaction limits.

The constant name and value are appropriate for pagination purposes.

---

59-61: LGTM: Interface extension follows consistent patterns.

The new methods GetOutputs and Sync are well-defined and follow the existing interface patterns. The GetOutputs method properly returns a slice of pointers to Output structs, and both methods have appropriate error handling.

---

78-78: LGTM: Simple and clear field addition.

The Legacy boolean field is a straightforward addition to the struct that aligns with the wallet credential requirements.

internal/cln/cln.go (2)

102-104: LGTM: Appropriate no-op implementation for Lightning.

The Sync() method correctly returns nil as a no-op, which is appropriate for CLN since Lightning nodes don't require explicit synchronization like onchain wallets.

---

358-362: LGTM: Correct unsupported operation handling.

The GetOutputs method appropriately returns lightning.ErrUnsupported since CLN is a Lightning implementation that doesn't manage individual UTXOs. The method signature correctly matches the interface and the fix for the missing closing brace resolves the syntax issue.

internal/nursery/refund.go (1)

35-35: LGTM: Method call simplified correctly.

The removal of the empty string parameter from checkExternalReverseSwaps aligns with the method signature change mentioned in the AI summary, making the call cleaner and more focused.

internal/nursery/nursery_test.go (1)

111-221: LGTM: Comprehensive test coverage for direct output selection.

The test function provides excellent coverage with well-structured table-driven tests covering three important scenarios:

1. Match: Successfully finding a suitable output within the expected range
2. NoMatch: Handling cases where no suitable output exists
3. Overpaid: Handling overpaid scenarios gracefully

The test data is well-constructed with realistic ReverseSwap configurations, fee estimations, and output values. The use of require.Eventually pattern would be beneficial here if the tested method involves any asynchronous operations, but the current synchronous assertion approach is appropriate for this logic.

internal/onchain/wallet/wallet_test.go (1)

262-275: LGTM: Improved test reliability with require.Eventually.

The replacement of the manual timeout loop with require.Eventually is an excellent improvement. This approach:

• Eliminates the need for explicit channel management
• Provides cleaner error reporting
• Uses consistent polling intervals
• Reduces complexity in the test logic

The 15-second timeout with 250ms intervals is appropriate for blockchain-related operations.

internal/database/migration.go (3)

24-24: LGTM on schema version increment.

The increment from 15 to 16 is correct for the new migration.

---

591-592: Good addition of logging for consistency.

Adding the logMigration(oldVersion) call to case 14 maintains consistency with other migration cases.

---

598-608: Migration columns look safe—confirm legacy column is read by your queries

We’ve verified that:

• lastIndex is represented as sql.NullInt64 in internal/database/wallet.go, and all codepaths check .Valid before using the value.
• The DDL sets legacy BOOLEAN DEFAULT FALSE and immediately flips existing rows to TRUE, so old wallets are correctly marked while new ones default to FALSE.

Please double-check that your data-access layer actually selects and scans the new legacy column so it’s surfaced in your Wallet structs. In particular, review:

• internal/database/wallet.go – ensure your SELECT clauses and Scan(...) calls include the legacy field.
• internal/rpcserver (or wherever you map DB rows to RPC models) – confirm the Legacy flag is being loaded and returned.

internal/onchain/wallet/wallet.go (3)

770-773: Good refactoring to use shared constant.

Replacing the local GetTransactionsMaxLimit with onchain.DefaultTransactionsLimit improves consistency across the codebase.

---

858-858: Consistent usage of shared constant in BumpTransactionFee.

The replacement of hardcoded limits with onchain.DefaultTransactionsLimit is consistent with the interface changes.

Also applies to: 868-868, 871-871

---

509-511: Appropriate no-op implementation of Sync method.

The no-op implementation is appropriate for this wallet type and satisfies the interface requirement. The method can be enhanced in the future if synchronization logic is needed.

internal/test/test.go (3)

50-50: Good addition of WalletId constant.

Adding the WalletId constant and using it in the WalletCredentials function provides consistency across tests.

Also applies to: 59-59

---

66-105: Well-implemented FundWallet function.

The FundWallet function is well-structured with:

• Early return if wallet already has funds
• Proper error handling
• Reasonable timeout (15 seconds)
• Clean separation of concerns

The function improves code reusability and maintainability.

---

135-137: Good refactoring of InitTestWallet.

Replacing the inline funding logic with the FundWallet function call improves code organization and reusability.

internal/nursery/nursery.go (3)

252-252: Appropriate parameter removal.

Removing the TxId parameter from the checkExternalReverseSwaps call simplifies the interface and aligns with the refactoring mentioned in the AI summary.

---

368-380: Well-implemented fee estimation method.

The GetFeeEstimations method is well-structured with:

• Clear separation of concerns
• Proper error handling for both currencies
• Clean return of fee estimations map
• Appropriate error propagation

This consolidates fee estimation logic effectively.

---

382-388: Good refactoring of CheckAmounts method.

The refactoring to use the new GetFeeEstimations method improves code organization while maintaining the same functionality. Error handling is preserved correctly.

internal/rpcserver/server.go (2)

19-19: Appropriate imports for Liquid wallet backend.

The addition of database and liquid_wallet imports are necessary for the new backend initialization.

Also applies to: 23-23

---

210-223: Well-implemented Liquid wallet backend initialization.

The backend initialization is well-structured with:

• Proper configuration setup using server's network and data directory
• Appropriate use of existing onchain Liquid transaction provider
• Database-backed wallet persister
• Conditional Electrum configuration based on availability
• Proper error handling and propagation

The initialization follows established patterns and integrates well with the existing server architecture.

pkg/boltzrpc/boltzrpc.proto (1)

108-108: Consistent deprecation of subaccount functionality.

The deprecation of subaccount-related RPCs, fields, and messages is properly implemented following protobuf conventions. This maintains backward compatibility while signaling to users that these features should no longer be used.

Also applies to: 113-113, 722-722, 747-747, 754-754, 759-759, 900-900

internal/rpcserver/rpcserver_test.go (4)

20-21: LGTM! Clean import additions.

The new imports for liquid wallet packages are properly organized and necessary for the new wallet implementation and serialization.

Also applies to: 34-34

---

1161-1186: Improved wallet retrieval and funding logic.

The refactored fundedWallet function now properly checks for existing wallets before importing, avoiding duplicate imports. The funding logic with require.Eventually is more robust than the previous implementation.

---

1740-1762: Test updates align with new wallet implementation.

The test correctly validates both mnemonic and the new CoreDescriptor field after wallet creation. The removal of the deprecated SetSubaccount call is appropriate.

---

3134-3171: Excellent backward compatibility test.

The test properly validates that wallets can switch between the new Liquid wallet implementation and the legacy implementation based on the database flag. This ensures smooth migration paths for existing users.

Makefile (3)

50-55: Proper Rust-Go binding generation setup.

The use of uniffi-bindgen-go with a specific version tag ensures reproducible builds. The conditional installation of the tool is a good practice.

---

96-101: Efficient conditional build for lwk library.

The build target properly checks for the existence of the library before building, preventing unnecessary rebuilds. Copying both .a and .so files ensures compatibility with different linking scenarios.

---

68-86: Improved test isolation with wallet data cleanup.

The addition of clear-wallet-data ensures clean test runs by removing any persisted wallet data. This prevents test pollution and makes tests more reliable.

internal/database/wallet.go (2)

10-41: Well-structured persistence layer for Liquid wallet.

The WalletPersister provides a clean implementation of the liquid_wallet.Persister interface with proper error handling and null value support for the lastIndex field.

---

84-106: Robust null handling for optional LastIndex field.

The use of sql.NullInt64 properly handles nullable database columns, and the error wrapping provides better context for debugging.

internal/mocks/onchain/Wallet_mock.go (1)

204-264: LGTM!

The auto-generated mock methods for GetOutputs and Sync follow the established patterns correctly.

Also applies to: 601-643

internal/mocks/lightning/LightningNode_mock.go (1)

446-506: LGTM!

The auto-generated mock methods for GetOutputs and Sync are correctly implemented.

Also applies to: 1130-1172

internal/onchain/liquid-wallet/wallet_test.go (5)

22-48: LGTM! Well-structured test helpers.

The helper functions provide clean test setup with in-memory database and proper configuration.

---

116-119: Good test coverage for spent outputs tracking.

The test properly verifies that spent outputs are tracked and cleared after sync, preventing double-spending.

Also applies to: 149-156

---

189-201: LGTM! Clean wallet creation helper.

The helper function properly handles credential generation and wallet initialization.

---

203-225: Excellent test for address generation and persistence.

The test thoroughly verifies address uniqueness and proper index persistence.

---

245-261: Well-designed consolidation test.

The test effectively verifies automatic UTXO consolidation behavior. The balance verification logic correctly accounts for the consolidation transaction.

internal/rpcserver/router.go (5)

72-72: LGTM!

The addition of liquidBackend field to the server struct is appropriate for managing Liquid wallet operations.

---

1042-1050: LGTM!

The condition correctly ensures both claimAddress and walletId are present before generating the address signature, which is necessary to prove ownership of the claim address.

---

1751-1776: LGTM!

The fee estimation logic is properly updated to be currency-aware, correctly using the wallet's currency for fee rate estimation and dummy address selection.

---

2396-2401: LGTM!

The loginWallet method correctly routes between liquid wallet and GDK wallet implementations based on currency and legacy flag.

---

2029-2036: Good error handling for wallet sync during unlock.

The implementation correctly handles sync failures by logging errors without preventing the wallet from being added. This allows the wallet to be used even if initial sync fails, with the sync loop retrying later.

internal/onchain/liquid-wallet/wallet.go (5)

20-37: LGTM!

The Persister interface and struct definitions are well-designed. The Wallet struct properly includes synchronization primitives and thread-safe spent outputs tracking.

---

77-131: LGTM!

The NewBlockchainBackend function has proper validation, sensible defaults, and good error handling. The network-specific configuration is well-structured.

---

226-240: Excellent implementation of the sync loop!

The random jitter (0.75-1.25x interval) is a thoughtful addition to prevent traffic spikes when multiple wallets sync simultaneously. The error handling appropriately logs failures without crashing the sync loop.

---

413-501: Excellent implementation of spent output tracking!

The caller-controlled locking pattern is correctly implemented:

• SendToAddress holds write lock throughout the entire operation
• GetSendFee uses read lock for concurrent access
• createTransaction operates under the caller's lock protection

This ensures atomicity and prevents double-spending of UTXOs.

---

336-350: LGTM!

The address generation correctly ensures uniqueness by persisting the incremented index immediately after generating the address. This prevents address reuse even in case of crashes.

Reviewing files that changed from the base of the PR and between e37fed2 and beb13e4.

• internal/onchain/liquid-wallet/wallet.go (1 hunks)
• internal/onchain/liquid-wallet/wallet_test.go (1 hunks)

• GitHub Check: CI (ubuntu-latest, 1.24.x)

Reviewing files that changed from the base of the PR and between ae49df8 and 2178897.

• internal/autoswap/chain.go (1 hunks)
• internal/onchain/liquid-wallet/wallet.go (1 hunks)
• internal/onchain/liquid-wallet/wallet_test.go (1 hunks)
• internal/rpcserver/rpcserver_test.go (9 hunks)
• internal/test/test.go (3 hunks)

• internal/autoswap/chain.go

• internal/test/test.go

• GitHub Check: CI (ubuntu-latest, 1.24.x)

internal/rpcserver/rpcserver_test.go (7)

20-21: LGTM: New imports support liquid wallet integration.

The imports for liquid_wallet and wallet packages are appropriate for the PR's objective of integrating liquid wallet functionality and supporting legacy wallet types.

---

34-34: LGTM: Serializers import supports currency parsing.

The serializers import is needed for the new currency parsing functionality used in the direct reverse swap payment tests.

---

1741-1747: LGTM: Wallet creation and credential handling improvements.

The changes properly capture the creation response and validate that both mnemonic and core descriptor are non-empty, which ensures the wallet credentials are properly generated.

---

1752-1753: LGTM: Consistent use of wallet ID and full credentials.

The changes correctly use the created wallet ID for removal and the full credentials object for import, which is more robust than the previous approach.

Also applies to: 1762-1763

---

1265-1266: LGTM: Proper recommendation validation.

The changes correctly check that the recommendation swap is nil, which aligns with the expected behavior after swap execution.

---

1849-1849: LGTM: Optional BroadcastTransaction mock expectation.

The .Maybe() call on the BroadcastTransaction expectation is appropriate since the call is optional depending on the test scenario.

---

3135-3171: LGTM: Comprehensive legacy wallet test coverage.

The new TestLegacyWallet test provides excellent coverage for the legacy wallet functionality:

1. Type verification: Properly checks that a new wallet returns liquid_wallet.Wallet
2. Database manipulation: Correctly marks the wallet as legacy
3. Reinitialization: Properly reinitializes the system to test the change
4. Legacy behavior: Verifies that the wallet now returns wallet.Wallet

The test aligns well with the retrieved learning about liquid wallet implementation and the PR's objective of supporting legacy wallet types.

internal/onchain/liquid-wallet/wallet.go (2)

271-298: Good implementation of auto-consolidation with proper fee estimation.

The auto-consolidation logic now properly uses the fee provider to estimate fees dynamically, which addresses the previous concern about hardcoded fee rates.

---

498-516: Correct implementation of the caller-controlled locking pattern.

The SendToAddress method properly holds the write lock throughout the transaction creation, broadcast, and marking inputs as spent, ensuring atomicity as per the established pattern.

Reviewing files that changed from the base of the PR and between 6eb6965 and da94b11.

• docs/grpc.md (1 hunks)
• internal/rpcserver/router.go (19 hunks)
• internal/rpcserver/rpcserver_test.go (9 hunks)

• docs/grpc.md
• internal/rpcserver/router.go

• GitHub Check: CI (ubuntu-latest, 1.24.x)

internal/rpcserver/rpcserver_test.go (8)

20-21: LGTM! Proper imports for new wallet implementations.

These imports are necessary for the new liquid wallet implementation and legacy wallet support being added in this PR.

---

34-34: LGTM! Required import for serializers functionality.

The serializers import is needed for the enhanced wallet functionality in the test suite.

---

1161-1186: Excellent refactoring of fundedWallet helper function.

The new implementation is more efficient and follows a logical flow:

1. First attempts to get existing wallet by name
2. Only imports if wallet doesn't exist
3. Only funds if wallet balance is zero

This eliminates unnecessary wallet imports and provides better test performance.

---

1741-1762: Improved TestWallet function with better resource management.

The changes properly capture the wallet creation response and use it consistently throughout the test. The credential retrieval with password assertion and using the full credentials object for import are good improvements.

---

1849-1849: Appropriate use of Maybe() for optional mock expectation.

Adding .Maybe() to the BroadcastTransaction mock makes it optional, which is reasonable since the broadcasting behavior may vary depending on test conditions and swap logic changes.

---

1884-1892: Enhanced confirmation logic for different currencies.

The conditional logic properly handles BTC vs other currencies for confirmation requirements. Mining a block for BTC when not zero-conf ensures proper transaction confirmation handling.

---

1265-1266: Correct assertion update for autoswap behavior.

The change from checking length to checking for nil swap aligns with the new autoswap behavior where the swap field can be nil rather than an empty array.

---

3133-3169: Excellent test coverage for legacy wallet functionality.

The new TestLegacyWallet function properly validates the wallet type switching mechanism:

1. Verifies initial liquid wallet implementation
2. Tests database legacy flag modification
3. Confirms wallet type switches to legacy implementation after reinitialization

This comprehensive test ensures the legacy wallet support works correctly.