Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/mutation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ jobs:

echo "Running mutation testing for changed files:"
echo "$changed_rs"
cargo mutants $file_args
make mutation-test ARGS="$file_args"

- name: Upload mutation report
uses: actions/upload-artifact@v4
Expand Down Expand Up @@ -195,7 +195,7 @@ jobs:
tool: cargo-mutants

- name: Run full mutation testing
run: cargo mutants
run: make mutation-test
# Note: Do NOT fail on low score initially (report only mode)
continue-on-error: true

Expand Down
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,7 @@ docker-build-startos:
cd docker && \
docker compose build mostro-startos

mutation-test:
@set -o pipefail; \
CARGO_MUTANTS_JOBS=2 MOSTRO_TEST_LN_PORT=18080 cargo mutants $(ARGS)
Comment on lines +69 to +71

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -eu

grep -nE '^[[:space:]]*(SHELL|\.SHELL)' Makefile || true
make -n mutation-test
/bin/sh -c 'set -o pipefail'
cargo mutants --help | grep -iE 'job|parallel' || true

Repository: MostroP2P/mostro

Length of output: 99


🏁 Script executed:

#!/usr/bin/env bash
set -eu

printf 'Makefile shell/config near mutation-test:\n'
grep -nE '^[[:space:]]*(SHELL|\.SHELL|mutation-test|CARGO_MUTANTS|MOSTRO_TEST_LN_PORT|Cargo\.toml|\.cargo/config)' Makefile Cargo.toml -r . 2>/dev/null || true

printf '\nMakefile lines 55-78 if present:\n'
sed -n '55,78p' Makefile 2>/dev/null || true

printf '\nCargo mutants config references:\n'
rg -n "mutants|CARGO_MUTANTS|MOSTRO_TEST_LN_PORT|18080" Makefile Cargo.toml src .cargo 2>/dev/null || true

printf '\nShell pipefail behavior:\n'
bash -c '/bin/sh -c "set -o pipefail; echo ok"' 2>/dev/null && echo sh_pipefail=works || echo sh_pipefail=failed_exit=$?
bash -c '/bin/bash -c "set -o pipefail; echo ok"' && echo bash_pipefail=works || echo bash_pipefail=failed_exit=$?

Repository: MostroP2P/mostro

Length of output: 1921


Make the mutation target portable and actually configurable.

Make may run this recipe with /bin/sh unless SHELL := /bin/bash is set earlier; Ubuntu’s default shell rejects set -o pipefail, so remove it unless another pipeline is added. This target also overwrites MOSTRO_TEST_LN_PORT, preventing callers from choosing a different port. With parallel mutation workers, either guard 18080 concurrency or use per-worker ports/serialized port-bound tests.

🧰 Tools
🪛 checkmake (0.3.2)

[warning] 69-69: Required target "all" is missing from the Makefile.

(minphony)


[warning] 69-69: Required target "clean" is missing from the Makefile.

(minphony)


[warning] 69-69: Required target "test" is missing from the Makefile.

(minphony)

🤖 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 `@Makefile` around lines 69 - 71, Update the mutation-test target to avoid the
Bash-only set -o pipefail under Make’s default shell, and preserve caller
configuration by not unconditionally overwriting MOSTRO_TEST_LN_PORT. Also
prevent parallel mutation workers from sharing the same fixed port by guarding
concurrency or assigning distinct worker ports while retaining configurable
overrides.


240 changes: 238 additions & 2 deletions src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,24 @@ async fn handle_message_action(
}
}

/// True when a rumor's `created_at` falls outside the 10s replay window.
/// Pulled out of `accept_event` so the boundary can be hit directly instead
/// of only through a live wrapped event.
fn is_stale(created_at: Timestamp, since_time: u64) -> bool {
created_at.as_secs() < since_time
}

