From f21de9d0d6632816356464250e00528e4c28c15a Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@z.cash>
Date: Fri, 21 Feb 2020 17:07:57 +0000
Subject: [PATCH] consensus: Check JoinSplit signatures against the previous
 network upgrade

We only check failing signatures against the previous epoch to minimise
the extra computational load on nodes.
---
 src/gtest/test_checktransaction.cpp | 48 +++++++++++++++++++++++++++++
 src/main.cpp                        | 20 +++++++++++-
 2 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/src/gtest/test_checktransaction.cpp b/src/gtest/test_checktransaction.cpp
index 1431c9388..d3f0f2f41 100644
--- a/src/gtest/test_checktransaction.cpp
+++ b/src/gtest/test_checktransaction.cpp
@@ -535,6 +535,54 @@ TEST(ChecktransactionTests, BadTxnsInvalidJoinsplitSignature) {
     ContextualCheckTransaction(tx, state, chainparams, 0, true, [](const CChainParams&) { return false; });
 }
 
+TEST(ChecktransactionTests, JoinsplitSignatureDetectsOldBranchId) {
+    SelectParams(CBaseChainParams::REGTEST);
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, 1);
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, 1);
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_BLOSSOM, 10);
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_HEARTWOOD, 20);
+    auto chainparams = Params();
+
+    auto saplingBranchId = NetworkUpgradeInfo[Consensus::UPGRADE_SAPLING].nBranchId;
+    auto blossomBranchId = NetworkUpgradeInfo[Consensus::UPGRADE_BLOSSOM].nBranchId;
+
+    // Create a valid transaction for the Sapling epoch.
+    CMutableTransaction mtx = GetValidTransaction(saplingBranchId);
+    CTransaction tx(mtx);
+
+    MockCValidationState state;
+    // Ensure that the transaction validates against Sapling.
+    EXPECT_TRUE(ContextualCheckTransaction(
+        tx, state, chainparams, 5, false,
+        [](const CChainParams&) { return false; }));
+
+    // Attempt to validate the inputs against Blossom. We should be notified
+    // that an old consensus branch ID was used for an input.
+    EXPECT_CALL(state, DoS(
+        10, false, REJECT_INVALID,
+        strprintf("old-consensus-branch-id (Expected %s, found %s)",
+            HexInt(blossomBranchId),
+            HexInt(saplingBranchId)),
+        false)).Times(1);
+    EXPECT_FALSE(ContextualCheckTransaction(
+        tx, state, chainparams, 15, false,
+        [](const CChainParams&) { return false; }));
+
+    // Attempt to validate the inputs against Heartwood. All we should learn is
+    // that the signature is invalid, because we don't check more than one
+    // network upgrade back.
+    EXPECT_CALL(state, DoS(
+        10, false, REJECT_INVALID,
+        "bad-txns-invalid-joinsplit-signature", false)).Times(1);
+    EXPECT_FALSE(ContextualCheckTransaction(
+        tx, state, chainparams, 25, false,
+        [](const CChainParams&) { return false; }));
+
+    // Revert to default
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_HEARTWOOD, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT);
+    RegtestDeactivateBlossom();
+}
+
 TEST(ChecktransactionTests, NonCanonicalEd25519Signature) {
     SelectParams(CBaseChainParams::REGTEST);
     auto chainparams = Params();
diff --git a/src/main.cpp b/src/main.cpp
index 25a3bbb42..66379557d 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -936,17 +936,20 @@ bool ContextualCheckTransaction(
         }
     }
 
+    auto consensusBranchId = CurrentEpochBranchId(nHeight, chainparams.GetConsensus());
+    auto prevConsensusBranchId = PrevEpochBranchId(consensusBranchId, chainparams.GetConsensus());
     uint256 dataToBeSigned;
+    uint256 prevDataToBeSigned;
 
     if (!tx.vJoinSplit.empty() ||
         !tx.vShieldedSpend.empty() ||
         !tx.vShieldedOutput.empty())
     {
-        auto consensusBranchId = CurrentEpochBranchId(nHeight, chainparams.GetConsensus());
         // Empty output script.
         CScript scriptCode;
         try {
             dataToBeSigned = SignatureHash(scriptCode, tx, NOT_AN_INPUT, SIGHASH_ALL, 0, consensusBranchId);
+            prevDataToBeSigned = SignatureHash(scriptCode, tx, NOT_AN_INPUT, SIGHASH_ALL, 0, prevConsensusBranchId);
         } catch (std::logic_error ex) {
             // A logic error should never occur because we pass NOT_AN_INPUT and
             // SIGHASH_ALL to SignatureHash().
@@ -965,6 +968,21 @@ bool ContextualCheckTransaction(
                                         dataToBeSigned.begin(), 32,
                                         tx.joinSplitPubKey.begin()
                                         ) != 0) {
+            // Check whether the failure was caused by an outdated consensus
+            // branch ID; if so, inform the node that they need to upgrade. We
+            // only check the previous epoch's branch ID, on the assumption that
+            // users creating transactions will notice their transactions
+            // failing before a second network upgrade occurs.
+            if (crypto_sign_verify_detached(&tx.joinSplitSig[0],
+                                            prevDataToBeSigned.begin(), 32,
+                                            tx.joinSplitPubKey.begin()
+                                            ) == 0) {
+                return state.DoS(
+                    dosLevelPotentiallyRelaxing, false, REJECT_INVALID, strprintf(
+                        "old-consensus-branch-id (Expected %s, found %s)",
+                        HexInt(consensusBranchId),
+                        HexInt(prevConsensusBranchId)));
+            }
             return state.DoS(
                 dosLevelPotentiallyRelaxing,
                 error("CheckTransaction(): invalid joinsplit signature"),