6 votes
Accepted

I would like to dig into the open Taproot PR to understand it, test it and possibly even review it. Any advice or recommendations?

Previous Bitcoin Core PR review club sessions A good starting point is the various Bitcoin Core PR review club sessions that have been held thus far on specific commits of the Taproot PR. The latest ...
Michael Folkson's user avatar
5 votes
Accepted

How can I efficiently search the bitcoin-dev mailing list for discussion of a specific term?

The simplest way to do this is use a search engine e.g. Google, DuckDuckGo and enter: "bip37 site:lists.linuxfoundation.org" This is also useful for searching GitHub issues, pull requests as ...
Michael Folkson's user avatar
4 votes
Accepted

what is the significance (if any) of "ACKs for top commit:" in the git commit messages of bitcoin core?

From https://github.com/bitcoin/bitcoin/blob/v0.18.1/CONTRIBUTING.md#peer-review: Anyone may participate in peer review which is expressed by comments in the pull request. Typically reviewers will ...
Pieter Wuille's user avatar
4 votes
Accepted

Why do some Bitcoin Core "good first issues" require scripted-diffs?

From the Bitcoin Core developer notes, when the bash script is included in the commit message the "Travis CI job checks that the result of the script is identical to the commit." It also ...
Michael Folkson's user avatar
4 votes
Accepted

Is commenting on github as reviewer good place to ask questions about source code?

I think you'll get different perspectives on this from different people depending on how "basic" we are talking, the PR author and whether the PR is high priority and already obtaining a lot ...
Michael Folkson's user avatar
4 votes
Accepted

How can I keep my disk usage under control when regularly running the Bitcoin Core functional tests?

This was answered on Twitter by various individuals. You should remove the directory /tmp/test_common_Bitcoin Core regularly. For Martin Leitner-Ankerl this was 6GB of leftover files (leftovers only ...
Michael Folkson's user avatar
3 votes
Accepted

Is it okay to open a pull request while there there's little done?

You can open a pull request in draft if it isn't ready to be reviewed yet but it should at least be a work in progress rather than its current state ("not done anything worth mentioning"). ...
Michael Folkson's user avatar
3 votes
Accepted

Should we inform others that all tests are passed in the course of reviewing a pull request?

If all you did was run the automatic tests on a standard system, and found they succeeded, there is little point in commenting at all. The continuous integration system already runs all tests (in a ...
Pieter Wuille's user avatar
3 votes
Accepted

What is fuzz testing?

In the paper The Art, Science and Engineering of Fuzzing the authors define fuzzing as “a process of repeatedly running a program with generated inputs that may be syntactically or semantically ...
Michael Folkson's user avatar
3 votes

I have written a test for an open Bitcoin Core PR. Should I open a PR to the author's fork or wait for it to be merged?

It depends on the situation. When the change is a few lines of code it is sufficient to show the author the patch and they will amend the pull request. You can do this using a ``` diff in the comments ...
Michael Folkson's user avatar
3 votes

How do I join the Bitcoin Core PR review club meetings on IRC?

On Ubuntu, I like weechat. On OSX I've used Colloquy before but have recently upgraded to Textual which is not free -- unless you build it from source :-). There is even a web interface for freenode, ...
pinhead's user avatar
  • 5,144
3 votes
Accepted

What is ccache? When should I use it when building and reviewing Bitcoin Core PRs?

The tool ccache helps store intermediate compilation results, so if you change branches often, and rebuild, it can significantly speed up doing so. To use it, you install ccache for your platform. ...
Pieter Wuille's user avatar
2 votes
Accepted

What is Valgrind? When should I use it for debugging Bitcoin Core over using gdb, lldb, pdb etc?

If you watched Fabian's presentation (or read the transcript) you'll have seen that he said you can use Valgrind for memory leak checks and that it can be used similarly to lldb. valgrind --leak-check=...
Michael Folkson's user avatar
2 votes

How do I join the Bitcoin Core PR review club meetings on IRC?

The Bitcoin Core PR review club site links to this beginner's guide to IRC. There are more impressive IRC setups such as Pieter Wuille's setup. My setup is: juicessh on my phone, ssh into my VPS (non-...
Michael Folkson's user avatar
2 votes
Accepted

How do I build and review a Bitcoin Core GUI pull request?

It is difficult to answer this question as I don't know what your current skillset is. You will need some basic command line and Git experience and you will need a GitHub account. I will assume these ...
Michael Folkson's user avatar
2 votes
Accepted

Should I run the tests every time I review an open Bitcoin Core PR?

The Bitcoin Core contributing guidelines recommend that post Concept ACK, Approach ACK: A review begins with ACK BRANCH_COMMIT, where BRANCH_COMMIT is the top of the PR branch, followed by a ...
Michael Folkson's user avatar
2 votes
Accepted

How much testing is done on each pull request before it is merged?

Each pull request usually isn't that big, so it's not hard to look through every line of code. Each pull goes through meticulous testing, in case of any malicious code or an unintended flaw. Why do ...
Whyte the Weeabear's user avatar
2 votes
Accepted

What is an uninitialized read? Can you point out some examples found in Bitcoin Core? How do you detect them?

An uninitialized read is when a variable is created without an initial value and then the value of this variable is read. This should be avoided as the resulting behavior is unpredictable and could be ...
Michael Folkson's user avatar
2 votes
Accepted

What is DrahtBot on Bitcoin Core pull requests?

The primary use of DrahtBot is to add automated comments to a PR that list other PRs that conflict with the PR you are viewing. It was written and is managed by Marco Falke but it is not currently ...
Michael Folkson's user avatar
2 votes
Accepted

What mistakes do first time contributors to Bitcoin Core often make?

The most common mistakes I have seen first time contributors make (and this includes me) is: Not opening the PR from a topic branch of the forked Bitcoin Core repo and instead opening it from the ...
Michael Folkson's user avatar
2 votes
Accepted

I'm digging further into the open Taproot PR. Can you explain the BIP340 test vectors?

These bip340_test_vectors are used in two places: the unit tests (src/test/key_tests.cpp) and the functional tests (test/functional/test_framework/key.py). The Python code for testing the ...
Michael Folkson's user avatar
2 votes

Should we inform others that all tests are passed in the course of reviewing a pull request?

As part of ACKing a top commit hash (post Concept ACK, Approach ACK) you are encouraged to explain what you did during your review that resulted in your ACK. As part of this comment it is perfectly ...
Michael Folkson's user avatar
1 vote
Accepted

What Git workflow do I need to use to be able to compare a PR branch before and after an author's force push?

Git is like a blockchain: it only keeps the "best-commit-chain". A git push -f "reorgs" the prior chain of commits to a new "best chain". If you checked out the PR before ...
Murch's user avatar
  • 75.2k
1 vote

What should I include in the description of a Bitcoin Core pull request?

There is currently some (limited) guidance in CONTRIBUTING.md. The title of the pull request should be prefixed by the component or area that the pull request affects e.g. consensus: Add new opcode ...
Michael Folkson's user avatar
1 vote

Should I always run "make clean" between building different PR branches of Bitcoin Core?

You could always run make clean between builds but a lot of the time it won't be necessary and it is slower. Pieter Wuille usually doesn't run make clean and only runs it when the build fails. If it ...
Michael Folkson's user avatar
1 vote

What is ccache? When should I use it when building and reviewing Bitcoin Core PRs?

ccache is a compiler cache. It speeds up recompilation by caching previous compilations and detecting when the same compilation is being done again. Unlike dbcache the decision of how much ccache to ...
Michael Folkson's user avatar

Only top scored, non community-wiki answers of a minimum length are eligible