fix notifier

[stutxo]

i wanted to understand how this notifier worked just in case the extra one i added for the tests would cause problems with the heartbeat function, and it looks like the wrong notifier is set in start_message_handler, its not actually doing anything.

I updated it to use notifier.notify_one();

Is this purpose of this to stop the heartbeat if a message has already been handled to reduce the amount of messages being sent?

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.03%. Comparing base (a6ea56b) to head (a9f9365).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main       #8      +/-   ##
==========================================
+ Coverage   91.74%   92.03%   +0.28%     
==========================================
  Files           9        9              
  Lines         933      929       -4     
==========================================
- Hits          856      855       -1     
+ Misses         77       74       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[stutxo]

il try and figure out how to remove this extra 'notify_one()' i put in for the test as it's not that clean

[pool2win]

il try and figure out how to remove this extra 'notify_one()' i put in for the test as it's not that clean

It looks like you are right here, we should be using notify_one and not notified here. Though I''d like to be able to replicate the error the bad usage of notified was causing. That will be nice too.

Also, generally speaking, I think this is good thing you are looking into. We might be able to completely do away with notifiers to close connections if there is an error downstream. TBH, that will be a much cleaner solution. I am writing some networking code for frost-federation and I have been able to avoid notifiers there, instead, I return Error back upstream and the connection then cleans up after itself on an downstream errors. I haven't wrapped up all the tests there, so maybe notifiers will come back.

[stutxo]

I removed the extra notify_one that i added to the start_listen function and am using the new notify_one that gets triggered when a message is received to progress the it_should_run_connect_without_errors without having to sleep.

I also had to use the addr "127.0.0.1:6680" instead of localhost, or it kept failing. Im not sure why that is right now, it might be something with my local machine.

[stutxo]

il try and figure out how to remove this extra 'notify_one()' i put in for the test as it's not that clean

It looks like you are right here, we should be using notify_one and not notified here. Though I''d like to be able to replicate the error the bad usage of notified was causing. That will be nice too.

Also, generally speaking, I think this is good thing you are looking into. We might be able to completely do away with notifiers to close connections if there is an error downstream. TBH, that will be a much cleaner solution. I am writing some networking code for frost-federation and I have been able to avoid notifiers there, instead, I return Error back upstream and the connection then cleans up after itself on an downstream errors. I haven't wrapped up all the tests there, so maybe notifiers will come back.

I haven't come across using notifiers before, but i don't have much experience in network code, it doesn't seem like its being used for errors here though, its just used to reset the start_heartbeat function when a message is received.