Skip to content

Commit acdbcf8

Browse files
committed
Merge #2492: [Consensus] v5.3 network upgrade.
1355b20 Validation: Add new multi-inputs and multi-empty-outputs coinstake rules. (furszy) 599bd42 Test: Add coinstake multi-inputs and multi-empty-outputs test coverage. (furszy) a195c2e [Test] Introduce PoS testing suite. (furszy) e98c1e8 Add and guard new max time window for mnb and mnp sigtime. (furszy) 7f8fb12 Introduce v5.3 network upgrade. (furszy) Pull request description: Grouped the consensus changes for the coming v5.3 upgrade. There is no enforcement height defined to not set it until everything is ready but.. we aren't far from be able to enforce it on testnet and start testing the release deeply, only have to merge #2411 and can be done. ACKs for top commit: random-zebra: ACK 1355b20 Fuzzbawls: ACK 1355b20 Tree-SHA512: 9282857e54f5a03ef43d9098a376bd403729304fb8a12be9a68c7dca4b6b29aba444f60158df1d50715f0b40db34225771f39770d05c9f9be16226cb4cb8975b
2 parents c2b36fe + 1355b20 commit acdbcf8

15 files changed

+232
-32
lines changed

src/Makefile.test.include

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,9 @@ BITCOIN_TEST_SUITE = \
8484
if ENABLE_WALLET
8585
BITCOIN_TEST_SUITE += \
8686
wallet/test/wallet_test_fixture.cpp \
87-
wallet/test/wallet_test_fixture.h
87+
wallet/test/wallet_test_fixture.h \
88+
wallet/test/pos_test_fixture.cpp \
89+
wallet/test/pos_test_fixture.h
8890
endif
8991

9092
# test_pivx binary #
@@ -174,7 +176,8 @@ SAPLING_TESTS +=\
174176
test/librust/sapling_rpc_wallet_tests.cpp \
175177
test/librust/sapling_wallet_tests.cpp \
176178
wallet/test/wallet_shielded_balances_tests.cpp \
177-
wallet/test/wallet_sapling_transactions_validations_tests.cpp
179+
wallet/test/wallet_sapling_transactions_validations_tests.cpp \
180+
wallet/test/pos_validations_tests.cpp
178181
endif
179182

180183
test_test_pivx_SOURCES = $(BITCOIN_TEST_SUITE) $(BITCOIN_TESTS) $(SAPLING_TESTS) $(JSON_TEST_FILES) $(RAW_TEST_FILES)

src/chainparams.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,8 @@ class CMainParams : public CChainParams
201201
consensus.vUpgrades[Consensus::UPGRADE_V4_0].nActivationHeight = 2153200;
202202
consensus.vUpgrades[Consensus::UPGRADE_V5_0].nActivationHeight = 2700500;
203203
consensus.vUpgrades[Consensus::UPGRADE_V5_2].nActivationHeight = 2927000;
204+
consensus.vUpgrades[Consensus::UPGRADE_V5_3].nActivationHeight =
205+
Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT;
204206
consensus.vUpgrades[Consensus::UPGRADE_V6_0].nActivationHeight =
205207
Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT;
206208

@@ -340,6 +342,8 @@ class CTestNetParams : public CChainParams
340342
consensus.vUpgrades[Consensus::UPGRADE_V4_0].nActivationHeight = 201;
341343
consensus.vUpgrades[Consensus::UPGRADE_V5_0].nActivationHeight = 201;
342344
consensus.vUpgrades[Consensus::UPGRADE_V5_2].nActivationHeight = 262525;
345+
consensus.vUpgrades[Consensus::UPGRADE_V5_3].nActivationHeight =
346+
Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT;
343347
consensus.vUpgrades[Consensus::UPGRADE_V6_0].nActivationHeight =
344348
Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT;
345349

@@ -472,6 +476,7 @@ class CRegTestParams : public CChainParams
472476
Consensus::NetworkUpgrade::ALWAYS_ACTIVE;
473477
consensus.vUpgrades[Consensus::UPGRADE_V5_0].nActivationHeight = 300;
474478
consensus.vUpgrades[Consensus::UPGRADE_V5_2].nActivationHeight = 300;
479+
consensus.vUpgrades[Consensus::UPGRADE_V5_3].nActivationHeight = 251;
475480
consensus.vUpgrades[Consensus::UPGRADE_V6_0].nActivationHeight =
476481
Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT;
477482

