nostr-protocol / nips

Nostr Implementation Possibilities
2.39k stars 582 forks source link

NIP-68 Lightning Debit Requests #1529

Open shocknet-justin opened 1 month ago

shocknet-justin commented 1 month ago

Ad-hoc requests for lightning payments that provide more intuitive and secure UX, while also leveraging Nostr as identity for the requesting service

Combined with prNIP69 this creates a more robust and scalable successor to NWC

https://demo.nip69.dev/nip68.html

shocknet-justin commented 1 month ago

UX vid clip

https://github.com/user-attachments/assets/528cacfc-d250-43de-976c-43354834abfe

vitorpamplona commented 1 month ago

I am confused. Isn't this exactly NWC, but just different event kinds and json payloads?

I can't see why would you implement another protocol if all it does is to pay the same old ln invoices that apps already gather and request payment via NWC...

shocknet-justin commented 1 month ago

I am confused. Isn't this exactly NWC, but just different event kinds and json payloads?

I can't see why would you implement another protocol if all it does is to pay the same old ln invoices that apps already gather and request payment via NWC...

It's the polar opposite, where NWC requires user side provisioning of keys for apps and services, this allows for persistent identities and ad-hoc connections. The static pointers then enable the discovery methods.

Both are for lightning invoices sure, invoices are the standard for the underlying payment protocol, but this addresses the many deficiencies of NWC's backward design at the app level.

vitorpamplona commented 1 month ago

where NWC requires user side provisioning of keys for apps and services, this allows for persistent identities and ad-hoc connections

But doesn't that create a massive security issue? Those keys were added to make sure that if the private key leaked, which is inevitable given the number of clients out there, the attacker could not spend the funds.

shocknet-justin commented 1 month ago

doesn't that create a massive security issue? Those keys were added to make sure that if the private key leaked, which is inevitable given the number of clients out there, the attacker could not spend the funds.

Not sure I follow, you mean the app/service key requesting the payment? In a purely client to client scenario that would still be ephemeral keys. In a service to wallet user scenario, the wallet user is still inherently a sub-scope to the nodein our implementation via the node middleware. Leaked service keys present no added risk to the node as a whole.

NWC fumbling over of keys on the otherhand creates intercept vectors from the start, that's the present security issue.

Also, by using this we enable co-established permissions which facilitates ACLs being more accurately scoped. The added WoT potential with reputational keys creates scaffolding for even more advanced rulesets and automated revocation (behavioral reporting mention in the doc)

vitorpamplona commented 1 month ago

In a purely client to client scenario that would still be ephemeral keys

What do you mean by ephemeral keys? There is no mention of ephemeral keys in this NIP. Where is that coming from? And how can ephemeral keys replace the usual NWC keys if they are ephemeral (one time only, right?)?

This feels like NWC without the extra security of the per-key/client budget selections, which is a non-starter to me.

On your process flow, can you clearly mark all the stakeholders involved? If user A is paying user B, who is running this service, and who is pinging what and when throughout the process?

shocknet-justin commented 1 month ago

In a purely client to client scenario that would still be ephemeral keys

What do you mean by ephemeral keys? There is no mention of ephemeral keys in this NIP. Where is that coming from? And how can ephemeral keys replace the usual NWC keys if they are ephemeral (one time only, right?)?

This feels like NWC without the extra security of the per-key/client budget selections, which is a non-starter to me.

On your process flow, can you clearly mark all the stakeholders involved? If user A is paying user B, who is running this service, and who is pinging what and when throughout the process?

I think I see the disconnect, we're talking disparate use-cases. I mean ephemeral as in per-key/client and not reused by a central service.

For an Amtheyst user there is no negative tradeoff security-wise, but this is infinitely better UX.

Because Amtheyst is just another client operated by the user, there's no central service, the privatekey would be generated locally in the Amtheyst client (much more secure than NWC where the user has to create it and then fumble it). This key created by their Amtheyst client is never re-used anywhere else.

To pair Amtheyst, user would enter a NIP-05 address, Amtheyst would look up their NIP-68 string, and use it (with a new key) to request permissions. User accepts in their wallet, thereby whitelisting (and optionally adding scope) the key created by Amtheyst. Better security, better UX.

Does this address your concerns? I'll revisit the process flow accordingly if so

