Expose contract-only RGB asset import

[Jainakin]

Problem

Fresh wallets need a way to register a trusted, network-specific RGB asset before the first transfer. An asset ID is only an identifier; it is not enough to reconstruct or validate the contract.

Changes

• export a wallet-known RGB contract as a serialized standalone contract
• import a validated standalone contract into RGB stock and SQL asset metadata
• return canonical metadata plus an already_imported result
• make import idempotent and repairable after either stock-only or database-only interrupted persistence
• replace the transfer-metadata import panic path with typed validation/network errors

Semantics and safety

• contract import registers public identity and metadata only
• it creates no allocations, transfers no units, and reports a zero asset balance
• the contract is validated against the wallet network and supported schema before mutation
• contracts declaring attachments are rejected because standalone contract payloads do not contain the attachment files
• RGB stock is persisted before the SQL transaction commits, so every interrupted state is recoverable by retry
• storage failures are returned to the caller instead of being hidden behind drop-time persistence
• concurrent duplicate imports serialize through the existing RGB runtime lock

Direct rgb-lib consumers should preserve the library's existing single-writer discipline for heterogeneous wallet mutations. The RLN integration serializes these operations through its unlocked-wallet lifecycle guard.

Validation

Validated at 95332c41fd715939ac6e078ad859d474b1f6fa9b.

• GitHub Actions: build matrix, no-default-feature builds, Electrum/Esplora feature builds, tests, docs, lint, format, and CodeQL are green
• contract import coverage: NIA, CFA, IFA, and UDA
• verifies zero balance and canonical metadata
• verifies idempotent repeat import
• verifies missing-attachment rejection
• verifies stock-only and database-only interrupted-state repair
• verifies concurrent duplicate import serialization

Dependency chain

1. This PR
2. UTEXO-Protocol/rust-lightning#31
3. UTEXO-Protocol/rgb-lightning-node#128

No ownership state can be created by this API; a real RGB transfer is still required before the wallet can hold or spend units.

[github-actions]

🌗 Pull Request Overview

This PR exposes new Rust APIs for exporting and importing RGB asset contracts without transferring allocations. It introduces explicit persistence control in RgbRuntime to support repairable, idempotent contract imports, adds idempotency checks to save_new_asset, and includes comprehensive tests for partial-state repair and concurrent import serialization.

Reviewed Changes
Kimi performed full review on 4 changed files and found 1 issue.

Show a summary per file

File	Description
src/lib.rs	Re-exports RgbContract from rgbstd::containers so it can be used in the new public API signatures
src/utils.rs	Adds persist_on_drop flag to RgbRuntime, plus require_explicit_persistence, persist, and export_contract methods; updates Drop to respect the flag
src/wallet/rust_only.rs	Adds ImportAssetContractResult struct, export_asset_contract, import_asset_contract methods; improves save_new_asset error handling and idempotency
src/wallet/test/rust_only.rs	Adds tests for successful import, missing attachment rejection, partial persistence repair, and concurrent call serialization

---

📋 Review Findings

📄 src/wallet/rust_only.rs

🔵 LOW code-quality: Inconsistent validation error mapping in import_asset_contract

Line 301

import_asset_contract maps every validation error to Error::InvalidConsignment, discarding resolver errors and losing diagnostic detail. In the same PR, save_new_asset was updated to explicitly distinguish InvalidConsignment from ResolverError. The same pattern should be used here for consistency and to preserve error context.

💡 Suggested fix:

Current code:

let valid_contract = contract
    .validate(&DumbResolver, &validation_config)
    .map_err(|_| Error::InvalidConsignment)?;

Improved code:

let valid_contract = match contract.validate(&DumbResolver, &validation_config) {
    Ok(contract) => contract,
    Err(ValidationError::InvalidConsignment(error)) => {
        error!(self.logger(), "Contract is invalid: {}", error);
        return Err(Error::InvalidConsignment);
    }
    Err(ValidationError::ResolverError(error)) => {
        return Err(Error::Network {
            details: error.to_string(),
        });
    }
};

