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

NIP-47: some clarifications #1827

Open
yukibtc opened this issue Mar 5, 2025 · 5 comments
Open

NIP-47: some clarifications #1827

yukibtc opened this issue Mar 5, 2025 · 5 comments

Comments

@yukibtc
Copy link

yukibtc commented Mar 5, 2025

Hi, @johngribbin noticed that coinos is not including the block_hash field in the get_info response, causing a deserialization error.
Checking the NIP-47, only notifications field is marked as "optional", so I would assume that the others are mandatory, right?

And, always related to the get_info response: is the pubkey field the "Lightning Node's public key"? Because I see coinos is sending the nostr public key (32-bytes, the same used in the NWC URI) and not the lightning node one (33-bytes).

@rolznz @benthecarman @Semisol @asoltys @secondl1ght

@rolznz
Copy link
Contributor

rolznz commented Mar 5, 2025

I think all the fields except for methods should probably be optional. I think for custodial wallet services like coinos and primal a lot of these fields cannot really be filled in because the node is shared by many users. Same for nodeless implementations such as Liquid swaps / Ark and other future technologies I think these fields will not be able to be filled in and I'm not sure if any app even relies on them?

But here the pubkey should be the lightning node pubkey, it's probably best to be clear with a comment there too.

I don't think there is any benefit of re-including the nostr public key (32-bytes, the same used in the NWC URI) in the response since the client must have in order to do the request in the first place.

@asoltys
Copy link

asoltys commented Mar 5, 2025

I just went ahead and added the block_hash and changed the pubkey to our node id for coinos

@johngribbin
Copy link

I just went ahead and added the block_hash and changed the pubkey to our node id for coinos

@asoltys Thanks! This works - I was able to successfully make a connection with NWC from Clams to my Coinos wallet.

I am seeing a similar error when I call the list_transactions method using the nwc crate:

JJmissing field `payment_hash`

Per the NIP, I think this is the relevant section - https://github.com/nostr-protocol/nips/blob/master/47.md#list_transactions

Looks like payment_hash is considered required also?

@asoltys
Copy link

asoltys commented Mar 5, 2025

Thanks @johngribbin I've fixed that as well now coinos/coinos-server@118386e

@johngribbin
Copy link

Thanks @johngribbin I've fixed that as well now coinos/coinos-server@118386e

Awesome, thanks @asoltys !

All looks good on our end, cheers for the quick fixes!

yukibtc added a commit to rust-nostr/nostr that referenced this issue Mar 11, 2025
Make all `GetInfoResponse` fields optional, except for `methods`.

Ref nostr-protocol/nips#1827
Closes #795

Signed-off-by: Yuki Kishimoto <[email protected]>
yukibtc added a commit to rust-nostr/nostr that referenced this issue Mar 12, 2025
 Make all `GetInfoResponse` fields optional, except for `methods`.

 Ref nostr-protocol/nips#1827
 Closes #795

 Signed-off-by: Yuki Kishimoto <[email protected]>

Signed-off-by: Yuki Kishimoto <[email protected]>
yukibtc added a commit to rust-nostr/nostr that referenced this issue Mar 12, 2025
Make all `GetInfoResponse` fields optional, except for `methods`.

Ref nostr-protocol/nips#1827
Closes #795

Signed-off-by: Yuki Kishimoto <[email protected]>
yukibtc added a commit to rust-nostr/nostr that referenced this issue Mar 12, 2025
Make all `GetInfoResponse` fields optional, except for `methods`.

Ref nostr-protocol/nips#1827
Closes #795

Pull-Request: #803
Signed-off-by: Yuki Kishimoto <[email protected]>
yukibtc added a commit to rust-nostr/nostr that referenced this issue Mar 12, 2025
Make all `GetInfoResponse` fields optional, except for `methods`.

Ref nostr-protocol/nips#1827
Closes #795

Pull-Request: #803
Signed-off-by: Yuki Kishimoto <[email protected]>
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

No branches or pull requests

4 participants