I like small improvements.
Don’t get me wrong; I like big improvements too. Yet some of the most satisfying work I’ve ever done is a couple of lines here and there that make someone’s life easier, or at least stops making someone’s life more difficult.
Things We Never Thought Would Happen
Hang around enough developers, and you’ll figure out most of us are a lot more optimistic than we should be. We think things work better than they do. We think things will take less time and effort to create than they do. We think people will read the documentation, error messages, and tool tips.
We think the things we don’t think will happen won’t happen.
I really like when people report small bugs in free and open source projects. It means people are using the code and they have an opinion about how it’s doing something.
Look at Dogecoin issue 1902, which reports some confusing behavior on the sync screen. When the network is slow to receive packets, some of the statistics and predictions are wrong. They’re so wrong they’re misleading and confusing.
This is a great bug report. It shows exactly what’s happening, gives clear details about the environment, and includes a screenshot that demonstrates the problem visually. No one has to reproduce this behavior with a complex setup scenario to see for themselves what’s going wrong here.
I have enough experience creating bugs that I was pretty sure I could figure out what the bug was before even looking at the code: somewhere the calculation for how much time and effort left to get this node up to date did some math that always expected the calculation to give a positive number.
Teach a Man to Find a Fish
As it turns out, PR 1908 is really small. That’s satisfying!
It’s probably more interesting to describe how I figured out where to look for this code than to describe how I fixed it.
Look at the screenshot the bug reporter provided. The misleading value has a label of “Progress increase per hour”. What are the odds that exact text string occurs in other places in the code?
Here’s the secret senior developers won’t tell you on your first day: it’s good to know a codebase, but it’s more important to be able to find things in a codebase.
I’ve been using Andy Lester’s ack for many years to
search for text in files from the Unix command line. There are lots of great
tools like this: grep, pt, ag, and built-in features in IDEs. It’s not
important which one you use as long as you know how to use it effectively.
My command line looked something like this
chromatic@ghostwheel:~/dev/dogecoin$ ack 'Progress increase per hour' src/qt/
src/qt/forms/modaloverlay.ui
295: <string>Progress increase per hour</string>
src/qt/forms/ui_modaloverlay.h
269: labelProgressIncrease->setText(QCoreApplication::translate("ModalOverlay", "Progress increase per hour", nullptr));
...
I’ve elided some of the output, which is all in translation files. (See my post on translation for more details.)
The first hit told me everything I needed to know. The literal text of this
string is in a Qt form called modaloverlay. That implies the existence of
file called src/qt/modaloverlay.cpp where the code probably exists that
populates the value I was looking for.
The literal text I searched for isn’t in that file, but if you open up the
Qt modaloverlay UI
file
and look for the literal text (line 295), you’ll see it’s associated with an
updatable widget called progressIncreasePerH (line 300).
In the C++ code, updating the text of a widget with that name will update the
percentage shown in the GUI. That’s line 107 in PR
1908.
All I had to do was look in the C++ file for anything that touched
ui->progressIncreasePerH, and that led me right there.
When In Doubt, Don’t Mislead
I don’t remember all the details, but I probably skimmed through the rest of
the method to see how that progressPerHour calculation happened, but the
minimal necessary change was to the display logic. If that number is less than
zero, show zero. Otherwise, show the percentage calculated as normal.
I could have stopped there, but why not continue? The bug report initially made me think about fixing the calculation of the estimated time left to sync, but when the estimated percentage of progress per hour is zero or negative, what’s the right answer? Throwing a shrug emoji in there is awfully cute and not helpful.
Look at the GUI definition file again. The default value is “Calculating…” which implies “We don’t know yet” and is both accurate and comforting.
I decided the simplest thing to do is not to update that value with a misleading number if the number is misleading, and I left a comment to that effect. I normally prefer to write code that doesn’t need comments, but “there’s no good sample here, so don’t mislead users” is a nuanced decision I don’t trust myself to make without thinking through the code really deeply, so the comment helps future me and other developers to slow down and say “Oh yeah, we need to be cautious here”.
… And Make Bugs Impossible
Could we make this kind of error impossible? It’s possible. If you’re a scrappy and enterprising developer, feel free to pick apart the block sync progress code to see if there’s a rewrite that never lets sync estimates go negative.
The easiest approach is to make all of those double values unsigned double
values. Maybe progressDelta or timeDelta is the culprit in the case that
caused slightlyskepticalpotat to
file this bug. I don’t know; I didn’t need to do anything to reproduce the bug
because the report was enough to point me in the right direction.
I could be wrong about my diagnosis, but if you’re looking for something relatively self-contained and potentially small and not risky but useful, poke at this code and see what you can find. You’ll get bonus points if you figure out a good way to test this.
I suspect there are other, similar cases of unexpected negative values throughout the entire codebase. Patrick and I were discussing one a while back in a PR that’s taken a lot more effort than I thought it would, but that’s a story for another day.
In the meantime, keep filing great bug reports like this, look in the code to figure out where you might want to make a change, and keep having fun!
