I went into the ledger layer to fix waste. A background sync was loading every transaction it had ever recorded in order to work out which ones were new, which is a silly way to ask for a set of hashes and was costing a full table read every ten minutes.
I came out having fixed something else, and the something else could have quietly rewritten a tax return.
The function
One function served every read of the transaction ledger. In substance it was this:
list: async (sinceTs = 0) =>
col.find(sinceTs ? { ts: { $gte: sinceTs } } : {})
.sort({ ts: -1 })
.limit(5000)
.toArray()
Newest first, capped at five thousand. That is a completely ordinary thing to write. A page that renders a transaction table should not try to render an unbounded number of rows, and a sort with no limit on a growing collection is how you eventually hang a page. If you were reviewing this you would approve it, and you would be right to.
The other function
Cost basis in this app is FIFO. First in, first out: when you dispose of something, you consume the oldest lot you still hold, then the next oldest, until the sale is covered. The matcher is about fifteen lines and the comment above it says what it does.
// Pure FIFO consumption. Oldest eligible lot first;
// a lot acquired after the disposal date is never eligible.
So one of these reads the ledger newest-first and throws away the tail. The other one starts at the oldest row and works forward.
Those are the same rows. What a newest-first cap discards is exactly what FIFO reaches for first.
What that does to a number
Say you bought one unit a long time ago for $100, bought a second unit recently for $900, and then sold one unit for $1,000. These are illustrative numbers and not a trade.
FIFO says the unit you sold is the old one. Your basis is $100, your gain is $900, and because you held it more than a year it is taxed at the long-term rate.
Now suppose the $100 purchase has scrolled off the end of the cap. FIFO cannot see it, so it consumes the $900 lot instead. Your basis is $900, your gain is $100, and it is short-term.
Both of those are well-formed answers. Both render in the same tile in the same font. Nothing errors, no row is visibly missing, the totals still add up, and the page still says the same reassuring things it says on a good day. The second answer is wrong twice over, once about how much you made and once about which rate applies to it, and there is nothing on the screen that distinguishes it from the first.
I have written before that the dangerous number is the one with no error bar and no provenance. This is a sharper version of the same thing. There was no number to distrust. There was a smaller one.
The direction of the error is the bad part
Losing the oldest lots biases the result one way. Old lots are usually the cheap ones, because you bought them before the thing went up. Dropping them means substituting a higher basis, which means reporting a smaller gain.
A tool that overstates your gains gets audited by its user immediately, because nobody accepts a surprise tax bill without checking the arithmetic. A tool that understates them is believed. It agrees with you. You close the tab.
The same substitution also resets the clock. A recent lot is a short-term lot, and the short-term rate is usually the larger of the two. So the failure would report a smaller gain than you actually made while quietly moving it into the more expensive bracket, and the two errors do not cancel: they are answers to different questions.
It arrives on a schedule
The thing I keep turning over is that this was not broken. Not yet.
Five thousand transactions is a lot of transactions. The ledger is nowhere near it, so every figure the app has ever produced was computed over a complete history and is fine. I checked before I wrote anything else, because "is this live" is a different question from "is this wrong" and I wanted the answer to the first one before I got interested in the second.
But it is not a bug that might happen. It is a bug with a date on it, and the date is set by how much you trade. Nothing announces it. There is no five thousand and first transaction that throws. The books just start losing their beginning, one row at a time, oldest first, and every gain computed after that is matched against the wrong lots while looking entirely normal.
That is the version of this failure I find hardest to defend against in general. A limit is a decision about the future written in the present, and the present is always the case where it does not matter.
The fix is naming
The cap was not the bug. The cap is correct and it is still there.
The bug is that one function answered two questions: "what should I show this person" and "what is true". Those have different correct answers, and a single list() gave them both the display one.
So the reads are split by purpose now, and each one says in its own comment what it is for and what it must not be used for.
list()is the newest rows, capped, for display only. Its comment says money math must not call it, and says why: what the cap drops is what FIFO consumes first.all()is the whole ledger, oldest first. Basis, balance history and payouts read this one.refs()projects only the id, hash, chain and contract, for pages that map a row to its transaction and have no business pulling every amount and counterparty to do it.hashes(chain)returns the set a sync actually wanted, which is where I came in.
Three of those four are performance. One of them is the tax return. They look identical at the call site, which is the whole problem, so the difference had to be moved somewhere a person reading the code cannot miss it: into the names, and into a comment on the function rather than a comment at the call.
The dedupe I originally came for got its own module. It memoises the expensive derivations for the life of one request, so a page that wants the basis report, the open lots and the pricing progress rebuilds FIFO once instead of three times. Request-scoped on purpose: a new request recomputes from the database, so it cannot serve a stale figure. And it is a separate file specifically so the background jobs keep calling the raw functions and nothing about their behaviour changes. A cache that changes what a cron job writes is not a cache.
The general form
This is the fourth or fifth time this blog has arrived at the same shape from a different direction, and I am starting to think it is the only interesting kind of bug.
A compass that reports a bearing with full confidence and points south. A swap that looks like dollars coming in and is not. Grit from a coarse stage that rides into a fine one and scratches everything for a week without making a sound. None of these produce an error. All of them produce a plausible result, on time, in the expected format.
Here is the part specific to this one. A performance limit and a correctness limit are written with the same keyword. limit(5000) cannot tell you whether the rows it is dropping were decorative or load-bearing, and neither can the reviewer, because the answer is not in the query. It is in what the caller is going to do with the result, which is somewhere else in the codebase entirely.
The rule I am taking from it: when a function's answer gets truncated, sorted, sampled, paginated or capped, the truncation is part of its contract and belongs in its name. Not in a comment at the call site, where it protects only the caller who was already thinking about it. In the name, where it reaches the caller who was not.
That caller is the one who was going to compute your gains with it.