Skip to content

NIP-97 Attachments (binary content and media) #694

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

Closed
wants to merge 3 commits into from

Conversation

ondra-novak
Copy link

Implemented on wss://nostr-test.novacisko.cz/

Very simple demo client: https://nostr-test.novacisko.cz/files/index.html

Note that above relay is reset everyday at 00:00:00 UTC . Don't use it for other purposes.

FAQ

Why binary messages. NOSTR is not designed to transmit binary messages

I believe this is simply an excuse by people who have become comfortable with the status quo. That's why they are inventing special ways of encoding messages to "pass" binary content without taking full advantage of the potential of the technology that was chosen for NOSTR.

WebSocket was finalized into RFC 6455 in 2011. Now we are in 2023, 12 years later, programmers are afraid to use binary messages to transfer binary content. It's about on par with some email systems still relying on 7-bit Internet.

Introducing the transfer of binary content using binary messages solves a lot of issues. And if there are developers who use languages or libraries that do not support binary messages in websocket, please consider changing the library or even changing the language. Binary messages are supported by javascript in every browser.

The advantage of NIP-95 is that it does not need a special relay implementation. This NIP requires massive development on both the relay and client side

That's not true. If you read NIP-95 carefully, there are several requirements for how the relay should handle binary content - which is stored as base64. These are not trivial requirements. For example, the actual binary content is transmitted as an event, but it must not be indexed. This means adding a bunch of exceptions to the indexes.

If I want to have some control over the binary content, as a relay developer I have to do more programming, which I have to do in this proposal anyway. Plus some things are more difficult to deal with than using binary messages. For example, I have to allow the same interface for presenting normal events and the same for presenting NIP-95 events, yet handle them diametrically differently.

In the case of NIP-95, the relay cannot control the size of the binary content. The relay can also be configured so that text messages are short, but the binary content can be large, for example for storing videos, because the relay places attachments on a separate repository. This NIP also specifies in more detail how limits are communicated to the client. If any of the attachments are too large, the client can reduce the resolution or quality of the stored media to meet the limits set

An unresolved issue relates to garbage collection. If I want to deal with garbage collecting within NIP-95, I encounter various situations where a race condition can occur, so there can be a situation where someone uploads files but they can be deleted before the actual event is published. This is because the protocol treats each event as a separate record with no linkage to other records.

Binary content should be transferred using the HTTP protocol see NIP-96

No, it didn't. We're mixing protocols. What if the relay is not an HTTP server? Yes this can happen, an http server can be actually an upstream proxy. The NIP-96 design is also very complex

What if a client sends an event with the tag "attachment" but sends it via the EVENT command?

The FETCH command simply fails. Attachments aren't available.

How a client that does not support this NIP work?

This client can't see the attachments, only the content of the event

What if two clients upload the same attachment (how to resolve hash collisions)

Same hash = same content. The file is stored only once. The relay can discard duplicate content

In this NIP, the files are not signed, it is not possible to verify who sent them to the relay. NIP-95, on the other hand, also signs the file

The hash is always signed, not the content. In the case of NIP-95, the hash includes the author's pubkey, whereas in this proposal only the file hash itself is signed. This allows sharing the same file without reuploading it. And if you are worried about someone "stealing" ownership of the file. That may be the case with NIP-95, no signature will prevent a copy&paste operation. The only way to protect the file is to encrypt it

Why the tags do not have the form proposed in NIP-94

That was originally considered. I liked the compatibility with NIP-94. It would be possible to link the file directly to the metadata above NIP-94. But this proposal would not allow to attach multiple attachments to a single event. The motivation was to have it like Twitter, where there can be up to 4 images. Or like in Reels, where you can have short videos, but they can be linked into a longer continuous video. You can also implement a form of streaming longer videos in this way

Images in Kind:0, image placement in text, smileys, etc.

Just as I can place anything from tags using #[index], I can reference an attachment this way

Why does it have to be uploaded atomically?

This is related to the garbage collection requirement. It could happen that the client uploads attachments, but before publishing the event, the garbage collector comes and because the new attachments don't have a reference, it deletes them

Can a client initiate multiple ATTACH requests at the same time (i.e. without completing the previous one)?

This situation is not defined. For simplicity, let us assume that the answer is no, i.e. that a new ATTACH command within the same connection invalidates the previous ATTACH command if the event of that command has not been completed and published. This can possibly be discussed.