src/consensus/params.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ enum UpgradeIndex : uint32_t {
3535
UPGRADE_V4_0,
3636
UPGRADE_V5_0,
3737
UPGRADE_V5_2,
38+
UPGRADE_V5_3,
3839
UPGRADE_V6_0,
3940
UPGRADE_TESTDUMMY,
4041
// NOTE: Also add new upgrades to NetworkUpgradeInfo in upgrades.cpp

src/consensus/upgrades.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,31 +31,35 @@ const struct NUInfo NetworkUpgradeInfo[Consensus::MAX_NETWORK_UPGRADES] = {
3131
},
3232
{
3333
/*.strName =*/ "Zerocoin_v2",
34-
/*.strInfo =*/ "new zerocoin serials and zPOS start",
34+
/*.strInfo =*/ "New zerocoin serials and zPOS start",
3535
},
3636
{
3737
/*.strName =*/ "BIP65",
3838
/*.strInfo =*/ "CLTV (BIP65) activation - start block v5",
3939
},
4040
{
4141
/*.strName =*/ "Zerocoin_Public",
42-
/*.strInfo =*/ "activation of zerocoin public spends (spend v3)",
42+
/*.strInfo =*/ "Activation of zerocoin public spends (spend v3)",
4343
},
4444
{
4545
/*.strName =*/ "PIVX_v3.4",
46-
/*.strInfo =*/ "new 256-bit stake modifier - start block v6",
46+
/*.strInfo =*/ "New 256-bit stake modifier - start block v6",
4747
},
4848
{
4949
/*.strName =*/ "PIVX_v4.0",
50-
/*.strInfo =*/ "new message sigs - start block v7 - time protocol - zc spend v4",
50+
/*.strInfo =*/ "New message sigs - start block v7 - time protocol - zc spend v4",
5151
},
5252
{
5353
/*.strName =*/ "v5_shield",
5454
/*.strInfo =*/ "Sapling Shield - start block v8 - start transaction v3",
5555
},
5656
{
5757
/*.strName =*/ "PIVX_v5.2",
58-
/*.strInfo =*/ "new cold-staking rules",
58+
/*.strInfo =*/ "New cold-staking rules",
59+
},
60+
{
61+
/*.strName =*/ "PIVX_v5.3",
62+
/*.strInfo =*/ "New staking rules",
5963
},
6064
{
6165
/*.strName =*/ "v6_evo",

src/masternode-payments.cpp

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -161,11 +161,11 @@ std::string CMasternodePaymentWinner::GetStrMessage() const
161161
return vinMasternode.prevout.ToStringShort() + std::to_string(nBlockHeight) + HexStr(payee);
162162
}
163163

164-
bool CMasternodePaymentWinner::IsValid(CNode* pnode, std::string& strError)
164+
bool CMasternodePaymentWinner::IsValid(CNode* pnode, std::string& strError, int chainHeight)
165165
{
166166
int n = mnodeman.GetMasternodeRank(vinMasternode, nBlockHeight - 100);
167-
168-
if (n < 1 || n > MNPAYMENTS_SIGNATURES_TOTAL) {
167+
bool guard = Params().GetConsensus().NetworkUpgradeActive(chainHeight, Consensus::UPGRADE_V5_3);
168+
if ((guard && n < 1) || n > MNPAYMENTS_SIGNATURES_TOTAL) {
169169
//It's common to have masternodes mistakenly think they are in the top 10
170170
// We don't want to print all of these messages, or punish them unless they're way off
171171
if (n > MNPAYMENTS_SIGNATURES_TOTAL * 2) {
@@ -176,6 +176,12 @@ bool CMasternodePaymentWinner::IsValid(CNode* pnode, std::string& strError)
176176
return false;
177177
}
178178

179+
// Must be a P2PKH
180+
if (guard && !payee.IsPayToPublicKeyHash()) {
181+
LogPrint(BCLog::MASTERNODE, "%s - payee must be a P2PKH\n", __func__);
182+
return false;
183+
}
184+
179185
return true;
180186
}
181187

@@ -219,9 +225,9 @@ bool IsBlockValueValid(int nHeight, CAmount& nExpectedValue, CAmount nMinted, CA
219225
}
220226
}
221227

222-
// !todo: remove after V6 enforcement and default it to true
223-
const bool isV6UpgradeEnforced = consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V6_0);
224-
return (!isV6UpgradeEnforced || nMinted >= 0) && nMinted <= nExpectedValue;
228+
// !todo: remove after V5.3 enforcement and default it to true
229+
const bool isUpgradeEnforced = consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V5_3);
230+
return (!isUpgradeEnforced || nMinted >= 0) && nMinted <= nExpectedValue;
225231
}
226232

