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

Adds Zap splits to NIP-57 #552

Merged
merged 9 commits into from
Jul 31, 2023
Merged

Conversation

vitorpamplona
Copy link
Collaborator

@vitorpamplona vitorpamplona commented May 23, 2023

  1. Changes NIP-57's zap tag in events from one to many
  2. Deprecates LNURL and LNAddress in the tag
  3. Deprecates type attribute
  4. Adds an optional percentage for each zap receiver.
  5. Adds dynamic resolution of the lightning address of the receiver by zap tagging with pubkey
  6. Adds the suggestion to clients to display the Zap split on the screen.

While using lud06 and lud16 removes the need to download a user's profile metadata, pubkeys make it safer for users to know if a given lightning address is in fact owned by the user they want to send to. Clients can display ln addresses, but it would be better to display a picture + the name of the receiving user before confirming the zap split.

@Egge21M
Copy link
Contributor

Egge21M commented May 23, 2023

Love it. Aligned with all of these changes. I proposed something similar in the original zap-tag PR and also made it part of ZapGates

I even think that the use of lud06 and 16 should be discouraged because it can lead to stale zap targets on non-replaceable events.

Copy link
Member

@v0l v0l left a comment

Choose a reason for hiding this comment

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

This should have been in the first PR

57.md Outdated Show resolved Hide resolved
@staab
Copy link
Member

staab commented May 24, 2023

Can we not do integer percents? Percents are really weird because they're essentially for display. I'd strongly prefer a number from 0 to 1.

@vitorpamplona
Copy link
Collaborator Author

Can we not do integer percents? Percents are really weird because they're essentially for display. I'd strongly prefer a number from 0 to 1.

Then we need to come up with rules for what happens when the amounts do not match 100%. The weight system is just a generalization of percentages to avoid worrying about mistakes in the client creating those weights.

@Egge21M
Copy link
Contributor

Egge21M commented May 24, 2023

Can we not do integer percents? Percents are really weird because they're essentially for display. I'd strongly prefer a number from 0 to 1.

They are not percents, but weights. So you could do 1, 1, 2 which would mean 0.25, 0.25, 0.5

@staab
Copy link
Member

staab commented May 24, 2023

The weight system is just a generalization of percentages

Interesting! Leave it as is then, that's a clean solution. Maybe explain that in the NIP.

@jb55
Copy link
Contributor

jb55 commented May 24, 2023

why do we need lud16,lud06? can't we just have pubkey so we don't have to handle N different UI variations for this?

@Egge21M
Copy link
Contributor

Egge21M commented May 24, 2023

Interesting! Leave it as is then, that's a clean solution. Maybe explain that in the NIP.

@vitorpamplona maybe using weights that add up to exactly 100 is not ideal, because some people might think that those actually are numbers in percent. If you make it 1,1,2 for example and explain that that equals 0.25,0.25,0.5 it gets much clearer

@vitorpamplona
Copy link
Collaborator Author

why do we need lud16,lud06? can't we just have pubkey so we don't have to handle N different UI variations for this?

I am in favor in killing lud16 and lud06 options.

@vitorpamplona
Copy link
Collaborator Author

If we kill lud16 and lud06 options, do we want a relay address as 3rd parameter for consistency with other hex + relay tags or should we just move weights to the 3rd?

Option 1:

   [ "zap", "460c25e682fda7832b52d1f22d3d22b3176d972f60dcdc3212ed8c92ef85065c", "wss://relay.damus.io", "2" ] // 50%

Option 2

   [ "zap", "460c25e682fda7832b52d1f22d3d22b3176d972f60dcdc3212ed8c92ef85065c", "2" ] // 50%

I think consistency is good.

@jb55
Copy link
Contributor

jb55 commented May 24, 2023

option 1 is fine

@pablof7z
Copy link
Member

yeah, I like option 1 too and I'm in favor of killing luds, which will make it easier to display the splits

@vitorpamplona
Copy link
Collaborator Author

@v0l @staab let us know your position in terms of removing lud06 and lud16 from the zap tag. I can quickly modify this and start changing Amethyst's code to do hexes instead.

@staab
Copy link
Member

staab commented May 24, 2023

I'm in favor of option 1 as well

@v0l
Copy link
Member

v0l commented May 24, 2023

What is the relay tag for

@vitorpamplona
Copy link
Collaborator Author

What is the relay tag for

A tip for where to get the author's metadata from

@vitorpamplona
Copy link
Collaborator Author

Moved text to Option 1.

@v0l
Copy link
Member

v0l commented May 24, 2023

What is the relay tag for

A tip for where to get the author's metadata from

Option 2 for me

Copy link
Member

@cameri cameri left a comment

Choose a reason for hiding this comment

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

Option 1

@rolznz
Copy link

rolznz commented May 25, 2023

This would be one lnurl pay request per zap tag, right? Does this require mobile clients to have NWC to work practically? (otherwise a user would need to manually pay multiple invoices?)

@Egge21M
Copy link
Contributor

Egge21M commented May 25, 2023

This would be one lnurl pay request per zap tag, right? Does this require mobile clients to have NWC to work practically? (otherwise a user would need to manually pay multiple invoices?)

Yes, that is correct. NWC or any other QOL wallet connection like an integrated one, a connection to a node or Webln

@jb55
Copy link
Contributor

jb55 commented May 25, 2023 via email

@vitorpamplona
Copy link
Collaborator Author

We should advocate with wallets to allow a uri scheme were we can send many invoices to be paid at the same time for those who don't want to use NWC.

@arthurfranca
Copy link
Contributor

arthurfranca commented May 25, 2023

Regarding zap splits without NWC, I propose a simple solution (edit: github couldn't link to exact text: https://github.com/nostr-protocol/nips/blob/b9424fd454b124b5a61941d611f71336623cf582/96.md#monetization:~:text=If%20NIP%2D47%20isn%27t%20configured%2C). Taking advantage of the popularity of this thread, I ask client authors to review the related NIP-96 PR part (previous link) that talks about how file storage servers should get a share of the money on zap splits.

@vitorpamplona
Copy link
Collaborator Author

@Semisol @fiatjaf can we merge this?

@staab staab merged commit 5d63b15 into nostr-protocol:master Jul 31, 2023
@jb55
Copy link
Contributor

jb55 commented Jul 31, 2023

I can't even add this to Damus, yet now I'm "out of spec" on my own spec. This should have been a different NIP.

@jb55
Copy link
Contributor

jb55 commented Jul 31, 2023

I see that it says "SHOULD", so I guess it's fine.

@fiatjaf
Copy link
Member

fiatjaf commented Jul 31, 2023

Yeah, this looks very complicated. I would prefer that people don't do it, but.

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.

10 participants