/// True when the inner rumor is missing a required signature. Full-privacy
/// clients reuse the trade key as identity and send unsigned rumors; any
/// other identity/sender split must carry a valid inner signature.
fn missing_inner_signature(
identity: PublicKey,
sender: PublicKey,
signature: Option<Signature>,
) -> bool {
identity != sender && signature.is_none()
}

/// Decode and fully validate one relay event into a dispatchable
/// `(action, message, unwrapped)` triple, or `None` if it must be skipped
/// (failed PoW, wrong kind, spam-gate drop, decrypt failure, stale, missing
Expand Down Expand Up @@ -375,7 +393,7 @@ async fn accept_event(
.checked_sub_signed(chrono::Duration::seconds(10))
.unwrap()
.timestamp() as u64;
if unwrapped.created_at.as_secs() < since_time {
if is_stale(unwrapped.created_at, since_time) {
return None;
}
let message = unwrapped.message.clone();
Expand All @@ -384,7 +402,7 @@ async fn accept_event(
// unsigned rumors. Any other shape must carry a valid inner
// signature — unwrap_message already verified it, so if identity
// and sender differ here without a signature we bail out.
if unwrapped.identity != unwrapped.sender && unwrapped.signature.is_none() {
if missing_inner_signature(unwrapped.identity, unwrapped.sender, unwrapped.signature) {
tracing::warn!(
"Missing inner signature: identity {} differs from trade key {}",
unwrapped.identity,
Expand Down Expand Up @@ -610,6 +628,46 @@ mod tests {
}
}

#[test]
fn is_stale_rejects_events_older_than_the_cutoff() {
assert!(is_stale(Timestamp::from(99), 100));
}

#[test]
fn is_stale_accepts_events_exactly_at_the_cutoff() {
assert!(!is_stale(Timestamp::from(100), 100));
}

#[test]
fn is_stale_accepts_events_newer_than_the_cutoff() {
assert!(!is_stale(Timestamp::from(101), 100));
}

#[test]
fn missing_inner_signature_true_when_identity_and_sender_differ_unsigned() {
let identity = create_test_keys().public_key();
let sender = create_test_keys().public_key();
assert!(missing_inner_signature(identity, sender, None));
}

#[test]
fn missing_inner_signature_false_when_identity_equals_sender_unsigned() {
let same = create_test_keys().public_key();
assert!(!missing_inner_signature(same, same, None));
}

#[test]
fn missing_inner_signature_false_when_signed_even_if_identity_differs() {
let identity = create_test_keys().public_key();
let sender_keys = create_test_keys();
let sig = sender_keys.sign_schnorr(&nostr_sdk::secp256k1::Message::from_digest([7u8; 32]));
assert!(!missing_inner_signature(
identity,
sender_keys.public_key(),
Some(sig)
));
}

#[test]
fn test_warning_msg_all_error_types() {
let action = Action::NewOrder;
Expand Down Expand Up @@ -866,6 +924,184 @@ mod tests {
}
}

mod accept_event_tests {
use super::*;
use crate::app::context::test_utils::{test_settings, TestContextBuilder};
use mostro_core::prelude::*;
use sqlx::SqlitePool;
use std::sync::Arc;

async fn create_test_ctx() -> AppContext {
let pool = Arc::new(SqlitePool::connect(":memory:").await.unwrap());
TestContextBuilder::new()
.with_pool(pool)
.with_settings(test_settings())
.build()
}

#[allow(deprecated)] // Transport::GiftWrap: still the tested default; see #786.
async fn wrap_test_order(
identity_keys: &Keys,
trade_keys: &Keys,
receiver: PublicKey,
) -> Event {
// RestoreSession is the one action whose payload is required to
// be None (mostro-core's `verify()`), which keeps this helper
// free of a hand-built `Payload::Order`.
wrap_message_with(
Transport::GiftWrap,
&create_test_message(Action::RestoreSession, None),
identity_keys,
trade_keys,
receiver,
WrapOptions::default(),
)
.await
.expect("gift wrap succeeds")
}

#[tokio::test]
async fn accepts_a_validly_wrapped_event_and_returns_the_action() {
let ctx = create_test_ctx().await;
let identity_keys = create_test_keys();
let trade_keys = create_test_keys();
let receiver_keys = create_test_keys();
let event =
wrap_test_order(&identity_keys, &trade_keys, receiver_keys.public_key()).await;

let result = accept_event(
&ctx,
&event,
&receiver_keys,
0,
0,
NostrKind::GiftWrap,
false,
)
.await;

let (action, _message, unwrapped) = result.expect("valid event should be accepted");
assert_eq!(action, Action::RestoreSession);
assert_eq!(unwrapped.sender, trade_keys.public_key());
assert_eq!(unwrapped.identity, identity_keys.public_key());
}

#[tokio::test]
async fn rejects_an_event_of_the_wrong_kind() {
let ctx = create_test_ctx().await;
let identity_keys = create_test_keys();
let trade_keys = create_test_keys();
let receiver_keys = create_test_keys();
let event =
wrap_test_order(&identity_keys, &trade_keys, receiver_keys.public_key()).await;

// The event is a real, validly-signed GiftWrap; accepted_kind is
// deliberately mismatched to exercise the kind-gate on its own.
let result = accept_event(
&ctx,
&event,
&receiver_keys,
0,
0,
NostrKind::TextNote,
false,
)
.await;

assert!(result.is_none());
}

#[tokio::test]
async fn rejects_an_event_not_addressed_to_the_receiver() {
let ctx = create_test_ctx().await;
let identity_keys = create_test_keys();
let trade_keys = create_test_keys();
let intended_receiver = create_test_keys();
let eavesdropper = create_test_keys();
let event =
wrap_test_order(&identity_keys, &trade_keys, intended_receiver.public_key()).await;

// unwrap_incoming can't decrypt a rumor sealed for someone else.
let result = accept_event(
&ctx,
&event,
&eavesdropper,
0,
0,
NostrKind::GiftWrap,
false,
)
.await;

assert!(result.is_none());
}

/// Ensures a spam gate is installed for this binary run without
/// clobbering one another test already installed — `SPAM_GATE` is a
/// process-wide `OnceLock`, so at most one instance ever wins, and
/// every test only relies on `is_known` being false for its own
/// freshly-generated (never-registered) pubkey.
fn ensure_spam_gate_installed() {
let _ = crate::spam_gate::SpamGate::new(crate::spam_gate::REPLAY_WINDOW_SECS)
.install_global();
}

#[tokio::test]
async fn unknown_first_contact_sender_clearing_the_pow_bar_is_accepted() {
ensure_spam_gate_installed();
let ctx = create_test_ctx().await;
let identity_keys = create_test_keys();
let trade_keys = create_test_keys();
let receiver_keys = create_test_keys();
let event =
wrap_test_order(&identity_keys, &trade_keys, receiver_keys.public_key()).await;

// is_v2 = true routes through the spam gate; a never-seen sender
// pubkey is always unknown, and pow_first_contact = 0 is
// trivially cleared, so the first-contact lane must let it
// through.
let result = accept_event(
&ctx,
&event,
&receiver_keys,
0,
0,
NostrKind::GiftWrap,
true,
)
.await;

assert!(result.is_some());
}

#[tokio::test]
async fn unknown_first_contact_sender_below_the_pow_bar_is_dropped() {
ensure_spam_gate_installed();
let ctx = create_test_ctx().await;
let identity_keys = create_test_keys();
let trade_keys = create_test_keys();
let receiver_keys = create_test_keys();
let event =
wrap_test_order(&identity_keys, &trade_keys, receiver_keys.public_key()).await;

// pow_first_contact = u8::MAX demands more leading-zero bits than
// any real event id will ever have, so this deterministically
// fails the first-contact PoW toll.
let result = accept_event(
&ctx,
&event,
&receiver_keys,
0,
u8::MAX,
NostrKind::GiftWrap,
true,
)
.await;

assert!(result.is_none());
}
}

mod handle_message_action_tests {
use super::*;
use crate::app::context::test_utils::{test_settings, TestContextBuilder};
Expand Down
7 changes: 6 additions & 1 deletion src/lightning/invoice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,12 @@ mod tests {
)
.layer(tower_http::cors::CorsLayer::permissive());

let listener = TcpListener::bind("127.0.0.1:8080").await.unwrap();
// Must match lnurl.rs's MOSTRO_TEST_LN_PORT override (default 8080).
let port = std::env::var("MOSTRO_TEST_LN_PORT")
.ok()
.and_then(|v| v.parse::<u16>().ok())
.unwrap_or(8080);
let listener = TcpListener::bind(("127.0.0.1", port)).await.unwrap();
let addr = listener.local_addr().unwrap();
let port = addr.port();

Expand Down
22 changes: 19 additions & 3 deletions src/lnurl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,14 @@ async fn extract_lnurl(address: &str) -> Result<String, MostroError> {
None => return Err(MostroInternalErr(ServiceError::LnAddressParseError)),
};
let base_url = if cfg!(test) {
format!("http://{domain}:8080")
// Overridable so test runs can dodge a port already taken on the
// host (e.g. cargo-mutants) without disturbing the 8080 default
// other tests assert on.
let port = std::env::var("MOSTRO_TEST_LN_PORT")
.ok()
.and_then(|v| v.parse::<u16>().ok())
.unwrap_or(8080);
format!("http://{domain}:{port}")
} else {
format!("https://{domain}")
};
Expand Down Expand Up @@ -260,11 +267,20 @@ mod tests {

#[tokio::test]
async fn extract_lnurl_builds_wellknown_url_for_lightning_address() {
// cfg!(test) pins lightning addresses to the local test host form.
// cfg!(test) pins lightning addresses to the local test host form,
// on MOSTRO_TEST_LN_PORT (default 8080) so this stays correct under
// a port override (e.g. cargo-mutants dodging a taken 8080).
let port = std::env::var("MOSTRO_TEST_LN_PORT")
.ok()
.and_then(|v| v.parse::<u16>().ok())
.unwrap_or(8080);
let extracted = extract_lnurl("alice@127.0.0.1")
.await
.expect("lightning address parses");
assert_eq!(extracted, "http://127.0.0.1:8080/.well-known/lnurlp/alice");
assert_eq!(
extracted,
format!("http://127.0.0.1:{port}/.well-known/lnurlp/alice")
);
}

#[tokio::test]
Expand Down
25 changes: 25 additions & 0 deletions src/nip33.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1117,6 +1117,31 @@ mod tests {
);
}

#[test]
fn create_event_does_not_duplicate_a_caller_supplied_expiration_tag() {
// Order events (kind 38383) always get an auto expiration tag from
// config when one isn't already present. Pre-supplying a real NIP-40
// `TagKind::Expiration` tag must suppress the auto-add, exercising
// the `||` in create_event's has_expiration_tag check on its
// `TagKind::Expiration` arm rather than only the `Custom("expiration")` arm.
init_test_settings();
let keys = Keys::generate();
let extra_tags = Tags::from_list(vec![Tag::expiration(Timestamp::from(123_456_u64))]);

let order = super::new_order_event(&keys, "", "order-id".to_string(), extra_tags)
.expect("order event");

let expiration_tags = order
.tags
.iter()
.filter(|t| matches!(t.kind(), TagKind::Expiration))
.count();
assert_eq!(
expiration_tags, 1,
"caller-supplied expiration tag must not be duplicated"
);
}
Comment on lines +1121 to +1143

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Cover the custom "expiration" tag branch too.

This test only exercises TagKind::Expiration; deleting the TagKind::Custom("expiration") predicate in create_event would still pass. Add a companion case using a custom "expiration" tag and assert no standard expiration tag is auto-added.

Proposed companion test
+    #[test]
+    fn create_event_does_not_duplicate_a_custom_expiration_tag() {
+        init_test_settings();
+        let keys = Keys::generate();
+        let extra_tags = Tags::from_list(vec![Tag::custom(
+            TagKind::Custom(Cow::Borrowed("expiration")),
+            vec!["123456".to_string()],
+        )]);
+
+        let order = super::new_order_event(&keys, "", "order-id".to_string(), extra_tags)
+            .expect("order event");
+
+        assert!(order.tags.iter().any(
+            |tag| matches!(tag.kind(), TagKind::Custom(ref name) if name == "expiration")
+        ));
+        assert!(!order
+            .tags
+            .iter()
+            .any(|tag| matches!(tag.kind(), TagKind::Expiration)));
+    }

As per PR objectives, this change is intended to kill NIP-33 expiration-tag mutants.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fn create_event_does_not_duplicate_a_caller_supplied_expiration_tag() {
// Order events (kind 38383) always get an auto expiration tag from
// config when one isn't already present. Pre-supplying a real NIP-40
// `TagKind::Expiration` tag must suppress the auto-add, exercising
// the `||` in create_event's has_expiration_tag check on its
// `TagKind::Expiration` arm rather than only the `Custom("expiration")` arm.
init_test_settings();
let keys = Keys::generate();
let extra_tags = Tags::from_list(vec![Tag::expiration(Timestamp::from(123_456_u64))]);
let order = super::new_order_event(&keys, "", "order-id".to_string(), extra_tags)
.expect("order event");
let expiration_tags = order
.tags
.iter()
.filter(|t| matches!(t.kind(), TagKind::Expiration))
.count();
assert_eq!(
expiration_tags, 1,
"caller-supplied expiration tag must not be duplicated"
);
}
fn create_event_does_not_duplicate_a_caller_supplied_expiration_tag() {
// Order events (kind 38383) always get an auto expiration tag from
// config when one isn't already present. Pre-supplying a real NIP-40
// `TagKind::Expiration` tag must suppress the auto-add, exercising
// the `||` in create_event's has_expiration_tag check on its
// `TagKind::Expiration` arm rather than only the `Custom("expiration")` arm.
init_test_settings();
let keys = Keys::generate();
let extra_tags = Tags::from_list(vec![Tag::expiration(Timestamp::from(123_456_u64))]);
let order = super::new_order_event(&keys, "", "order-id".to_string(), extra_tags)
.expect("order event");
let expiration_tags = order
.tags
.iter()
.filter(|t| matches!(t.kind(), TagKind::Expiration))
.count();
assert_eq!(
expiration_tags, 1,
"caller-supplied expiration tag must not be duplicated"
);
}
#[test]
fn create_event_does_not_duplicate_a_custom_expiration_tag() {
init_test_settings();
let keys = Keys::generate();
let extra_tags = Tags::from_list(vec![Tag::custom(
TagKind::Custom(Cow::Borrowed("expiration")),
vec!["123456".to_string()],
)]);
let order = super::new_order_event(&keys, "", "order-id".to_string(), extra_tags)
.expect("order event");
assert!(order.tags.iter().any(
|tag| matches!(tag.kind(), TagKind::Custom(ref name) if name == "expiration")
));
assert!(!order
.tags
.iter()
.any(|tag| matches!(tag.kind(), TagKind::Expiration)));
}
🤖 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/nip33.rs` around lines 1121 - 1143, Add a companion test alongside
create_event_does_not_duplicate_a_caller_supplied_expiration_tag that supplies a
custom "expiration" tag through new_order_event, then assert the resulting order
contains no auto-added standard TagKind::Expiration tag. Keep the existing
standard-tag test unchanged and verify the custom branch in create_event's
has_expiration_tag logic.


// ── create_rating_tag ────────────────────────────────────────────────

#[test]
Expand Down
Loading