nbd-wtf / go-nostr

Nostr library for Golang
MIT License
364 stars 102 forks source link

nip44.Encrypt() fails without a custom salt #131

Closed chebizarro closed 2 weeks ago

chebizarro commented 4 months ago

Hi there,

thanks for your hard work on this extremely useful library. I'm trying to implement nip59 and I've noticed that nip44.Encrypt() will fail unless you provide a custom salt through the applyOptions function. I'm not sure if this is intentional or not, but looking at the source code I noticed on line 56:

https://github.com/nbd-wtf/go-nostr/blob/3862333dfaad2b71909d4727109624a6b07f4284/nip44/nip44.go#L55-L60

that the randomly generated salt is assigned to a shadow variable named salt and not the variable named salt at the top level scope. I assume this is an error because there would be no need to generate a random salt if it's never used. It's a fairly trivial fix and if you agree that this is indeed an error, I can make a pull request. Thanks again!

chebizarro commented 2 weeks ago

Circling back on this I see that recent commits have made this issue redundant, so I'm closing it now

fiatjaf commented 2 weeks ago

I'm sorry, I have missed this issue completely. Hopefully this was fixed then, let me know if it wasn't.