nostr: synchronize nip47 fields with current spec

[daywalker90]

Description

notably adding state field for transactions : nostr-protocol/nips#1933

Checklist

• I followed the contribution guidelines
• I ran just precommit or just check before committing
• I updated the CHANGELOG (if applicable)

[yukibtc]

Thanks, I've fixed the conflicts and merged at 4e8e015

[afilini]

FYI: this breaks NWC with lnbits instances because they don't include a state field. It's probably a bug of theirs but I would suggest next time adding the fields as options so they don't completely break if the server is not compliant

[daywalker90]

FYI: this breaks NWC with lnbits instances because they don't include a state field. It's probably a bug of theirs but I would suggest next time adding the fields as options so they don't completely break if the server is not compliant

Well the spec change made it non-optional so best to comment there nostr-protocol/nips#1933 imo

[afilini]

Yes I understand you are just following the spec, I wanted to point out that not everybody upgrades the spec at the same time unfortunately

[yukibtc]

Checking again the NIP-47 spec, I think that doesn't really make sense to have the type, state and fees_paid fields in the MakeInvoiceResponse.

IMO those fields should be removed from the make_invoice response. In the other responses they make sense, but I'll probably update the state to be optional until everybody support it. I'm going to open a PR to revert some of these changes. I'll open also a PR in the nips repo to adj. the spec.

@afilini, thanks for raising this issue.

[yukibtc]

I've opened a PR here to update the code, and I'm waiting for a reply regarding the make_invoice response here.

[yukibtc]

@afilini, from commit 36cc4bb the issue should be fixed. I've removed type, state and fees_paid, as they don't make sense there, and made all other fields optional, apart for invoice.