ngit-grasp/1bb0-event-signature-verification.md

# Event Signature Verification

**ID:** 1bb0

## Issue

We need to verify that event signatures are being checked for every event we store or issue over websockets. Currently unclear if rust-nostr is doing this automatically or if we need explicit verification.

## Requirements

1. **Verify signatures for all events** - Both stored events and events sent over websockets
2. **Check signatures before expensive operations** - Validate signatures before database queries for efficiency
3. **Avoid duplicate verification** - Don't verify the same event multiple times
4. **NaughtyList integration** - If an event with invalid signature comes via sync, add the source Relay to the NaughtyList

## Investigation Questions

- [x] Does rust-nostr automatically verify signatures when parsing events?
- [x] Where do events enter the system? (websocket, sync, etc.)
- [x] Where should verification happen in the event flow?
- [x] What is the current NaughtyList implementation status?

## Investigation Findings

### Event Entry Points

Events enter ngit-grasp through two paths:

1. **WebSocket (User-submitted)** - Handled by `nostr-relay-builder`'s `LocalRelay`
2. **Sync (From other relays)** - Handled by `nostr-sdk`'s `RelayPool` via `RelayConnection`

### Current Signature Verification Status

#### Path 1: WebSocket Events (LocalRelay) - VERIFIED

`nostr-relay-builder/src/local/inner.rs` line ~340-380 in `handle_client_msg`:

```rust
if !event.verify_id() {
    return send_msg(..., "invalid event ID", ...);
}

// ... check expired, POW ...

if !event.verify_signature() {
    return send_msg(..., "invalid event signature", ...);
}

// THEN check write policy (our Nip34WritePolicy)
if let Some(policy) = self.write_policy.as_ref() {
    if let WritePolicyResult::Reject { ... } = policy.admit_event(&event, addr).await {
        // ...
    }
}
```

**Validation order for WebSocket events:**
1. Rate limiting
2. ID verification (`verify_id()`)
3. Expiration check
4. POW check (if configured)
5. **Signature verification (`verify_signature()`)** - BEFORE any DB queries
6. NIP42 auth check
7. Protected event check
8. Database duplicate check
9. Mode check
10. **WritePolicy** (our `Nip34WritePolicy.admit_event()`)

**Conclusion:** WebSocket events ARE verified before reaching our code. Order is correct.

#### Path 2: Sync Events (RelayPool) - VERIFIED

`nostr-relay-pool/src/relay/inner.rs` in `handle_event_msg`:

```rust
// Check if the event was already verified.
// This is useful if someone continues to send the same invalid event:
// since invalid events aren't stored in the database,
// skipping this check would result in the re-verification of the event.
// This is important since event signature verification is a heavy job!
if !self.state.verified(&event.id)? {
    event.verify()?;  // This calls verify_id() AND verify_signature()
}

// Save into the database
match self.state.database().save_event(&event).await? { ... }
```

**Important:** nostr-sdk has a "verified events" cache to avoid re-verifying the same event.

**Validation order for Sync events:**
1. Event size check
2. Tags limit check
3. Subscription filter matching (if configured)
4. Expiration check
5. Admission policy check
6. Database status check (already saved/deleted?)
7. **Signature verification (event.verify())** - uses cache to avoid duplicates
8. Save to database

**Conclusion:** Sync events ARE verified by nostr-sdk before being stored.

### NaughtyList Status

The `NaughtyListTracker` exists at `src/sync/naughty_list.rs` (lines 27-557) and is used for:
- DNS lookup failures
- TLS certificate issues  
- Protocol errors

**Categories defined:**
- `DnsLookupFailed` - Domain doesn't resolve
- `TlsCertificateInvalid` - SSL/TLS issues
- `ProtocolError` - WebSocket protocol violations

**NOT currently used for:** Invalid signatures from relays during sync.

### Gap Analysis

| Entry Point | Sig Verified? | Correct Order? | By Whom | NaughtyList on Invalid? |
|-------------|---------------|----------------|---------|-------------------------|
| WebSocket   | YES           | YES            | nostr-relay-builder | N/A (direct client) |
| Sync        | YES           | YES            | nostr-sdk relay pool | **NO - GAP** |

## Recommended Actions

### 1. No Code Changes Needed for Verification

