Skip to content

Commit f977194

Browse files
PastaPastaPastaPastaPastaPasta
committed
fix: address masternode payout share review findings
1 parent 46c5fb4 commit f977194

12 files changed

Lines changed: 211 additions & 41 deletions

‎src/evo/providertx.cpp‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -106,15 +106,9 @@ bool IsPayoutListKeySafe(const MasternodePayoutShares& payouts, const CTxDestina
106106
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-collateral-reuse");
107107
}
108108

109-
const auto* collateral_pkhash = std::get_if<PKHash>(&collateral_dest);
110-
if (!collateral_pkhash) {
111-
return true;
112-
}
113-
114109
for (const auto& payout : payouts) {
115110
CTxDestination payout_dest;
116-
if (ExtractDestination(payout.scriptPayout, payout_dest) &&
117-
payout_dest == CTxDestination(*collateral_pkhash)) {
111+
if (ExtractDestination(payout.scriptPayout, payout_dest) && payout_dest == collateral_dest) {
118112
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-payee-reuse");
119113
}
120114
}
@@ -222,7 +216,10 @@ std::string CProRegTx::MakeSignString() const
222216

223217
// We only include the important stuff in the string form...
224218

225-
const std::string strPayout = PayoutListToString(GetOwnerPayouts(nVersion, scriptPayout, payouts));
219+
CTxDestination dest;
220+
const std::string strPayout = nVersion >= ProTxVersion::MultiPayout
221+
? PayoutListToString(payouts)
222+
: (ExtractDestination(scriptPayout, dest) ? EncodeDestination(dest) : HexStr(scriptPayout));
226223

227224
s += strPayout + "|";
228225
s += strprintf("%d", nOperatorReward) + "|";

‎src/evo/specialtxman.cpp‎

Lines changed: 77 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,69 @@
3030
#include <util/system.h>
3131
#include <validation.h>
3232

33+
static bool AddNetInfoEntries(const std::shared_ptr<NetInfoInterface>& net_info, NetInfoPurpose purpose,
34+
const NetInfoList& entries, BlockValidationState& state)
35+
{
36+
for (const auto& entry : entries) {
37+
if (const auto ret{net_info->AddEntry(purpose, entry.ToStringAddrPort())}; ret != NetInfoStatus::Success) {
38+
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-netinfo-version");
39+
}
40+
}
41+
return true;
42+
}
43+
44+
static bool SetStateVersion(CDeterministicMNState& state_mn, uint16_t nVersion, MnType nType,
45+
BlockValidationState& state)
46+
{
47+
const bool needs_extended = nVersion >= ProTxVersion::ExtAddr;
48+
if (state_mn.nVersion == nVersion && state_mn.netInfo->CanStorePlatform() == needs_extended) {
49+
return true;
50+
}
51+
52+
auto converted_netinfo{NetInfoInterface::MakeNetInfo(nVersion)};
53+
if (needs_extended) {
54+
if (!AddNetInfoEntries(converted_netinfo, NetInfoPurpose::CORE_P2P,
55+
state_mn.netInfo->GetEntries(NetInfoPurpose::CORE_P2P), state)) {
56+
return false;
57+
}
58+
if (state_mn.netInfo->CanStorePlatform()) {
59+
if (!AddNetInfoEntries(converted_netinfo, NetInfoPurpose::PLATFORM_P2P,
60+
state_mn.netInfo->GetEntries(NetInfoPurpose::PLATFORM_P2P), state) ||
61+
!AddNetInfoEntries(converted_netinfo, NetInfoPurpose::PLATFORM_HTTPS,
62+
state_mn.netInfo->GetEntries(NetInfoPurpose::PLATFORM_HTTPS), state)) {
63+
return false;
64+
}
65+
} else if (nType == MnType::Evo && !state_mn.netInfo->IsEmpty()) {
66+
const CNetAddr addr{state_mn.netInfo->GetPrimary()};
67+
if ((state_mn.platformP2PPort != 0 &&
68+
converted_netinfo->AddEntry(NetInfoPurpose::PLATFORM_P2P,
69+
CService(addr, state_mn.platformP2PPort).ToStringAddrPort()) != NetInfoStatus::Success) ||
70+
(state_mn.platformHTTPPort != 0 &&
71+
converted_netinfo->AddEntry(NetInfoPurpose::PLATFORM_HTTPS,
72+
CService(addr, state_mn.platformHTTPPort).ToStringAddrPort()) != NetInfoStatus::Success)) {
73+
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-netinfo-version");
74+
}
75+
}
76+
state_mn.platformP2PPort = 0;
77+
state_mn.platformHTTPPort = 0;
78+
} else {
79+
if (!AddNetInfoEntries(converted_netinfo, NetInfoPurpose::CORE_P2P,
80+
state_mn.netInfo->GetEntries(NetInfoPurpose::CORE_P2P), state)) {
81+
return false;
82+
}
83+
if (nType == MnType::Evo && state_mn.netInfo->CanStorePlatform() && !state_mn.netInfo->IsEmpty()) {
84+
const auto p2p_entries{state_mn.netInfo->GetEntries(NetInfoPurpose::PLATFORM_P2P)};
85+
const auto http_entries{state_mn.netInfo->GetEntries(NetInfoPurpose::PLATFORM_HTTPS)};
86+
state_mn.platformP2PPort = p2p_entries.empty() ? 0 : p2p_entries.front().GetPort();
87+
state_mn.platformHTTPPort = http_entries.empty() ? 0 : http_entries.front().GetPort();
88+
}
89+
}
90+
91+
state_mn.nVersion = nVersion;
92+
state_mn.netInfo = std::move(converted_netinfo);
93+
return true;
94+
}
95+
3396
static bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex, const Consensus::Params& consensus_params,
3497
const CChain& chain, const llmq::CQuorumManager& qman,
3598
const chainlock::Chainlocks& chainlocks, BlockValidationState& state)
@@ -345,6 +408,8 @@ bool CSpecialTxProcessor::RebuildListFromBlock(const CBlock& block, gsl::not_nul
345408
}
346409