Why does the client have to explicitly link existing attachments to the new event (FETCH+ATTACH), wouldn't it be enough for the relay to do this automatically when it sees that some attachments are already present in the relay?

It would be possible, but it would significantly complicate the protocol flow. The client knows how many attachments it has to send. If the relay would add some attachments automatically, it would have to communicate this fact to the client. Presumably via an ATTACH response.

However, this still does not address the situation where there may be a race condition, where the same attachment appears on a relay before the request is completed. Therefore, it is better if the client handles the attachment management and the relay behaves passively in this case.

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Aug 5, 2023

I think we could just define a binary protocol. We don't need to mix text and binary and create a stateful mess to manage between the two. It could be a TLV with length of hash, hash as byte array, length of content, content. Clients would send a TLV in the binary channel.

@ondra-novak
Copy link
Author

You forget that everything revolves around the event and it is, was and will always be text-based. The text channel also controls the protocol communication and it doesn't make sense to me to create a secondary, binary channel with the same purpose

I also remind you that the attachment metadata are part of the event which is signed. So they have a double role. Firstly, you are announcing in advance how many attachments you are going upload, how big they are going to be and what type, but later also verifying who uploaded them and that hasn't been changed along the way (because the signature is in the event).

So in the end you have to send this metadata in text form anyway.

And what you're left with is just the binary content. The entire binary message is used for that without any extra processing (no TLV is needed). The whole binary message is the content.

I also completely disagree with the "stateful mess" criticism. The whole NOSTR is stateful. The REQ statement defines a certain state. Each relay must manage a state of the connected client. In my relay implementation, the class "Peer" (peer.h) is responsible for this. You can judge the complexity of the attachment state implementation by yourself (attachments.h, attachments.cpp)

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Aug 7, 2023

My point was that you don't need to "announce in advance" that you are going to send something. The server can just be listening to new attachments in the binary channel. We just need to move the hash in there so that the server can validate the payload.

And once you do that, you can also have a protocol in place to transfer any nostr event or filter over the binary protocol.

everything revolves around the event and it is, was and will always be text-based

The text is just the easiest serialization of the abstract data model. Nothing forbids Nostr from implementing other serialization structures.

@ondra-novak
Copy link
Author

"Announce in advance" is the key point!

It is part of a secure and controlled data transfer. The client has the possibility to find out that his attachment cannot be accepted before it is sent, for example because the relay has a limit defined. I can't allow clients to send a gigabyte binary message only to have the relay tell them at the end that their message was too big. I can also force regulation on content type, for example having a relay that only allows images (mime: image/*)

Announcing large transfers in advance is a technique that is commonly used, for example in the HTTP protocol itself (Except: 100-Continue).

And then there is the purely abstract view. We can look at an event as a text part and attachments. The whole event needs to be sent at once, and since part of it is text and part of it is binary, the whole transfer needs to be split into a text transfer and a binary transfer. It doesn't matter how it's implemented on the relay then, if the attachments are together with the event or separated.

And once you do that, you can also have a protocol in place to transfer any nostr event or filter over the binary protocol.
The text is just the easiest serialization of the abstract data model. Nothing forbids Nostr from implementing other serialization structures

But that's for another proposal.

What I'm a little sad about is that in the time it's taken to create a lot of relays and clients, no one has come up with a good proposal for how to transfer binary content over NOSTR. No one.

Who is this network for? Users prefer pictures, videos, instagram, tiktok.

Send your own NIP, we can discuss

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Aug 7, 2023

If the goal is just to check if the relay can receive it, you should use NIP-11 and advertise binary limits there, together with all the other limits. There is no need to check each individual upload.

@ondra-novak
Copy link
Author

ondra-novak commented Aug 7, 2023

If the goal is just to check if the relay can receive it, you should use NIP-11 and advertise binary limits there, together with all the other limits. There is no need to check each individual upload.

This one of goals. Not only one, see my first reply. Are you author of Amethyst? What about your application want to know in advance, how big attachment will process. As I remember, some large allocations on android can crash the application.

I still don't understand the arguments. What exactly are you arguing? If the whole message is about "we have to do it differently", then please write how to do it differently. Please write it as a proposal with a detailed description. I'm not going to do your homework.

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Aug 7, 2023

