feat(contracts): per-type ContractScope (on-chain/off-chain)

[Kukks]

What

Makes a contract's layer — on-chain (boarding UTXOs) vs off-chain (VTXOs) — a first-class property instead of scattered Type == "Boarding" checks.

• [Flags] ContractScope { Onchain = 1, Offchain = 2 } (Both = Onchain | Offchain).
• ArkContract.DefaultScope (abstract — every contract type must declare its scope, so a new type can't silently inherit the wrong default). Boarding ⇒ Onchain; Payment, HashLockPayment, Delegate, VHTLC, Note, Unknown, Generic ⇒ Offchain.
• Resolved scope persisted on ArkContractEntity.Scope / ArkWalletContractEntity (indexed). ToEntity(scopeOverride:) is the per-instance escape hatch (Scope = scopeOverride ?? DefaultScope); nothing sets it yet (YAGNI).
• IContractStorage.GetContracts(scope:) filters via the SQL-translatable bitwise predicate (Scope & s) == s — not HasFlag (EF Core doesn't translate it).
• Consumers migrated off the boarding-type check: BoardingUtxoPollService, BoardingUtxoSyncService, OnchainSweepService → scope: Onchain; WalletRecoveryService → off-chain bitwise filter.

Tests (TDD)

• Per-type DefaultScope, ToEntity override resolution, flags semantics.
• A SQLite-backed GetContracts(scope:) test (real SQLite, not InMemory — InMemory client-evaluates and would hide a non-SQL-translatable query). Verifies the bitwise predicate translates and that a dual-scope contract matches both filters.
• dotnet build NArk.sln clean; dotnet test NArk.Tests → 437 passing.

Schema / migration note

NArk.Storage.EfCore is provider-agnostic with no migrations history (consumers own their schema via EnsureCreated or their own migrations). The Scope column therefore ships via entity configuration (ArkWalletContractEntity.Configure, indexed). Consumers that manage their schema with EF migrations should add a migration creating the column (default Offchain) and backfilling existing boarding rows to Onchain — documented in the README.

Notes

• GenericArkContract (a concrete : ArkContract subclass) also implements DefaultScope ⇒ Offchain (mandatory since the property is abstract); preserves its prior implicit behavior.
• README gains a 'Contract scope' section.

[arkanaai]

Arkana Code Review — feat(contracts): per-type ContractScope

Verdict: Approve ✅

Clean, well-designed abstraction. The abstract DefaultScope forcing compile-time decisions on every contract subclass is the right call — no contract can silently inherit wrong layer semantics. Tests are solid, especially the real-SQLite storage test that catches the HasFlag vs bitwise translation trap. The 4 internal consumer migrations are correct.

---

Findings

1. ArkContractEntity.Scope defaults to (ContractScope)0 — invisible ghost rows (Low severity)

NArk.Abstractions/Contracts/ArkContractEntity.cs — Scope is { get; init; } with no explicit default. default(ContractScope) = 0 = no flags set. Any ArkContractEntity constructed without explicitly setting Scope will be invisible to all scope filters ((0 & Onchain) == Onchain → false, (0 & Offchain) == Offchain → false).

The ArkWalletContractEntity correctly defaults to Offchain, and ArkContract.ToEntity() always sets Scope — so the normal code path is safe. But anyone constructing ArkContractEntity directly (test code, manual recovery, deserialization from external sources) hits this trap silently.

Suggestion: Default the record property to Offchain to match ArkWalletContractEntity:

public ContractScope Scope { get; init; } = ContractScope.Offchain;

Or define None = 0 on the enum and add a guard in SaveContract that rejects scope == 0. Either way, make the zero-value pit-of-success not a pit-of-silence.

2. No Both named constant (Nit)

NArk.Abstractions/Contracts/ContractScope.cs — The PR description mentions Both = Onchain | Offchain but the enum doesn't define it. Tests use ContractScope.Onchain | ContractScope.Offchain directly. A named Both = 3 member would improve readability at call sites and in debugger output.

3. WalletRecoveryService semantic shift is correct but worth a comment

NArk.Swaps/Recovery/WalletRecoveryService.cs:84 — Changed from c.Type != ArkBoardingContract.ContractType to (c.Scope & ContractScope.Offchain) != 0.

These produce identical results for all current contract types. For a hypothetical future on-chain non-boarding contract, the old code would incorrectly include it in off-chain recovery; the new code correctly excludes it. The new semantics are strictly better. 👍

4. Downstream impact: btcpay-arkade needs migration work

btcpay-arkade has ~5 client-side Type == "Boarding" / Type != "Boarding" filters and ~4 contractTypes: [ArkBoardingContract.ContractType] call sites. The interface change is backward-compatible (optional param), so nothing breaks on submodule bump, but:

• btcpay-arkade must add an EF migration to create the Scope column (default Offchain) and backfill boarding rows to Onchain
• Its client-side type checks should migrate to scope: for efficiency

This is documented in the PR description and README — just flagging for tracking.

5. Existing contractTypes: callers inside the SDK not migrated (Intentional/OK)

BoltzSwapProvider and SwapsManagementService filter by VHTLCContract.ContractType post-query. These are type-specific filters (not layer-specific), so they correctly remain on contractTypes: / post-query .Type == checks. No action needed.

---

Protocol Safety Assessment

This PR does not touch VTXO handling, transaction signing, forfeit paths, round lifecycle, connector trees, or exit paths. It changes how contracts are categorized and queried, not how they execute. The scope assignments are correct:

• ArkBoardingContract → Onchain ✅ (boarding UTXOs live on L1)
• All others → Offchain ✅ (VTXOs live in the Ark virtual layer)

A misclassification would cause a contract to be missed by sync/sweep/recovery services (funds stuck, not lost), and the abstract property ensures no new contract type can skip the decision.

No protocol-critical concerns. No human sign-off required beyond normal review.

---

Summary

Category	Status
Correctness	✅ All scope assignments match layer semantics
Tests	✅ Unit + real-SQLite integration, flags semantics covered
API design	✅ Backward-compatible, compile-time enforcement via abstract
Cross-repo	⚠️ btcpay-arkade needs EF migration + optional consumer migration
Protocol safety	✅ No protocol-critical code touched

Well-crafted PR. The ArkContractEntity.Scope default-zero footgun (finding #1) is the only thing I'd want addressed before merge — everything else is nit-level.

🤖 Reviewed by Arkana

[d4rp4t]

rebased onto latest master