vitorpamplona commented 1 month ago

Ok, so the main point is to create the secret in the client instead of the wallet app.

I mean ephemeral as in per-key/client and not reused by a central service.

The secret on NIP-47 is not reused by the central service. It is created by the service, but every connection gets a new one with a budget for it.

Wouldn't that be solved by re-using NIP-46's client-generated nostrconnect URI? I think multiple people have suggested that and we could easily add it to NIP-47 to create local secrets. If that is the only improvement over NIP-47, we should probably just change NIP-47 instead.

Why does the user have to put the NIP-68 string publically in the metadata? That reveals the wallet address (via relay) to everybody else and an attacker may try to connect to that relay and watch the payments. We could just place the pubkey + relay in the app when setting up payments and save it locally, in the same way we do it today. Otherwise, the relay used by the wallet can just be spammed with connection requests from attackers out there.

shocknet-justin commented 1 month ago

secret on NIP-47 is not reused by the central service

Right, and that's a downside both in terms of UX and the inability to leverage reputation in crafting policies... for a service it's also not scalable.

NIP-46's client-generated nostrconnect URI?

The UX of that is not great generally and the reliance on a new protocol handler is a non-starter for web-first apps

Why does the user have to put the NIP-68 string publically in the metadata?

They don't have to, its just for optimal UX, they could paste a string instead as done today sure... but because its a listener pointer it can be reused in autocomplete etc too. In our wallet the metadata/nip05 option is a check box thats unchecked by default, they must opt-in to publishing. The NIP-05 address itself need not necessarily be linked to a primary identity, our nip05s are explicitly wallet based not social... like a random named lightning address from a custodial wallet... only not custodial.

the relay used by the wallet can just be spammed with connection requests from attackers

If someone decides to publish their string they're still not going to be adding any spam attack vector that doesn't already exist on relays, its true of all services including lightning nodes themselves. Fortunately Nostr is a great tool for mitigating griefing attacks with reputation systems, there's a lot of upside to lean into that. Privacy is not unlike DM metadata but with even more deniability because a payment relationship is indistinguishable from a req/gfy, and might even encourage more read authenticated relays.

vitorpamplona commented 1 month ago

that's a downside both in terms of UX and the inability to leverage reputation in crafting policies

In NIP-47, the connection is started by the wallet with the user already logged in. The reputation is already provided by the login of the wallet itself. There is no need to check the reputation of the incoming request.

for a service it's also not scalable.

I don't get this. Alby seems to have scaled this just fine.

If someone decides to publish their string they're still not going to be adding any spam attack vector that doesn't already exist on relays

Sure, but there is no need to let everybody else know which relay you are using for wallet services. Adding it publically is one more metadata leak in Nostr.

--

But besides this, is there anything else here that is different? The rest seems like the same old NIP-47. So, why re-inventing the wheel?

shocknet-justin commented 1 month ago

In NIP-47, the connection is started by the wallet with the user already logged in.

This is part of the bad UX I'm trying to solve, its very uintuitive. Remember we're not talking nerds purposefully wiring up a client, this is a more OAuth-like flow that's more palatable to services doing retail billing.

Alby seems to have scaled this just fine.

Lightning barely has any users and the ones it has barely do any transactions. Down the road, you can scale bad code horizontally for sure, doesn't make it cost effective.

adding it publically is one more metadata leak in Nostr

That's why it's opt-in, because even without this part of it the flow is still superior. I don't think we'll agree on this concern specifically because there is no objective answer, may as well give users these options and see. NWC UX is failing people bigly today.

The rest seems like the same old NIP-47. So, why re-inventing the wheel?

The only similarity is that they use Lightning and Nostr, 47 is just bad for all the reasons mentioned already. One failed experiment doesn't mean we're finished. If its similar as you perceive it then it wont be hard for apps to upgrade when they start losing share to apps with better UX.

vitorpamplona commented 1 month ago

This is part of the bad UX I'm trying to solve, its very intuitive.

I don't think this PR makes it any better. In the end, the user must approve the creation of the new budget as soon as the first connect happens. This is either via relay in this PR or via the regular NIP-47 process, which is just a link to the browser for login.

