Skip to content

Commit 63a87f9

Browse files
random-zebrarandom-zebra
committed
[Refactoring] move zerocoin contextual checks out of CheckTransaction
1 parent bfd3fe4 commit 63a87f9

12 files changed

+90
-71
lines changed

‎src/chainparams.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ class CMainParams : public CChainParams
183183
consensus.ZC_MinMintFee = 1 * CENT;
184184
consensus.ZC_MinStakeDepth = 200;
185185
consensus.ZC_TimeStart = 1508214600; // October 17, 2017 4:30:00 AM
186+
consensus.ZC_HeightStart = 863735;
186187

187188
// Network upgrades
188189
consensus.vUpgrades[Consensus::BASE_NETWORK].nActivationHeight =
@@ -302,6 +303,7 @@ class CTestNetParams : public CChainParams
302303
consensus.height_last_ZC_AccumCheckpoint = -1;
303304
consensus.height_last_ZC_WrappedSerials = -1;
304305
consensus.height_ZC_RecalcAccumulators = 999999999;
306+
consensus.ZC_HeightStart = 0;
305307

306308
// Zerocoin-related params
307309
consensus.ZC_Modulus = "25195908475657893494027183240048398571429282126204032027777137836043662020707595556264018525880784"
@@ -441,6 +443,7 @@ class CRegTestParams : public CChainParams
441443
consensus.ZC_MinMintFee = 1 * CENT;
442444
consensus.ZC_MinStakeDepth = 10;
443445
consensus.ZC_TimeStart = 0; // not implemented on regtest
446+
consensus.ZC_HeightStart = 0;
444447

445448
// Network upgrades
446449
consensus.vUpgrades[Consensus::BASE_NETWORK].nActivationHeight =

