Skip to content

Add silent payments scan label support - #11

Merged
praveenperera merged 1 commit into
masterfrom
spscan
Jul 8, 2026
Merged

Add silent payments scan label support#11
praveenperera merged 1 commit into
masterfrom
spscan

Conversation

@praveenperera

@praveenperera praveenperera commented Jul 8, 2026

Copy link
Copy Markdown
Member

Support BIP329 spscan records in Label and LabelRef, and add import options to ignore unsupported record types.

This keeps existing parsing strictness configurable per callsite while preserving compatibility.

Summary by CodeRabbit

  • New Features
    • Added support for silent payments scan labels in imports and display.
    • Added new import options to handle unsupported record types by skipping them.
  • Bug Fixes
    • Existing label parsing now better tolerates extra fields in supported records.
    • Unknown record types can now be ignored during file and text import when enabled.
  • Chores
    • Updated the changelog and test data to include the new label type.

Support BIP329 spscan records in Label and LabelRef, and add

import options to ignore unsupported record types. This keeps

existing parsing strictness configurable per callsite while preserving

compatibility.
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a new Label::SilentPaymentsScan variant backed by SilentPaymentsScanRecord, wires it through From, label(), ref_(), and Display, and introduces LabelParseOptions with ignore_unknown_types support across try_from_str, try_from_str_with_metadata, and try_from_file parsing APIs, plus updated tests and changelog.

Changes

Silent Payments Scan Label & Parse Options

Layer / File(s) Summary
Core data model additions
src/lib.rs
Adds LabelParseOptions struct with ignore_unknown_types builder, Label::SilentPaymentsScan and LabelRef::SilentPaymentsScan variants, Display support, and SilentPaymentsScanRecord struct.
From conversion for new record type
src/from.rs
Reformats crate imports and adds From<SilentPaymentsScanRecord> for Label.
Option-aware label parsing implementation
src/label.rs
Extends try_from_str/_with_metadata/_from_file into option-aware variants, adds parse_label_line/parse_label_line_with_metadata helpers that skip unknown record types, extends ParsedLabelLine tagging/conversion, and updates label()/ref_() for spscan records.
Tests, changelog, and test vector updates
src/label.rs, CHANGELOG.md, tests/data/test_vector.jsonl
Updates deserialization tests/assertions for the new spscan record, adds tests for ignore_unknown_types, updates the changelog, and adds a new spscan entry to the test vector.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant Labels
  participant parse_label_line
  participant LabelType

  Caller->>Labels: try_from_str_with_options(labels, options)
  loop for each line
    Labels->>parse_label_line: parse_label_line(line, options)
    alt ignore_unknown_types enabled
      parse_label_line->>LabelType: parse type field only
      LabelType-->>parse_label_line: type value
      parse_label_line-->>Labels: None (skip) if unknown type
    end
    parse_label_line-->>Labels: Some(Label) if known type
  end
  Labels-->>Caller: Labels collection
Loading

Possibly related PRs

  • bitcoinppl/bip329#9: Both PRs modify the metadata-preserving label JSONL parsing path in src/label.rs, including try_from_str_with_metadata/...with_options and ParsedLabelLine deserialization helpers.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding silent payments scan label support.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch spscan

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds support for silent payments scan labels and configurable import strictness. The main changes are:

  • New spscan record support in Label and LabelRef.
  • New SilentPaymentsScanRecord type and conversion into Label.
  • Import options for skipping unsupported record types.
  • Metadata-aware parsing support for skipped unknown records.
  • Test vector and unit test coverage for the new paths.

Confidence Score: 4/5

The changed flow looks mergeable after a small cleanup to spscan ref validation.

  • Existing strict parsing behavior is preserved by default.
  • Unknown record skipping is scoped to unsupported string type values.
  • The new spscan record can carry malformed scan key strings into parsed labels and exports.

src/lib.rs

Important Files Changed

Filename Overview
src/lib.rs Adds the new parse options, spscan record type, and enum variants; the new scan key ref is accepted as an opaque string.
src/label.rs Adds option-aware JSONL parsing, unknown-type filtering, metadata parsing support, and helper handling for spscan labels.
src/from.rs Adds the direct conversion from SilentPaymentsScanRecord into Label.
tests/data/test_vector.jsonl Adds a valid spscan record to the shared test vector.
CHANGELOG.md Documents the new spscan support, import option, and enum expansion.

Reviews (1): Last reviewed commit: "Add silent payments scan label support" | Re-trigger Greptile

Comment thread src/lib.rs
#[derive(Clone, Debug, Serialize, Deserialize, Hash, PartialEq, Eq, PartialOrd, Ord)]
pub struct SilentPaymentsScanRecord {
#[serde(rename = "ref")]
pub ref_: String,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Invalid Scan Keys Parse

When an import contains {"type":"spscan","ref":"not-a-valid-scan-key"}, serde accepts it as a normal SilentPaymentsScanRecord because ref_ is just a String. That invalid ref then reaches Label::ref_(), map keys, and export output as if it were a valid silent payments scan key expression, so downstream wallets only see the failure later when they try to decode or use it.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/label.rs (1)

219-224: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep is_known_label_type in sync with enum variants

The hardcoded type list in is_known_label_type must manually stay in sync with the #[serde(rename = "...")] attributes on both Label and ParsedLabelLine. If a new record type is added to either enum but omitted here, ignore_unknown_types would silently skip records of that type during imports — a subtle data-loss bug.

Consider adding a test that iterates all known type strings and verifies they deserialize successfully as both Label and ParsedLabelLine, or at minimum a comment near both sites flagging the coupling.

🧪 Suggested sync test
#[test]
fn known_label_types_match_enum_variants() {
    let known_types = ["tx", "addr", "pubkey", "input", "output", "xpub", "spscan"];
    for ty in known_types {
        assert!(is_known_label_type(ty), "{ty} should be known");
        // Verify each known type round-trips through the enum's serde rename
        let minimal = format!(r#"{{"type":"{ty}"}}"#);
        let _: Result<LabelType, _> = serde_json::from_str(&minimal);
    }
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/label.rs` around lines 219 - 224, Keep is_known_label_type synchronized
with the serde-renamed variants on Label and ParsedLabelLine: update the
hardcoded matches list whenever either enum gains a new record type, and add a
test around is_known_label_type/serde deserialization to verify every known type
string still parses for both enums so ignore_unknown_types cannot silently drop
new records.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/label.rs`:
- Around line 219-224: Keep is_known_label_type synchronized with the
serde-renamed variants on Label and ParsedLabelLine: update the hardcoded
matches list whenever either enum gains a new record type, and add a test around
is_known_label_type/serde deserialization to verify every known type string
still parses for both enums so ignore_unknown_types cannot silently drop new
records.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 56efe526-d0a0-4953-bf2d-7aa7b995f0bb

📥 Commits

Reviewing files that changed from the base of the PR and between 05e4880 and 15591dd.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • src/from.rs
  • src/label.rs
  • src/lib.rs
  • tests/data/test_vector.jsonl

@praveenperera
praveenperera merged commit 3d596b7 into master Jul 8, 2026
10 checks passed
@praveenperera
praveenperera deleted the spscan branch July 8, 2026 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant