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

Proposal of a very simple spec for p2p events #1331

Merged
merged 6 commits into from
Nov 4, 2024

Conversation

grunch
Copy link
Contributor

@grunch grunch commented Jun 27, 2024

This kind of events are being used on Mostro and lnp2pbot, the idea is to have an easy to implement specs for p2p platforms

@melvincarvalho

This comment was marked as spam.

@grunch
Copy link
Contributor Author

grunch commented Jun 27, 2024

n Network: The network used for the trade, it can be lightning, liquid, onchain, etc.

can it be used with testnet3/4?

sure

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Jun 27, 2024

I would change the goal from "p2p events" to "order events"

After all, the kind 38383 used here only defines an order to be encoded as an event, not a generic P2P message.

@KoalaSat
Copy link
Member

KoalaSat commented Jun 27, 2024

Ey @grunch this is awesome, I'm actively investigating exactly this for Robosats, I was taking a look to see if I could just use NIP-15 but would be awesome to work on a specific NIP for this, count with me 😁

@KoalaSat
Copy link
Member

KoalaSat commented Jun 27, 2024

n Network: The network used for the trade, it can be lightning, liquid, onchain, etc.

can it be used with testnet3/4?

Maybe it's worth to split it

  • n Network: mainnet
  • layer Layer: lightning

@KoalaSat
Copy link
Member

KoalaSat commented Jun 27, 2024

These are some suggestions

  • name or u Username
  • g Geohash: For F2F
  • bond Bond: The bond (in %) the maker and taker will lock to formalize the trade (Robosats specific)

@grunch
Copy link
Contributor Author

grunch commented Jun 27, 2024

Ey @grunch this is awesome, I'm actively investigating exactly this for Robosats, I was taking a look to see if I could just use NIP-15 but would be awesome to work on a specific NIP for this, count with me 😁

Hi @KoalaSat Thanks for your feedback, yes I think we should work together to implement this solution in more p2p to have a bigger pool of orders, let's go!

@grunch
Copy link
Contributor Author

grunch commented Jun 28, 2024

I would change the goal from "p2p events" to "order events"

After all, the kind 38383 used here only defines an order to be encoded as an event, not a generic P2P message.

yeah, makes sense

xx.md Outdated Show resolved Hide resolved
grunch added a commit to lnp2pBot/bot that referenced this pull request Jul 5, 2024
Following this nip proposal nostr-protocol/nips#1331
grunch added a commit to lnp2pBot/bot that referenced this pull request Jul 5, 2024
@jerryfletcher21
Copy link

This is really nice!

Two other tags that could be added can be escrow_duration (robosats specific but used in similar ways also in other platforms) and order_duration.
In case the maker and the taker do not lock the funds when creating and taking the offer (usually for lightning trades with bonds and not for onchain ones), escrow_duration is the time that they have to lock the funds before the trade effectivly starts (the chat opens).
order_duration is the maximum duration that the order can have after the trade started (in case escrow_duration is present, after the maker and the taker have locked the funds).

An other suggestion can be to replace layer with something like trade-protocol.
Right now most platforms have just one trading protocol non interoperable with other platforms, so it can be kind of redundand with platform. But in the future each platform could have different trading protocols (see for example in bisq 2 and in robosats), and maybe there may be even some platforms that share the same interoperable trading protocol.
Also a trade protocol can use two or more different layers (for example paying the bond on lightning and the rest onchain, or using a combination of lightning with liquid swaps), so layer is not enough.

@grunch
Copy link
Contributor Author

grunch commented Jul 15, 2024

This is really nice!

Two other tags that could be added can be escrow_duration (robosats specific but used in similar ways also in other platforms) and order_duration. In case the maker and the taker do not lock the funds when creating and taking the offer (usually for lightning trades with bonds and not for onchain ones), escrow_duration is the time that they have to lock the funds before the trade effectivly starts (the chat opens). order_duration is the maximum duration that the order can have after the trade started (in case escrow_duration is present, after the maker and the taker have locked the funds).

Hi @jerryfletcher21 thanks for your thoughts, I agree with this two new fields.

Now that you brought this, we should also let know to the client if the order will be deleted from the list after a period of time, we have this on lnp2pbot and mostro, we can call it pending_duration.

An other suggestion can be to replace layer with something like trade-protocol. Right now most platforms have just one trading protocol non interoperable with other platforms, so it can be kind of redundand with platform. But in the future each platform could have different trading protocols (see for example in bisq 2 and in robosats), and maybe there may be even some platforms that share the same interoperable trading protocol. Also a trade protocol can use two or more different layers (for example paying the bond on lightning and the rest onchain, or using a combination of lightning with liquid swaps), so layer is not enough.

I disagree with this one because there is not a trade-protocol, the trade it's over bitcoin protocol and other layers (protocols on top of the main protocol), if in the future we have the case that one trade have different layers we can add it to the layer field like this

{
...
   "layer": ["onchain", "lightning", "liquid"]
...
}

@KoalaSat
Copy link
Member

@grunch @jerryfletcher21 Don´t you think order_duration can be calculated from expiration - created_at

@jerryfletcher21
Copy link

jerryfletcher21 commented Jul 15, 2024

Now that you brought this, we should also let know to the client if the order will be deleted from the list after a period of time, we have this on lnp2pbot and mostro, we can call it pending_duration.

Is this not what the expiration tag does? I thought it can work like this: in the first event when the order is created expiration is set to current time plus what you call pending_duration, then when it is taken it is set to current time plus escrow_duration, and then finally, or directly if the escrow period is not present, to current time plus order_duration.

I disagree with this one because there is not a trade-protocol, the trade it's over bitcoin protocol and other layers (protocols on top of the main protocol), if in the future we have the case that one trade have different layers we can add it to the layer field