347410
auto newState = std::make_shared<CDeterministicMNState>(*dmn->pdmnState);
411+
const uint16_t current_version{static_cast<uint16_t>(newState->nVersion)};
412+
const uint16_t target_version{is_v24_deployed ? std::max<uint16_t>(current_version, opt_proTx->nVersion) : current_version};
348413
if (is_v24_deployed) {
349414
// Extended addresses support in v24 means that the version can be updated
350415
newState->nVersion = opt_proTx->nVersion;
@@ -358,6 +423,9 @@ bool CSpecialTxProcessor::RebuildListFromBlock(const CBlock& block, gsl::not_nul
358423
newState->platformHTTPPort = opt_proTx->platformHTTPPort;
359424
}
360425
}
426+
if (is_v24_deployed && !SetStateVersion(*newState, target_version, dmn->nType, state)) {
427+
return false;
428+
}
361429
if (newState->IsBanned()) {
362430
// only revive when all keys are set
363431
if (newState->pubKeyOperator != CBLSLazyPublicKey() && !newState->keyIDVoting.IsNull() &&
@@ -385,20 +453,22 @@ bool CSpecialTxProcessor::RebuildListFromBlock(const CBlock& block, gsl::not_nul
385453
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash");
386454
}
387455
auto newState = std::make_shared<CDeterministicMNState>(*dmn->pdmnState);
456+
const uint16_t old_version{static_cast<uint16_t>(newState->nVersion)};
457+
uint16_t target_version{std::max<uint16_t>(old_version, opt_proTx->nVersion)};
388458
if (newState->pubKeyOperator != opt_proTx->pubKeyOperator) {
389459
// reset all operator related fields and put MN into PoSe-banned state in case the operator key changes
390460
newState->ResetOperatorFields();
391461
newState->BanIfNotBanned(nHeight);
392-
// we update pubKeyOperator here, make sure state version matches
393-
// Make sure we don't accidentally downgrade the state version if using version after basic BLS
394-
newState->nVersion = newState->nVersion > ProTxVersion::BasicBLS ? newState->nVersion : opt_proTx->nVersion;
395-
newState->netInfo = NetInfoInterface::MakeNetInfo(newState->nVersion);
396462
newState->pubKeyOperator = opt_proTx->pubKeyOperator;
397463
}
398464
newState->keyIDVoting = opt_proTx->keyIDVoting;
399-
newState->nVersion = opt_proTx->nVersion;
400-
if (opt_proTx->nVersion >= ProTxVersion::MultiPayout) {
401-
newState->payouts = opt_proTx->payouts;
465+
if (!SetStateVersion(*newState, target_version, dmn->nType, state)) {
466+
return false;
467+
}
468+
if (target_version >= ProTxVersion::MultiPayout) {
469+
newState->payouts = opt_proTx->nVersion >= ProTxVersion::MultiPayout
470+
? opt_proTx->payouts
471+
: LegacyPayoutAsList(opt_proTx->scriptPayout);
402472
newState->scriptPayout.clear();
403473
} else {
404474
newState->scriptPayout = opt_proTx->scriptPayout;

‎src/interfaces/node.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ class MnEntry
8080
virtual const CKeyID& getKeyIdVoting() const = 0;
8181
virtual const COutPoint& getCollateralOutpoint() const = 0;
8282
virtual const CScript& getScriptPayout() const = 0;
83+
virtual std::vector<CScript> getScriptPayouts() const = 0;
8384
virtual const CScript& getScriptOperatorPayout() const = 0;
8485
virtual const int32_t& getLastPaidHeight() const = 0;
8586
virtual const int32_t& getPoSePenalty() const = 0;

‎src/node/interfaces.cpp‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -101,16 +101,29 @@ using interfaces::WalletLoader;
101101

102102
namespace node {
103103
namespace {
104+
std::vector<CScript> GetOwnerPayoutScripts(const CDeterministicMN& dmn)
105+
{
106+
std::vector<CScript> ret;
107+
for (const auto& payout : GetOwnerPayouts(dmn.pdmnState->nVersion, dmn.pdmnState->scriptPayout,
108+
dmn.pdmnState->payouts)) {
109+
ret.emplace_back(payout.scriptPayout);
110+
}
111+
return ret;
112+
}
113+
104114
class MnEntryImpl : public MnEntry
105115
{
106116
private:
107117
CDeterministicMNCPtr m_dmn;
108-
mutable CScript m_script_payout;
118+
const std::vector<CScript> m_script_payouts;
119+
const CScript m_script_payout;
109120

110121
public:
111122
MnEntryImpl(const CDeterministicMNCPtr& dmn) :
112123
MnEntry{dmn},
113-
m_dmn{Assert(dmn)}
124+
m_dmn{Assert(dmn)},
125+
m_script_payouts{GetOwnerPayoutScripts(*m_dmn)},
126+
m_script_payout{m_script_payouts.front()}
114127
{
115128
}
116129
~MnEntryImpl() = default;
@@ -123,12 +136,8 @@ class MnEntryImpl : public MnEntry
123136
const CKeyID& getKeyIdOwner() const override { return m_dmn->pdmnState->keyIDOwner; }
124137
const CKeyID& getKeyIdVoting() const override { return m_dmn->pdmnState->keyIDVoting; }
125138
const COutPoint& getCollateralOutpoint() const override { return m_dmn->collateralOutpoint; }
126-
const CScript& getScriptPayout() const override
127-
{
128-
m_script_payout = GetOwnerPayouts(m_dmn->pdmnState->nVersion, m_dmn->pdmnState->scriptPayout,
129-
m_dmn->pdmnState->payouts).front().scriptPayout;
130-
return m_script_payout;
131-
}
139+
const CScript& getScriptPayout() const override { return m_script_payout; }
140+
std::vector<CScript> getScriptPayouts() const override { return m_script_payouts; }
132141
const CScript& getScriptOperatorPayout() const override { return m_dmn->pdmnState->scriptOperatorPayout; }
133142
const int32_t& getLastPaidHeight() const override { return m_dmn->pdmnState->nLastPaidHeight; }
134143
const int32_t& getPoSePenalty() const override { return m_dmn->pdmnState->nPoSePenalty; }

‎src/qt/masternodelist.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,10 +220,14 @@ void MasternodeList::updateMasternodeList()
220220

221221
QSet<QString> owned_mns;
222222
for (const auto& entry : feed->m_entries) {
223+
const auto script_payouts{entry->scriptPayoutsRaw()};
224+
const bool owns_payout{std::any_of(script_payouts.begin(), script_payouts.end(), [&](const auto& script) {
225+
return walletModel->wallet().isSpendable(script);
226+
})};
223227
bool fMyMasternode{setOutpts.count(entry->collateralOutpointRaw()) ||
224228
walletModel->wallet().isSpendable(PKHash(entry->keyIdOwnerRaw())) ||
225229
walletModel->wallet().isSpendable(PKHash(entry->keyIdVotingRaw())) ||
226-
walletModel->wallet().isSpendable(entry->scriptPayoutRaw()) ||
230+
owns_payout ||
227231
walletModel->wallet().isSpendable(entry->scriptOperatorPayoutRaw())};
228232
if (fMyMasternode) {
229233
owned_mns.insert(entry->proTxHash());

‎src/qt/masternodemodel.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,13 @@ MasternodeEntry::MasternodeEntry(const interfaces::MnEntryCPtr& dmn, const QStri
5656
auto addr_key = dmn->getNetInfoPrimary().GetKey();
5757
m_service_key = QByteArray(reinterpret_cast<const char*>(addr_key.data()), addr_key.size());
5858

59-
if (CTxDestination dest; ExtractDestination(dmn->getScriptPayout(), dest)) {
60-
m_payout_address = QString::fromStdString(EncodeDestination(dest));
61-
} else {
62-
m_payout_address = QObject::tr("UNKNOWN");
59+
QStringList payout_addresses;
60+
for (const auto& script_payout : dmn->getScriptPayouts()) {
61+
if (CTxDestination dest; ExtractDestination(script_payout, dest)) {
62+
payout_addresses << QString::fromStdString(EncodeDestination(dest));
63+
}
6364
}
65+
m_payout_address = payout_addresses.isEmpty() ? QObject::tr("UNKNOWN") : payout_addresses.join(", ");
6466

6567
if (m_operator_reward_pct) {
6668
m_operator_reward = QString::number(m_operator_reward_pct / 100.0, 'f', 2) + "%";

‎src/qt/masternodemodel.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ class MasternodeEntry
7070
const COutPoint& collateralOutpointRaw() const { return m_dmn->getCollateralOutpoint(); }
7171
const CScript& scriptOperatorPayoutRaw() const { return m_dmn->getScriptOperatorPayout(); }
7272
const CScript& scriptPayoutRaw() const { return m_dmn->getScriptPayout(); }
73+
std::vector<CScript> scriptPayoutsRaw() const { return m_dmn->getScriptPayouts(); }
7374
const QByteArray& serviceKey() const { return m_service_key; }
7475
const QString& collateralAddress() const { return m_collateral_address; }
7576
const QString& collateralOutpoint() const { return m_collateral_outpoint; }

‎src/rpc/client.cpp‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,34 +279,45 @@ static const CRPCConvertParam vRPCConvertParams[] =
279279
{ "protx list", 3, "height" },
280280
{ "protx register", 2, "collateralIndex" },
281281
{ "protx register", 3, "coreP2PAddrs", true },
282+
{ "protx register", 8, "payoutAddress", true },
282283
{ "protx register", 10, "submit" },
283284
{ "protx register_legacy", 2, "collateralIndex" },
284285
{ "protx register_legacy", 3, "coreP2PAddrs", true },
286+
{ "protx register_legacy", 8, "payoutAddress", true },
285287
{ "protx register_legacy", 10, "submit" },
286288
{ "protx register_evo", 2, "collateralIndex" },
287289
{ "protx register_evo", 3, "coreP2PAddrs", true },
290+
{ "protx register_evo", 8, "payoutAddress", true },
288291
{ "protx register_evo", 10, "platformP2PAddrs", true },
289292
{ "protx register_evo", 11, "platformHTTPSAddrs", true },
290293
{ "protx register_evo", 13, "submit" },
291294
{ "protx register_fund", 2, "coreP2PAddrs", true },
295+
{ "protx register_fund", 7, "payoutAddress", true },
292296
{ "protx register_fund", 9, "submit" },
293297
{ "protx register_fund_legacy", 2, "coreP2PAddrs", true },
298+
{ "protx register_fund_legacy", 7, "payoutAddress", true },
294299
{ "protx register_fund_legacy", 9, "submit" },
295300
{ "protx register_fund_evo", 2, "coreP2PAddrs", true },
301+
{ "protx register_fund_evo", 7, "payoutAddress", true },
296302
{ "protx register_fund_evo", 9, "platformP2PAddrs", true },
297303
{ "protx register_fund_evo", 10, "platformHTTPSAddrs", true },
298304
{ "protx register_fund_evo", 12, "submit" },
299305
{ "protx register_prepare", 2, "collateralIndex" },
300306
{ "protx register_prepare", 3, "coreP2PAddrs", true },
307+
{ "protx register_prepare", 8, "payoutAddress", true },
301308
{ "protx register_prepare_legacy", 2, "collateralIndex" },
302309
{ "protx register_prepare_legacy", 3, "coreP2PAddrs", true },
310+
{ "protx register_prepare_legacy", 8, "payoutAddress", true },
303311
{ "protx register_prepare_evo", 2, "collateralIndex" },
304312
{ "protx register_prepare_evo", 3, "coreP2PAddrs", true },
313+
{ "protx register_prepare_evo", 8, "payoutAddress", true },
305314
{ "protx register_prepare_evo", 10, "platformP2PAddrs", true },
306315
{ "protx register_prepare_evo", 11, "platformHTTPSAddrs", true },
307316
{ "protx revoke", 3, "reason" },
308317
{ "protx revoke", 5, "submit" },
318+
{ "protx update_registrar", 4, "payoutAddress", true },
309319
{ "protx update_registrar", 6, "submit" },
320+
{ "protx update_registrar_legacy", 4, "payoutAddress", true },
310321
{ "protx update_registrar_legacy", 6, "submit" },
311322
{ "protx update_service", 2, "coreP2PAddrs", true },
312323
{ "protx update_service", 6, "submit" },

‎src/rpc/evo.cpp‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -147,13 +147,31 @@ static RPCArg GetRpcArg(const std::string& strParamName)
147147
"The address must be unused and must differ from the collateralAddress."}
148148
},
149149
{"payoutAddress_register",
150-
{"payoutAddress", RPCArg::Type::STR, RPCArg::Optional::NO,
151-
"The Dash address to use for masternode reward payments."}
150+
{"payoutAddress", RPCArg::Type::ARR, RPCArg::Optional::NO,
151+
"The Dash address to use for masternode reward payments, or for v4 provider transactions, "
152+
"an array of payout shares.",
153+
{
154+
{"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "",
155+
{
156+
{"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The Dash payout address."},
157+
{"reward", RPCArg::Type::NUM, RPCArg::Optional::NO, "The payout share in basis points."},
158+
}},
159+
},
160+
"\"payoutAddress\"|[{\"address\",\"reward\"},...]", {"string or array", "string or array"}}
152161
},
153162
{"payoutAddress_update",
154-
{"payoutAddress", RPCArg::Type::STR, RPCArg::Optional::NO,
155-
"The Dash address to use for masternode reward payments.\n"
156-
"If set to an empty string, the currently active payout address is reused."}
163+
{"payoutAddress", RPCArg::Type::ARR, RPCArg::Optional::NO,
164+
"The Dash address to use for masternode reward payments, or for v4 provider transactions, "
165+
"an array of payout shares.\n"
166+
"If set to an empty string, the currently active payout address is reused.",
167+
{
168+
{"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "",
169+
{
170+
{"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The Dash payout address."},
171+
{"reward", RPCArg::Type::NUM, RPCArg::Optional::NO, "The payout share in basis points."},
172+
}},
173+
},
174+
"\"payoutAddress\"|[{\"address\",\"reward\"},...]", {"string or array", "string or array"}}
157175
},
158176
{"proTxHash",
159177
{"proTxHash", RPCArg::Type::STR, RPCArg::Optional::NO,
@@ -1191,6 +1209,9 @@ static RPCHelpMan protx_update_registrar_wrapper(const bool specific_legacy_bls_
11911209
if (!dmn) {
11921210
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("masternode %s not found", ptx.proTxHash.ToString()));
11931211
}
1212+
if (dmn->pdmnState->nVersion == ProTxVersion::LegacyBLS && ptx.nVersion > ProTxVersion::BasicBLS) {
1213+
ptx.nVersion = ProTxVersion::BasicBLS;
1214+
}
11941215

11951216
ptx.keyIDVoting = dmn->pdmnState->keyIDVoting;
11961217
ptx.scriptPayout = dmn->pdmnState->scriptPayout;
@@ -1355,7 +1376,9 @@ static RPCHelpMan protx_revoke()
13551376
// Using funds from previousely specified masternode payout address
13561377
CTxDestination txDest;
13571378
const auto owner_payouts = GetOwnerPayouts(dmn->pdmnState->nVersion, dmn->pdmnState->scriptPayout, dmn->pdmnState->payouts);
1358-
ExtractDestination(owner_payouts.front().scriptPayout, txDest);
1379+
if (owner_payouts.empty() || !ExtractDestination(owner_payouts.front().scriptPayout, txDest)) {
1380+
throw JSONRPCError(RPC_INVALID_PARAMETER, "No payout or fee source addresses found, can't revoke");
1381+
}
13591382
FundSpecialTx(*pwallet, tx, ptx, txDest);
13601383
}
13611384

0 commit comments

Comments
 (0)