Skip to content

p2p: Misc TODOs and graceful handling of errors #2195

@iostat

Description

@iostat

p2p_channels_signaling_discovery_reducer.rs, lines 342-354

If OfferSendSuccess is not enabled at dispatch time, state becomes
inconsistent. The dialer believes the offer was sent, but the outgoing
connection state machine hasn't advanced.

// TODO(binier): this action might not be enabled, in
// which case we sshould be rejecting discovered peer.
dispatcher.push(P2pConnectionOutgoingAction::OfferSendSuccess {
    peer_id: target_public_key.peer_id(),
});

p2p_channels_effectful_effects.rs, line 121

bug_condition! logs a warning (and doesn't panic unless a specific env is set) but does not propagate an error or transition the state machine. If encryption fails, the answer is silently dropped. The
dialer waits forever (compounded by #2190). The assumption that "decryption succeeded so encryption must succeed" is fragile.

Err(_) => bug_condition!(
    "Failed to encrypt webrtc answer. \
     Shouldn't happen since we managed to decrypt sent offer."
),

channels/signaling/mod.rs, line 54

Peer selection shuffle is seeded from the redux timestamp. This is deterministic and reproducible. An attacker who knows (or can influence) the timestamp can predict which peer will be selected as a relay target. Should use a better PRNG seed source.

let mut rng = rand::rngs::StdRng::seed_from_u64(time.into());

p2p_channels_signaling_discovery_actions.rs, line 136

// Allow one discovery request per minute.
// TODO(binier): make configurable
now.checked_sub(*time)
    .is_some_and(|dur| dur.as_secs() >= 60)

The one-minute cooldown between discovery requests is hardcoded with a TODO to make it configurable. In production, different deployment scenarios (high-churn networks, bootstrap servers) may need different values.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

Projects

Status

Todo

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions