dtonon / nostr-ruby

A ruby library to interact with the Nostr Protocol
MIT License
47 stars 7 forks source link

Start refactoring the original poc code #8

Closed dtonon closed 3 months ago

dtonon commented 7 months ago

Ok, I did a big refactoring, structuring the code in a more readable form.

Now there is a Nostr::Event class that permits raw creation of events, also using NIP-13 (pow), NIP-4 (old dm) and NIP-26 (delegation) as options. Then I added specific classes to quickly operate with specific events: Nostr::ShortNote, Nostr::Medatada, Nostr::Deletion, Nostr::Reaction, Nostr::ContactList, Nostr::DirectMessage.

Finally, there is Nostr::Client that manages the keys and can sign events; in the future it will be enabled to post events and query relays, too.

A typical usage with the new code is the follow:

require_relative 'lib/nostr_ruby'
private_key = "a727ffd7c6ffefa2a0b9f8a238885ea64caf6e0fe8f4daed590496f96999e341"
client = Nostr::Client.new(private_key)
puts "My npub is: #{client.bech32_public_key}"

# Create a note
note = Nostr::ShortNote.new(
  "00bf316f2e2fc64dd4c502b281a2fdd945519164920720b788ece9746e5d3c65",
  "Hello world!"
)
client.sign(note)

# Create a dm
dm = Nostr::DirectMessage.new(
  "00bf316f2e2fc64dd4c502b281a2fdd945519164920720b788ece9746e5d3c65",
  "Hello, how are you?",
  "62f57fb64b3ab4475a03cc1804f856a45f12e7d25c8177a597b1962d309ee25a"
)
client.sign(dm)
client.decrypt_nip4(dm)

The refactor is quite dirty, I just migrated the current code with some small additions; I still need to reorganize the crypto functions (maybe in a Nostr::Signer that can be extended with more modes, e.g. NIP-46) and the utilities (e.g. the betch32 lib). Then I plan to add some methods to manage tags, automatically create e and p tags from nostr: entities found in the content, and some methods to easily manage a kind:1 thread (reply, browse children, parent, sibling), the new NIP-44 encryption and more NIPs. All validations and errors handling have been removed, so they need a complete review.

Some points I'm reasoning on:

@anthony-robin @erskingardner @alexgleason I would like to have your feedback before going on, so we can have a solid and scalable base on which to continue development.

dtonon commented 7 months ago

I'm pondering that it might give the idea that it doesn't make sense to pass the author's pubkey in creating an event, since it can be obtained at signing time; but we may also need to handle unsigned events (e.g., a draft), so probably the best approach is to leave the pubkey param and then check for congruity at signing. If it is left blank instead, it will be valued at signing.

dtonon commented 7 months ago

Another structural point is whether, in classes to interact directly with a specific event, e.g., Nostr::Reaction, it is preferable to pass data in a raw manner (as now, e.g., event id and author pubkey), or to prefer a structured data, in this case an instance of Nostr::Event. The advantage of the first approach is obvious in Nostr::Deletion, where I simply need the event id without needing the full instance.

erskingardner commented 7 months ago

@dtonon I think we should probably change the approach to be more modular and try to match how users would want to use things.

The way that you're starting here will mean that we'll have LOTS of potentially deeply namespaced modules or modules that have somewhat arbitrary names. For example, creating a new DM with Nostr::DirectMessage.new() makes way less sense to me than creating events with something like Nostr::Event.new(kind: Kinds::Nip04Dm, content: "blah", ...), the fewer things to remember about how to do the most common operations, the better IMO.

Off the top of my head I would probably categorize the breakdown of the library as follows:

Happy to hear other ideas though, that's just my thoughts about how it would make sense to organize it.

dtonon commented 7 months ago

The way that you're starting here will mean that we'll have LOTS of potentially deeply namespaced modules or modules that have somewhat arbitrary names. For example, creating a new DM with Nostr::DirectMessage.new() makes way less sense to me than creating events with something like Nostr::Event.new(kind: Kinds::Nip04Dm, content: "blah", ...), the fewer things to remember about how to do the most common operations, the better IMO.

The classes under nips/ are just shortcuts to Nostr::Event with sensible values for quick operations, if we wipe them, the library is still totally usable. Sometime using the raw Nostr::Event would be necessary, for example to manipulate tags in custom ways (notification); and actually offer more way to achieve the same goal can be confusing. Btw, this is an easy detail to solve (just delete the files), we don't have other deeply namespaced modules.

I agree with the overall structure you proposed, I would add Client to manage all the activities with the relays.

anthony-robin commented 6 months ago

Hi Daniele,

Sorry for taking so long to reply 🙏 , here are the feedback I can give you after testing your refactoring:

I like the way to have classes that abstract the setup and not explicitly specifying the NIP. However, both ways should be doable according to the user preferences and we should ensure users could create a manual event payload in case the gem doesn’t have a support for an event yet.

I would also prefer to have the Nostr event class instance as light as possible and not have to pass the npub each time. An initializer with the private nsec/npub config could be a good way to have it as optional at the event class level but still overridable on the fly (I’m thinking on how the aws-sdk-s3 ruby gem handle it).

# config/initializers/nostr.rb
Nostr.config.update(
  private_key: '<private_key>',
  public_key: '<public_key>'
)

# any classes or services
client = Nostr::Client.new
# => Client use default config from initializer

client = Nostr::Client.new('another_private_key')
# => Client use the given key in initializer

I do not have strong opinion for other questions you asked in the discussion, let’s see how it can go ! Also note that I’m not working on a Nostr Rails project anymore so feel free to move forward according to your and other ruby developers wishes :)

dtonon commented 6 months ago

@anthony-robin

we should ensure users could create a manual event payload

This is already possible to with the Event class.

An initializer with the private nsec/npub config

The Client class works like that. I just need to decide how to link this class to the Event one.

Thank you for your support, le me know if you start another nostr/ruby project! :)

dtonon commented 3 months ago

I just pushed some commits in master that update the API to the new structure. Now I'm going to think how to organize the classes and their relationship.

/cc @anthony-robin @erskingardner @alexgleason