Your ledger is losing money and your tests will never tell you

Zubair Khan — Fri, 25 Sep 2026 10:53:47 +0000

Most of the bugs I have chased in payment systems were not logic bugs. The formula was right. The rounding was right. The tests passed. Money still went missing.

It went missing because two people did something at the same time.

I have spent the last few years on transaction and settlement systems, and the thing I did not appreciate early on is how quietly this class of bug fails. A null reference throws. A wrong formula shows up in the first test. A lost update just makes the number slightly wrong, on one day, for one account, and nobody notices until reconciliation runs and somebody asks where four hundred dollars went.

So I built a small repository to make the failure visible: github.com/sunny56/ledger-core. It is a double-entry ledger with three concurrency strategies behind one interface, and a test suite that proves which ones conserve money and which one does not.

The one law

A ledger has exactly one rule that matters:

sum(all balances) after N transfers == sum(all balances) before

Transfers move money. They never create it and never destroy it. Everything else in a ledger is policy. This is physics.

And it is trivially easy to satisfy with one writer, which is exactly why it is so easy to get wrong.

The code everybody writes first

var balance = store.Read(account).Balance;
store.Write(account, balance + delta);

Read the balance, work out the new one, write it back. It is the obvious thing. It is also broken.

Two threads read 1000 at the same moment. One adds +100 and writes 1100. The other adds -50 and writes 950. The second write lands last, so the first transfer is gone. Not failed, not logged, not retried. Gone.

In a CRUD app this is a stale profile field and somebody edits it again. In a ledger it is money that existed a second ago and does not exist now.

Making the bug fail loudly

The part of the repository I am most attached to is the strategy I deliberately did not fix.

NaiveStrategy is the read-modify-write above, kept in the codebase, with this test pointed at it:

[Fact]
public void NaiveStrategy_LosesMoney_UnderConcurrency()
{
    var harness = new LedgerHarness();
    var service = harness.ServiceFor(new NaiveStrategy(harness.Store));

    // 8 threads, 250 transfers each, all hitting the same account pair
    var result = harness.RunConcurrently(service, Threads, OperationsPerThread,
        (thread, op) => harness.Transfer(0, 1, 100, $"naive-{thread}-{op}"));

    Assert.Empty(result.Failures);
    Assert.NotEqual(harness.OpeningTotal, result.ClosingTotal);
}

Read that assertion again. It asserts the total changed. The test passes when money is lost.

Two things make this work reliably rather than one run in fifty. The harness releases every thread from a Barrier, so they all start at the same instant instead of staggering. And the naive strategy has a Thread.SpinWait(50) between the read and the write, which widens the race window on purpose.

That SpinWait is the honest part. The window exists in real code too. It is just narrower, which means you hit it on a Tuesday in production instead of every time in CI.

If that test ever starts conserving, I do not want somebody deleting it. I want them widening the window, because the test has stopped proving anything.

Three ways to actually fix it

Lock everything first

var accounts = entry.TouchedAccounts.ToArray();
using var _ = store.Lock(accounts);

Take an exclusive lock on every account the entry touches, then do the read-modify-write inside it. This is the SELECT ... FOR UPDATE shape and it is correct and easy to explain to anyone.

It also has a trap in it. Look at the store:

public IDisposable Lock(IReadOnlyCollection<AccountId> accounts)
{
    var gates = accounts.Distinct().OrderBy(a => a).Select(a => RowFor(a).Gate).ToArray();
    return new MultiLock(gates);
}

The OrderBy is the entire reason this does not deadlock. Without it, a transfer from A to B takes A then B, a transfer from B to A takes B then A, and if both start together they sit there forever holding half of what the other one needs.

Deterministic ordering is a one-line fix for a class of bug that is otherwise very hard to reproduce. There is a test for exactly that shape, with a join timeout, so a deadlock fails the build instead of hanging it.

The cost of this strategy is throughput. Every writer queues behind the hottest account. If one account is popular, you have built a single-threaded system with extra steps.

Assume nobody else is writing, then check