---

📄 src/lib.rs

✅ No issues found. The re-export addition is correct and necessary for the new public API surface.

---

📄 src/utils.rs

✅ No issues found. The explicit persistence control is well-designed:

• persist_on_drop is initialized correctly in load_rgb_runtime
• persist() disables drop-time persistence before storing, preventing double-persistence and ensuring errors propagate
• Drop now safely skips persistence when the caller has opted out

---

📄 src/wallet/test/rust_only.rs

✅ No issues found. The tests are thorough and cover the right edge cases:

• Happy path for all four asset schemas
• Attachment rejection for CFA contracts
• Repair of both "stock-only" and "database-only" partial states
• Concurrent import serialization via std::thread::scope

---

---

_{Powered by Kimi | Model: kimi-k2.6}

[claude]

Review: Expose contract-only RGB asset import

Thanks for the thorough PR description — the reasoning around "persist the stock before committing SQL so every interrupted state is repairable on retry" is the right shape, and the require_explicit_persistence() / persist() pair is a genuine improvement over drop-time .expect("unable to save stock"). The error path where save_new_asset_internal fails is clean: the stock is not persisted, the txn rolls back, and the caller sees the error.

A few things I think need attention before merge, roughly in order of importance.

---

1. import_asset_contract_serializes_concurrent_calls will be flaky

LOCK_FILE_TIMEOUT_SECS is 1.0 in test builds (src/utils.rs:40), and write_rgb_runtime_lockfile retries with 400 ms sleeps — so the losing thread gives up after ~1.2 s with Err(Error::Internal { details: "unreleased lock file" }).

The winning thread holds that lockfile across: load_rgb_runtime (full stock read from disk), export_contract (see 5 below — serializes the whole contract), begin_transaction + get_asset, import_contract, save_new_asset_internal + update_backup_info, runtime.persist() (full stock write to disk), and txn.commit().

Two disk round-trips of the whole stock plus a SQLite commit inside a ~1.2 s budget, on a CI runner also running regtest and other parallel tests, is not a safe bet. When it loses, second.join().unwrap().unwrap() panics and the assert_ne! never even runs.

Suggestion: assert the invariant that actually matters rather than the cross-thread interleaving — after both threads finish, exactly one asset row exists, metadata is canonical, balance is zero — and tolerate Err(Error::Internal { .. }) from the loser (or retry it). As written the test also deliberately violates the single-writer discipline the PR description asks consumers to keep, which makes a failure hard to interpret.

2. The attachment guard has a hole for UDA

extract_attachments UDA arm (src/wallet/online.rs:896-911) collects only token_data.media and token_data.attachments. But extract_asset_data UDA arm (src/wallet/offline.rs:1655-1658) turns contract.contract_terms().media into a Media DB row:

let media = contract.contract_terms().media.map(|a| Media::from_attachment(&a, media_dir));

rgb-lib own UDA issuance always passes None for terms media (src/wallet/offline.rs:577), which is why this asymmetry has been harmless so far. But import_asset_contract accepts a caller-supplied contract from any issuer. A UDA with terms.media = Some(..) and no token media passes your guard, gets a media row inserted, and Metadata.media / list_assets then point at media_dir/<digest> for a file that will never exist — the receive path at online.rs:1360-1411 also skips the download, because by then the asset row exists.

This PR makes extract_attachments load-bearing as a completeness check, so it is worth closing: either add contract.contract_terms().media to the UDA arm, or gate the import on everything extract_asset_data would materialize as a media row. MOCK_CONTRACT_DATA / mock_asset_terms (src/wallet/test/mod.rs:232) gives you a way to construct exactly this case in a test.

3. contract.validate(&DumbResolver, ..) on caller-supplied data

DumbResolver::resolve_witness is unreachable!() (src/utils.rs:743). Everywhere else in production code DumbResolver is only handed to import_contract for contracts the wallet just issued itself (genesis only, nothing to resolve). This is the first production use of it with validate() on an externally supplied consignment.

