<p><strong>Previous Bitcoin Core PR review club sessions</strong></p>
<p>A good starting point is the various Bitcoin Core PR review club sessions that have been held thus far on specific commits of the <a href="https://github.com/bitcoin/bitcoin/pull/19953" rel="nofollow noreferrer">Taproot PR</a>. The latest <a href="https://bitcoincore.reviews/17977-3" rel="nofollow noreferrer">one</a> was on &quot;Implement Taproot validation&quot; (hosted by John Newbery) and there are various notes and a transcript of the meeting there. There are also links to the previous review club sessions on the Taproot PR on that page.</p>
<p><strong>Build the PR</strong></p>
<p>When reviewing a PR it is always a good idea to build the PR. For simplified guidance on building a PR see this StackExchange <a href="https://bitcoin.stackexchange.com/questions/99014/how-do-i-build-and-review-a-bitcoin-core-gui-pull-request">question</a>. For more detailed guidance see this <a href="https://jonatack.github.io/articles/how-to-compile-bitcoin-core-and-run-the-tests" rel="nofollow noreferrer">resource</a> from Jon Atack.</p>
<p>Obviously the <a href="https://github.com/sipa/bitcoin" rel="nofollow noreferrer">fork</a> of Bitcoin Core you will want to clone is sipa's (Pieter Wuille's) and the branch you will want to check out is <code>taproot</code>.</p>
<p>There is no GUI or wallet support in this Taproot PR so you will want to use the following flags when configuring to save time.</p>
<pre><code>./configure --without-gui --disable-wallet
</code></pre>
<p><strong>Understand what the PR does (and what it doesn't)</strong></p>
<p>On the PR Pieter Wuille states:</p>
<blockquote>
<p>No signing or wallet support of any kind is included, as testing is done entirely through the Python test framework.</p>
</blockquote>
<p>This is pretty limited scope. All signing and validation of (Schnorr) signatures is already handled by the libsecp256k1 <a href="https://github.com/bitcoin-core/secp256k1" rel="nofollow noreferrer">library</a> which has its own separate repo. But what Pieter is saying here is that with the current Taproot <a href="https://github.com/sipa/bitcoin/tree/taproot" rel="nofollow noreferrer">branch</a> you <strong>can't</strong> create a Taproot (pay-to-taproot or P2TR) address or Taproot spend from your Bitcoin Core wallet or using <code>bitcoin-cli</code>. The only way you can create Taproot transactions is using the Python code in the functional test framework. So if you know some Python you can adjust the Python code to create different Taproot transactions but other than that you can't create Taproot UTXOs or spend from them.</p>
<p>Once Taproot transactions are created from the Python functional test framework this PR branch will allow you to validate Taproot spends with the C++ validation code.</p>
<p><strong>Functional tests</strong></p>
<p>Given the only Taproot transactions that are validated by your Bitcoin Core full node are coming from the functional test framework it is a good idea to start your code review there. Some existing functional tests are edited in this PR but there is a new functional test added called <a href="https://github.com/sipa/bitcoin/blob/taproot/test/functional/feature_taproot.py" rel="nofollow noreferrer">feature_taproot.py</a>. This test generates random Taproot transactions and verifies they are accepted into the mempool (when appropriate) and accepted (when appropriate) into Python-constructed blocks.</p>
<p>You can run this new functional test directly by running:</p>
<pre><code>test/functional/feature_taproot.py
</code></pre>
<p>After a few minutes this test will very likely pass. So here is where you can be creative in making changes to the test or the C++ code it is testing before running the test again to see what you need to do to make this test fail.</p>
<p><strong>What to avoid</strong></p>
<p>There are a few things to avoid on this PR. It is still possible that there is a <strong>conceptual</strong> problem with the Taproot design but at this stage it is unlikely. There was an in depth discussion of the evolution of the conceptual design of Taproot at this <a href="https://diyhpl.us/wiki/transcripts/london-bitcoin-devs/2020-07-21-socratic-seminar-bip-taproot/" rel="nofollow noreferrer">Socratic Seminar</a> at London BitDevs covering years of discussions. I would recommend watching that or reading the transcript of that if you think you have discovered a conceptual problem. Alternatively, you are free to ask questions on the ##taproot-bip-review channel on IRC.</p>
<p>Another thing to avoid on this PR is discussion of <strong>activation</strong> (or deployment). This PR completely avoids any decision on activation method and so there should be no discussion of activation on this PR. If you look at the commit on <a href="https://github.com/bitcoin/bitcoin/commit/c13431268e675c3431e819b0edd90a52964bb356" rel="nofollow noreferrer">Activate Taproot/Tapscript on regtest (BIP 341, BIP 342)</a> there is a deployment date set of January 1 2008 for regtest but other than that no activation or deployment parameters are set. If you would like to discuss activation there is another IRC channel ##taproot-activation for that purpose.</p>
<pre><code>// Deployment of Taproot (BIPs 340-342)
    consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].bit = 2;
    consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nStartTime = 1199145601; // January 1, 2008
    consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nTimeout = 1230767999; // December 31, 2008
</code></pre>
<p>The PR is at a reasonably advanced stage and what it is really seeking is code review, testing and (maybe!) additional tests.</p>
<p>For guidance on debugging Taproot functional tests see this other StackExchange <a href="https://bitcoin.stackexchange.com/questions/99318/how-do-i-debug-taproot-functional-tests-using-pdb">question</a>.</p>

