Adds support for NIP-28 #178

masterial · 2024-09-12T19:18:44Z

tyiu

Thanks for the contribution! I left a few comments around supporting properties of each kind in a first-class way, and using a Builder protocol instead of continuing with the soon-to-be-deprecated EventCreating protocol.

Also, there seems to be a few SwiftLint errors that should be addressed.

tyiu · 2024-09-14T02:06:27Z

Sources/NostrSDK/Events/Channels/CreateChannelEvent.swift

+}
+
+public extension EventCreating {
+    func createChannelEvent(withContent content: String, signedBy keypair: Keypair) throws -> CreateChannelEvent {


I'm in the process of deprecating the EventCreating protocol actually, and moving towards a Builder pattern. The former is too rigid for building Nostr events, whereas the latter gives us a lot of flexibility.

See https://github.com/nostr-sdk/nostr-sdk-ios/pull/175/files#diff-c87e105b65b1d7c9512acf2ee8c7e3fc7582648811025c959a37b36d97403b67R170 as an example.

Also, it would be better to have an API where the developer can specify the basic channel metadata (name, about, picture and relays) rather than needing to enter in raw JSON in content.
https://github.com/nostr-protocol/nips/blob/master/28.md#kind-40-create-channel

Hi, @tyiu, thanks for your time and looking over my commit!

Yes, I am on board with the later. The interface in this PR needs further definition and polish.
I created this PR because I am using your SDK with my project (SwiftUI/Swift 5). Thanks for putting it together!
So far, I was able to implement and successfully field test Encrypted Message, Group Chat and Key Management features.
As far as, my PR goes I will clean it up soon for the linter. I am thinking doing it in parallel while I’m working on replies for my chat application.

I don't quite understand what you mean by ‘deprecating EventCreating protocol’. I do use it a lot but I did find it somewhat rigid.
I also think there needs to be some better way to manage subscriptions. I do have them on about 5 Views on my app. Yikes...! 🦟

https://github.com/SkatePay/skatepay/blob/main/SkateConnect/SkateConnect/Views/Skatepark/Lobby/SpotFeed/ChannelFeed.swift#L145

Regards!

When I say deprecating the EventCreating protocol in favor of the Builder protocol, I mean it would look something like this when creating this event:

try CreateChannelEvent.Builder() .channelMetadata(channelMetadata, merging: rawChannelMetadata) .build(signedBy: keypair)

Take a look at what we do here for kind 0, as an example:

nostr-sdk-ios/Sources/NostrSDK/Events/MetadataEvent.swift

Lines 161 to 192 in 8b40eb9

public extension MetadataEvent {

/// Builder of ``MetadataEvent``.

final class Builder: NostrEvent.Builder<MetadataEvent>, CustomEmojiBuilding {

public init() {

super.init(kind: .metadata)

}

/// Sets the user metadata by merging ``UserMetadata`` with a dictionary of raw metadata.

///

/// - Parameters:

/// - userMetadata: The ``UserMetadata`` to set.

/// - rawUserMetadata: The dictionary of raw metadata to set that can contain fields unknown to any implemented NIPs.

///

/// > Note: If `rawUserMetadata` has fields that conflict with `userMetadata`, `userMetadata` fields take precedence.

public final func userMetadata(_ userMetadata: UserMetadata, merging rawUserMetadata: [String: Any] = [:]) throws -> Self {

let userMetadataAsData = try JSONEncoder().encode(userMetadata)

let allUserMetadataAsData: Data

if rawUserMetadata.isEmpty {

allUserMetadataAsData = userMetadataAsData

} else {

var userMetadataAsDictionary = try JSONSerialization.jsonObject(with: userMetadataAsData, options: []) as? [String: Any] ?? [:]

userMetadataAsDictionary.merge(rawUserMetadata) { (current, _) in current }

allUserMetadataAsData = try JSONSerialization.data(withJSONObject: userMetadataAsDictionary, options: .sortedKeys)

}

let allUserMetadataAsString = String(decoding: allUserMetadataAsData, as: UTF8.self)

content(allUserMetadataAsString)

return self

}

}

@masterial Turns out the SwiftLint errors were on the main branch, due to how CI pulls in the latest SwiftLint version and it had some breaking changes in what it validates. I just fixed the issue on the main branch so the lint errors should go away after you pull it into your branch.

Merged, thanks for the heads up, @tyiu!
I plan to address other issues you've highlighted very soon

tyiu · 2024-09-14T02:08:33Z

Sources/NostrSDK/Events/Channels/CreateChannelMessageEvent.swift

+}
+
+public extension EventCreating {
+    func createChannelMessageEvent(withContent content: String, eventId: String, relayUrl: String, signedBy keypair: Keypair) throws -> CreateChannelMessageEvent {


Same here. Again, look at how TextNoteEvent's Builder handles NIP-10 marker event tags. Maybe you could refactor or at least follow what we do in that code.

NIP-28 says that root or reply event tags are supported.

Yes, I will take care of this when I am working on replies in my app.

FYI I'm working on NIP-17 - Private Direct Messages which requires threading as well, so I ended up refactoring the logic for building events with NIP-10 tags. It hasn't been merged yet -- still working on my branch but it'll be ready soon. Hope you haven't gotten to it yet, don't want to make your changes redundant.

Great! Thank you so much.

Merged: #186

tyiu · 2024-09-14T02:11:30Z

Sources/NostrSDK/Events/Channels/CreateChannelEvent.swift

+
+/// Create a public chat channel.
+/// See [NIP-28](https://github.com/nostr-protocol/nips/blob/master/28.md#kind-40-create-channel).
+public class CreateChannelEvent: NostrEvent {


It would be good to expose the basic channel metadata (name, about, picture and relays) properties.

Take a look at https://github.com/nostr-sdk/nostr-sdk-ios/blob/main/Sources/NostrSDK/Events/MetadataEvent.swift as it also does JSON encoding and decoding for the kind 0 event.

Should I make a MetadataChannel class here or what?

In a separate file, you could create a ChannelMetadata struct with the properties. Then, you could create a ChannelMetadataInterpreting protocol that decodes the JSON.

Kinds 40 CreateChannelEvent and kind 41 SetChannelMetadataEvent could extend from ChannelMetadataInterpreting.

Something like this, it'll be pretty similar to what we do with kind 0:

public struct ChannelMetadata: Codable { public let name: String? public let about: String? ... enum CodingKeys: String, CodingKey { ... } } public protocol ChannelMetadataInterpreting: NostrEvent {} public extension ChannelMetadataInterpreting { public var channelMetadata: ChannelMetadata { guard let data = content.data(using: .utf8) else { return nil } return try? JSONDecoder().decode(ChannelMetadata.self, from: data) } }

tyiu · 2024-09-14T02:12:29Z

Sources/NostrSDK/Events/Channels/HideChannelMessageEvent.swift

+
+/// User no longer wants to see a certain message.
+/// See [NIP-28](https://github.com/nostr-protocol/nips/blob/master/28.md#kind-43-hide-message).
+public class HideChannelMessageEvent: NostrEvent {


Same here for the reason metadata in the content field.

tyiu · 2024-09-14T02:12:41Z

Sources/NostrSDK/Events/Channels/HideChannelMessageEvent.swift

+}
+
+public extension EventCreating {
+    func hideChannelMessageEvent(withContent content: String, signedBy keypair: Keypair) throws -> HideChannelMessageEvent {


tyiu · 2024-09-14T02:13:26Z

Sources/NostrSDK/Events/Channels/MuteChannelUserEvent.swift

+
+/// User no longer wants to see messages from another user.
+/// See [NIP-28](https://github.com/nostr-protocol/nips/blob/master/28.md#kind-44-mute-user).
+public class MuteChannelUserEvent: NostrEvent {


Same here about reason.

Yes, implementing this feature soon too.

tyiu · 2024-09-14T02:13:32Z

Sources/NostrSDK/Events/Channels/MuteChannelUserEvent.swift

+}
+
+public extension EventCreating {
+    func createMuteChannelUserEvent(withContent content: String, signedBy keypair: Keypair) throws -> MuteChannelUserEvent {


tyiu · 2024-09-14T02:14:10Z

Sources/NostrSDK/Events/Channels/SetChannelMetadataEvent.swift

+
+/// Update a channel's public metadata.
+/// See [NIP-28](https://github.com/nostr-protocol/nips/blob/master/28.md#kind-41-set-channel-metadata).
+public class SetChannelMetadataEvent: NostrEvent {


Same here about exposing the basic metadata fields as properties on this class.

tyiu · 2024-09-14T02:14:41Z

Sources/NostrSDK/Events/Channels/SetChannelMetadataEvent.swift

+}
+
+public extension EventCreating {
+    func setChannelMetadataEvent(withContent content: String, signedBy keypair: Keypair) throws -> SetChannelMetadataEvent {


Same here about the builder pattern and exposing an API to set the metadata fields instead of the raw content field.

* Fix SwiftLint errors from latest SwiftLint release (nostr-sdk#180) * Fix SwiftLint errors from latest SwiftLint release * Fix GitHub action workflows to trigger on only pull_request * Fix GitHub workflows to also run on main branch (nostr-sdk#181) * Add Swift 6.0 to unit tests (nostr-sdk#179) --------- Co-authored-by: Terry Yiu <[email protected]>

Adds relevant class files and definitions.

3b5391b

masterial mentioned this pull request Sep 12, 2024

Implement NIP-28 Public Chat #88

Open

tyiu requested changes Sep 14, 2024

View reviewed changes

masterial and others added 3 commits September 18, 2024 10:26

Adds builder and metadata type for CreateChannelEvent.

9fc355b

Adds hashtag labeling.

ac73725

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds support for NIP-28 #178

Adds support for NIP-28 #178

masterial commented Sep 12, 2024

tyiu left a comment

tyiu Sep 14, 2024

masterial Sep 14, 2024

tyiu Sep 14, 2024

tyiu Sep 18, 2024

masterial Sep 18, 2024

tyiu Sep 14, 2024

masterial Sep 14, 2024

tyiu Oct 17, 2024

masterial Oct 17, 2024

tyiu Oct 19, 2024

tyiu Sep 14, 2024

masterial Sep 14, 2024

tyiu Sep 14, 2024

tyiu Sep 14, 2024

tyiu Sep 14, 2024

tyiu Sep 14, 2024

masterial Sep 14, 2024

tyiu Sep 14, 2024

tyiu Sep 14, 2024

tyiu Sep 14, 2024

	public extension MetadataEvent {
	/// Builder of ``MetadataEvent``.
	final class Builder: NostrEvent.Builder<MetadataEvent>, CustomEmojiBuilding {
	public init() {
	super.init(kind: .metadata)
	}

	/// Sets the user metadata by merging ``UserMetadata`` with a dictionary of raw metadata.
	///
	/// - Parameters:
	/// - userMetadata: The ``UserMetadata`` to set.
	/// - rawUserMetadata: The dictionary of raw metadata to set that can contain fields unknown to any implemented NIPs.
	///
	/// > Note: If `rawUserMetadata` has fields that conflict with `userMetadata`, `userMetadata` fields take precedence.
	public final func userMetadata(_ userMetadata: UserMetadata, merging rawUserMetadata: [String: Any] = [:]) throws -> Self {
	let userMetadataAsData = try JSONEncoder().encode(userMetadata)

	let allUserMetadataAsData: Data
	if rawUserMetadata.isEmpty {
	allUserMetadataAsData = userMetadataAsData
	} else {
	var userMetadataAsDictionary = try JSONSerialization.jsonObject(with: userMetadataAsData, options: []) as? [String: Any] ?? [:]
	userMetadataAsDictionary.merge(rawUserMetadata) { (current, _) in current }
	allUserMetadataAsData = try JSONSerialization.data(withJSONObject: userMetadataAsDictionary, options: .sortedKeys)
	}

	let allUserMetadataAsString = String(decoding: allUserMetadataAsData, as: UTF8.self)
	content(allUserMetadataAsString)

	return self
	}
	}

Adds support for NIP-28 #178

Are you sure you want to change the base?

Adds support for NIP-28 #178

Conversation

masterial commented Sep 12, 2024

tyiu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment