Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

establish NostrEvent subclassing pattern with basic event kinds #56

Merged
merged 4 commits into from
Jul 25, 2023

Conversation

bryanmontz
Copy link
Collaborator

This PR implements a class hierarchy architecture pattern for the parent event type NostrEvent and all of its many specific kinds.

The general pattern is that kinds will map to specific subclasses, which will then interpret the "raw" information contained in the parent class, primarily from the the content and tags properties.

Closes #48
Closes #49
Closes #50
Closes #51

Copy link
Contributor

@tyiu tyiu left a comment

Choose a reason for hiding this comment

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

Hype! This is great.

///
/// > Note: [NIP-01 Specification](https://github.com/nostr-protocol/nips/blob/b503f8a92b22be3037b8115fe3e644865a4fa155/01.md#basic-event-kinds)
final class RecommendServerEvent: NostrEvent {
var relayURL: URL? {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would there be any validation we want to perform here to ensure it's a websocket URL (i.e., ws:// or wss://) and return nil if not?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure. We could if we want because the spec specifically says it's for relays and those will be websockets for now.

Copy link
Contributor

@tyiu tyiu Jul 24, 2023

Choose a reason for hiding this comment

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

For this specific case, I think we can be opinionated here because any other non-websocket protocol URL does not make sense on this field.

@tyiu
Copy link
Contributor

tyiu commented Jul 24, 2023

One more thought: this change adds support for decoding -- what are your thoughts about encoding subclass instances for signing for submission to relays?

@bryanmontz
Copy link
Collaborator Author

I don't think the subclasses will need to worry about signing at all. The base event type can handle that after simple, specific-kind initializers pack all the right fields of an event. I'll tackle that for these first kinds next.

@tyiu
Copy link
Contributor

tyiu commented Jul 24, 2023

I don't think the subclasses will need to worry about signing at all. The base event type can handle that after simple, specific-kind initializers pack all the right fields of an event. I'll tackle that for these first kinds next.

Sorry, I didn't mean that the subclasses would be responsible for signing. I just meant that the initialization of the subclass instance with the kind specific fields would/should also call the base NostrEvent class constructor initializer (so that when event signing does happen, it can just rely on the base NostrEvent fields when encoding and signing the message). Basically, what you said in the latter part.

@joelklabo
Copy link
Collaborator

Looks awesome! I think we picked the right strategy 👍

@bryanmontz bryanmontz merged commit 4da2bbc into main Jul 25, 2023
3 checks passed
@bryanmontz bryanmontz deleted the montz/note-classes branch July 25, 2023 11:30
RandyMcMillan pushed a commit to RandyMcMillan/nostr-sdk-ios that referenced this pull request Sep 1, 2024
…r-sdk#56)

* establish NostrEvent subclassing pattern with basic event kinds

Closes nostr-sdk#48
Closes nostr-sdk#49
Closes nostr-sdk#50
Closes nostr-sdk#51

* lint

* validate relay URLs

* add external interface for creating set metadata and recommend server events, plus tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants