lightninglabs / lightninglabs/taproot-assets

[feature]: Retire `proof.Archiver.ImportProofs` in favour of the verified-import pattern

Open
#1,992 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement proofs
Dominant language
Go
Stars
525
Forks
150
Avg merge
2d 15h
Merged PRs (30d)
31

Description

The `proof.Archiver` interface currently requires an `ImportProofs` method that accepts a `VerifierCtx` argument, implying verification will occur before storage. In practice, most implementations ignore this argument:

- **`proof.FileArchiver.ImportProofs`** (`proof/archive.go:738`) — ignores the `VerifierCtx` entirely (parameter is `_`), writing proofs directly to disk with no verification.
- **`tapdb.AssetStore.ImportProofs`** (`tapdb/assets_store.go:2049`) — also ignores the `VerifierCtx`, inserting proofs into the database without verification.
- **`proof.MultiArchiver.ImportProofs`** (`proof/archive.go:1004`) — the only implementation that actually verifies proofs before delegating to its backends.

This inconsistency is misleading. Callers pass a `VerifierCtx` believing verification will happen, but depending on the archiver wired in, it may be silently skipped.

Meanwhile, a **verified-import pattern** has been introduced: callers explicitly verify proofs via `proof.VerifyAnnotatedProofs()` to obtain `VerifiedAnnotatedProof` values, then call `ImportVerifiedProofs()` which stores without re-verification. `ChainPorter.storeProofs` already uses this pattern. This is clearer and safer because verification is explicit at the call site rather than hidden (or not) inside the archiver.

### Proposal

1. **Remove `ImportProofs` from the `proof.Archiver` interface** (`proof/archive.go:198`).

2. **Add `ImportVerifiedProofs` to the `proof.Archiver` interface** (or a new composed interface) so all archivers accept only pre-verified proofs.

3. **Remove proof verification from `MultiArchiver.ImportProofs`** and replace the method with `ImportVerifiedProofs`. The `MultiArchiver` should no longer hold a `proofVerifier` field; verification is the caller's responsibility.

4. **Add `ImportVerifiedProofs` to `tapdb.AssetStore`**, following the same pattern as `FileArchiver.ImportVerifiedProofs` — unwrap the `VerifiedAnnotatedProof` values and delegate to the internal storage logic.

5. **Migrate all call sites** from `ImportProofs` to the verified-import pattern (`proof.VerifyAnnotatedProofs` → `ImportVerifiedProofs`):

| Call site | File | Line |
|-----------|------|------|
| `rpcServer.ImportProof` (dev RPC) | `rpcserver.go` | 2339 |
| `rpcServer.ProveFetchedAssetProvenance` | `rpcserver.go` | 10659 |
| `Custodian` (address proof import) | `tapgarden/custodian.go` | 934 |
| `Custodian` (non-interactive proof import) | `tapgarden/custodian.go` | 1567 |
| `ReOrgWatcher` (updated proof re-import) | `tapgarden/re-org_watcher.go` | 633 |
| `BatchCaretaker` (minting proof storage) | `tapgarden/caretaker.go` | 1219 |
| `ChainPlanter` (updated proof re-import) | `tapgarden/planter.go` | 3121 |
| `AuxSweeper` (funding proof import) | `tapchannel/aux_sweeper.go` | 1526 |

6. **Update tests** that call `ImportProofs` directly (in `proof/archive_test.go`, `tapdb/assets_store_test.go`, `tapdb/sqlutils_test.go`) to use the new API.

7. **Remove the `proofVerifier` field from `MultiArchiver`** and simplify its constructor since verification is no longer its concern.

### Notes

- The `ChainPorter` already follows the verified-import pattern (`chain_porter.go:528-539`, `chain_porter.go:620-635`) and uses the `VerifiedProofImporter` interface, so it requires no changes.
- Each migrated call site should call `proof.VerifyAnnotatedProofs(ctx, vCtx, ...)` before import. Since every existing call site already constructs and passes a `VerifierCtx`, the verification context is readily available.
- This change makes it impossible to accidentally import unverified proofs — the type system enforces that `ImportVerifiedProofs` only accepts values produced by the verification step.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the proof.Archiver interface and implementations in proof/archive.go, then compare FileArchiver.ImportVerifiedProofs with the verified-import flow in chain_porter.go. Migrate the listed rpcserver.go, tapgarden, and tapchannel call sites, add the tapdb.AssetStore method, and update tests in proof/archive_test.go, tapdb/assets_store_test.go, and tapdb/sqlutils_test.go. Done means ImportProofs and MultiArchiver verification are removed and the affected tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.