What about your application want to know in advance, how big attachment will process.

There is no need for it. First, the attachment tag already has size and the app will always have the nostr event before it downloads the attachment. Second, neither relays, not apps, can trust the other party or a nostr event to send or contain the right size, or hash, or mime-type. You have to design for conflict. This means the app or relay would just cut you off if the amount of data is too large. It doesn't matter what you claim the size is.

I still don't understand the arguments. What exactly are you arguing?

That the protocol can be a lot simpler and easier to implement without an additional state.

Please write it as a proposal with a detailed description. I'm not going to do your homework.

Relax man. I am giving you feedback from a client developer mindset. If you don't want it, why are you even publishing this here...

@ondra-novak
Copy link
Author

I cannot accept the feedback because I do not find it relevant. Based on our discussion, I have decided not to change anything in the proposal for now.

Copy link
Collaborator

@vitorpamplona vitorpamplona left a comment

Choose a reason for hiding this comment

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

Additional comments

The proposal introduces a new tag "attachment" with the following format

```
["attachment","hash_hex","size_in_bytes","mime_type","features1", "feature2", ....]
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could use a FileMetadata Event instead of this tag. https://github.com/nostr-protocol/nips/blob/master/94.md You can just use the already common x tag to be the hash of the attachment. There is no need to redefine all possible metadata tags here again.

Additionally, in that way, we don't run the risk of breaking relays out there that don't support infinite tag columns.

Copy link
Author

Choose a reason for hiding this comment

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

This is replied in FAQ section

Copy link
Author

@ondra-novak ondra-novak Aug 7, 2023

Choose a reason for hiding this comment

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

I can see struggle #697

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think all relays that use a relational database have a limit on the number of columns. We just don't know what those are :(

```


* One event can have more than one of these tags, then it carries more attachments.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using NIP-94 you would have just one event per attachment, which enables more interesting features like defining owners, liking, zapping, and reporting each attachment individually.

Copy link
Author

Choose a reason for hiding this comment

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

So in summary, you suggest one binary content to one event. You will need to define kind for such event. Similar to NIP-95
This is also addressed in FAQ section.

This will need to two indirections which can complicate some things. How do you implement stories (very popular facebook stories). How do you manage replaceable events. I want to have nice icon and background image in "Kind: 0" Who is responsible to erase suich unused events with attachments? Will your client implement a gallery or a file manager?

Copy link
Author

Choose a reason for hiding this comment

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

NIP-95 can't be used for Kind:4

Copy link
Collaborator

@vitorpamplona vitorpamplona Aug 7, 2023

Choose a reason for hiding this comment

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

you suggest one binary content to one event. You will need to define kind for such event.

NIP-94 IS that event. It already exists, it's being used and can easily do what you need here. There is no need to define a new one.

This will need to two indirections which can complicate some things.

Zaps have 1-2 indirections. The New DMs will have 4. Some group chats will have 5. NIP-94 also has redirections. It's really not a big deal.

Will your client implement a gallery or a file manager?

We already do. There is an online client as well: https://filestr.vercel.app/

Who is responsible to erase suich unused events with attachments?

Garbage collectors already work for Zaps that don't have an event, or likes that don't have an event. This shouldn't be a big problem. Orphan events are EXTREMELY common in Nostr and some relays already deal with them.

NIP-95 can't be used for Kind:4

They allow encryption, so I already used them like that. Same for NIP-94. I don't know why you would say it can't be used.

Copy link
Author

Choose a reason for hiding this comment

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

OK I can accept this, you suggest binary version of NIP-95 (in summary, thank for feedback)

Copy link
Author

Choose a reason for hiding this comment

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

Garbage collectors already work for Zaps that don't have an event, or likes that don't have an event. This shouldn't be a big problem. Orphan events are EXTREMELY common in Nostr and some relays already deal with them.

Which NIP defines garbage collection of orphan of events?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Which NIP defines garbage collection of orphan of events?

I don't think there is an agreed NIP for that and we might actually not need an agreement on how GCs are run. For clients, GC doesn't really matter. It's purely a relay software/operator choice. I assume most of them just do it to reduce costs (memory and database sizes).

Paid relays usually have tons of orphans because a paid user can cite an event that was signed by an unpaid user. Since the "reply" event author has paid, they don't delete those events. Thus, orphans are everywhere.