for (var attempt = 1; attempt <= maxAttempts; attempt++)
{
    var snapshots = accounts.ToDictionary(a => a, store.Read);

    using (store.Lock(accounts))
    {
        if (accounts.All(a => store.Read(a).Version == snapshots[a].Version))
        {
            // apply every posting, then return
        }
    }

    Thread.SpinWait(Random.Shared.Next(10, 10 * Math.Min(attempt, 16)));
}

throw new ConcurrencyRetriesExhaustedException(maxAttempts);

Read with a version, do your thinking without holding anything, then commit only if no version moved underneath you. Rowversion in SQL Server, xmin in Postgres, an ETag in a document store, same idea everywhere.

Two details in there that took me longer than they should have.

The commit phase still takes a lock. People ask me why, given the whole point is not locking. The answer is that a journal entry must land completely or not at all. If I compare-and-swap account A successfully and then fail on account B, the ledger is now unbalanced, which is worse than the problem I started with. So the CAS is what detects an interleaved writer, and the short commit-phase lock is only there to stop one entry being torn in half across two accounts. It is held for microseconds, not across the think time.

The backoff has jitter. Without it, every writer that loses a round wakes up at the same moment and collides again. You get a thundering herd that looks like a deadlock but is actually just very bad luck, repeatedly.

The bound on maxAttempts matters too. When you start hitting it, the right move is to shard the hot account, not to raise the number. Raising it turns a fast failure into a slow one.

Let the database do it

The third option is to stop writing concurrency control and open the transaction at SERIALIZABLE.

I have not implemented this one in the repository yet, and I want to be honest about why it is on the roadmap rather than in the code: it behaves so differently across engines that a single implementation would be misleading. Postgres uses SSI and surfaces conflicts as an error at COMMIT (SQLSTATE 40001), so a retry loop is mandatory, not optional. SQL Server takes range locks instead and blocks. Identical application code, completely different performance profile.

It is the least code and the most portable correctness argument, which is often the right answer for a team that does not want to own this problem. I just do not want to publish a benchmark of it until I have run it on both.

Making bad states unrepresentable

Separate from concurrency, one thing I would do again on any ledger: enforce the invariants in the constructor.

var sum = postings.Aggregate(0L, (acc, p) => checked(acc + p.Amount.MinorUnits));
if (sum != 0)
    throw new UnbalancedEntryException($"Postings must sum to zero, summed to {sum}.");

Postings sum to zero. One currency per entry. At least two postings. Transfers must be positive.

Validate at construction, not at the API edge, and there is no code path anywhere in the system that can hand an unbalanced entry to anything. Not a controller, not a background job, not a test helper somebody wrote in a hurry.

The other decision in that direction: money is stored as integer minor units, never decimal. Conversion happens at the edges. The ledger itself cannot introduce a rounding error because it never divides anything.

Idempotency is not optional

A retry, a queue redelivery, an impatient user clicking twice. Same thing as far as your ledger is concerned, and all three will happen.

Every entry carries a key, claimed before the strategy runs. The test fires 800 concurrent posts of the same key and asserts exactly one applied:

Assert.Equal(1, result.Applied);
Assert.Equal(799, result.Duplicates);

Note that a duplicate returns false rather than throwing. Callers retry a lot, and an exception per retry is noise, not information.

What I would tell myself four years ago

Write the failing test first. Not a test for the fix, a test for the bug. Until you have watched conservation break in CI, you are guessing about whether your mitigation does anything, and you will pick one based on what you read rather than what your workload does.

Then pick a strategy based on your actual contention, not on which one sounds most senior. Pessimistic locking is fine and boring and correct if your accounts are not hot. Optimistic is better if they are not contended and worse if they are. Serializable is the least code you will write. None of them is the clever answer.

The clever answer is the test that tells you when you got it wrong.

The repository is at github.com/sunny56/ledger-core. It runs with dotnet test, no Docker and no database needed. The store is in-memory but uses real locks and a real version counter, so the semantics line up with what a row lock and a rowversion column give you.

Next on it: a Postgres adapter behind the same interface, the serializable strategy, a single-writer-per-partition strategy, and a BenchmarkDotNet harness so the throughput claims in this post stop being qualitative.

If you have shipped a settlement system and solved this differently, I would genuinely like to hear it.

DEV Community: Zubair Khan