Today, for instance, the process for Alby is very easy. Without any previous setup, the user clicks in the alby icon in the app, which takes him to https://nwc.getalby.com/apps/new?c=Amethyst. User proceeds to log into alby in the browser or their app, approve the budget and then the wallet creates the URI back to Amethyst.

Either way, the user must be logged into the wallet to approve the transaction and set up the budget with all the restrictions for this app.

That's why it's opt-in,

There should be no option to do this. It just makes things worse. Users should never reveal which relay they use for payments to external parties. It doesn't make any sense.

47 is just bad for all the reasons mentioned already.

All you talked about was the kickstart of the process. There are no other "reasons".

One failed experiment doesn't mean we're finished.

NWC is not a failed experiment. I don't understand why you are so negative about it given that thousands of users do NWC just fine.


Again... This PR doesn't seem to create anything new. There is no "better UX" here. In fact, one might even say that the more secure idea of generating the secret by the client before trying to connect makes the UX actually worse for users. Now there are more points of attack than before.

shocknet-justin commented 1 month ago

the user must approve the creation of the new budget as soon as the first connect happens

This is async because it introduces a budget request, something NWC doesn't have. I also think you're still focused on the decentralized client use-case, services are a much bigger problem.

Alby

They run a lot of custodial infrastructure to make it work and get around its flaws, the protocol handler requirement doesn't work for webapps without the extra infrastructure they capture users with.

There should be no option to do this

You're concerns on this are inconsistent as you were very enthusiastic about NIP-69 which is the same just reversed payment flow. The spam/metadata concerns are also not specific to this, the arguments you're using are arguments against nostr, not this.

kickstart of the process

UX of the process, but also the scalability and insecurity of fumbling keys. Most importantly, flexibility.

NWC is not a failed experiment. I don't understand why you are so negative about it

Failed is subjective, sure, but your arguments seem irrationally complacent with it. Are user complaints and irrelevant adoption a success story? We're not talking about uprooting DNS or TCP/IP here.

more points of attack than before

All concerns about Nostr, not this. You're just arguing against using Nostr for things, but at the same time this doesnt do anything new?

vitorpamplona commented 1 month ago

This is async because it introduces a budget request, something NWC doesn't have

We do request budgets right now, so it does have. It's just not through a relay. You clearly haven't coded NIP-47. Otherwise, you wouldn't get basic things wrong like this. Which is not helping your case here.

Also opening the possibility for anyone to make a budget request adds risk. It allows anyone who gets hold of your private key to submit a budget request and steal funds. Currently, NIP-47 requires the wallet to be in the same device or for the user to start from the wallet. This adds extra security for the inevitable time when the user's key leaks.

I also think you're still focused on the decentralized client use-case, services are a much bigger problem.

Can you talk more about this? You haven't really explained what this is.

They run a lot of custodial infrastructure to make it work and get around its flaws, the protocol handler requirement doesn't work for webapps without the extra infrastructure they capture users with.

I don't understand what any of this have to do with their centralized infra. Mutiny Wallet was running NWC without any custodial service, directly from the app in the phone or the PWA. So, I don't really understand what you are trying to say.

UX of the process, but also the scalability and insecurity of fumbling keys. Most importantly, flexibility.

You keep saying UX, but you don't explain what it actually is this UX improvement. Do you mean just the kickstart process or is there something else (I have asked this question 4 times already and you have not responded).

If it is just a kick-start, you can submit a change to NIP-47 with a way to start the request from the client. The rest is the same.

Are user complaints and irrelevant adoption a success story?

I literally don't have ANY user complaint that this specific PR will solve, even after using NWC for 1.5 years with over 200,000 users. I have lots of other issues that need to be solved, but this PR definitely doesn't solve them.

I am trying to understand what you want to improve, but you clearly do not want to explain it. You just keep saying that it will improve UX without actually saying what is this UX that you are talking about.

All concerns about Nostr, not this. You're just arguing against using Nostr for things

Not really. On Nostr, you can choose to make things available or not to other users. It's a choice of every NIP. Here you are just making things available that have no usage for other users, besides enabling scammers and phishing attacks.

vitorpamplona commented 1 month ago

Strong NACK on this idea.

First of all, this is not a replacement for NIP-47 as the author implies. NIP-47 is doing much more than just paying invoices at the moment and we can't have 2 NWC-like NIPs.