* Each relay defines the maximum attachment size and also the maximum number of attachments within an event
* Attachment can be attached to any "kind". Attachments for `kind:4` are always encrypted

The presence of the `attachment` attribute only informs the client that the relay should also have its own attachment content available in binary form. The client must download the attachments from the same relay to display them. See below
Copy link
Collaborator

Choose a reason for hiding this comment

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

SHOULD or MUST? If relays have the event, they MUST have the attachment as well. Otherwise, it gets a little messy on where to find it.

Relays that don't support this NIP will all have the event, but not the attachment. Thus, I don't think there is a good solution for MUST. Clients must always assume the relays might not have it and figure out a way to discover which relay has the attachment.

Copy link
Author

Choose a reason for hiding this comment

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

SHOULD as your client must be fault tolerant. Similarly, the image you point to via the URL may be missing

If the relay does not have an attachment, the client is under no obligation to look for it elsewhere. He just doesn't have it, it's not available. Broken image icon.


Upload
------
Single event must be published simultaneously with attachments. It is necessary to prevent the client from publishing the event but not uploading the relevant attachments. Similarly, it should not be possible to upload attachments first and then publish the event that references them. The operation must be atomic
Copy link
Collaborator

Choose a reason for hiding this comment

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

These are unnecessary rules. The order of events shouldn't matter. The garbage collection will take care of orphan attachments anyway. Why bother with the order?

Copy link
Author

Choose a reason for hiding this comment

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

It is not rule, it is an explanation.
You ran into the problem yourself below

Copy link
Collaborator

Choose a reason for hiding this comment

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

So you are saying that "Single event MUST be published simultaneously with attachments" is not a MUST? Can you change it?

Copy link
Author

Choose a reason for hiding this comment

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

There is nothing to force here, this is simply an explanation of why it is necessary. I don't understant

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then just replace MUST with SHOULD. It's just a better word.

relay: ["OK","<event-id">,true,"continue"]
client: <binary message for first attachment>
relay: ["ATTACH",<attachment-id>, true, "continue"]
client: <binary message for second attachment>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add the attachment-id to the binary content to avoid race conditions in multi-threaded, multi-connection environments. Nostr doesn't have a notion of atomic transaction for a sequence of messages and there is no need to create one for this.

Copy link
Author

Choose a reason for hiding this comment

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

No one here is talking about a multithreading environment.
The word atomic is meant in the sense of database transactions.
It means that in one state the relay does not have an event and attachments and in the other state it has the event and attachments at the same time. There should not be a situation where the relay will have an event without attachments or attachments without the event (in the latter case, it is more of a problem with the GC)

Copy link
Collaborator

@vitorpamplona vitorpamplona Aug 7, 2023

Choose a reason for hiding this comment

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

There should not be a situation where the relay will have an event without attachments or attachments without the event.

But there are.... In 2 particular ways:

  1. All relays that do not implement this NIP will have events without attachments and there is nothing you can do about it. You might not care about it, but I HAVE to. The client always has to deal with multiple relays out there. Even the crazy ones.

  2. Deleting events happen all the time these days. Deleting attachment tags from Replaceable events is also common. Either way, the GC is going to take its time. Meanwhile, for clients, your relay is offering attachments without events. As a client, I must ALWAYS expect that the relay is broken, because the GC hasn't run yet.

Because of 1 and 2, your atomic lock doesn't work.

There is no need to make guarantees you cannot keep as a relay. If you can't absolutely guarantee them at all times, there is no need to have them in the first place.

In other words: It's ok if you have orphan attachments. We can deal with it on the client side. You don't need to make a solution for that.