In my opinion simply specifing onchain or lightning (or any other lists of layers) is not descriptive enough for the user.
Maybe the user is using a platform that does lightning trades without bonds, and they are used to that trading protocol. This platform implements this spec and lists orders from other platforms that for example supports lightning trades with bonds. If the trading protocol is not specified, both orders will be categorized as lightning.
My idea is to instead categorize the firsts for example as lightning-no-bonds and the seconds as lightning-with-bonds.

@jerryfletcher21
Copy link

@grunch @jerryfletcher21 Don´t you think order_duration can be calculated from expiration - created_at

If order_duration is the duration that the order can have after it is taken then no, because when you create the order you can not know when the order will be taken.

@grunch
Copy link
Contributor Author

grunch commented Jul 15, 2024

Is this not what the expiration tag does?

Not necessarily, right now in lnp2pbot and Mostro is like this, we set expiration to now+24 hours, then the order is taken and then with the change to status is in-progress the expiration is now+24 hours but if the order takes more than 24h since that moment, the order will be discarded by relays.

I thought it can work like this: in the first event when the order is created expiration is set to current time plus what you call pending_duration, then when it is taken it is set to current time plus escrow_duration, and then finally, or directly if the escrow period is not present, to current time plus order_duration.

makes sense, it would be great to see if someone else have an opinion about this, I want to cover most use cases but I don't want to overcharge this spec

In my opinion simply specifing onchain or lightning (or any other lists of layers) is not descriptive enough for the user. Maybe the user is using a platform that does lightning trades without bonds, and they are used to that trading protocol. This platform implements this spec and lists orders from other platforms that for example supports lightning trades with bonds. If the trading protocol is not specified, both orders will be categorized as lightning. My idea is to instead categorize the firsts for example as lightning-no-bonds and the seconds as lightning-with-bonds.

Although I think it is not necessary to go so much to the specific ones I think adding this kind of detail doesn't add complexity so let's do it.

@KoalaSat
Copy link
Member

KoalaSat commented Jul 16, 2024

My idea is to instead categorize the firsts for example as lightning-no-bonds and the seconds as lightning-with-bonds.

not sure if I understood the intention of this, but doesn´t the tags "layer": ["lightning"]and "bond": 3 do the job? if the event does not contain "bond" you can assume no bond is needed.

@jerryfletcher21
Copy link

not sure if I understood the intention of this, but doesn´t the tags "layer": ["lightning"]and "bond": 3 do the job? if the event does not contain "bond" you can assume no bond is needed.

You are right maybe lightning-no-bonds and lightning-with-bonds are not the best examples.

For example with onchain there are different trading protocols whose differences are not covered by the other tags, for example onchain-2-of-2-mediation-arbitration (bisq), onchain-2-of-3-arbitration (peach and hodlhodl), onchain-custodial (what normal exchanges do, not sure if a p2p platform does it).
In the future there may be trading protocols that mix different layers in various ways (see for example Bisq Lightning in bisq2).
Also with lightning for example an other trading protocol (not supported by any platform at the moment as far as I know) can be lightning-full-trustless (see here).

I agree with @grunch though that is better to not add too many details and complexities, a trade-protocol tag string is sufficient I think.

@KoalaSat
Copy link
Member

The PR for Robosats is ready to be merged https://github.com/RoboSats/robosats/pull/1362/files#diff-7568a241dea682393e9d377f0f3426c46b9c6a7b5759d2835e0b60888a6f3a4bR39, it will stay "hidden" in the back-end so we can test it out in deep with real data, but after 24h coordinators should have published all available orders :)

In future PRs I'll implement an additional system to publish notes in relays out of the federation.

xx.md Outdated Show resolved Hide resolved
grunch added a commit to lnp2pBot/bot that referenced this pull request Sep 2, 2024
We were publishing all the movements on nostr but this could
be used to deanonymize users, so now we are working only with
those status on the specs

nostr-protocol/nips#1331
@KoalaSat
Copy link
Member

Hi all. After 1 week on production, I can confirm Robosats' federation is now fully publishing their global orders book to their own relays.

I'm extending the event with a couple of extra tags, but I consider them specific enough to not be taken into consideration for this NIP.

I need to work on a sync to wss://satstralia.com so the orders are available in clearnet.

In the mid-time, if any strfry runner is interested on automatically receiving these specific events, I'm happy to configure my coordinator for that 🙂

grunch added a commit to lnp2pBot/bot that referenced this pull request Oct 18, 2024
* Fixes to follow order NIP proposal

We were publishing all the movements on nostr but this could
be used to deanonymize users, so now we are working only with
those status on the specs

nostr-protocol/nips#1331

* Remove non used element from require

* avoid duplicate events

---------

Co-authored-by: Catrya <140891948+Catrya@users.noreply.github.com>
@pablof7z
Copy link
Member

pablof7z commented Nov 2, 2024

There seem to be a few implementations of this; is this ready to merge @grunch ? Pls update the NIP number and README 👌

@grunch
Copy link
Contributor Author

grunch commented Nov 4, 2024

There seem to be a few implementations of this; is this ready to merge @grunch ? Pls update the NIP number and README 👌

Done, thanks @pablof7z and all who contributed with this proposal 😃

@pablof7z pablof7z merged commit 1cb4a1f into nostr-protocol:master Nov 4, 2024
@grunch grunch deleted the p2p-nip branch November 4, 2024 19:51
@shocknet-justin
Copy link
Contributor

shocknet-justin commented Nov 8, 2024

requesting a change to the number, 69 has multiple implementations including nostr-tools and development resources like demo.nip69.dev, and r-sequential companion nip 68

#1460

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.

7 participants