CC BY-SA 4.0
2020-10-03T12:28:20.470Z
2020-10-06T11:04:51.267Z
2020-10-06T11:04:51.267Z
Michael Folkson
<p>I would like to dig into the open Taproot PR (<a href="https://github.com/bitcoin/bitcoin/pull/19953" rel="nofollow noreferrer">PR #19953</a>) to understand it, test it and possibly even review it. Any advice or recommendations?</p>

<bitcoin-core-development><testing><taproot><bitcoincore-review><functional-test>
I would like to dig into the open Taproot PR to understand it, test it and possibly even review it. Any advice or recommendations?
Michael Folkson
# I would like to dig into the open Taproot PR to understand it, test it and possibly even review it. Any advice or recommendations?

(Asked by: username_0 on 2020-10-03)

I would like to dig into the open Taproot PR ([PR #19953](https://github.com/bitcoin/bitcoin/pull/19953)) to understand it, test it and possibly even review it. Any advice or recommendations?

**Tags:** bitcoin-core-development, testing, taproot, bitcoincore-review, functional-test


---

## 1 answers

### Answer by username_0 on 2020-10-03. Score: 6 | Accepted answer

**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](https://github.com/bitcoin/bitcoin/pull/19953). The latest [one](https://bitcoincore.reviews/17977-3) was on "Implement Taproot validation" (hosted by John Newbery) and there are various notes and a transcript of the meeting there. There are also links to the previous review club sessions on the Taproot PR on that page.

**Build the PR**

When reviewing a PR it is always a good idea to build the PR. For simplified guidance on building a PR see this StackExchange [question](https://bitcoin.stackexchange.com/questions/99014/how-do-i-build-and-review-a-bitcoin-core-gui-pull-request). For more detailed guidance see this [resource](https://jonatack.github.io/articles/how-to-compile-bitcoin-core-and-run-the-tests) from Jon Atack.

Obviously the [fork](https://github.com/sipa/bitcoin) of Bitcoin Core you will want to clone is sipa's (Pieter Wuille's) and the branch you will want to check out is `taproot`.

There is no GUI or wallet support in this Taproot PR so you will want to use the following flags when configuring to save time.

```
./configure --without-gui --disable-wallet

```

**Understand what the PR does (and what it doesn't)**

On the PR Pieter Wuille states:

> No signing or wallet support of any kind is included, as testing is done entirely through the Python test framework.

This is pretty limited scope. All signing and validation of (Schnorr) signatures is already handled by the libsecp256k1 [library](https://github.com/bitcoin-core/secp256k1) which has its own separate repo. But what Pieter is saying here is that with the current Taproot [branch](https://github.com/sipa/bitcoin/tree/taproot) you **can't** create a Taproot (pay-to-taproot or P2TR) address or Taproot spend from your Bitcoin Core wallet or using `bitcoin-cli`. The only way you can create Taproot transactions is using the Python code in the functional test framework. So if you know some Python you can adjust the Python code to create different Taproot transactions but other than that you can't create Taproot UTXOs or spend from them.

Once Taproot transactions are created from the Python functional test framework this PR branch will allow you to validate Taproot spends with the C++ validation code.

**Functional tests**

Given the only Taproot transactions that are validated by your Bitcoin Core full node are coming from the functional test framework it is a good idea to start your code review there. Some existing functional tests are edited in this PR but there is a new functional test added called [feature\_taproot.py](https://github.com/sipa/bitcoin/blob/taproot/test/functional/feature_taproot.py). This test generates random Taproot transactions and verifies they are accepted into the mempool (when appropriate) and accepted (when appropriate) into Python-constructed blocks.

You can run this new functional test directly by running:

```
test/functional/feature_taproot.py

```

After a few minutes this test will very likely pass. So here is where you can be creative in making changes to the test or the C++ code it is testing before running the test again to see what you need to do to make this test fail.

**What to avoid**

There are a few things to avoid on this PR. It is still possible that there is a **conceptual** problem with the Taproot design but at this stage it is unlikely. There was an in depth discussion of the evolution of the conceptual design of Taproot at this [Socratic Seminar](https://diyhpl.us/wiki/transcripts/london-bitcoin-devs/2020-07-21-socratic-seminar-bip-taproot/) at London BitDevs covering years of discussions. I would recommend watching that or reading the transcript of that if you think you have discovered a conceptual problem. Alternatively, you are free to ask questions on the ##taproot-bip-review channel on IRC.

Another thing to avoid on this PR is discussion of **activation** (or deployment). This PR completely avoids any decision on activation method and so there should be no discussion of activation on this PR. If you look at the commit on [Activate Taproot/Tapscript on regtest (BIP 341, BIP 342)](https://github.com/bitcoin/bitcoin/commit/c13431268e675c3431e819b0edd90a52964bb356) there is a deployment date set of January 1 2008 for regtest but other than that no activation or deployment parameters are set. If you would like to discuss activation there is another IRC channel ##taproot-activation for that purpose.

```
// Deployment of Taproot (BIPs 340-342)
    consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].bit = 2;
    consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nStartTime = 1199145601; // January 1, 2008
    consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nTimeout = 1230767999; // December 31, 2008

```

The PR is at a reasonably advanced stage and what it is really seeking is code review, testing and (maybe!) additional tests.

For guidance on debugging Taproot functional tests see this other StackExchange [question](https://bitcoin.stackexchange.com/questions/99318/how-do-i-debug-taproot-functional-tests-using-pdb).


---