This PR does propose a new protocol for client-based initiations of the Nostr Wallet Collect protocol. It's just mixed with an unnecessary re-specification of the current NIP-47 payment calls.

My suggestion is to rewrite this NIP, delete most of it, and resubmit the client-initiation protocol alone as a new section on the NIP-47 spec. In that way, all NWC implementers can weigh in on the privacy and security of the new client-initiation procedures.

bumi commented 1 month ago

I could not fully follow the comments above and I hope we can collaborate on one spec and not add some fragmentation which just makes it hard for any developer.

The motivation sounds to me it builds on invalid assumptions. The claim that in NWC the wallet MUST create the secret and share the secret is wrong. In fact several applications do that differently. With improvements on the one-click connection flow (NWA and other ideas) there is already a lot of work being done to improve the UX of the initial handshake. I invite everybody in contributing to that. Recurring and non-interactive payments are one of the main features why NWC was started. Having a key for each app connection is a security and privacy feature and allows users to easily revoke any app connection at any point.

shocknet-justin commented 1 month ago

We do request budgets right now

47 mentions budgets exactly 1 time and in reference to it being set wallet side. Are you referring to NWA? That might be close but its still dependent on a protocol handler.

the possibility for anyone to make a budget request adds risk

So now you're saying 47 doesn't do what you just said it does?

anyone who gets hold of your private key

So we shouldn't use Nostr, SSL, SSH, or anything else? This is an absurd objetion in this context, everything uses private keys. This NIP does not use them in some special way, each connection is a different key that can be scoped and revoked just as before.

NIP-47 requires the wallet to be in the same device or for the user to start from the wallet. This adds extra security for the inevitable time when the user's key leaks.

Sorry but I still have no idea what you're talking about here, in both scenarios it's child keys being explicitly whitelisted and scoped and revocable.

You haven't really explained what this is.

Example: A hosting provider invoices a user every month for services. This allows that provider to, using their well-reputed key, ask a user (via their wallet) to agree to these terms. It's also flexible enough for this amount to adjust based on fiat exchange rates. You say NWC/NWA can do this? I've see no evidence of that.

Mutiny Wallet was running NWC without any custodial service

They're dead because their infra was one workaround after another. This is a fatal flaw or reliance on protocol handlers, you either rely on app store approval or a centralized namespace like Mutiny's PWA or Alby service enrollment, the exact type of thing Nostr is great at solving.

what it actually is this UX improvement

I haven't used NWA so maybe they can be similar, but as for straight NWC the requirement of users to install a native app or be outsourced to a 3rd party website to trigger deep linking.

If there's something that achieves the equivalent of a DM to a wallet where all the user has to do is accept in the wild, let me know.

submit a change to NIP-47 with a way to start the request from the client. The rest is the same.

By your own admission its more than that, now NWA for budgets enters the conversation and that's not used anywhere. Even our error codes create more flexibility for things like fiat exchange rates and more.

after using NWC for 1.5 years with over 200,000 users

How many of those downloads actually use it? I doubt its more than a few dozen, thats 199k+ complaints then because they're non-users. I have Amtheyst on my phone and don't even see anything related to NWC, attempting a zap appears to just provide a regular lnurl/invoice in my wallet. Don't take a victory lap just yet this stuff is all irrelevant still.

You just keep saying that it will improve UX

You keep saying this does nothing new then complain about all the new things you don't like. Pick one narrative and stick with it.

this is not a replacement for NIP-47

Would you prefer I call it a replacement for LNURL-W? You seemed to like my replacement for LNURL-P which is exactly the same thing.

new section on the NIP-47 spec