If a contract consignment produced by Stock::export_contract for an asset that has already been transferred carries bundles whose witnesses need resolving, validation panics rather than returning an error — a hard abort across the FFI bindings, which is exactly the class of thing this PR removes from save_new_asset. Every test here exports immediately after issuance, so the post-transfer shape is untested. Please add a test that exports and imports a contract after at least one transfer of that asset. If witnesses are reachable there, use the blockchain resolver (or a resolver that returns WitnessResolverError instead of panicking).

Two smaller points in the same expression:

• .map_err(|_| Error::InvalidConsignment) throws away the failure detail, while the save_new_asset change in this same PR carefully error!-logs it. Worth logging here too.
• DumbResolver::check_chain_net returns Ok(()) unconditionally, so "the contract is validated against the wallet network" rests entirely on the in-validator chain_net comparison. A negative test (import a testnet contract into a regtest wallet) would pin down that claim.

4. save_new_asset now silently succeeds where it used to error

Asset::Id is string_uniq (migration/src/m20230608_071249_init_db.rs:55) and set_asset is a plain insert, so a repeat save_new_asset for a known asset previously failed loudly on the unique constraint. The new early return turns that into Ok(()).

That is probably the behaviour you want, but it is an unannounced contract change on an existing public API: the doc comment still says "Extract the metadata of a new RGB asset and save the asset into the DB", and neither new branch has a test — not the idempotent success, not the runtime.export_contract(contract_id)? "stock disagrees with the DB" rejection. Given CLAUDE.md "keep changes minimal and scoped to the PR goal" and "prefer explicit errors over silent fallback", I would either split this into its own commit with tests and updated docs, or keep the error. The ? on export_contract also surfaces as an opaque Error::Internal, where a typed error would let callers branch.

5. export_contract(..).is_ok() is an expensive existence check

Both import_asset_contract and the new save_new_asset guard build a full contract consignment — genesis plus every bundle — purely to compute a boolean. RgbRuntime::genesis(contract_id) (src/utils.rs:830) already exists and goes straight to the stash provider:

let contract_in_stock = runtime.genesis(contract_id).is_ok();

6. Critical section is longer than needed, and repeat imports rewrite the whole stock

The attachment rejection depends only on valid_contract, asset_schema and asset_in_database. Hoisting the contract-only part above self.rgb_runtime()? shortens the lock hold (which also helps 1). As written, a rejected import acquires the lockfile, opens a transaction, and then on return Err drops the runtime with persist_on_drop still true — so Drop runs a full stock.store() for a call that changed nothing, on an .expect() panic path.

The same applies to the already_imported early return: it drop(runtime)s with persist_on_drop == true, so every repeat import rewrites the entire stock to disk. Your own repeat-import assertions exercise this. A runtime.require_explicit_persistence() before those two returns (or restructuring so the read-only checks happen before the runtime is taken) would avoid it.

7. Minor

• No info!(self.logger(), ...) start/end lines on either new public method, unlike every neighbouring method in rust_only.rs.
• export_asset_contract returns Error::Internal for an unknown contract. Checking the DB first would let it return Error::AssetNotFound, which is what callers can actually match on. (The ContractId::from_str -> Error::Internal mapping is consistent with offline.rs:2776, so that part is fine.)
• import_asset_contract is feature-gated on electrum/esplora but ImportAssetContractResult and export_asset_contract are not — so a no-default-features build exports a public result type nothing can produce. Worth gating the struct too.

8. Design question

Contracts declaring attachments are rejected, and no companion "supply and verify the files" flow ships here. That means any NIA/CFA/IFA issued with a media file, and any UDA with token media, is simply not importable — and the doc comment points at a flow that does not exist yet. If the RLN use case involves media-bearing assets, the payload likely needs to carry the attachment bytes alongside the contract. Worth stating explicitly in the doc comment which asset shapes are in scope today.

---

Nothing here is structural — the persistence ordering is sound and the repair semantics are well thought through. Items 1 (flaky test), 2 (UDA media hole) and 3 (panicking resolver on caller-supplied input) are the ones I would want resolved before merge.