Add address filtering to websocket block confirmation subscription #37
Conversation
apps/server/net/nanows_client.go
Outdated
|
|
||
| addresses, err := w.GetAllAccountAddresses() | ||
| if err != nil { | ||
| // Handle error |
There was a problem hiding this comment.
I know gpt wrote this, but this should be handled. Maybe modify the function to return an error and consider panicking if there's an error.
apps/server/net/nanows_client.go
Outdated
| Action: "subscribe", | ||
| Topic: "confirmation", | ||
| Ack: false, | ||
| Id: guuid.New().String(), |
There was a problem hiding this comment.
My understanding is the point of the "id" here is for the node to respond with an acknowledgement that it received the message. It would be nice to actually respond to it, or just remove this all together and maybe add a todo type of thing.
|
I have a couple of questions that I annotated, but did you try to run this code yourself with success? I'm curious as to the motivation for creating this PR. |
The motivation for fixing this issue is the websockets subscription doesn't work for public nodes because they don't support subscribing to all block confirmations. It does work and I'm using it currently. Thanks for those reviews I'll try fix it up a bit. |
still need to update the subscription when new accounts are created
dylan1951
force-pushed
the
websockets-filter-by-addresses
branch
from
51e1827
to
fe25284
Compare
Disclaimer: I have never written or seen go before and ChatGPT wrote all of the code.
Resolves #3