Transaction validation rejects post-Blossom transactions

[teor2345]

Version

zebrad main branch as of 2020-11-24.

Description

When I sync zebrad using checkpoint_sync = true, the TransactionVerifier fails almost immediately with the following errors:

• InternalDowncastError("downcast to redjubjub::Error failed, original error: ScriptInvalid")
• RedJubjub(InvalidSignature)
• Ed25519(InvalidSignature)

Suspected Causes

The sighash function call hard-codes the Sapling network upgrade for each block:

                    let sighash = tx.sighash(
                        NetworkUpgrade::Sapling, // TODO: pass this in
                        HashType::ALL,           // TODO: check these
                        None,                    // TODO: check these
                    );

https://github.com/ZcashFoundation/zebra/blob/main/zebra-consensus/src/transaction.rs#L150

Transaction verifier creation hard-codes the Sapling network upgrade:

        let branch = NetworkUpgrade::Sapling.branch_id().unwrap();
        let script_verifier = script::Verifier::new(state_service.clone(), branch);
        let transaction_verifier = transaction::Verifier::new(script_verifier);

https://github.com/ZcashFoundation/zebra/blob/main/zebra-consensus/src/block.rs#L86

These hard-coded Sapling values mean that full block verification will fail past Sapling. (But the cached state tests will work, because they start at Sapling.)

I've checked for other hard-coded network upgrades across Zebra, and they all seem to be correct.

Suggested Solution

To fix these bugs, we need to call NetworkUpgrade::current(network, height) for each block, and use that network upgrade's consensus branch ID. This means that the APIs and data structures for the transaction verifier and script verifier are incorrect, because:

• the transaction verifier needs to be created for a specific Network, and remember that network
• we need to pass each block height to the transaction verifier
• the script verifier should accept a consensus branch ID as part of each request

TODO

• Revert the temporary workaround PR Disable transaction checks that depend on a broken API #1370
• Redesign the transaction and script verifier APIs
• Test the fixed code on post-Sapling and post-Blossom blocks

Test hint:

Some testnet blocks might only spend UTXOs from a single previous block - look for a testnet block with only 2 transactions, and only 1 transaction input. Then test with that block and its dependent block.

Full Logs

Nov 24 19:22:40.807  INFO sync: zebrad::components::sync: starting sync, obtaining new tips                                                                                          
Nov 24 19:22:40.808  INFO sync:obtain_tips:state: zebra_state::util: created block locator tip_height=Height(1019249) min_locator_height=1019150 locators=[Height(1019249), Height(1019248), Height(1019247), Height(1019245), Height(1019241), Height(1019233), Height(1019217), Height(1019185), Height(1019150)]                                                                           
Nov 24 19:22:46.525  WARN sync: zebrad::components::sync: error downloading and verifying block e=Block(Transaction(InternalDowncastError("downcast to redjubjub::Error failed, original error: ScriptInvalid")))                                                                                                                                                                             
Nov 24 19:22:46.525  INFO sync: zebrad::components::sync: waiting to restart sync timeout=45s                                                                                                  
Nov 24 19:23:31.525  INFO sync: zebrad::components::sync: starting sync, obtaining new tips                                                                                                    
Nov 24 19:23:31.526  INFO sync:obtain_tips:state: zebra_state::util: created block locator tip_height=Height(1019249) min_locator_height=1019150 locators=[Height(1019249), Height(1019248), Height(1019247), Height(1019245), Height(1019241), Height(1019233), Height(1019217), Height(1019185), Height(1019150)]                                                                           
Nov 24 19:23:38.414  WARN sync: zebrad::components::sync: error downloading and verifying block e=Block(Transaction(Ed25519(InvalidSignature)))                                                
Nov 24 19:23:38.414  INFO sync: zebrad::components::sync: waiting to restart sync timeout=45s                                                                                                  
Nov 24 19:24:23.414  INFO sync: zebrad::components::sync: starting sync, obtaining new tips                                                                                                    
Nov 24 19:24:23.415  INFO sync:obtain_tips:state: zebra_state::util: created block locator tip_height=Height(1019249) min_locator_height=1019150 locators=[Height(1019249), Height(1019248), Height(1019247), Height(1019245), Height(1019241), Height(1019233), Height(1019217), Height(1019185), Height(1019150)]
Nov 24 19:24:33.336  WARN sync: zebrad::components::sync: error downloading and verifying block e=Block(Transaction(RedJubjub(InvalidSignature)))
Nov 24 19:24:33.336  INFO sync: zebrad::components::sync: waiting to restart sync timeout=45s
Nov 24 19:25:18.336  INFO sync: zebrad::components::sync: starting sync, obtaining new tips
Nov 24 19:25:18.337  INFO sync:obtain_tips:state: zebra_state::util: created block locator tip_height=Height(1019249) min_locator_height=1019150 locators=[Height(1019249), Height(1019248), Height(1019247), Height(1019245), Height(1019241), Height(1019233), Height(1019217), Height(1019185), Height(1019150)]
Nov 24 19:25:24.804  WARN sync: zebrad::components::sync: error downloading and verifying block e=Block(Transaction(RedJubjub(InvalidSignature)))

[hdevalence]

There are at least two errors here:

• the consensus branch ID is hardcoded
• the errors are being cast improperly

[teor2345]

We didn't write any tests in the PR that closed this issue, so I opened #1378 for the tests.