diff --git a/src/coins.cpp b/src/coins.cpp index f2999df13..e568e7674 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -11,6 +11,8 @@ #include +#include + /** * calculate number of bytes for the bitmask, and its number of non-zero bytes * each bit in the bitmask represents the availability of one output, but the @@ -914,7 +916,7 @@ CAmount CCoinsViewCache::GetValueIn(const CTransaction& tx) const return nResult; } -bool CCoinsViewCache::HaveShieldedRequirements(const CTransaction& tx) const +std::optional CCoinsViewCache::HaveShieldedRequirements(const CTransaction& tx) const { boost::unordered_map intermediates; @@ -925,7 +927,12 @@ bool CCoinsViewCache::HaveShieldedRequirements(const CTransaction& tx) const if (GetNullifier(nullifier, SPROUT)) { // If the nullifier is set, this transaction // double-spends! - return false; + auto txid = tx.GetHash().ToString(); + auto nf = nullifier.ToString(); + TracingWarn("consensus", "Sprout double-spend detected", + "txid", txid.c_str(), + "nf", nf.c_str()); + return UnsatisfiedShieldedReq::SproutDuplicateNullifier; } } @@ -934,7 +941,12 @@ bool CCoinsViewCache::HaveShieldedRequirements(const CTransaction& tx) const if (it != intermediates.end()) { tree = it->second; } else if (!GetSproutAnchorAt(joinsplit.anchor, tree)) { - return false; + auto txid = tx.GetHash().ToString(); + auto anchor = joinsplit.anchor.ToString(); + TracingWarn("consensus", "Transaction uses unknown Sprout anchor", + "txid", txid.c_str(), + "anchor", anchor.c_str()); + return UnsatisfiedShieldedReq::SproutUnknownAnchor; } BOOST_FOREACH(const uint256& commitment, joinsplit.commitments) @@ -946,16 +958,27 @@ bool CCoinsViewCache::HaveShieldedRequirements(const CTransaction& tx) const } for (const SpendDescription &spendDescription : tx.vShieldedSpend) { - if (GetNullifier(spendDescription.nullifier, SAPLING)) // Prevent double spends - return false; + if (GetNullifier(spendDescription.nullifier, SAPLING)) { // Prevent double spends + auto txid = tx.GetHash().ToString(); + auto nf = spendDescription.nullifier.ToString(); + TracingWarn("consensus", "Sapling double-spend detected", + "txid", txid.c_str(), + "nf", nf.c_str()); + return UnsatisfiedShieldedReq::SaplingDuplicateNullifier; + } SaplingMerkleTree tree; if (!GetSaplingAnchorAt(spendDescription.anchor, tree)) { - return false; + auto txid = tx.GetHash().ToString(); + auto anchor = spendDescription.anchor.ToString(); + TracingWarn("consensus", "Transaction uses unknown Sapling anchor", + "txid", txid.c_str(), + "anchor", anchor.c_str()); + return UnsatisfiedShieldedReq::SaplingUnknownAnchor; } } - return true; + return std::nullopt; } bool CCoinsViewCache::HaveInputs(const CTransaction& tx) const diff --git a/src/coins.h b/src/coins.h index 10999e8d9..67cba90e8 100644 --- a/src/coins.h +++ b/src/coins.h @@ -447,6 +447,14 @@ public: friend class CCoinsViewCache; }; +/** The set of shielded requirements that might be unsatisfied. */ +enum class UnsatisfiedShieldedReq { + SproutDuplicateNullifier, + SproutUnknownAnchor, + SaplingDuplicateNullifier, + SaplingUnknownAnchor, +}; + /** CCoinsView that adds a memory cache for transactions to another CCoinsView */ class CCoinsViewCache : public CCoinsViewBacked { @@ -566,7 +574,7 @@ public: bool HaveInputs(const CTransaction& tx) const; //! Check whether all joinsplit and sapling spend requirements (anchors/nullifiers) are satisfied - bool HaveShieldedRequirements(const CTransaction& tx) const; + std::optional HaveShieldedRequirements(const CTransaction& tx) const; //! Return priority of tx at height nHeight double GetPriority(const CTransaction &tx, int nHeight) const; diff --git a/src/main.cpp b/src/main.cpp index 63316eaf3..a8c0a33e6 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -115,6 +115,28 @@ const string strMessageMagic = "Zcash Signed Message:\n"; // Internal stuff namespace { + unsigned char ShieldedReqRejectCode(UnsatisfiedShieldedReq shieldedReq) + { + switch (shieldedReq) { + case UnsatisfiedShieldedReq::SproutDuplicateNullifier: + case UnsatisfiedShieldedReq::SaplingDuplicateNullifier: + return REJECT_DUPLICATE; + case UnsatisfiedShieldedReq::SproutUnknownAnchor: + case UnsatisfiedShieldedReq::SaplingUnknownAnchor: + return REJECT_INVALID; + } + } + + std::string ShieldedReqRejectReason(UnsatisfiedShieldedReq shieldedReq) + { + switch (shieldedReq) { + case UnsatisfiedShieldedReq::SproutDuplicateNullifier: return "bad-txns-sprout-duplicate-nullifier"; + case UnsatisfiedShieldedReq::SproutUnknownAnchor: return "bad-txns-sprout-unknown-anchor"; + case UnsatisfiedShieldedReq::SaplingDuplicateNullifier: return "bad-txns-sapling-duplicate-nullifier"; + case UnsatisfiedShieldedReq::SaplingUnknownAnchor: return "bad-txns-sapling-unknown-anchor"; + } + } + /** Abort with a message */ bool AbortNode(const std::string& strMessage, const std::string& userMessage="") { @@ -1540,10 +1562,18 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa return state.Invalid(error("AcceptToMemoryPool: inputs already spent"), REJECT_DUPLICATE, "bad-txns-inputs-spent"); - // are the joinsplits' and sapling spends' requirements met in tx(valid anchors/nullifiers)? - if (!view.HaveShieldedRequirements(tx)) - return state.Invalid(error("AcceptToMemoryPool: shielded requirements not met"), - REJECT_DUPLICATE, "bad-txns-shielded-requirements-not-met"); + // Are the shielded spends' requirements met? + auto unmetShieldedReq = view.HaveShieldedRequirements(tx); + if (unmetShieldedReq) { + auto txid = tx.GetHash().ToString(); + auto rejectCode = ShieldedReqRejectCode(*unmetShieldedReq); + auto rejectReason = ShieldedReqRejectReason(*unmetShieldedReq); + TracingError( + "main", "AcceptToMemoryPool(): shielded requirements not met", + "txid", txid.c_str(), + "reason", rejectReason.c_str()); + return state.Invalid(false, rejectCode, rejectReason); + } // Bring the best block into scope view.GetBestBlock(); @@ -2174,9 +2204,18 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins if (!inputs.HaveInputs(tx)) return state.Invalid(error("CheckInputs(): %s inputs unavailable", tx.GetHash().ToString())); - // are the JoinSplit's requirements met? - if (!inputs.HaveShieldedRequirements(tx)) - return state.Invalid(error("CheckInputs(): %s JoinSplit requirements not met", tx.GetHash().ToString())); + // Are the shielded spends' requirements met? + auto unmetShieldedReq = inputs.HaveShieldedRequirements(tx); + if (unmetShieldedReq) { + auto txid = tx.GetHash().ToString(); + auto rejectCode = ShieldedReqRejectCode(*unmetShieldedReq); + auto rejectReason = ShieldedReqRejectReason(*unmetShieldedReq); + TracingError( + "main", "CheckInputs(): shielded requirements not met", + "txid", txid.c_str(), + "reason", rejectReason.c_str()); + return state.Invalid(false, rejectCode, rejectReason); + } CAmount nValueIn = 0; CAmount nFees = 0; @@ -2859,10 +2898,18 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin return state.DoS(100, error("ConnectBlock(): inputs missing/spent"), REJECT_INVALID, "bad-txns-inputs-missingorspent"); - // are the JoinSplit's requirements met? - if (!view.HaveShieldedRequirements(tx)) - return state.DoS(100, error("ConnectBlock(): JoinSplit requirements not met"), - REJECT_INVALID, "bad-txns-joinsplit-requirements-not-met"); + // Are the shielded spends' requirements met? + auto unmetShieldedReq = view.HaveShieldedRequirements(tx); + if (unmetShieldedReq) { + auto txid = tx.GetHash().ToString(); + auto rejectCode = ShieldedReqRejectCode(*unmetShieldedReq); + auto rejectReason = ShieldedReqRejectReason(*unmetShieldedReq); + TracingError( + "main", "ConnectBlock(): shielded requirements not met", + "txid", txid.c_str(), + "reason", rejectReason.c_str()); + return state.DoS(100, false, rejectCode, rejectReason); + } // insightexplorer // https://github.com/bitpay/bitcoin/commit/017f548ea6d89423ef568117447e61dd5707ec42#diff-7ec3c68a81efff79b6ca22ac1f1eabbaR2597 diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 01a4bf818..a092ca03e 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -644,7 +644,7 @@ BOOST_AUTO_TEST_CASE(chained_joinsplits) CMutableTransaction mtx; mtx.vJoinSplit.push_back(js2); - BOOST_CHECK(!cache.HaveShieldedRequirements(mtx)); + BOOST_CHECK(cache.HaveShieldedRequirements(mtx) == std::optional(UnsatisfiedShieldedReq::SproutUnknownAnchor)); } { @@ -654,7 +654,7 @@ BOOST_AUTO_TEST_CASE(chained_joinsplits) mtx.vJoinSplit.push_back(js2); mtx.vJoinSplit.push_back(js1); - BOOST_CHECK(!cache.HaveShieldedRequirements(mtx)); + BOOST_CHECK(cache.HaveShieldedRequirements(mtx) == std::optional(UnsatisfiedShieldedReq::SproutUnknownAnchor)); } { @@ -662,7 +662,7 @@ BOOST_AUTO_TEST_CASE(chained_joinsplits) mtx.vJoinSplit.push_back(js1); mtx.vJoinSplit.push_back(js2); - BOOST_CHECK(cache.HaveShieldedRequirements(mtx)); + BOOST_CHECK(cache.HaveShieldedRequirements(mtx) == std::nullopt); } { @@ -671,7 +671,7 @@ BOOST_AUTO_TEST_CASE(chained_joinsplits) mtx.vJoinSplit.push_back(js2); mtx.vJoinSplit.push_back(js3); - BOOST_CHECK(cache.HaveShieldedRequirements(mtx)); + BOOST_CHECK(cache.HaveShieldedRequirements(mtx) == std::nullopt); } { @@ -681,7 +681,7 @@ BOOST_AUTO_TEST_CASE(chained_joinsplits) mtx.vJoinSplit.push_back(js2); mtx.vJoinSplit.push_back(js3); - BOOST_CHECK(cache.HaveShieldedRequirements(mtx)); + BOOST_CHECK(cache.HaveShieldedRequirements(mtx) == std::nullopt); } }