Both entry points already verify signatures correctly:
- WebSocket: `nostr-relay-builder` verifies before calling our `WritePolicy`
- Sync: `nostr-sdk` verifies before storing and has dedup cache

### 2. Add NaughtyList for Invalid Signatures (Enhancement)

Currently, if a relay sends us an event with an invalid signature during sync, we simply reject the event but don't track the relay as "naughty". This could be valuable for:
- Detecting malicious/buggy relays
- Avoiding wasted bandwidth re-syncing with bad relays
- Metrics/monitoring

**Implementation approach:**
1. Add `InvalidEventSignature` to `NaughtyCategory` enum
2. Hook into sync error handling to detect signature verification failures
3. Record the source relay when signature verification fails

**Challenge:** nostr-sdk's relay pool handles verification internally. We'd need to either:
- Check for verification errors in the `RelayNotification` stream
- Or wrap events with pre-verification before they reach the pool (not recommended - duplicates work)

### 3. Optional: Add Explicit Verification in process_event_static (Defense in Depth)

Currently `src/sync/mod.rs:2389` `process_event_static()` trusts that events are pre-verified. While nostr-sdk does verify, we could add explicit verification as defense-in-depth:

```rust
// At start of process_event_static:
if event.verify().is_err() {
    // Log and add to naughty list
    return ProcessResult::Rejected;
}
```

**Trade-off:** This would re-verify events that nostr-sdk already verified (wasted CPU), unless nostr-sdk exposes its verified-events cache.

## Performance Issue: Verification Cache Not Shared Across Sync Connections

### Finding

nostr-sdk's verification cache is **per-RelayPool**, not global. The cache lives in `SharedState`:

```rust
// nostr-relay-pool/src/shared.rs
pub struct SharedState {
    // ...
    verification_cache: Arc<Mutex<LruCache<u64, ()>>>,  // 128k entries max
    // ...
}
```

When a `RelayPool` is created, it creates one `SharedState` and shares it (via Arc clone) with all relays in that pool. This means relays **within the same pool** share the verification cache.

### The Problem in ngit-grasp

In `src/sync/relay_connection.rs:112-114`:

```rust
pub fn new(url: String, keys: Keys) -> Self {
    let client = Client::new(keys);  // Creates a NEW Client for EACH relay connection!
    // ...
}
```

**ngit-grasp creates a separate `Client` instance for each `RelayConnection`**. Each `Client` has its own `RelayPool`, which has its own `SharedState` with its own verification cache.

### Result

**If the same event is received from multiple relays during sync, it WILL be verified multiple times** because:

1. Each `RelayConnection` uses a separate `Client`
2. Each `Client` has its own isolated `RelayPool` and `SharedState`
3. The verification caches are NOT shared across `RelayConnection` instances

### Impact

- Wasted CPU cycles on redundant signature verification (expensive cryptographic operation)
- The more relays we sync from, the worse the duplication
- Popular events (seen on many relays) get verified N times instead of once

### Potential Solutions (Not Yet Researched)

**Open question:** How trivial is it to share the verification cache?

Options to investigate:
1. **Single shared `Client`/`RelayPool`** - Use one pool for all sync operations instead of per-relay clients
2. **Shared `SharedState`** - Would require upstream changes or workarounds to inject a shared state into multiple clients
3. **Application-level verification cache** - Implement our own cache at the ngit-grasp level, verify events before they reach the clients (but this duplicates the verification work on first-seen events)
4. **Upstream contribution** - Add ability to share verification cache across multiple `Client` instances

## Conclusion

**Good news:** Signature verification IS happening correctly for both entry points via rust-nostr libraries.

**Performance issue:** Verification cache is not shared across sync connections, causing redundant signature verification for events seen from multiple relays.

**Potential enhancement:** Add NaughtyList tracking for relays that send invalid signatures during sync. This requires monitoring for verification errors from nostr-sdk, which may require upstream changes or a custom wrapper.

## Status

**Created:** 2026-01-10
**Status:** Investigation Complete
**Finding:** No immediate action required - signatures are verified by upstream libraries
**Performance:** Verification cache isolation causes redundant verification across sync connections (needs further research on fix complexity)
**Enhancement:** Consider NaughtyList integration for invalid signatures from sync relays