zecfoundation
/
zcashd
mirror of https://github.com/ZcashFoundation/zcashd.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Auto merge of #4371 - str4d:4260-nu-branch-id-errors, r=str4d 

Check failing transparent and JoinSplit signatures against the previous network upgrade

This change improves usability across network upgrades, by informing
users when their new transactions are being created with the consensus
branch ID from the previous epoch.

We only check failing signatures against the previous epoch to minimise
the extra computational load on nodes.

A future refactor is needed to similarly check Sapling signatures.

Part of #4260.
This commit is contained in:
Homu 2020-04-09 04:27:29 +00:00
parent e2624328ec 4216319ee6
commit 726bd2a2c8
8 changed files with 260 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/consensus/upgrades.cpp
View File

@ -84,6 +84,16 @@ uint32_t CurrentEpochBranchId(int nHeight, const Consensus::Params& params) {
return NetworkUpgradeInfo[CurrentEpoch(nHeight, params)].nBranchId;
}
uint32_t PrevEpochBranchId(uint32_t currentBranchId, const Consensus::Params& params) {
for (int idx = Consensus::BASE_SPROUT + 1; idx < Consensus::MAX_NETWORK_UPGRADES; idx++) {
if (currentBranchId == NetworkUpgradeInfo[idx].nBranchId) {
return NetworkUpgradeInfo[idx - 1].nBranchId;
}
}
// Base case
return NetworkUpgradeInfo[Consensus::BASE_SPROUT].nBranchId;
}
bool IsConsensusBranchId(int branchId) {
for (int idx = Consensus::BASE_SPROUT; idx < Consensus::MAX_NETWORK_UPGRADES; idx++) {
if (branchId == NetworkUpgradeInfo[idx].nBranchId) {

6
src/consensus/upgrades.h
View File

@ -53,6 +53,12 @@ int CurrentEpoch(int nHeight, const Consensus::Params& params);
*/
uint32_t CurrentEpochBranchId(int nHeight, const Consensus::Params& params);
/**
* Returns the branch ID that preceded currentBranchId, or 0 if no upgrade
* matches currentBranchId.
*/
uint32_t PrevEpochBranchId(uint32_t currentBranchId, const Consensus::Params& params);
/**
* Returns true if a given branch id is a valid nBranchId for one of the network
* upgrades contained in NetworkUpgradeInfo.

72
src/gtest/test_checktransaction.cpp
View File

@ -7,6 +7,7 @@
#include "consensus/validation.h"
#include "transaction_builder.h"
#include "utiltest.h"
#include "zcash/JoinSplit.hpp"
#include <librustzcash.h>
@ -51,10 +52,21 @@ public:
void CreateJoinSplitSignature(CMutableTransaction& mtx, uint32_t consensusBranchId);
CMutableTransaction GetValidTransaction() {
uint32_t consensusBranchId = SPROUT_BRANCH_ID;
CMutableTransaction GetValidTransaction(uint32_t consensusBranchId=SPROUT_BRANCH_ID) {
CMutableTransaction mtx;
if (consensusBranchId == NetworkUpgradeInfo[Consensus::UPGRADE_OVERWINTER].nBranchId) {
mtx.fOverwintered = true;
mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID;
mtx.nVersion = OVERWINTER_TX_VERSION;
} else if (consensusBranchId == NetworkUpgradeInfo[Consensus::UPGRADE_SAPLING].nBranchId) {
mtx.fOverwintered = true;
mtx.nVersionGroupId = SAPLING_VERSION_GROUP_ID;
mtx.nVersion = SAPLING_TX_VERSION;
} else if (consensusBranchId != SPROUT_BRANCH_ID) {
// Unsupported consensus branch ID
assert(false);
}
mtx.vin.resize(2);
mtx.vin[0].prevout.hash = uint256S("0000000000000000000000000000000000000000000000000000000000000001");
mtx.vin[0].prevout.n = 0;
@ -70,6 +82,12 @@ CMutableTransaction GetValidTransaction() {
mtx.vJoinSplit[1].nullifiers.at(0) = uint256S("0000000000000000000000000000000000000000000000000000000000000002");
mtx.vJoinSplit[1].nullifiers.at(1) = uint256S("0000000000000000000000000000000000000000000000000000000000000003");
if (mtx.nVersion >= SAPLING_TX_VERSION) {
libzcash::GrothProof emptyProof;
mtx.vJoinSplit[0].proof = emptyProof;
mtx.vJoinSplit[1].proof = emptyProof;
}
CreateJoinSplitSignature(mtx, consensusBranchId);
return mtx;
}
@ -520,6 +538,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();

2
src/gtest/test_mempool.cpp
View File

@ -11,7 +11,7 @@
#include "util.h"
// Implementation is in test_checktransaction.cpp
extern CMutableTransaction GetValidTransaction();
extern CMutableTransaction GetValidTransaction(uint32_t consensusBranchId=SPROUT_BRANCH_ID);
// Fake the input of transaction 5295156213414ed77f6e538e7e8ebe14492156906b9fe995b242477818789364
// - 532639cc6bebed47c1c69ae36dd498c68a012e74ad12729adbd3dbb56f8f3f4a, 0

140
src/gtest/test_validation.cpp
View File

@ -1,8 +1,10 @@
#include <gtest/gtest.h>
#include <gmock/gmock.h>
#include "consensus/upgrades.h"
#include "consensus/validation.h"
#include "main.h"
#include "transaction_builder.h"
#include "utiltest.h"
extern ZCJoinSplit* params;
@ -21,10 +23,14 @@ void ExpectOptionalAmount(CAmount expected, boost::optional<CAmount> actual) {
}
}
// Fake an empty view
class FakeCoinsViewDB : public CCoinsView {
// Fake a view that optionally contains a single coin.
class ValidationFakeCoinsViewDB : public CCoinsView {
public:
FakeCoinsViewDB() {}
boost::optional<std::pair<std::pair<uint256, uint256>, std::pair<CTxOut, int>>> coin;
ValidationFakeCoinsViewDB() {}
ValidationFakeCoinsViewDB(uint256 blockHash, uint256 txid, CTxOut txOut, int nHeight) :
coin(std::make_pair(std::make_pair(blockHash, txid), std::make_pair(txOut, nHeight))) {}
bool GetSproutAnchorAt(const uint256 &rt, SproutMerkleTree &tree) const {
return false;
@ -39,16 +45,33 @@ public:
}
bool GetCoins(const uint256 &txid, CCoins &coins) const {
return false;
if (coin && txid == coin.get().first.second) {
CCoins newCoins;
newCoins.vout.resize(2);
newCoins.vout[0] = coin.get().second.first;
newCoins.nHeight = coin.get().second.second;
coins.swap(newCoins);
return true;
} else {
return false;
}
}
bool HaveCoins(const uint256 &txid) const {
return false;
if (coin && txid == coin.get().first.second) {
return true;
} else {
return false;
}
}
uint256 GetBestBlock() const {
uint256 a;
return a;
if (coin) {
return coin.get().first.first;
} else {
uint256 a;
return a;
}
}
uint256 GetBestAnchor(ShieldedType type) const {
@ -72,6 +95,23 @@ public:
}
};
class MockCValidationState : public CValidationState {
public:
MOCK_METHOD5(DoS, bool(int level, bool ret,
unsigned char chRejectCodeIn, std::string strRejectReasonIn,
bool corruptionIn));
MOCK_METHOD3(Invalid, bool(bool ret,
unsigned char _chRejectCode, std::string _strRejectReason));
MOCK_METHOD1(Error, bool(std::string strRejectReasonIn));
MOCK_CONST_METHOD0(IsValid, bool());
MOCK_CONST_METHOD0(IsInvalid, bool());
MOCK_CONST_METHOD0(IsError, bool());
MOCK_CONST_METHOD1(IsInvalid, bool(int &nDoSOut));
MOCK_CONST_METHOD0(CorruptionPossible, bool());
MOCK_CONST_METHOD0(GetRejectCode, unsigned char());
MOCK_CONST_METHOD0(GetRejectReason, std::string());
};
TEST(Validation, ContextualCheckInputsPassesWithCoinbase) {
// Create fake coinbase transaction
CMutableTransaction mtx;
@ -80,7 +120,7 @@ TEST(Validation, ContextualCheckInputsPassesWithCoinbase) {
ASSERT_TRUE(tx.IsCoinBase());
// Fake an empty view
FakeCoinsViewDB fakeDB;
ValidationFakeCoinsViewDB fakeDB;
CCoinsViewCache view(&fakeDB);
for (int idx = Consensus::BASE_SPROUT; idx < Consensus::MAX_NETWORK_UPGRADES; idx++) {
@ -91,7 +131,91 @@ TEST(Validation, ContextualCheckInputsPassesWithCoinbase) {
}
}
TEST(Validation, ContextualCheckInputsDetectsOldBranchId) {
SelectParams(CBaseChainParams::REGTEST);
UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, 10);
UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, 20);
UpdateNetworkUpgradeParameters(Consensus::UPGRADE_BLOSSOM, 30);
auto consensusParams = Params(CBaseChainParams::REGTEST).GetConsensus();
auto overwinterBranchId = NetworkUpgradeInfo[Consensus::UPGRADE_OVERWINTER].nBranchId;
auto saplingBranchId = NetworkUpgradeInfo[Consensus::UPGRADE_SAPLING].nBranchId;
auto blossomBranchId = NetworkUpgradeInfo[Consensus::UPGRADE_BLOSSOM].nBranchId;
CBasicKeyStore keystore;
CKey tsk = AddTestCKeyToKeyStore(keystore);
auto destination = tsk.GetPubKey().GetID();
auto scriptPubKey = GetScriptForDestination(destination);
// Create a fake block. It doesn't need to contain any transactions; we just
// need it to be in the global state when our fake view is used.
CBlock block;
block.hashMerkleRoot = block.BuildMerkleTree();
auto blockHash = block.GetHash();
CBlockIndex fakeIndex {block};
mapBlockIndex.insert(std::make_pair(blockHash, &fakeIndex));
chainActive.SetTip(&fakeIndex);
// Fake a view containing a single coin.
CAmount coinValue(50000);
COutPoint utxo;
utxo.hash = uint256S("4242424242424242424242424242424242424242424242424242424242424242");
utxo.n = 0;
CTxOut txOut;
txOut.scriptPubKey = scriptPubKey;
txOut.nValue = coinValue;
ValidationFakeCoinsViewDB fakeDB(blockHash, utxo.hash, txOut, 12);
CCoinsViewCache view(&fakeDB);
// Create a transparent transaction that spends the coin, targeting
// a height during the Overwinter epoch.
auto builder = TransactionBuilder(consensusParams, 15, &keystore);
builder.AddTransparentInput(utxo, scriptPubKey, coinValue);
builder.AddTransparentOutput(destination, 40000);
auto tx = builder.Build().GetTxOrThrow();
ASSERT_FALSE(tx.IsCoinBase());
// Ensure that the inputs validate against Overwinter.
CValidationState state;
PrecomputedTransactionData txdata(tx);
EXPECT_TRUE(ContextualCheckInputs(
tx, state, view, true, 0, false, txdata,
consensusParams, overwinterBranchId));
// Attempt to validate the inputs against Sapling. We should be notified
// that an old consensus branch ID was used for an input.
MockCValidationState mockState;
EXPECT_CALL(mockState, DoS(
10, false, REJECT_INVALID,
strprintf("old-consensus-branch-id (Expected %s, found %s)",
HexInt(saplingBranchId),
HexInt(overwinterBranchId)),
false)).Times(1);
EXPECT_FALSE(ContextualCheckInputs(
tx, mockState, view, true, 0, false, txdata,
consensusParams, saplingBranchId));
// Attempt to validate the inputs against Blossom. All we should learn is
// that the signature is invalid, because we don't check more than one
// network upgrade back.
EXPECT_CALL(mockState, DoS(
100, false, REJECT_INVALID,
"mandatory-script-verify-flag-failed (Script evaluated without error but finished with a false/empty top stack element)",
false)).Times(1);
EXPECT_FALSE(ContextualCheckInputs(
tx, mockState, view, true, 0, false, txdata,
consensusParams, blossomBranchId));
// Tear down
chainActive.SetTip(NULL);
mapBlockIndex.erase(blockHash);
// Revert to default
RegtestDeactivateBlossom();
}
TEST(Validation, ReceivedBlockTransactions) {
SelectParams(CBaseChainParams::REGTEST);
auto chainParams = Params();
auto sk = libzcash::SproutSpendingKey::random();

46
src/main.cpp
View File

@ -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"),
@ -1327,6 +1345,8 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
*pfMissingInputs = false;
int nextBlockHeight = chainActive.Height() + 1;
// Grab the branch ID we expect this transaction to commit to.
auto consensusBranchId = CurrentEpochBranchId(nextBlockHeight, Params().GetConsensus());
if (pool.IsRecentlyEvicted(tx.GetHash())) {
@ -1475,11 +1495,9 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
}
}
// Grab the branch ID we expect this transaction to commit to. We don't
// yet know if it does, but if the entry gets added to the mempool, then
// it has passed ContextualCheckInputs and therefore this is correct.
auto consensusBranchId = CurrentEpochBranchId(chainActive.Height() + 1, Params().GetConsensus());
// We don't yet know if the transaction commits to consensusBranchId,
// but if the entry gets added to the mempool, then it has passed
// ContextualCheckInputs and therefore this is correct.
CTxMemPoolEntry entry(tx, nFees, GetTime(), dPriority, chainActive.Height(), mempool.HasNoInputsOf(tx), fSpendsCoinbase, consensusBranchId);
unsigned int nSize = entry.GetTxSize();
@ -2139,6 +2157,22 @@ bool ContextualCheckInputs(
pvChecks->push_back(CScriptCheck());
check.swap(pvChecks->back());
} else if (!check()) {
// Check whether the failure was caused by an outdated
// consensus branch ID; if so, don't trigger DoS protection
// immediately, and 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.
auto prevConsensusBranchId = PrevEpochBranchId(consensusBranchId, consensusParams);
CScriptCheck checkPrev(*coins, tx, i, flags, cacheStore, prevConsensusBranchId, &txdata);
if (checkPrev()) {
return state.DoS(
10, false, REJECT_INVALID, strprintf(
"old-consensus-branch-id (Expected %s, found %s)",
HexInt(consensusBranchId),
HexInt(prevConsensusBranchId)));
}
if (flags & STANDARD_NOT_MANDATORY_VERIFY_FLAGS) {
// Check whether the failure was caused by a
// non-mandatory script verification check, such as

2
src/transaction_builder.cpp
View File

@ -209,7 +209,7 @@ void TransactionBuilder::AddTransparentInput(COutPoint utxo, CScript scriptPubKe
tIns.emplace_back(scriptPubKey, value);
}
void TransactionBuilder::AddTransparentOutput(CTxDestination& to, CAmount value)
void TransactionBuilder::AddTransparentOutput(const CTxDestination& to, CAmount value)
{
if (!IsValidDestination(to)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid output address, not a valid taddr.");

2
src/transaction_builder.h
View File

@ -135,7 +135,7 @@ public:
// Assumes that the value correctly corresponds to the provided UTXO.
void AddTransparentInput(COutPoint utxo, CScript scriptPubKey, CAmount value);
void AddTransparentOutput(CTxDestination& to, CAmount value);
void AddTransparentOutput(const CTxDestination& to, CAmount value);
void SendChangeTo(libzcash::SaplingPaymentAddress changeAddr, uint256 ovk);