Re: [PATCH notedeck 0/2] Replace nostr-sdk with nostr-types

[William Casarin]

Hey Mike!

On Sat, May 04, 2024 at 03:13:51AM GMT, Mike Dilger wrote:
>On Friday, May 3rd, 2024 at 12:37 PM, William Casarin <jb...@jb55.com> wrote:
>>
>> On Thu, May 02, 2024 at 04:52:58PM GMT, kernelkind wrote:
>>
>>> I don't like the Keys/PublicKey impl in nostr-sdk since the PublicKey
>>> bytes aren't accessible by reference.
>>>
>>> the nostr-types implementation of PublicKey is more useable for notedeck
>>> in my opinion since the bytes are accessible by reference. It is very
>>> similar to the enostr/src/pubkey.rs implementation, but it has more
>>> features.
>>
>> It looks like the nostr-types is structually similar to our PubKey since
>> 0.7.0 or spicifically 099077afc81def588b8655f010a15d77baaff7ca:
>>
>> commit 099077afc81def588b8655f010a15d77baaff7ca (test)
>> Author: Mike Dilger mi...@mikedilger.com
>>
>> [BREAKING] PublicKey major change - internal format changed; functions changed:
>>
>> [..]
>>
>> I thought this was an obvious thing but it seems nostr-sdk also makes
>> this mistake. It was also why I never used nostr-types at the start when
>> I was first evaluating it. I am more open to using nostr-types now as I
>> didn't realize Mike made this change.
>>
>> We will still likely need an unowned Pubkey type if we want to represent
>> nostrdb pubkeys in a typesafe way without a copy, but maybe nostrdb-rs
>> should be exporting that one.
>>
>> [..]
>>
>> So yeah at first this patchset spooked me but now that I see the pubkey
>> has been updated to just raw bytes I think it's fine.
>
>Just FYI, while nostr-types is ok, it's not my best work and I'm moving
>away from it. It underlies the gossip client, but I will either be
>switching to rust-nostr (Yuki's crates) or to pocket-types which I've
>recently extracted from chorus relay.
>
>Pocket-types uses borrowed and owned types for large variable-sized
>things that would normally require multiple memory allocations (tags,
>events, filters), but Copy types for fixed-sized things small enough to
>be on the stack (IMHO copying 64 bytes onto the stack for a Sig is
>cheap whereas allocating 64 bytes on the heap might not be). Yes, you
>could go further and borrow those 32 bytes of the pubkey but it might
>not offer any noticeable improvement, I'm not sure. Anyhow that's
>something we could experiment with.
>
>For the other types like Event, it has Event and OwnedEvent which work
>similarly to str and String or to Path vs PathBuf, in that the first
>one is unsized and must be a reference (&Event or &mut Event) and the
>second one is owned.
>
>It also has custom rapid JSON deserialization from JSON bytes into
>these types in a pre-allocated buffer that you supply (or if using the
>Owned type, it works out the buffer for you).
>
>Since pocket is really just starting out, maybe we could work together on this.

Hmm most of the data we access is in virtual memory from data paged in
from nostrdb, so I'm not sure if we can make too much use of this. Our
note type is an enum of Owned or Unowned, but I don't know if this is a
good idea or if I should be separating it. It is nice to have a uniform
type across our codebase that doesn't care about ownership and without
using Into<Note<'_>> everywhere, but then you are stuck with carrying
around lifetimes even if they are static. So I may switch to NoteBuf in
the future for owned stuff, which we don't use too much yet.

Re: kernelkind: given this info and considering how trivial it is to just
improve our own types, maybe we should stick to enostr? I still want a
place where we can maintain owned variants of the nostrdb types. Maybe
that should be in nostrdb itself, but for simple things like keys where
we need unowned references with type safety, its not hard to simply wrap
it in a newtype.

I want to reduce the amount of package dependencies as much as possible.
Some mistakenly attributed this to NIH, but every dependency is a
potentially liability and many times pulls in much more dependencies.

I tried to pull in fedimint into damus recently and it added hundreds of
packages and increased the build time by like 10 minutes. This is an
extreme example but still, it's an important thing to keep in mind.

Normally I'm fine with it for things that are complicated like http
libs, but pubkey types? This code is not difficult. Let's just improve
our own types where they are deficient.


Will

[kernelkind]

Re: kernelkind: given this info and considering how trivial it is to just
improve our own types, maybe we should stick to enostr? 

 

Normally I'm fine with it for things that are complicated like http

libs, but pubkey types? This code is not difficult. Let's just improve
our own types where they are deficient.

 Yeah we can stick to enostr. At first it seemed like a shame to duplicate effort like this, but because of nostrdb's architecture, our requirements for types are pretty strict so it makes sense that we can't really make use of common libraries with other rust nostr projects.