Reduce client list growth

[irriden]

With this PR, the client list grows with the number of unique devices that a user connects to broker.

The client id now has two parts, a random, but fixed "device id", an a random "session id" generated fresh for each connection.

The two parts are separated by a _. I tried /, but the rumqttd plumbing complains.

EDIT: closes #100 depends on https://github.com/stakwork/sphinx-rs/pull/30

[Evanfeenstra]

Remove any expects ... for example now cid.split_once("_").expect would cause the connection handler thread to crash... so someone could bring down the broker just by connecting with a client id with no _

or the String::from_utf8(existing.to_vec()).expect Err could be handled as well

[Evanfeenstra]

Also... why the double pub_timeout here https://github.com/stakwork/sphinx-key/blob/e408866e737800daa83f62c972c4e8ebba80d6f7/broker/src/mqtt.rs#L179?

Can you just set current to the next client? Then the loop will go around again and try the next one

[irriden]

The idea is "try the current connection," then try all other devices by iterating through the full map.

If I just set current to the next client, I have to keep the iterator over the entire map across separate iterations no ?

[Evanfeenstra]

Ok.. thinking about this more... we shouldn't just loop through all clients, since some (mobile signers) will usually not be online so there is no point in trying them.

Should we prefix the client IDs with esp or something to let the broker know they are "always-on" signers?

Then u cant just look in client_list to find the next one that starts with esp right? and set_current to that... then on the next time around the loop it will try the next one right?

[irriden]

client-list could be made solely of always on signers, so no need to "look for" esp, just try them all?

[irriden]

Ephemeral signers like the phone go straight to setting the current value, but don't get added to the client_list.

[Evanfeenstra]

How would the phone get sent VLS requests if its not in the client list?

Eventually we can filter by "send" and "receive" VLS calls (like all keysend signatures will go to the phone, since we can assume its on since they just sent a keysend), but all "receive" and other random things like AnnounceChannel etc can go to the hardware

[irriden]

The phone ID will be stored in the current member of conns.

[Evanfeenstra]

Hmmm ya i guess that works. But what if there are multiple ephemeral signers (phone and laptop for example)...

Maybe we should assume mobile signers will always need an LSS sync and actually shouldn't store their ID to re-use?

[irriden]

Ok.. thinking about this more... we shouldn't just loop through all clients, since some (mobile signers) will usually not be online so there is no point in trying them.

How about we keep all clients in the client list? It seems to me each attempt at making a connection is cheap no?

[Evanfeenstra]

Ya lets keep the all in the list. We can fiddle with choosing mobile signer vs ESP in the next PR

[irriden]

Seems to me a single persistent ID will work. Do we want to revert back to a vector ?

[Evanfeenstra]

HashMap + current is fine i think. We may want to add more things to the value of the hashmap later... like timestamp, signer type, etc. Also HashMap is more efficient

[irriden]

So for now key is client ID, and value is empty ?

[Evanfeenstra]

Just do a <string, bool> for now and set true for any signer... next PR we can use that bool to store if its an always-on signer or not

[irriden]

@Evanfeenstra this is what it looks like now let me know what you think. Single reconnects looking good gonna test multi signer mode next.

[irriden]

Multi signer tested - working all good.