-
Notifications
You must be signed in to change notification settings - Fork 17
Implement NIP-17 Private Direct Messages #90
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
Implement NIP-17 Private Direct Messages #90
Conversation
|
#88 needs to be merged first for the convience reviewing this PR furthermore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using Event/Types for specific Event kinds would lead to a large list eventually. That's why I had chosen to name the subdirectory the same as the classname (just like the Profile.php class for kind 0 in Event/Profile).
|
|
||
| print PHP_EOL . "===== PUBLISHING GIFT WRAPS TO RELAYS =====" . PHP_EOL; | ||
|
|
||
| // Normally, you would get the relay list from the NIP-10050 event |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean NIP-65 here with event kind 10002? Or the relays returned by a NIP-05 lookup? NIP-10050 does not exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops... I meant kind 10050, as described in NIP-51
|
I've fixed the typo and renamed the Event classes so it is now consistent with the Profile namespace. |
|
|
||
| // Check that the gift wrap is addressed to the receiver via p tag | ||
| // Handle both Event objects and stdClass objects from relays | ||
| $tags = method_exists($giftWrap, 'getTags') ? $giftWrap->getTags() : ($giftWrap->tags ?? []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In which case the $giftWrap variable is a object of the GiftWrap class? Why is this method_exists check needed? The tags property is protected is should be only accessed via the getTags() method of the GiftWrap class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's in the comment above, in the case the event is a stdClass, a received event is not automatically deserialized as a GiftWrap class. In the nip-17 example where the created gift wrap is a giftwrap it already is an instance of GiftWrap
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would make it so that if the object is a stdClass convert it to a Giftwrap (Event) object. Does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think in general it would be good to have helper methods (factory class?) that convert stdClass objects to the correct kind-specific classes.
| // Extract the encrypted content from the gift wrap | ||
| $encryptedContent = method_exists($giftWrap, 'getContent') ? | ||
| $giftWrap->getContent() : | ||
| ($giftWrap->content ?? ''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as my previous comment for line 105.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same answer ;)
| ($giftWrap->content ?? ''); | ||
|
|
||
| // Extract the gift wrap's random public key | ||
| $giftWrapPubkey = method_exists($giftWrap, 'getPublicKey') ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as my previous comment for line 105.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same answer ;)
|
Work in progress, see my latest commit pushed 1a6072e.
|
…arate DmRelaysList class (as an event object), added tests for DmRelaysList class + updated the Nip17Test.php
src/Nip17/DirectMessageInterface.php
Outdated
| * @param string $pubkey The public key to get relays for | ||
| * @return array Array of relay URLs | ||
| */ | ||
| // public function getPreferredRelaysForPubkey(string $pubkey): array; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dsbaars I've replaced this (fetching event kind 10050 for a given pubkey) with b6b96d5#diff-dbd4ad9a1f506981b2675768f436001f7b0d7cb87bed236ea765facad540cf95
Please have a look and feel free to share your feedback on this :)
src/Event/List/DmRelaysList.php
Outdated
| foreach ($response as $relayResponses) { | ||
| foreach ($relayResponses as $relayResponse) { | ||
| if ($relayResponse instanceof RelayResponseEose) { | ||
| // TODO this does not work to get to the next relay in $other_relays_to_query |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is handled better in #95
Implements NIP-17 Private Direct Messages based on #88
Includes tests and example