227233
bool IsBlockPayeeValid(const CBlock& block, const CBlockIndex* pindexPrev)
@@ -326,7 +332,7 @@ bool CMasternodePayments::GetLegacyMasternodeTxOut(int nHeight, std::vector<CTxO
326332
if (!GetBlockPayee(nHeight, payee)) {
327333
//no masternode detected
328334
const Consensus::Params& consensus = Params().GetConsensus();
329-
const uint256& hash = consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V6_0) ?
335+
const uint256& hash = consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V5_3) ?
330336
mnodeman.GetHashAtHeight(nHeight - 1) : consensus.hashGenesisBlock;
331337
MasternodeRef winningNode = mnodeman.GetCurrentMasterNode(hash);
332338
if (winningNode) {
@@ -457,7 +463,7 @@ void CMasternodePayments::ProcessMessageMasternodePayments(CNode* pfrom, std::st
457463
}
458464

459465
std::string strError = "";
460-
if (!winner.IsValid(pfrom, strError)) {
466+
if (!winner.IsValid(pfrom, strError, nHeight)) {
461467
// if(strError != "") LogPrint(BCLog::MASTERNODE,"mnw - invalid message - %s\n", strError);
462468
return;
463469
}

src/masternode-payments.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ class CMasternodePaymentWinner : public CSignedMessage
177177
std::string GetStrMessage() const override;
178178
CTxIn GetVin() const { return vinMasternode; };
179179

180-
bool IsValid(CNode* pnode, std::string& strError);
180+
bool IsValid(CNode* pnode, std::string& strError, int chainHeight);
181181
void Relay();
182182

183183
void AddPayee(const CScript& payeeIn)

src/masternode.cpp

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,13 @@ int MasternodeRemovalSeconds()
6262
return Params().IsRegTestNet() ? MASTERNODE_REMOVAL_SECONDS_REGTEST : MASTERNODE_REMOVAL_SECONDS;
6363
}
6464

65+
// Used for sigTime < maxTimeWindow
66+
int64_t GetMaxTimeWindow(int chainHeight)
67+
{
68+
bool isV5_3 = Params().GetConsensus().NetworkUpgradeActive(chainHeight, Consensus::UPGRADE_V5_3);
69+
return GetAdjustedTime() + (isV5_3 ? (60 * 2) : (60 * 60));
70+
}
71+
6572

6673
CMasternode::CMasternode() :
6774
CSignedMessage()
@@ -136,7 +143,7 @@ std::string CMasternode::GetStrMessage() const
136143
//
137144
// When a new masternode broadcast is sent, update our information
138145
//
139-
bool CMasternode::UpdateFromNewBroadcast(CMasternodeBroadcast& mnb)
146+
bool CMasternode::UpdateFromNewBroadcast(CMasternodeBroadcast& mnb, int chainHeight)
140147
{
141148
if (mnb.sigTime > sigTime) {
142149
// TODO: lock cs. Need to be careful as mnb.lastPing.CheckAndUpdate locks cs_main internally.
@@ -147,7 +154,7 @@ bool CMasternode::UpdateFromNewBroadcast(CMasternodeBroadcast& mnb)
147154
protocolVersion = mnb.protocolVersion;
148155
addr = mnb.addr;
149156
int nDoS = 0;
150-
if (mnb.lastPing.IsNull() || (!mnb.lastPing.IsNull() && mnb.lastPing.CheckAndUpdate(nDoS, false))) {
157+
if (mnb.lastPing.IsNull() || (!mnb.lastPing.IsNull() && mnb.lastPing.CheckAndUpdate(nDoS, chainHeight, false))) {
151158
lastPing = mnb.lastPing;
152159
mnodeman.mapSeenMasternodePing.emplace(lastPing.GetHash(), lastPing);
153160
}
@@ -393,10 +400,10 @@ bool CMasternodeBroadcast::CheckDefaultPort(CService service, std::string& strEr
393400
return true;
394401
}
395402

396-
bool CMasternodeBroadcast::CheckAndUpdate(int& nDos)
403+
bool CMasternodeBroadcast::CheckAndUpdate(int& nDos, int nChainHeight)
397404
{
398405
// make sure signature isn't in the future (past is OK)
399-
if (sigTime > GetAdjustedTime() + 60 * 60) {
406+
if (sigTime > GetMaxTimeWindow(nChainHeight)) {
400407
LogPrint(BCLog::MASTERNODE,"mnb - Signature rejected, too far into the future %s\n", vin.prevout.hash.ToString());
401408
nDos = 1;
402409
return false;
@@ -444,7 +451,7 @@ bool CMasternodeBroadcast::CheckAndUpdate(int& nDos)
444451
return false;
445452

446453
// incorrect ping or its sigTime
447-
if(lastPing.IsNull() || !lastPing.CheckAndUpdate(nDos, false, true)) {
454+
if(lastPing.IsNull() || !lastPing.CheckAndUpdate(nDos, nChainHeight, false, true)) {
448455
return false;
449456
}
450457

@@ -470,7 +477,7 @@ bool CMasternodeBroadcast::CheckAndUpdate(int& nDos)
470477
if (pmn->pubKeyCollateralAddress == pubKeyCollateralAddress && !pmn->IsBroadcastedWithin(MasternodeBroadcastSeconds())) {
471478
//take the newest entry
472479
LogPrint(BCLog::MASTERNODE,"mnb - Got updated entry for %s\n", vin.prevout.hash.ToString());
473-
if (pmn->UpdateFromNewBroadcast((*this))) {
480+
if (pmn->UpdateFromNewBroadcast((*this), nChainHeight)) {
474481
if (pmn->IsEnabled()) Relay();
475482
}
476483
masternodeSync.AddedMasternodeList(GetHash());
@@ -491,7 +498,7 @@ bool CMasternodeBroadcast::CheckInputsAndAdd(int nChainHeight, int& nDoS)
491498
}
492499

493500
// incorrect ping or its sigTime
494-
if(lastPing.IsNull() || !lastPing.CheckAndUpdate(nDoS, false, true)) {
501+
if(lastPing.IsNull() || !lastPing.CheckAndUpdate(nDoS, nChainHeight, false, true)) {
495502
return false;
496503
}
497504

@@ -589,9 +596,9 @@ std::string CMasternodePing::GetStrMessage() const
589596
return vin.ToString() + blockHash.ToString() + std::to_string(sigTime);
590597
}
591598

592-
bool CMasternodePing::CheckAndUpdate(int& nDos, bool fRequireAvailable, bool fCheckSigTimeOnly)
599+
bool CMasternodePing::CheckAndUpdate(int& nDos, int nChainHeight, bool fRequireAvailable, bool fCheckSigTimeOnly)
593600
{
594-
if (sigTime > GetAdjustedTime() + 60 * 60) {
601+
if (sigTime > GetMaxTimeWindow(nChainHeight)) {
595602
LogPrint(BCLog::MNPING,"%s: Signature rejected, too far into the future %s\n", __func__, vin.prevout.hash.ToString());
596603
nDos = 30;
597604
return false;

src/masternode.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ class CMasternodePing : public CSignedMessage
5858
const CTxIn GetVin() const { return vin; };
5959
bool IsNull() const { return blockHash.IsNull() || vin.prevout.IsNull(); }
6060

61-
bool CheckAndUpdate(int& nDos, bool fRequireAvailable = true, bool fCheckSigTimeOnly = false);
61+
bool CheckAndUpdate(int& nDos, int nChainHeight, bool fRequireAvailable = true, bool fCheckSigTimeOnly = false);
6262
void Relay();
6363

6464
CMasternodePing& operator=(const CMasternodePing& other) = default;
@@ -156,7 +156,7 @@ class CMasternode : public CSignedMessage
156156
Unserialize(s);
157157
}
158158

159-
bool UpdateFromNewBroadcast(CMasternodeBroadcast& mnb);
159+
bool UpdateFromNewBroadcast(CMasternodeBroadcast& mnb, int chainHeight);
160160

161161
CMasternode::state GetActiveState() const;
162162

@@ -243,7 +243,7 @@ class CMasternodeBroadcast : public CMasternode
243243
CMasternodeBroadcast(CService newAddr, CTxIn newVin, CPubKey newPubkey, CPubKey newPubkey2, int protocolVersionIn, const CMasternodePing& _lastPing);
244244
CMasternodeBroadcast(const CMasternode& mn);
245245

246-
bool CheckAndUpdate(int& nDoS);
246+
bool CheckAndUpdate(int& nDoS, int nChainHeight);
247247
bool CheckInputsAndAdd(int chainHeight, int& nDos);
248248

249249
uint256 GetHash() const;

src/masternodeman.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,7 @@ int CMasternodeMan::ProcessMNBroadcast(CNode* pfrom, CMasternodeBroadcast& mnb)
713713
}
714714

715715
int nDoS = 0;
716-
if (!mnb.CheckAndUpdate(nDoS)) {
716+
if (!mnb.CheckAndUpdate(nDoS, GetBestHeight())) {
717717
return nDoS;
718718
}
719719

@@ -747,7 +747,7 @@ int CMasternodeMan::ProcessMNPing(CNode* pfrom, CMasternodePing& mnp)
747747
if (mapSeenMasternodePing.count(mnpHash)) return 0; //seen
748748

749749
int nDoS = 0;
750-
if (mnp.CheckAndUpdate(nDoS)) return 0;
750+
if (mnp.CheckAndUpdate(nDoS, GetBestHeight())) return 0;
751751

752752
if (nDoS > 0) {
753753
// if anything significant failed, mark that node
@@ -894,7 +894,7 @@ void CMasternodeMan::UpdateMasternodeList(CMasternodeBroadcast& mnb)
894894
CMasternode mn(mnb);
895895
Add(mn);
896896
} else {
897-
pmn->UpdateFromNewBroadcast(mnb);
897+
pmn->UpdateFromNewBroadcast(mnb, GetBestHeight());
898898
}
899899
}
900900

src/test/CMakeLists.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ set(BITCOIN_TEST_SUITE
8888
${CMAKE_CURRENT_SOURCE_DIR}/librust/sapling_test_fixture.cpp
8989
${CMAKE_SOURCE_DIR}/src/wallet/test/wallet_test_fixture.h
9090
${CMAKE_SOURCE_DIR}/src/wallet/test/wallet_test_fixture.cpp
91+
${CMAKE_SOURCE_DIR}/src/wallet/test/pos_test_fixture.h
92+
${CMAKE_SOURCE_DIR}/src/wallet/test/pos_test_fixture.cpp
9193
${CMAKE_CURRENT_SOURCE_DIR}/librust/utiltest.h
9294
${CMAKE_CURRENT_SOURCE_DIR}/librust/utiltest.cpp
9395
${CMAKE_CURRENT_SOURCE_DIR}/librust/json_test_vectors.h
@@ -175,6 +177,7 @@ set(BITCOIN_TESTS
175177
${CMAKE_SOURCE_DIR}/src/wallet/test/crypto_tests.cpp
176178
${CMAKE_SOURCE_DIR}/src/wallet/test/wallet_shielded_balances_tests.cpp
177179
${CMAKE_SOURCE_DIR}/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp
180+
${CMAKE_SOURCE_DIR}/src/wallet/test/pos_validations_tests.cpp
178181
)
179182

180183
set(test_test_pivx_SOURCES ${BITCOIN_TEST_SUITE} ${BITCOIN_TESTS} ${JSON_TEST_FILES})

0 commit comments

Comments
 (0)