The client to validate the message before sending it to the relay

[tcheeric]

See spec: https://github.com/nostr-protocol/nips/blob/master/01.md#from-client-to-relay-sending-events-and-creating-subscriptions

[avlo]

hi, eric. perhaps a good software engineering consideration re: above-

if/when you've got a nostr-java message validator in place, seems sensible for SC to use that same mechanism instead of coding up a separate one... and if you haven't yet started an implementation and/or have other/urgent/etc TODO's, feel free to assign this task to me.

[tcheeric]

Ok, noted. I am assigning to you, as I have not touched yet. Thanks.

[avlo]

starting this item now & btw- do you have a label/equivalent to indicate an item/issue/etc is "in progress"? i don't see one/similar in the current set:

if not, i think worthwhile & informative to add one.

[tcheeric]

Good idea. I thought I did, but I will create the necessary labels, if missing -- Secured with Tuta Mail: https://tuta.com/free-email 3 Jan 2025, 17:58 by ***@***.***:

…

starting this item now & btw- do you have a > label <https://github.com/tcheeric/nostr-java/issues/labels>> /equivalent to indicate an item/issue/etc is "in progress"? i don't see one/similar in the current set: labels-03.png (view on web) <https://github.com/user-attachments/assets/10b0f41b-f44c-41df-ab69-7dc53bcf0186> if not, i think worthwhile & informative to add one. — Reply to this email directly, > view it on GitHub <#187 (comment)>> , or > unsubscribe <https://github.com/notifications/unsubscribe-auth/ABQMG7FL2OMS4WVGT75XCAT2I3FVTAVCNFSM6AAAAABTZPISDCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNRZGYYTEMJQGY>> . You are receiving this because you were assigned.> Message ID: > <tcheeric/nostr-java/issues/187/2569612106> @> github> .> com>

[avlo]

PR #195 submitted

[tcheeric]

This looks good, thanks!

Re. the test failures.

[ERROR] Errors:
[ERROR] ApiEventTest.testNIP04EncryptDecrypt:144 » IllegalArgument Invalid hex string: [0256adf01ca1aa9d6f1c35953833bbe6d99a0c85b73af222e6bd305b51f2749f6f], length: [66], target length: [64]
[ERROR] ApiEventTest.testNIP04SendDirectMessage:107 » IllegalArgument Invalid hex string: [0256adf01ca1aa9d6f1c35953833bbe6d99a0c85b73af222e6bd305b51f2749f6f], length: [66], target length: [64]

I think the validation is not appropriate or incorrect in this specific scenario. If you look at the code sample in the spec:
https://github.com/nostr-protocol/nips/blob/master/04.md#encrypted-direct-message

let sharedPoint = secp.getSharedSecret(ourPrivateKey, '02' + theirPublicKey)

It expects that you add the '02' prefix to the public key when calculating the sharedpoint, which is why the length of 66 (and not 64).

My implementation is in nostr.crypto.nip04.EncryptedDirectMessage

    private static byte[] getSharedSecret(String privateKeyHex, String publicKeyHex) {

        SecP256K1Curve curve = new SecP256K1Curve();
        ECPoint pubKeyPt = curve.decodePoint(NostrUtil.hexToBytes("02" + publicKeyHex));
        BigInteger tweakVal = new BigInteger(1, NostrUtil.hexToBytes(privateKeyHex));
        return pubKeyPt.multiply(tweakVal).getEncoded(true);
    }

In general, if my understanding is correct, private and public key hex representations have different lengths:
Private keys: 64
Public keys: 66

[avlo]

gotcha & thx for pointing out that specification variant. to accommodate it, i've added a NostrUtil NIP04-specific (66-character, under the hood) method:

public static byte[] nip04PubKeyHexToBytes(String s)

and updated the PR to reflect its addition.

note: if testing message-validation related changes using SC, you'll need the most recent version of req_message-filters_validation branch

---

per your last paragraph, quick disambiguation on my part re: "in general":

In general, if my understanding is correct, private and public key hex representations have different lengths:
Private keys: 64
Public keys: 66

the "in general" context you're referring to is constrained to NIP-04, correct?

if i'm mistaken, pls advise as "in general (as per NIP-01)", public key lengths are spec'd as:

"pubkey": <32-bytes lowercase hex-encoded public key of the event creator>,

where each hex-encoded byte is composed of 2 hex digits (00, 01, ..., FE, FF) resulting in total string length of 32 * 2 == 64 characters as illustrated throughout the documentation:

