Replies: 12 comments 6 replies
-
|
hi, eric. happy to help troubleshoot. if not using superconductor for your relay, can you point to where i can obtain either an (linux) executable to your relay or source-code/repo so i can build and run one locally? i'll then try an replicate your parse-error issue and get back to you. re:your bottom 8 Errors related to:
these are NIP-42, Authentication of clients to relays related. nostr-java tests are not (currently) implemented with NIP-42 considerations but it appears your relay requires NIP-42 before it'll accept client events/requests. so you'll need to either:
|
Beta Was this translation helpful? Give feedback.
-
|
btw, i've pushed a minor change to ApiEventTestUsingSpringWebSocketClientTest to obtain then iterate relay values from relays.properties file instead of the hardcoded localhost:5555 i'd had in tests before. doing a this should help us both troubleshoot your current issue a bit easier and once we've resolved current issue, i'll update all remaining tests containing hardcoded localhost:5555 to also use relays.properties |
Beta Was this translation helpful? Give feedback.
-
|
@tcheeric, also btw- if using superconductor for (any one of your) relays- i've updated its Relay Endpoint for Clients documentation with wss/ws configuration instructions. |
Beta Was this translation helpful? Give feedback.
-
|
@tcheeric, something just came to mind re: your relay log first line:
appears to have JSON event message id ("2b613d13abd9be78fd7d335b351d0ebcf8224fd120ed46b342043d851a1aab61") element occurring before JSON event ("EVENT") element instead of after it. can you retry doing if you're still having same test failure after that, you might also want to completely start your xyz/tcheeric .m2 repo fresh, even if you've done so already, no harm in re-doing to establish certainty:
followed by any variant of your btw, i've also just updated EventMessage.encode() to use Optional.ofNullable instead of explicit null check when performing |
Beta Was this translation helpful? Give feedback.
-
|
Thanks Nick. I'll follow your instructions and will switch to superconductor before testing again.These changes will greatly simplify relay communication.
…--
Secured with Tuta Mail:
https://tuta.com/free-email
17 Dec 2024, 07:06 by ***@***.***:
@tcheeric <https://github.com/tcheeric>> , something just came to mind re: your relay log first line:
>
> "["2b613d13abd9be78fd7d335b351d0ebcf8224fd120ed46b342043d851a1aab61","EVENT",
>
>
appears to have JSON > event message id> ("2b613d13abd9be78fd7d335b351d0ebcf8224fd120ed46b342043d851a1aab61") element occurring > before> JSON > event> ("EVENT") element instead of > after> it.
can you retry doing > git pull> of my repo > java21 branch <https://github.com/avlo/nostr-java-avlo-fork/tree/java21>> before doing any variant of > make clean install> , then retry your tests.
if you're still having same test failure after that, you might also want to completely start your xyz/tcheeric .m2 repo fresh, even if you've done so already, no harm in re-doing to establish certainty:
>
> rm -rf <path><to><your>/.m2/repository/xyz/tcheeric
>
>
followed by any variant of your > make clean install
btw, i've also just updated > EventMessage.encode() to use Optional.ofNullable instead of explicit null check <avlo@711ec0c>> when performing > getSubscriptionId()> , so you'll want to confirm for yourself you have that change after performing git pull and before building.
—
Reply to this email directly, > view it on GitHub <#186 (comment)>> , or > unsubscribe <https://github.com/notifications/unsubscribe-auth/ABQMG7GT2ZPVQAG4RFXSY732F7EQNAVCNFSM6AAAAABTWUAPH2VHI2DSMVQWIX3LMV43URDJONRXK43TNFXW4Q3PNVWWK3TUHMYTCNJYHE2DIMY>> .
You are receiving this because you were mentioned.> Message ID: > <tcheeric/nostr-java/repo-discussions/186/comments/11589443> @> github> .> com>
|
Beta Was this translation helpful? Give feedback.
-
|
yw & let me know how it goes, glad to help with any issues (related or otherwise). worth noting- if using superconductor as your relay, which is heavily dependent on nostr-java classes:
nostr-java(java21)$ mvn clean install -Dmaven.test.skip=true
superconductor(develop)$ mvn clean install
superconductor(develop)$ mvn spring-boot:run -P local
nostr-java(java21)$ mvn verify |
Beta Was this translation helpful? Give feedback.
-
|
Ok, I am seeing your last message only now and will try this again, as I was getting some errors. On a related note, here are two issues I have noticed:
This is causing an error with notr-rs-relay, . See: https://github.com/nostr-protocol/nips/blob/master/01.md#from-client-to-relay-sending-events-and-creating-subscriptions and https://github.com/nostr-protocol/nips/blob/master/01.md#from-relay-to-client-sending-events-and-notices in the NIP01 spec. This may need to be enforced in superconductor, and I am happy to make the change if you agree with the recommendation.
Thoughts? |
Beta Was this translation helpful? Give feedback.
-
|
re: #1) good catch! your test line 49 variant is indeed correct- and an oversight on my part when writing that test. please update as you see fit 👍👍 re: the same consideration for superconductor- rest assured it neither requires nor uses a subscriptionId for EventMessage payload, as seen in below websocketclient and superconductor server logs screenshots: websocketclient console log: superconductor server log: if you'd like to verify the same, i've updated superconductor develop branch with an optional debug line to display event message payload, activatable via 2 options:
$ mvn spring-boot:run -P local -Dspring-boot.run.arguments=--logging.level.com.prosilion.superconductor.controller.NostrEventController=debugor
re #2) yes, absolutely agreed- requiring 4th parameter returned value as "success:..." is unnecessarily too strict as defined in NIP-01. please feel free to change as you see fit. |
Beta Was this translation helpful? Give feedback.
-
|
forgot to mention- the reason superconductor still properly functions with superfluous subscriptionId JSON element is because NostrEventController.java#handleTextMessage(...) makes a call to: messageServiceMap.get(message.getCommand()).processIncoming(message, session.getId()); which- in it's current implementation- ignores existence of subscriptionId within a payload. that being said, unless superconductor might/should also have flexibility to function as both a relay and a client (which if i recall, might have been the reason i included it in the first place) i am certainly open to enforcing the same JSON validation constraint (i.e., no subscriptionId JSON element allowed) as the nostr relay you're using. thoughts? |
Beta Was this translation helpful? Give feedback.
-
|
@avlo |
Beta Was this translation helpful? Give feedback.
-
|
this is up next, thx for counsel |
Beta Was this translation helpful? Give feedback.
-
|
linking related issue (& discussion therein) #187 (The client to validate the message before sending it to the relay) |
Beta Was this translation helpful? Give feedback.
-
|
hi, eric. related to this thread, i've added single parameter constructor:
to EventMessageFactory & related update for its create() method, pushed to same java21 branch |
Beta Was this translation helpful? Give feedback.
-
|
Hi Nick, I have made some progress on this branch. Here are the three major changes:
I have been working a separate branch (
It seems you need to set up your repository to allow others to push changes. On a related note. With SC, what about disabling the SSL requirement for the local environment by default, so it can be run on ws and http (instead of wss and https)? |
Beta Was this translation helpful? Give feedback.
-
|
hi, eric. understood & sounds good on all counts. i've added you as collaborator to my fork of your repo for PR submission at your convenience. disabling of WSS/HTTPS by default seems fine for now- since quick WSS-related NIPs examination appears to be limited to NIP-17 Private Direct Messages- although i/we might/should revisit socket-level security-by-default considerations in the (non-immediate) future. i'll notify you of SC default updates shortly, and btw- you've got superconductor collaborator permissions if useful/needed. |
Beta Was this translation helpful? Give feedback.
-
|
With SSL mode enabled, you can't use self-signed certificates, is that correct? Also, PR submitted on java21-merge-sig. Thank you! |
Beta Was this translation helpful? Give feedback.
-
|
ah, yes- apologies for lack of formal/README documentation re: trusted/self-signed certificate use . i've actually got a TODO for that documentation and will get it in ASAP. as a quick answer to your question, one can indeed use self-signed certificates with SSL mode enabled. in fact:
all that being said, i think your initial suggestion for SSL/WSS disabled by default is correct given no NOSTR NIPS (other than NIP-17, as far as i can tell) specify/require SSL/WSS. lmk if i've left any of your questions insufficiently answered and/or pls advise if anything additional worthwhile to consider/correct/remove/etc and i'll be sure to do so. btw, much thx for your input on SC, eric. having another informed & discerning mind on this is genuinely invaluable & welcome 🙏 |
Beta Was this translation helpful? Give feedback.
-
|
Beta Was this translation helpful? Give feedback.
-
Hi Nick @avlo , I am having troubles building the branch.
I am getting errors with the new ws clients. Here's one:
My dev setup includes a nostr-rs-relay instance, and I can see this error in my relay logs:
2024-12-16T17:03:41.500545Z TRACE nostr_rs_relay::server: parse error on message: "[\"2b613d13abd9be78fd7d335b351d0ebcf8224fd120ed46b342043d851a1aab61\",\"EVENT\",{\"id\":\"2b613d13abd9be78fd7d335b351d0ebcf8224fd120ed46b342043d851a1aab61\",\"kind\":30018,\"content\":\"{\\\"id\\\":\\\"aa491561-9a8b-42cd-b0cc-f85e91c957be\\\",\\\"stall_id\\\":\\\"d50e1dba-930e-4b4d-9f4f-07967c9eaf4e\\\",\\\"name\\\":\\\"Bague\\\",\\\"description\\\":\\\"Un bijou en or\\\",\\\"currency\\\":\\\"USD\\\",\\\"price\\\":450.0,\\\"quantity\\\":4,\\\"specs\\\":[[\\\"couleur\\\",\\\"or\\\"],[\\\"poids\\\",\\\"150g\\\"]]}\",\"pubkey\":\"21043242ec258dbc65fa2e427e89bce9c943ded20f12eb5ff7c90403e1fc466b\",\"created_at\":1734368621,\"tags\":[[\"d\",\"aa491561-9a8b-42cd-b0cc-f85e91c957be\"],[\"t\",\"bijoux\"],[\"t\",\"Hommes\"]],\"sig\":\"79738a2863df372bbf9b0999494c8d1822baca250f9cb320eeb0bf49959a050f62cb0078807475810a6c54c4c07223808f9dfdd13691861cb2c7fbcba7f5a5e7\"}]"Results:
Tests run: 70, Failures: 8, Errors: 7, Skipped: 0
Beta Was this translation helpful? Give feedback.
All reactions