#cln #dev
So, we've had this annoying intermittent bug where UTXO spends would get missed. Sometimes it meant that we would keep gossip for channels which had been spent, and sometimes we'd miss opportunities to sweep funds (more concerning!). Eventually I started to suspect our (my!) hash table implementation. It's extremely efficient, but if it had some bug it could explain the issues: it has a random seed for the hash function, so weird corner cases would appear random.
I wrote some random churn tests, nothing. I could get more elaborate, of course, but then something else happened.
Shahana wrote some code to create all our new documentation examples, which involved getting nodes into all kinds of weird states, and hit a strange bug. I tracked it down to a case where the recovery code was putting a new peer into the hash table, where one already exists. Easy bug fix, but it made me wonder: were we doing this elsewhere?
My hash table code allows duplicate keys just fine. But it's actually unusual to want that, and there are APIs (get, delkey) which only handle the first one vs getfirst, getnext which are fully generic.
So, I wondered. Did we make this mistake anywhere else? I bit the bullet and split the APIs: up front you now declare what type of hash table you want (duplicate keys or nodups) and you don't even get the deceptive APIs for each case.
As you might expect, the only code which had a problem was the various places where we watch UTXOs. You can absolutely be watching for the same thing in multiple places, and indeed the code was not iterating, but only handling the "first" one.
And this was all my own code, front to back. Mea culpa.
APIs matter. The natural use of an API should be the correct one. And of course "don't patch bad code, rewrite it" a-la Elements of Programming Style.