This is just bikeshedding event kinds and fields, which are irrelevant to DX because its all abstracted into Nostr-tools (which we've provided) anyway

shocknet-justin commented 1 month ago

improve the UX of the initial handshake

I'm glad we agree work is needed in this area, I submitted this only after implementing to ensure we have a flexible and complete solution.

Having a key for each app connection is a security and privacy feature and allows users to easily revoke any app connection at any point.

This does that as well, I'm not sure where the impression comes from that it doesn't. Even the video demo shows this.

vitorpamplona commented 1 month ago

How many of those downloads actually use it? I doubt its more than a few dozen,

You are kidding, right? This is one of the most used features for years.

I have Amtheyst on my phone and don't even see anything related to NWC, attempting a zap appears to just provide a regular lnurl/invoice in my wallet. Don't take a victory lap just yet this stuff is all irrelevant still.

Click and hold the Zap button. Click on the alby icon, log into Alby, approve a budget, Alby will send you back to Amethyst, hit Save and done.

shocknet-justin commented 1 month ago

Click and hold the Zap button. Click on the alby icon, log into Alby, approve a budget, Alby will send you back to Amethyst, hit Save and done.

Cool easter-egg, but sending users to a custodial account system or scanning a QR code is an abomination before god and man.

I do see a clipboard icon, but it doesn't seem to do anything? Nothing is getting clipped, lmk if I should file an issue.

The 3 input fields below it should be 1 input field that accepts either a NIP-05 (publish based enhanced ux) or NIP-19 (copypasta UX that addresses your privacy concerns).

vitorpamplona commented 1 month ago

Sure, we can implement those screen tweaks. None of that requires recreating NIP-47.

If you can imagine a better client initiation procedure for NIP-47, I am happy to code it there.

shocknet-justin commented 1 month ago

better client initiation procedure

change the nostr-tools import from 47 to 68, voila!

I enjoy Amtheyst already so I'm happy to kick in a bounty for it.

vitorpamplona commented 1 month ago

change the nostr-tools import from 47 to 68, voila!

It took us 1.5 years to replace NIP-04 with NIP-44. They have exactly the same API and most people still haven't changed their clients (we are 8 months after this "nostr-tools" change).

This PR is not a simple switch like that, it requires new screens, modified calls, modified caching, etc. You said yourself, it's a UX change.

It's going to move a lot faster if you simply add the section to NIP-47

shocknet-justin commented 1 month ago

Fair point re: NIP-44, but to take full advantage of the UX improvement is still going to require those front-end changes regardless of whether we call it 47 or 68.

I think it would create even more confusion and effort to publish this as a 47++ / v2. There's enough new concepts here it warrants dis-ambiguity, and a distinct NIP has the side benefit clearer communication to users that there's been progress. Let users decide whether they're happy with the NIP # they know, or want try something new. Since NIP-44 didn't change UX at all, that may be a factor in the slow upgrades that wouldn't apply here.

As shown in the demo app I've included, it's extremely simple to implement from scratch anyway, so simple that apps with existing 47 would be better served by simply dropping this alongside it and just auto-detecting whether the metadata exists or regex in an input field for the NIP-19 pointer.

vitorpamplona commented 1 month ago

There's enough new concepts here it warrants dis-ambiguity

I don't know if that's true. I do not see anything more than the new client-initiated handshake. The other stuff has been tried in NIP-47 and the many PRs that followed. Even a client-initiated handshake has been tried there, like https://github.com/nostr-protocol/nips/pull/851 which was debated at length and is already implemented in at least 4 clients, for instance.

And keep in mind, there is a rule in this repo that there should be no more than one way of doing the same thing.

shocknet-justin commented 1 month ago

I do not see anything more than the new client-initiated handshake

The budget request, even if sharing aspirations with pr67, is new vs 47 as it sits.

NWC strings would also be confused with ndebit strings, a lack of dis-ambiguity by their co-existence in one NIP sounds like a colloquial nightmare.

other stuff has been tried in NIP-47

Maybe this stuff keeps coming up because back-porting things into 47 complicates things unnecessarily, adds bureaucracy, and creates re-work. The string example above is just the beginning.

4 clients

Two are by the author and dead. Even so, this has at least as many, I referenced our 3 plus a library that has more TBA projects building on it.

there is a rule in this repo that there should be no more than one way of doing the same thing

And that makes perfect sense, particularly when talking about public display things like avatars and notes that are ubiquitous user properties. Fortunately there is no public side of 47, so 68 is not redundant. This exactly why 69 hasn't gotten this push back, we're introducing new user properties in both 68 & 69.

The documents are also very clearly oriented toward different use-cases, client pairing vs. service pairing, despite both being broadly applicable. Well, not actually, 47 could be more broadly applicable, and make this PR unnecessary, if only there wasn't such bureaucracy in fixing it to begin with.

I agree that having had to implement this from scratch was unfortunate, but the body trail of 47 amendments makes it clear that it was in fact necessary.