‎src/consensus/params.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ struct Params {
169169
CAmount ZC_MinMintFee;
170170
int ZC_MinStakeDepth;
171171
int ZC_TimeStart;
172+
int ZC_HeightStart;
172173

173174
libzerocoin::ZerocoinParams* Zerocoin_Params(bool useModulusV1) const
174175
{

‎src/consensus/tx_verify.cpp‎

Lines changed: 5 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& in
5252
return nSigOps;
5353
}
5454

55-
bool CheckTransaction(const CTransaction& tx, bool fZerocoinActive, CValidationState& state, bool fFakeSerialAttack, bool fColdStakingActive)
55+
bool CheckTransaction(const CTransaction& tx, CValidationState& state, bool fColdStakingActive)
5656
{
5757
// Basic checks that don't depend on any context
5858
// Transactions containing empty `vin` must have non-empty `vShieldedSpend`.
@@ -110,60 +110,21 @@ bool CheckTransaction(const CTransaction& tx, bool fZerocoinActive, CValidationS
110110
}
111111

112112
std::set<COutPoint> vInOutPoints;
113-
std::set<CBigNum> vZerocoinSpendSerials;
114-
int nZCSpendCount = 0;
115-
116113
for (const CTxIn& txin : tx.vin) {
117114
// Check for duplicate inputs
118115
if (vInOutPoints.count(txin.prevout))
119116
return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputs-duplicate");
120-
121-
//duplicate zcspend serials are checked in CheckZerocoinSpend()
122117
if (!txin.IsZerocoinSpend()) {
123118
vInOutPoints.insert(txin.prevout);
124-
} else if (!txin.IsZerocoinPublicSpend()) {
125-
nZCSpendCount++;
126-
}
127-
}
128-
129-
if (fZerocoinActive) {
130-
if (nZCSpendCount > consensus.ZC_MaxSpendsPerTx)
131-
return state.DoS(100, error("CheckTransaction() : there are more zerocoin spends than are allowed in one transaction"));
132-
133-
//require that a zerocoinspend only has inputs that are zerocoins
134-
if (tx.HasZerocoinSpendInputs()) {
135-
for (const CTxIn& in : tx.vin) {
136-
if (!in.IsZerocoinSpend() && !in.IsZerocoinPublicSpend())
137-
return state.DoS(100,
138-
error("CheckTransaction() : zerocoinspend contains inputs that are not zerocoins"));
139-
}
140-
141-
// Do not require signature verification if this is initial sync and a block over 24 hours old
142-
bool fVerifySignature = !IsInitialBlockDownload() && (GetTime() - chainActive.Tip()->GetBlockTime() < (60*60*24));
143-
if (!CheckZerocoinSpend(tx, fVerifySignature, state, fFakeSerialAttack))
144-
return state.DoS(100, error("CheckTransaction() : invalid zerocoin spend"));
145119
}
146120
}
147121

148122
if (tx.IsCoinBase()) {
149123
if (tx.vin[0].scriptSig.size() < 2 || tx.vin[0].scriptSig.size() > 150)
150124
return state.DoS(100, false, REJECT_INVALID, "bad-cb-length");
151-
} else if (fZerocoinActive && tx.HasZerocoinSpendInputs()) {
152-
if (tx.vin.size() < 1)
153-
return state.DoS(10, false, REJECT_INVALID, "bad-zc-spend-min-inputs");
154-
if (tx.HasZerocoinPublicSpendInputs()) {
155-
// tx has public zerocoin spend inputs
156-
if(static_cast<int>(tx.vin.size()) > consensus.ZC_MaxPublicSpendsPerTx)
157-
return state.DoS(10, false, REJECT_INVALID, "bad-zc-spend-max-inputs");
158-
} else {
159-
// tx has regular zerocoin spend inputs
160-
if(static_cast<int>(tx.vin.size()) > consensus.ZC_MaxSpendsPerTx)
161-
return state.DoS(10, false, REJECT_INVALID, "bad-zc-spend-max-inputs");
162-
}
163-
164125
} else {
165126
for (const CTxIn& txin : tx.vin)
166-
if (txin.prevout.IsNull() && (fZerocoinActive && !txin.IsZerocoinSpend()))
127+
if (txin.prevout.IsNull() && !txin.IsZerocoinSpend())
167128
return state.DoS(10, false, REJECT_INVALID, "bad-txns-prevout-null");
168129
}
169130

@@ -178,7 +139,9 @@ bool ContextualCheckTransaction(const CTransactionRef& tx, CValidationState& sta
178139
}
179140

180141
// Dispatch to ZerocoinTx validator
181-
// !TODO
142+
if (!ContextualCheckZerocoinTx(tx, state, chainparams.GetConsensus(), nHeight)) {
143+
return false; // Failure reason has been set in validation state object
144+
}
182145

183146
return true;
184147
}

‎src/consensus/tx_verify.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class CValidationState;
1818
/** Transaction validation functions */
1919

2020
/** Context-independent validity checks */
21-
bool CheckTransaction(const CTransaction& tx, bool fZerocoinActive, CValidationState& state, bool fFakeSerialAttack, bool fColdStakingActive);
21+
bool CheckTransaction(const CTransaction& tx, CValidationState& state, bool fColdStakingActive);
2222
/** Context-dependent validity checks */
2323
bool ContextualCheckTransaction(const CTransactionRef& tx, CValidationState& state, const CChainParams& chainparams, int nHeight, bool isMined, bool fIBD);
2424

‎src/consensus/zerocoin_verify.cpp‎

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@
1818
#include "zpiv/zpivmodule.h"
1919

2020

21-
bool CheckZerocoinSpend(const CTransaction& tx, bool fVerifySignature, CValidationState& state, bool fFakeSerialAttack)
21+
bool CheckZerocoinSpend(const CTransactionRef _tx, bool fVerifySignature, CValidationState& state, bool fFakeSerialAttack)
2222
{
23+
const CTransaction& tx = *_tx;
2324
//max needed non-mint outputs should be 2 - one for redemption address and a possible 2nd for change
2425
if (tx.vout.size() > 2) {
2526
int outs = 0;
@@ -135,6 +136,65 @@ bool CheckPublicCoinSpendVersion(int version) {
135136
return version == CurrentPublicCoinSpendVersion();
136137
}
137138

139+
bool ContextualCheckZerocoinTx(const CTransactionRef& tx, CValidationState& state, const Consensus::Params& consensus, int nHeight)
140+
{
141+
// zerocoin enforced via block time. First block with a zc mint is 863735
142+
const bool fZerocoinEnforced = (nHeight >= consensus.ZC_HeightStart);
143+
const bool fPublicSpendEnforced = consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_ZC_PUBLIC);
144+
const bool fRejectMintsAndPrivateSpends = !fZerocoinEnforced || fPublicSpendEnforced;
145+
const bool fRejectPublicSpends = !fPublicSpendEnforced || consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V5_0);
146+
147+
const bool hasPrivateSpendInputs = !tx->vin.empty() && tx->vin[0].IsZerocoinSpend();
148+
const bool hasPublicSpendInputs = !tx->vin.empty() && tx->vin[0].IsZerocoinPublicSpend();
149+
const std::string& txId = tx->GetHash().ToString();
150+
151+
int nSpendCount{0};
152+
for (const CTxIn& in : tx->vin) {
153+
if (in.IsZerocoinSpend()) {
154+
if (fRejectMintsAndPrivateSpends)
155+
return state.DoS(100, error("%s: zerocoin spend tx %s not accepted at height %d",
156+
__func__, txId, nHeight), REJECT_INVALID, "bad-txns-zc-private-spend");
157+
if (!hasPrivateSpendInputs)
158+
return state.DoS(100, error("%s: zerocoin spend tx %s has mixed spend inputs",
159+
__func__, txId), REJECT_INVALID, "bad-txns-zc-private-spend-mixed-types");
160+
if (++nSpendCount > consensus.ZC_MaxSpendsPerTx)
161+
return state.DoS(100, error("%s: zerocoin spend tx %s has more than %d inputs",
162+
__func__, txId, consensus.ZC_MaxSpendsPerTx), REJECT_INVALID, "bad-txns-zc-private-spend-max-inputs");
163+
164+
} else if (in.IsZerocoinPublicSpend()) {
165+
if (fRejectPublicSpends)
166+
return state.DoS(100, error("%s: zerocoin public spend tx %s not accepted at height %d",
167+
__func__, txId, nHeight), REJECT_INVALID, "bad-txns-zc-public-spend");
168+
if (!hasPublicSpendInputs)
169+
return state.DoS(100, error("%s: zerocoin spend tx %s has mixed spend inputs",
170+
__func__, txId), REJECT_INVALID, "bad-txns-zc-public-spend-mixed-types");
171+
if (++nSpendCount > consensus.ZC_MaxPublicSpendsPerTx)
172+
return state.DoS(100, error("%s: zerocoin spend tx %s has more than %d inputs",
173+
__func__, txId, consensus.ZC_MaxPublicSpendsPerTx), REJECT_INVALID, "bad-txns-zc-public-spend-max-inputs");
174+
175+
} else {
176+
// this is a transparent input
177+
if (hasPrivateSpendInputs || hasPublicSpendInputs)
178+
return state.DoS(100, error("%s: zerocoin spend tx %s has mixed spend inputs",
179+
__func__, txId), REJECT_INVALID, "bad-txns-zc-spend-mixed-types");
180+
}
181+
}
182+
183+
if (hasPrivateSpendInputs || hasPublicSpendInputs) {
184+
if (!CheckZerocoinSpend(tx, false, state, false))
185+
return false; // failure reason logged in validation state
186+
}
187+
188+
for (const CTxOut& o : tx->vout) {
189+
if (o.IsZerocoinMint() && fRejectMintsAndPrivateSpends) {
190+
return state.DoS(100, error("%s: zerocoin mint tx %s not accepted at height %d",
191+
__func__, txId, nHeight), REJECT_INVALID, "bad-txns-zc-mint");
192+
}
193+
}
194+
195+
return true;
196+
}
197+
138198
bool ContextualCheckZerocoinSpend(const CTransaction& tx, const libzerocoin::CoinSpend* spend, int nHeight, const uint256& hashBlock)
139199
{
140200
if(!ContextualCheckZerocoinSpendNoSerialCheck(tx, spend, nHeight, hashBlock)){

‎src/consensus/zerocoin_verify.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,14 @@
1010
#include "zpivchain.h"
1111

1212
/** Context-independent validity checks */
13-
bool CheckZerocoinSpend(const CTransaction& tx, bool fVerifySignature, CValidationState& state, bool fFakeSerialAttack = false);
13+
bool CheckZerocoinSpend(const CTransactionRef _tx, bool fVerifySignature, CValidationState& state, bool fFakeSerialAttack = false);
1414
// Fake Serial attack Range
1515
bool isBlockBetweenFakeSerialAttackRange(int nHeight);
1616
// Public coin spend
1717
bool CheckPublicCoinSpendEnforced(int blockHeight, bool isPublicSpend);
1818
int CurrentPublicCoinSpendVersion();
1919
bool CheckPublicCoinSpendVersion(int version);
20+
bool ContextualCheckZerocoinTx(const CTransactionRef& tx, CValidationState& state, const Consensus::Params& consensus, int nHeight);
2021
bool ContextualCheckZerocoinSpend(const CTransaction& tx, const libzerocoin::CoinSpend* spend, int nHeight, const uint256& hashBlock);
2122
bool ContextualCheckZerocoinSpendNoSerialCheck(const CTransaction& tx, const libzerocoin::CoinSpend* spend, int nHeight, const uint256& hashBlock);
2223

‎src/qt/walletmodel.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ WalletModel::SendCoinsReturn WalletModel::sendCoins(WalletModelTransaction& tran
520520
// Double check the tx before doing anything
521521
CTransactionRef& newTx = transaction.getTransaction();
522522
CValidationState state;
523-
if (!CheckTransaction(*newTx, true, state, true, fColdStakingActive)) {
523+
if (!CheckTransaction(*newTx, state, fColdStakingActive)) {
524524
return TransactionCheckFailed;
525525
}
526526

‎src/test/sighash_tests.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ BOOST_AUTO_TEST_CASE(sighash_from_data)
228228
stream >> tx;
229229

230230
CValidationState state;
231-
BOOST_CHECK_MESSAGE(CheckTransaction(*tx, false, state, false, false), strTest);
231+
BOOST_CHECK_MESSAGE(CheckTransaction(*tx, state, false), strTest);
232232
BOOST_CHECK(state.IsValid());
233233

234234
std::vector<unsigned char> raw = ParseHex(raw_script);

‎src/test/transaction_tests.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ BOOST_AUTO_TEST_CASE(tx_valid)
139139
CTransaction tx(deserialize, stream);
140140

141141
CValidationState state;
142-
BOOST_CHECK_MESSAGE(CheckTransaction(tx, false, state, false, false), strTest);
142+
BOOST_CHECK_MESSAGE(CheckTransaction(tx, state, false), strTest);
143143
BOOST_CHECK(state.IsValid());
144144

145145
PrecomputedTransactionData precomTxData(tx);
@@ -215,7 +215,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid)
215215
CTransaction tx(deserialize, stream);
216216

217217
CValidationState state;
218-
fValid = CheckTransaction(tx, false, state, false, false) && state.IsValid();
218+
fValid = CheckTransaction(tx, state, false) && state.IsValid();
219219

220220
PrecomputedTransactionData precomTxData(tx);
221221
for (unsigned int i = 0; i < tx.vin.size() && fValid; i++)
@@ -246,11 +246,11 @@ BOOST_AUTO_TEST_CASE(basic_transaction_tests)
246246
CMutableTransaction tx;
247247
stream >> tx;
248248
CValidationState state;
249-
BOOST_CHECK_MESSAGE(CheckTransaction(tx, false, state, false, false) && state.IsValid(), "Simple deserialized transaction should be valid.");
249+
BOOST_CHECK_MESSAGE(CheckTransaction(tx, state, false) && state.IsValid(), "Simple deserialized transaction should be valid.");
250250

251251
// Check that duplicate txins fail
252252
tx.vin.push_back(tx.vin[0]);
253-
BOOST_CHECK_MESSAGE(!CheckTransaction(tx, false, state, false, false) || !state.IsValid(), "Transaction with duplicate txins should be invalid.");
253+
BOOST_CHECK_MESSAGE(!CheckTransaction(tx, state, false) || !state.IsValid(), "Transaction with duplicate txins should be invalid.");
254254
}
255255

256256
//

‎src/test/validation_tests.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ void test_simple_sapling_invalidity(CMutableTransaction& tx)
5757
CMutableTransaction newTx(tx);
5858
CValidationState state;
5959

60-
BOOST_CHECK(!CheckTransaction(newTx, false, state, false, false));
60+
BOOST_CHECK(!CheckTransaction(newTx, state, false));
6161
BOOST_CHECK(state.GetRejectReason() == "bad-txns-vin-empty");
6262
}
6363
{
@@ -67,7 +67,7 @@ void test_simple_sapling_invalidity(CMutableTransaction& tx)
6767
newTx.sapData->vShieldedSpend.emplace_back();
6868
newTx.sapData->vShieldedSpend[0].nullifier = GetRandHash();
6969

70-
BOOST_CHECK(!CheckTransaction(newTx, false, state, false, false));
70+
BOOST_CHECK(!CheckTransaction(newTx, state, false));
7171
BOOST_CHECK(state.GetRejectReason() == "bad-txns-vout-empty");
7272
}
7373
{
@@ -104,7 +104,7 @@ void test_simple_sapling_invalidity(CMutableTransaction& tx)
104104

105105
newTx.sapData->vShieldedSpend.emplace_back();
106106

107-
BOOST_CHECK(!CheckTransaction(newTx, false, state, false, false));
107+
BOOST_CHECK(!CheckTransaction(newTx, state, false));
108108
BOOST_CHECK(state.GetRejectReason() == "bad-txns-invalid-sapling");
109109
}
110110
{
@@ -123,7 +123,7 @@ void test_simple_sapling_invalidity(CMutableTransaction& tx)
123123

124124
newTx.sapData->vShieldedSpend.emplace_back();
125125

126-
BOOST_CHECK(!CheckTransaction(newTx, false, state, false, false));
126+
BOOST_CHECK(!CheckTransaction(newTx, state, false));
127127
BOOST_CHECK(state.GetRejectReason() == "bad-txns-invalid-sapling");
128128
}
129129
}

0 commit comments

Comments
 (0)