crates/mdk-core/src/welcomes.rs (1)

371-377: Update MIP‑02 test docs to reflect 4‑tag welcome structure

The tests now correctly assert 4 tags on welcome rumors (relays, event reference, client, encoding), but the test_welcome_event_structure_mip02_compliance doc comment still claims “exactly 3 tags”. Consider updating that comment (and any similar references) to avoid confusion between spec text and the enforced structure.

Also applies to: 425-430, 599-603, 695-696, 940-941

crates/mdk-core/src/key_packages.rs (1)

93-96: Minor style nit: prefer .to_string() over .into() for &str → String

Per the Rust guidelines you’re following, consider switching "encoding".into() to "encoding".to_string() in the TagKind::Custom("encoding".into()) construction here (and the similar pattern in groups.rs) to keep &str→String conversions consistent:

-            Tag::custom(
-                TagKind::Custom("encoding".into()),
-                [encoding.as_tag_value()],
-            ),
+            Tag::custom(
+                TagKind::Custom("encoding".to_string()),
+                [encoding.as_tag_value()],
+            ),

This is purely stylistic and doesn’t affect behavior.

Also applies to: 2109-2117, 2143-2151

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

crates/mdk-core/src/welcomes.rs (1)

291-325: Centralized welcome decoding and format logging look solid

Using decode_content here with the extra debug log on the actual decoded format keeps welcome handling consistent with key-package decoding and makes failures persistently tracked via ProcessedWelcome::Failed. The duplicated failure-handling block shared with the later parse error path is fine, but could be factored into a small helper in the future if this pattern appears more often.

crates/mdk-core/src/key_packages.rs (2)

122-141: Unified key‑package decoding via decode_content is correct

parse_serialized_key_package now delegates to decode_content(key_package_str, encoding, "key package") and logs the decoded format, which matches the util contract and keeps base64/hex behavior centralized. Error mapping to Error::KeyPackage plus the updated tests for invalid encodings/inputs (test_invalid_key_package_parsing) ensure both formats fail fast with the right variant.

You might consider renaming the local format binding in the match to something like decoded_format to avoid shadowing the format! macro name and improve readability, but it’s not functionally problematic.

Also applies to: 950-971

---

2087-2117: Encoding‑tag tests for base64 and hex cover the new behavior well

The base64 and hex encoding tests now both ensure:

• Content decodes under the expected encoding.
• An "encoding" tag is present.
• The second element of that tag is exactly "base64" or "hex".

Combined with the cross‑format compatibility test, this gives good coverage of the new always-present encoding tag behavior for key packages.

If you want slightly clearer intent, matching on tag.kind() == TagKind::Custom("encoding".into()) (like other tests do) instead of peeking at as_slice().first() would reduce reliance on the underlying slice layout, but the current assertions are still robust.

Also applies to: 2125-2151, 2153-2156, 2159-2212

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

crates/mdk-core/src/util.rs (1)

212-228: Move nostr::Tag import out of the test function

use nostr::Tag; inside test_from_tags_returns_encoding violates the “no inner use” guideline; please move this import to the top of the tests module instead.

As per coding guidelines, all use statements should live at the top of the file/module.

crates/mdk-core/src/welcomes.rs (1)

291-324: Centralized welcome decoding via decode_content looks correct; consider simplifying error formatting

Using ContentEncoding::from_tags and decode_content to drive decoding (with a hex default) is consistent with the new encoding‑tag design, and logging the decoded format is helpful. In the error branch, you can drop the debug formatter and just use {} for e to avoid quoting the already‑formatted error string:

- let error_string = format!(
-     "Error decoding welcome event content ({}): {:?}",
-     encoding.as_tag_value(),
-     e
- );
+ let error_string = format!(
+     "Error decoding welcome event content ({}): {}",
+     encoding.as_tag_value(),
+     e
+ );

crates/mdk-core/src/key_packages.rs (2)

24-41: Encoding/tag handling in key‑package creation/parsing looks correct; align "encoding" conversions with style guide

The refactored create_key_package_for_event correctly:

• Picks ContentEncoding from use_base64_encoding,
• Uses shared encode_content,
• Returns a fixed [Tag; 7] including a trailing encoding tag, and
• Leaves parse_key_package free to derive the format via ContentEncoding::from_tags.

parse_serialized_key_package’s use of decode_content(..., "key package") and the format debug log are also consistent with the new util API and error expectations.

To match the “prefer .to_string() over .into()” guideline for &str → String, consider updating the custom kind construction (and matching test) like this:

- Tag::custom(
-     TagKind::Custom("encoding".into()),
-     [encoding.as_tag_value()],
- ),
+ Tag::custom(
+     TagKind::Custom("encoding".to_string()),
+     [encoding.as_tag_value()],
+ ),

and

- assert_eq!(tags[6].kind(), TagKind::Custom("encoding".into()));
+ assert_eq!(tags[6].kind(), TagKind::Custom("encoding".to_string()));

As per coding guidelines, .to_string() is preferred here.

Also applies to: 48-52, 72-85, 86-97, 127-133

---

2087-2117: Base64 and hex encoding tests validate both content and encoding tag; minor optional cleanup

The new test_key_package_base64_encoding and test_key_package_hex_encoding correctly:

• Exercise both encoding modes via MdkConfig::use_base64_encoding,
• Verify that content decodes as base64/hex, and
• Assert that the ["encoding", "..."] tag is present with the expected value.

If you want to trim a tiny allocation in the tag lookup, you could compare against "encoding" as &str instead of constructing a String each time:

- .find(|t| t.as_slice().first() == Some(&"encoding".to_string()));
+ .find(|t| t.as_slice().first().map(String::as_str) == Some("encoding"));

Also applies to: 2125-2151

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: marmot-protocol/mdk PR: 0
File: .cursor/rules/rust.mdc:0-0
Timestamp: 2025-12-03T10:18:10.432Z
Learning: Applies to **/*.rs : Use inline `mod tests { #[cfg(test)] ... }` for test modules rather than creating separate `tests.rs` files