["p", "f7234bd4c1394dda46d09f35bd384dd30cc552ad5541990f98844fb06676e9ca"]

"pubkey": "b0635d6a9851d3aed0cd6c495b282167acf761729078d975fc341b22650b07b9"

"pubkey":"00000000827ffaa94bfea288c3dfce4422c794fbb96625b6b31e9049f729d700"

"pubkey":"97c70a44366a6535c145b333f973ea86dfdc2d7a99da618c40c64705ad98e322"

pubkey: 8e0d3d3eb2881ec137a11debe736a9086715a8c8beeeda615780064d68bc25dd

"pubkey": "477318cfb5427b9cfc66a9fa376150c1ddbc62115ae27cef72417eb959691396"

[avlo]

perhaps worth noting, an obvious alternative to:

public static byte[] nip04PubKeyHexToBytes(String s)

could be a more generalized method requriing hex-string length as 2nd parameter:

public static byte[] hexToBytes(String s, int length)

which, although more flexible, i'd decided against since its potential misuse yields downstream repercussions that are safeguarded against by the submitted implementation

[tcheeric]

per your last paragraph, quick disambiguation on my part re: "in general":

In general, if my understanding is correct, private and public key hex representations have different lengths:
Private keys: 64
Public keys: 66

the "in general" context you're referring to is constrained to NIP-04, correct?

if i'm mistaken, pls advise as "in general (as per NIP-01)", public key lengths are spec'd as:

"pubkey": <32-bytes lowercase hex-encoded public key of the event creator>,

where each hex-encoded byte is composed of 2 hex digits (00, 01, ..., FE, FF) resulting in total string length of 32 * 2 == 64 characters as illustrated throughout the documentation:

["p", "f7234bd4c1394dda46d09f35bd384dd30cc552ad5541990f98844fb06676e9ca"]

"pubkey": "b0635d6a9851d3aed0cd6c495b282167acf761729078d975fc341b22650b07b9"

"pubkey":"00000000827ffaa94bfea288c3dfce4422c794fbb96625b6b31e9049f729d700"

"pubkey":"97c70a44366a6535c145b333f973ea86dfdc2d7a99da618c40c64705ad98e322"

pubkey: 8e0d3d3eb2881ec137a11debe736a9086715a8c8beeeda615780064d68bc25dd

"pubkey": "477318cfb5427b9cfc66a9fa376150c1ddbc62115ae27cef72417eb959691396"

You are definitely correct regarding key lengths. I had based my comment on cashu's implementation, and it seems there is a slight difference there...
Thanks.

[avlo]

just noticed this NIP-04 deprecation notification, if of any relevance to you:

wherein NIP-17 event spec appears to use 64-character pubkey hex strings (non-conclusive, brief examination) among additional encryption/security options & their related considerations.

[tcheeric]

Good to know, thanks. Nip 04, until recently, was still used in nip 46 to encrypt contents, but I've just checked and noticed 46 has now been updated to use 44 encryption instead.  We will probably have to completely remove 04 from the codebase... -- Secured with Tuta Mail: https://tuta.com/free-email 13 Jan 2025, 10:42 by ***@***.***:

…

just noticed this NIP-04 deprecation notification, if of any relevance to you: Screenshot_20250113_053143_GitHub.01.png (view on web) <https://github.com/user-attachments/assets/f05d6084-4ce7-438a-a71e-051d19af4373> wherein NIP-17 uses 64-character length pubkey hex-string lengths. — Reply to this email directly, > view it on GitHub <#187 (comment)>> , or > unsubscribe <https://github.com/notifications/unsubscribe-auth/ABQMG7FHONUCYSU2UYHRHKL2KOKA3AVCNFSM6AAAAABTZPISDCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKOBWG42TMMJRGI>> . You are receiving this because you were assigned.> Message ID: > <tcheeric/nostr-java/issues/187/2586756112> @> github> .> com>

[avlo]

You are definitely correct regarding key lengths. I had based my comment on cashu's implementation, and it seems there is a slight difference there...
Thanks.

i wonder if implementation/spec-enforcement shortcomings will have downstream data-validity/integrity consequences for cashu/users... with further concern correlated relay implementations must therefore necessarily also exist with incorrect/absent spec enforcement.

props to you pre-empting that defect early for us w/ validations. i'll do the same moving forward.