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

EncryptedDirectMessage class; simplify Event #39

Merged
merged 17 commits into from
Feb 4, 2023

Conversation

kdmukai
Copy link
Contributor

@kdmukai kdmukai commented Jan 24, 2023

This one probably should have been separated out into 3-4 small PRs. I can pull these apart if need be.

EncryptedDirectMessage class

Derived from the parent Event class to provide a more human-friendly way to construct a DM:

from nostr.event import EncryptedDirectMessage

dm = EncryptedDirectMessage(
  recipient_pubkey=recipient_pubkey,
  cleartext_content="Secret message!"
)
private_key.sign_event(dm)
relay_manager.publish_event(dm)
  • The recipient's 'p' reference tag is automatically inserted for us.
  • 'cleartext_contentprovides a clean way to differentiate the cleartext content from the ultimate encryptedcontent`.
  • Trying to instantiate a DM by specifying content throws an exception.
  • PrivateKey.sign() encrypts the message for us and populates the content field.
  • Explicit PrivateKey.encrypt_dm() convenience method added while preserving the more general encrypt_message() in case that finds other uses(?).
  • The DMs are still Event types and so can be passed straight on to the RelayManager.
  • Relevant tests added.
  • Example added to README.

Event.__init__ boilerplate removed via dataclass

Boilerplate bad. DRY good.

Simplified Event instantiation

We're going to sign an Event with our PrivateKey anyway so there's no need to populate the Event.public_key field on instantiation.

event = Event("Hello Nostr")
private_key.sign_event(event)
relay_manager.publish_event(event)

This same simplification is also demonstrated in the EncryptedDirectMessage example above.

README examples updated accordingly.

Event explicit 'e' and 'p' tag support

Similar to the discussion in #12:

  • Add explicit add_event_ref(event_id) to generate 'e' tags
  • Add explicit add_pubkey_ref(pubkey) to generate 'p' tags
  • README example added for replying to a note.

@callebtc
Copy link
Contributor

ACK

Type annotations good

Copy link
Owner

@jeffthibault jeffthibault left a comment

Choose a reason for hiding this comment

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

Thanks @kdmukai! Did a first pass and added a few comments and questions.

nostr/event.py Outdated Show resolved Hide resolved
nostr/event.py Show resolved Hide resolved
nostr/event.py Show resolved Hide resolved
nostr/key.py Show resolved Hide resolved
nostr/key.py Outdated Show resolved Hide resolved
nostr/key.py Outdated Show resolved Hide resolved
nostr/event.py Outdated Show resolved Hide resolved
nostr/event.py Outdated Show resolved Hide resolved
* Simpler Event instantiation w/content string w/out  kwarg.
* `tags` attr default value via factory.
* `id` changed to a compute-on-the-fly property; eliminates all the redundant `compute_id()` calls.
* Simpler DM instantiation (same as `Event`); simply copies the `content` field over to `cleartext_content` and no longer throws Exception.
* Asking for a DM's `id` raises an Exception if its message hasn't been encrypted yet.
* Adds/updates relevant Event tests.
* Restores key.mine_vanity_key.
@kdmukai
Copy link
Contributor Author

kdmukai commented Feb 2, 2023

@jeffthibault great notes! All my updates are now committed.

  • Simpler Event instantiation w/content string w/out kwarg.
  • tags attr default value via factory.
  • id changed to a compute-on-the-fly property; eliminates all the redundant compute_id() calls.
  • Simpler DM instantiation (same as Event); simply copies the content field over to cleartext_content and no longer throws Exception.
  • Asking for a DM's id raises an Exception if its message hasn't been encrypted yet.
  • Adds/updates relevant Event tests.
  • Restores key.mine_vanity_key.

@jeffthibault
Copy link
Owner

Thanks @kdmukai. This looks great!

Tested ACK.

@jeffthibault jeffthibault merged commit 69ff17b into jeffthibault:main Feb 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants