Getting Oriented: How I Approach Open-Source Contributions to Bitcoin
Field Notes on Lightning Lab's Protocol Infrastructure
Before I open a PR in an unfamiliar codebase, I read. Beyond the README to the review threads. Recent merges, maintainer pushback, and issue comments where scope was narrowed. I build two mental models at once: of the code, and the people who live in it.
This habit shaped the contributions I’ve made to Lightning Labs’ taproot-assets and loop repos over the past few months. Since my first merged PR I’ve shipped changes across both repos, written a design gist proposing a phased multi-PR refactor before touching a line of code, and reviewed infrastructure that’s now part of the loop test suite. This post is about how I approach that work and what I’ve learned about contributing at protocol depth.1
My previous post about Bitcoin Lightning covered getting a PR merged into Lightning Labs’ taproot-assets. It included the mechanics of navigating an unfamiliar codebase, finding a real issue, and delivering something reviewers could actually merge. That post explained the foundation I started with. This post explains how I’ve built up from that foundation.2
Getting oriented in an unfamiliar codebase
It’s easy to skip on reading the fine print. But this determines whether your first PR is a good one or a mess that never gets reviewed.
The issue backlog mixed clearly-scoped bugs with open-ended architectural questions. That second category is a trap for new contributors. They present interesting problems, but impossible first targets.
The house style surprised me too. Functions run long with plain-language comments explaining both the “what” and the “why”. It’s different from what I was used to in enterprise power systems code. But Bitcoin is meant to be money for humans. The codebase reads like it was written to be audited.
Contributors come from everywhere; acronyms are ubiquitous (HTLC, UTXO — look them up); and AI bots do a first-pass review on every PR, observable from any thread.
Most importantly: failure case testing is expected, not optional. You are the QA team.
I searched for contributions that were small enough to be mergeable, real enough to matter, and connected enough to the codebase that working on it would teach me something...
Taproot assets issue #1942 — surfacing errors that the Custodian daemon was silently dropping — was that. The mechanism already existed (AssetReceiveErrorEvent, SubscribeReceiveEvents). The problem was that three call sites hadn’t wired the code in. That gap is easy to miss in a large daemon and straightforward to fix once you see it.3
I thought over the scope and how to handle edge cases for error events that occur before full context is available. Added comments regarding my design decisions in the issue and PR notes. Then included unit tests for the missing error event publishing failure cases.
The same pattern held for subsequent issues. Loop issue #1088 (adding a Secret type so database passwords can be read from files instead of being passed in CLI arguments) came directly out of a maintainer’s nit comment in a prior PR. It was a well-scoped suggestion sitting in the open, explicitly labeled as a “freebie”.
Taproot-assets issue #2086 (RPC handlers returning codes.Unknown for every error, even predictable ones like a malformed bech32 address) was bigger. But it had a clear problem statement, a concrete example table, and an obvious first cut.
Reading the codebase before touching it is more than good practice. It’s how you learn what the maintainers actually care about, which is not always the same as what the issue tracker says.
My contribution process, from issue to merged Pull Request
When a maintainer reads your PR, they should review the idea - not have to clean up AI slop. My goal is always to raise the bar on quality, even being relatively new to a codebase and programming language like Go.
That’s why I work with a checklist. The engineers I respect most are disciplined about process because discipline frees them to focus on the hard problems. Discipline and rigor are not opposites of creativity: they’re the precondition for it.4
Create a plan based on the original issue
Implement the fix or feature. Stay inside the scope of the issue and resist the urge to fix adjacent code
Add tests for new or missing failure cases, then run the tests and linting
Write a self-review document mapping each test to the failure case it covers — this forces you to notice gaps before the reviewer does
Document the non-obvious decisions: both what you’re doing and why — especially on new types and interfaces
Open as a draft PR first; update the release notes doc. The draft signals you're not asking for review yet and keeps the thread clean.
Self-review and refactor. At least two passes; first to catch logic gaps, the second improves clarify
Move PR to code review
Let’s walk through what this actually looked like for Loop PR #1131, the Secret type implementation.5
The plan: Issue #1088 described exactly what was needed: a custom Secret string type implementing go-flags‘ Unmarshaler interface, with @-prefixed values treated as file paths. The nolint comment suppressing the gosec lint on PostgresConfig.Password could then be removed, replaced by an actual fix rather than a suppression.
The implementation: The Secret type went into loopdb alongside the PostgresConfig it was fixing. UnmarshalFlag reads the file if the value starts with @, then strips trailing whitespace. This is important — a trailing newline from echo will silently break your DSN otherwise.
Then it falls back to the raw string otherwise. DSN() gets a cast back to string. The field description documents the @file convention. Backward compatibility is preserved: existing --postgres.password=mypassword invocations still work.
The Gemini review-bot pointed out that there are trade-offs here and design decisions need to be made. For example, it’s usually best practice for a sensitive type to implement the fmt.Stringer interface. But I explicitly decided not to because the DSN() method already handles password redaction explicitly. Implementing Stringer would introduce unnecessary complexity.
Tests: The test cases covered both paths — raw string passthrough, file read on the happy path, file read on a missing file, and the trailing whitespace stripping. Each test was annotated with which failure case it covered.
Self-review: I took two full passes before moving out of draft. My first pass caught a missing error wrap. The second pass was mostly about comment clarity. For example the UnmarshalFlag doc needed to be explicit about the @ convention since it’s not obvious to someone reading the type for the first time.
I also added comments in the PR for questions for the maintainers. Chasing down a CI failure revealed there are flaky tests in the repo. It’s always worth investigating further before bugging the maintainers to re-run the job.
What Surprised Me
Two things landed as being more important than I expected.
The first was how much of the real work happens before the PR. The taproot-assets issue #2086 - replacing fmt.Errorf returns across the RPC layer with proper status.Errorf calls — is a bigger scope than it looks. The codebase has hundreds of fmt.Errorf returns in rpcserver/rpcserver.go. PR #2112 proposes to add 11 status.Error returns. 6
Changing all of them at once would produce a PR that’s gnarly to review. The right approach was incremental: start with InvalidArgument cases (malformed addresses, bad proof bytes, invalid keys), which are the clearest mapping; build out a taperrors package for sentinel errors; establish the pattern; then let the codebase grow into it over multiple PRs.
Before opening the PR, I wrote a design gist laying out this phased approach. This included a table of the current state and a specific recommended sequence. The LND repo team been working on the same problem incrementally since 2021, which set a precedent.
The gist went into the issue thread as context before I started working on the code. That step — proposing the design before coding it — was something I’d undervalued in earlier contributions. On a large refactor, maintainers need to see that you understand the scope before they’ll invest review time.
The second surprise was how much code review teaches you. I reviewed Loop PR #1072 by starius - a CLI session recording framework spanning 80+ JSON fixture files, a replay transport, and a recording layer.7
The goal of my review was more than to leave a few helpful comments. It was to understand how a serious engineer approached a non-trivial design problem under real constraints. Passive reading of existing tests would give me the what. Reviewing starius’s PR gave me the why: how the loop team thinks about test infrastructure, where they draw abstraction boundaries, what they consider over-engineered. That's harder to get another way.
Two wrinkles came up with taproot-assets #2086. The Gemini bot recommended adding happy path tests, but wiring in the mocked dependencies required significant infrastructure for minimal gain. Those cases were already covered by integration tests. Knowing when to push back on automated review suggestions is its own skill.
The other was that jtobin pushed back on the incremental approach entirely, asking for the full rpcserver/rpcserver.go and authmailbox/server.go coverage in a single PR. Even a well-scoped, well-argued design doesn’t guarantee maintainer alignment. Senior reviewers have their own read on what’s manageable.
Both surprises point to the same conclusion: the most valuable work isn’t simply writing code.
What I’m working on now
Loop PR #1131 (the Secret type) is open and in review. So is taproot-assets PR #2112 (the first phase of the gRPC status code refactor — InvalidArgument cases). Both are incremental steps towards making the Lightning gRPC system easier to reason about, harder to misuse, and more resilient when things go wrong.
This through-line matters. A sequence of unrelated fixes is a list of chores. A sequence of PRs that each add a layer — observability, then correctness, then fault tolerance — presents a story about how an engineer thinks.
Architecture-level decisions like design documents and improvement proposals are the next layer of that story. That's where I'm headed next.
I’d be happy to chat if you’re interested in open source contribution and protocol engineering
David Farley’s book Modern Software Engineering is worth a read if you work on any complex system. The core argument is that software engineering is primarily a discipline of managing complexity and learning under uncertainty. This maps well onto protocol work specifically. He recommends fast feedback loops and incremental change, which is directly applicable to open source contribution.