Skip to content

Conversation

@dsbaars
Copy link
Collaborator

@dsbaars dsbaars commented Feb 14, 2025

Added NIP-04 and NIP-44 encryption (version 2) based on nostr-tools, also added examples and tests, based on the vectors provided at https://github.com/paulmillr/nip44/blob/main/nip44.vectors.json.
It does show some depreciated notices because of not yet updated dependencies.

Implements #83

@Sebastix Sebastix self-requested a review February 21, 2025 13:50
@Sebastix Sebastix self-assigned this Feb 21, 2025
@Sebastix
Copy link
Member

Sebastix commented Mar 9, 2025

@dsbaars thanks for this great PR for adding NIP-04 and NIP-44! In general these are my first comments when checking the code:

For consistency, in line with the other examples:

  • I would suggest to replace all newlines \n with PHP_EOL.
  • Surround the examples code in a try-catch

The NIP-04 and NIP-44 classes require the hex formatted key values, but there are no specific checks or expections returned when you enter a bech32 formatted key for example.
If you look at the Sign class at line 32, there is a check which converts a bech32 key to a hex formatted key (using the convertToHex() function in the Key class).

* Decrypt a message using NIP-04 (AES-CBC).
*/
public static function decrypt(string $ciphertext, string $privateKey, string $publicKey): string
{
Copy link
Member

Choose a reason for hiding this comment

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

If $privateKey and/or $publicKey are bech32 formatted:

  1. convert to hex formatted key
  2. throw Exception

* Encrypt a message using NIP-04 (AES-CBC).
*/
public static function encrypt(string $text, string $privateKey, string $publicKey): string
{
Copy link
Member

Choose a reason for hiding this comment

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

If $privateKey and/or $publicKey are bech32 formatted:

  1. convert to hex formatted key
  2. throw Exception

use swentel\nostr\Sign\Sign;
use swentel\nostr\Key\Key;

// Initialize key generator
Copy link
Member

Choose a reason for hiding this comment

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

You could surround the runtime code with a try-catch.

use swentel\nostr\Sign\Sign;
use swentel\nostr\Key\Key;

// Initialize key generator
Copy link
Member

Choose a reason for hiding this comment

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

You could surround the runtime code with a try-catch.

$event->setCreatedAt(time());
$signer->signEvent($event, $alicePrivKey);

echo "Original message: $message\n";
Copy link
Member

Choose a reason for hiding this comment

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

You could replace all \n newlines with a PHP_EOL

$charliePubKey = $keyGenerator->getPublicKey($charliePrivKey);
$charlieNpub = $keyGenerator->convertPublicKeyToBech32($charliePubKey);

echo "Generated keys:\n";
Copy link
Member

Choose a reason for hiding this comment

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

You could replace all \n newlines with a PHP_EOL

@Sebastix Sebastix assigned dsbaars and unassigned Sebastix Mar 9, 2025
@dsbaars dsbaars requested a review from Sebastix March 11, 2025 08:47
@Sebastix Sebastix merged commit c1edb2f into nostrver-se:main Mar 17, 2025
4 checks passed
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.

2 participants