The PR branch HEAD was fa3e0da06 at the time of this review club meeting.
Policy or transaction relay policy describes which transactions are
considered for P2P relay by a node. Generally, any transactions added to the
transaction memory pool are valid according to consensus and policy rules.
While all nodes on the network are assumed to agree on consensus rules, they
might not agree on policy rules. For example, at times of high transaction
throughput, some peers might reject consensus-valid transactions with
Moreover, policy rules often change between software releases. For example if
a node receives a transaction that requires validation rules of a consensus
change it is not yet aware of, it may reject the loose transaction out of
caution even though the transaction will be valid in a block.
This pull request changes a policy rule of Taproot-spending transactions.
Previously it was only possible to send Taproot-spends after the Taproot
deployment activated, now it is possible at any time.
Which places use the status of the Taproot deployment? Which of those
are policy related? Hint: Search for Consensus::DEPLOYMENT_TAPROOT, for
example with git grep 'Consensus::DEPLOYMENT_TAPROOT'.
Which function was responsible to check if a transaction has a Taproot
input? Hint: Start looking in
How does the pull request change the function? Is the return value or
the function signature changed?
Does this pull request change the handling of consensus-invalid
Are there any (theoretical) issues with the change? If yes, give examples of
adverse effects. Hint: Is the Taproot activation set in stone? What happens
if a wallet creates a transaction with Taproot inputs before Taproot is
(bonus question) Is it theoretically possible for a mainnet chain to exist
that has Taproot not activated or activated at a different block height?
(bonus question) Does this change affect miners running this code? Assume
that the miner is running on a chain that has Taproot not active. Would the
miner attempt to include the transaction in a block? Hint: Look at
<MarcoFalke> Which places use the status of the Taproot deployment? Which of those are policy related? Hint: Search for Consensus::DEPLOYMENT_TAPROOT, for example with git grep 'Consensus::DEPLOYMENT_TAPROOT'.
<stickies-v> The function signature is changed because the last argument `taproot_active` is removed. The return type is unchanged. The function does not check anymore if taproot has been activated. As long as the longest chain has a taproot activation block height of 709,632, there should be no behaviour change.
<MarcoFalke> So to summarize the behavior change of the function: Previously AreInputsStandard returned false when it encountered a taproot spend, now it returns true if all inputs are otherwise standard.
<stickies-v> MarcoFalke : it would only return false if taproot was not actually activated yet, right? I.e. the current master branch would still return true at this point in time when there are taproot inputs?
<MarcoFalke> Are there any (theoretical) issues with the change? If yes, give examples of adverse effects. Hint: Is the Taproot activation set in stone? What happens if a wallet creates a transaction with Taproot inputs before Taproot is active?
<stickies-v> Taproot outputs that predate taproot activation height can be spent by anyone. In case of a huge reorg (going back to before block 709,632) to a chain that has no or a later taproot activation block, taproot outputs can be stolen.
<MarcoFalke> OliverOffing: Currently the taproot activation is not (yet) hardcoded at a blockheight. While it has a minimum activatio height, the status is determined by version bits signalling in the chain.
<raj_> MarcoFalke, hmm makes sense.. So is the reasoning for "disabling spend until activation" is like, we wanna make sure a taproot witness should only appear on chain when large majority of the network already knows how to deal with it?
<MarcoFalke> raj_: Good q. In theory it should be possible to include spends *before* activation (with or without witness), that is "invalid" or "valid". Though, obviously this is not safe (in the same way that sending to a taproot address is not safe in the first place). Though, it would be really messy relay-wise (as everyones mempool or just txs looked different).
<MarcoFalke> So to summarize the answer of negative effects if this patch was run with taproot being active: (1) One issue is that the wallet now allows import of taproot descriptors at any time. If someone were to send to those descriptors without taproot being active on the chain, miners could claim the outputs.
<MarcoFalke> Ok last question: Does this change affect miners running this code? Assume that the miner is running on a chain that has Taproot not active. Would the miner attempt to include the transaction in a block? Hint: Look at CreateNewBlock.