//event is published
```

The protocol flow is designed in such a way that it is possible to post other commands between individual phases. Possible implementation on a relay:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Without transaction states, there is no need to define this. It will be natively supported.

Copy link
Author

Choose a reason for hiding this comment

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

This rule is important because it allows you to send e.g. [REQ] commands at the same time as the upload. At the same time, you cannot influence whether the relay itself sends you an event in the meantime as a response to your REQ. It's just important to mention that the requests don't have to follow each other. That's why each answer contains the identifier of what it answers

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree, my point is that concurrent filters and sends are the default behavior of Nostr. There is no need to specify it.

You only have the need to specify because you are creating a temporary atomic transaction substate in each submission, which doesn't exist anywhere else in the Nostr protocol.


```
"limitation": {
"max_attachment_count": 4,
Copy link
Collaborator

Choose a reason for hiding this comment

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

The max count is unnecessary. Since each attachment is stored individually, events should have as many attachments as they like.

Copy link
Author

Choose a reason for hiding this comment

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

This rule is purely to protect the relay against bad users who can send an endless stream of attachments in order to carry out a DoS attack. Assume that the resulting transaction can have some maximum size. However, the limit is not mandatory, this rule only defines the existence of the given error message (and option). Relay may not require any limit. Alternatively, we can define a limit on the total size of all attachments in the event

Copy link
Collaborator

Choose a reason for hiding this comment

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

protect the relay against bad users who can send an endless stream of attachments in order to carry out a DoS attack.

But that rule doesn't protect against it. Remember, creating new users and new events has cost zero in Nostr. One can create millions of users while uploading the first attachment :)

Copy link
Author

@ondra-novak ondra-novak Aug 7, 2023

Choose a reason for hiding this comment

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

Yeah, but i mean transaction size


```
client: ["FETCH","<attachment-id>"]
relay: ["FETCH","<attachment-id>", true, "mime/type"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume the mime/type comes an the event id using the attachment. What happens when there are two events using the same attachment but with divergent time types?

Copy link
Author

Choose a reason for hiding this comment

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

I agree, this is not well defined.

The original purpose was to make work easier for the demo, where (see the link above) where you enter the ID of the attachment for fetch and it is not clear how it should be displayed. And also previously there was a LINK command that allowed you to get the URL to the attachment (see demo), and where the relay must know the content-type to correctly build HTTP response. However, here I might rewrite the parameters of the FETCH command like this

["FETCH",<event-id>,<tag-index>]

This further separates the presentation from the implementation. Attachment is visually part of the event, the fact that it is addressed internally by a hash is implementation issue

In this case, you will get mime from the attachment tag

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think the mime type is needed here. The app already has it from the event and tag.

```

* `"continue"` - accepted, continue sending attachments
* `"complete"` - event has been posted
Copy link
Collaborator

Choose a reason for hiding this comment

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

If a client uploads an event with 3 attachments but uploads only 2, and posts it, what happens? And what happens if the client tries to send the last attachment a few seconds later? Will the relay have deleted the event and attachments already? Will it allow a new attachment in? Will it just remain incomplete forever?

Copy link
Author

Choose a reason for hiding this comment

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

This is why there is a requirement for it to have transactional behavior (atomic)

By issuing the command ATTACH you open an transaction on the relay
while the transaction is active, garbage collector cannot delete attachments.
Transaction is automatically committed by posting the last attachment (complete status)
Transaction is automatically rollbacked when you close connection or when you issue another ATTACH command.

In my implementation, I store all attachments in the main storage immediately, however the transaction holds a special object AttachmentLock, which serves as reference to an attachment (so GC can see that reference). This lock lives on the transaction until it is committed or rollbacked.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would just not have the transaction scope at all and use a timeout for new files. GC can only delete after 5 minutes since posted. It's the same as a lock but you don't need to expect clients to behave (release the lock)

Copy link
Author

Choose a reason for hiding this comment

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

I have always problem with magic constants in a code.

Where I write anything about releasing lock?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think you can avoid the magic constant. You will need a time-out because the client might never finish uploading everything (it might/will crash)

As a client, the specific timeout a relay uses doesn't matter. I have to assume a broken database nevertheless.

@ondra-novak
Copy link
Author

To summarize. You would like to see the binary implementation as a binary version of NIP-95 and an extension of NIP-94.

Why haven't you written something like that already.Overall, it bothers me guys that NOSTR is in a state where we all know how not to do it but few are able to put together something meaningful. Please solve binary content, better than base64 sent over text. And better than using third party services.

I'll withdraw my proposal when an alternative proposal appears. I will not post another proposal

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Aug 7, 2023

You would like to see the binary implementation as a binary version of NIP-95 and an extension of NIP-94.

I'd like to see you building on top of things that already exist instead of recreating everything.

Why haven't you written something like that already

Mostly because I think NIP95 is not that bad. But you presented something new (thank you for that) and I am evaluating if we code it or not. In the end, every client will do the same evaluation I am doing.

Your proposal is not bad. It's also not that far out from NIP-94/95/96. But it significantly changes the base protocol and must be evaluated carefully.

I think your proposal here is smart and should be considered. But it also could use some help to fit in better with the others. That was my feedback.

@mikedilger
Copy link
Contributor

I like this NIP better than NIP-95 which is wasteful using base64.
Thank you for not trying to stuff everything into the Event structure.

programmers are afraid to use binary messages to transfer binary content

I've wanted to use binary since day-1, but nostr was already in use with a JSON object.

I have no problem using the binary websocket channel. We use Text, Ping, Pong and Close. It's not a stretch to use Binary. My code currently reads:

 WsMessage::Binary(_) => tracing::warn!("{}, Unexpected binary message", &self.url),

All my prior warnings about binary content making nostr more of a target are still of a concern (relay operators we will have to deal with CSAM once binary uploads are easy). But I'm not going to stand in the way of progress. It's just a sad prediction.

I haven't dug into the details of this proposal, but I wanted to make my position clear that this is IMHO on the right path for dealing with binary content.

@ondra-novak
Copy link
Author

ondra-novak commented Aug 8, 2023

You would like to see the binary implementation as a binary version of NIP-95 and an extension of NIP-94.

I'd like to see you building on top of things that already exist instead of recreating everything.

Believe it or not the original intent was to build it over the NIP-94. (link)

However, I wanted to address the current inability to add media to direct messages (kind 4). I wasn't sure if simply linking a file event to a direct message event was a sufficient solution, and how the encryption between the two would be handled. If I could see that encrypted file passing via direct messages works over, for instance, NIP-94 or NIP-95, I would probably have no reason to suggest different approach. But I haven't found any client that implements it that way

The second reason was the idea that messages with attachments are easier to understand because it is a common feature of email clients. If I built nostr on top of the email protocol, everything would be email and it could have attachments. Actually wait a minute, that already exists, it's called Network News Transfer Protocol.

Okay, so I'll summarize the original idea of expanding NIP-94 in the following points:

  • allocate a new kind
  • use tags from NIP-94 for metadata
  • one event is associated with only one binary file

However, the following issues still remain to be resolved

How to optimally pass binary content to relay. I still think the simplest way is the way I suggested using a temporary transaction existing in the client session (tagged as "stateful mess"). I'm looking at this from the perspective of the demo client creator, as I'm not a javascript expert, but from that perspective I found it much easier to minimize the necessary work with binary content, which is limited to just collecting the data, generating the metadata (calculating the SHA256 hash, using a library of course) and passing the binary data directly to the websocket. I have no idea how complicated it would be for me to construct some binary message using TLV notation

I still believe that we need to define new commands at the protocol level, because that way we can easily detect relays that don't support the new feature.

This also applies to download. NIP-95 relies on obtaining this special (base64 file) event via REQ and ID. Searching the index on my relay is not a trivial task given all the combinations the filter structure offers. At some level, I'm not even able to identify what event the the search found because it only works with internal IDs. So I would rather not mix event retrieval and binary content download. That's why I remain convinced that FETCH (or something similar) has a place here

@ondra-novak
Copy link
Author

(relay operators we will have to deal with CSAM once binary uploads are easy)

This already exists in the current implementation, but the burden of responsibility is shifted to the repository operators.

As a relay operator, I am aware of this responsibility and I am going to implement a feature that deletes any such content based on, for example, user reports. Therefore, the client must also be prepared for the eventuality that the referenced attachment/binary content is suddenly unavailable

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Aug 8, 2023

I wanted to address the current inability to add media to direct messages (kind 4).

I see. That is indeed not well defined in NIP-94 (or anywhere else). But the idea was to use a new tag to represent a nip04 encryption, similar to that aes-256-gcm but for the iv + recipient pub key of the NIP04 procedure. An important part was to separate the iv from the content and store only the content with external servers just to make it more difficult to decrypt.

Keep in mind there are lots of conversations to use new encryption procedures in DMs. Nothing settled yet, but there will be changes/new kinds for sure.

How to optimally pass binary content to relay.

Your approach seems to be the best indeed. But keep in mind that most Websocket implementations allow devs to send and listen to text and byte arrays independently. We could use that in our favor to modularize things. A relay could listen for text as usual and simply add a binary content receiver. That receiver receives the hash and content and stores it in disk, independent if there is an event for it or not. Later on, the GC cleans things up. Once the hash and the content is up, it doesn't really matter how other events use it.

Separating the two might allow operators to create relays software for binary content only. They wouldn't even implement the full Nostr protocol. They just need to understand the hash+content byte format.

I still believe that we need to define new commands at the protocol level.

I agree. The question is: are these commands the right ones? And do they block any future use of the binary channel?

So I would rather not mix event retrieval and binary content download. That's why I remain convinced that FETCH (or something similar) has a place here

Very interesting. I agree.

As a relay operator, I am aware of this responsibility and I am going to implement a feature that deletes any such content based on, for example, user reports.

If this is the case, then making a NIP-94 event for each attachment is better because it's super easy for Clients to implement reports on each NIP-94 attachment. You can also use per-attachment content labeling from NIP-31 as well as reactions/zaps for the attachments themselves. With an event per attachment, you also get a way to cite/quote the attachment in any other Nostr message via NIP-19. If attachments are individual events, they can also specify licensing restrictions and a zap-split that can be re-used when the same attachment NIP-94 event is cited somewhere else.

Now, NIP-94 is specifically designed for external content on other servers. Even though we are talking about that event kind here, I suggest we copy-paste the kind and make it an attachment header kind. Clients that receive this particular new event kind would then use the FETCH protocol to get the content.

Just to clarify, I am not against creating new tags. I am just against this way of coupling them together: ["attachment","hash_hex","size_in_bytes","mime_type","features1", "feature2", ....]. I would prefer type-value pairs of NIP-94 which are better if we have one event per attachment.

@ondra-novak
Copy link
Author

ondra-novak commented Aug 8, 2023

They just need to understand the hash+content byte format.

If the hash is just SHA256 of the content, you don't need to transfer the hash.

@vitorpamplona
Copy link
Collaborator

They just need to understand the hash+content byte format.

If the hash is just SHA256 of the content, you don't need to transfer the hash.

Sure, as long as there are no issues with partial/broken contents or other attack vectors, it should be fine. I assumed some large files (videos) will need to be transferred in multiple binary messages, but I don't have information on how big byte buffers in websockets are.

@ondra-novak
Copy link
Author

A very simple approach can be to allow the client to upload anything as binary content, store it, and after a period of time when no related event is sent, collect it as garbage. The related event should contain valid hash and valid size

But there's a catch. What if someone wants to use the NOSTR protocol at the binary level in the future, then it may be a problem to recognize whether the client is only sending binary content to be stored or whether this binary message is to be interpreted. That's why I went the " announce first, send later" approach. Personally, I think that any switching of the NOSTR protocol to the binary alternative will be handled differently, for example in the appropriate HTTP header in the initial handshake of the websocket protocol

@ondra-novak
Copy link
Author

But keep in mind that most Websocket implementations allow devs to send and listen to text and byte arrays independently

In my relay it at one place (link)

@ondra-novak
Copy link
Author

Sure, as long as there are no issues with partial/broken contents or other attack vectors,

Broken content should not happen in websocket realm. This is a fairly secure protocol. Even if you use fragmented message (continuation frame). The question of the buffers is more complicated. For example, my implementation concatenates all fragments of the message before sending it to the caller. There is, I think, a limit on the maximum size of the message that causes an exception that results to disconnect (with discarding the transmitted content)

@ondra-novak
Copy link
Author

I'm going to incorporate some changes into the proposal, and then probably resubmit it, closing this thread with a link to the new pull request (since there will be more changes).

If you have any more ideas, post them here (I'm reading this).

I'm currently figuring out the optimal names for the new commands.

  • Keep ATTACH and FETCH (however, the new proposal will not be about attachments)
  • Leave just FETCH and come up with another name for ATTACH
  • Don't seek a replacement for ATTACH, use EVENT and let the whole communication be more loosely defined, including the order of when to send binary content and when to send a tied event (and let the relay writers to suffer)
  • BIN_POST / BIN_GET
  • ???

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Aug 9, 2023

The simpler you make them, the easier to get traction.

STORE/RETRIEVE ?

We use EVENT to send JSON text. We could use BLOB to send binary.

Or if you find a name for this, you can use it directly, like "EVENT".

Keep pushing!

@ondra-novak
Copy link
Author

Closing for #719

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