Skip to content

feat: introduce type-flexible NetInfoEntry to allow non-CService entries, use in MnNetInfo #6629

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 28, 2025
Merged

feat: introduce type-flexible NetInfoEntry to allow non-CService entries, use in MnNetInfo #6629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
069583d
evo: expand error codes for `netInfo` validation
kwvg May 17, 2025
6d97bda
evo: introduce type-flexible `NetInfoEntry` to allow non-`CService` data
kwvg May 27, 2025
b0a634e
evo: ensure the ADDRV2 serialization is always used in `NetInfoEntry`
kwvg May 9, 2025
6286c9b
evo: change internal type of `MnNetInfo` to `NetInfoEntry`
kwvg May 15, 2025
06cf4ee
evo: return `MnNetInfo::GetEntries()` with `NetInfoEntry`
kwvg May 22, 2025
c69184b
evo: utilize `NetInfoEntry::IsTriviallyValid()` in ProTx trivial checks
kwvg May 3, 2025
e0a1c64
evo: prohibit overwriting entry in `MnNetInfo`
kwvg May 9, 2025
3a72f2f
evo: fast-fail `MnNetInfo::AddEntry()` if invalid characters found
kwvg May 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 92 additions & 36 deletions src/evo/deterministicmns.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,11 +470,18 @@ void CDeterministicMNList::AddMN(const CDeterministicMNCPtr& dmn, bool fBumpTota
throw(std::runtime_error(strprintf("%s: Can't add a masternode %s with a duplicate collateralOutpoint=%s", __func__,
dmn->proTxHash.ToString(), dmn->collateralOutpoint.ToStringShort())));
}
for (const CService& entry : dmn->pdmnState->netInfo.GetEntries()) {
if (!AddUniqueProperty(*dmn, entry)) {
for (const NetInfoEntry& entry : dmn->pdmnState->netInfo.GetEntries()) {
if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) {
const CService& service{service_opt.value()};
if (!AddUniqueProperty(*dmn, service)) {
mnUniquePropertyMap = mnUniquePropertyMapSaved;
throw std::runtime_error(strprintf("%s: Can't add a masternode %s with a duplicate address=%s",
__func__, dmn->proTxHash.ToString(), service.ToStringAddrPort()));
}
} else {
mnUniquePropertyMap = mnUniquePropertyMapSaved;
throw(std::runtime_error(strprintf("%s: Can't add a masternode %s with a duplicate address=%s", __func__,
dmn->proTxHash.ToString(), entry.ToStringAddrPort())));
throw std::runtime_error(
strprintf("%s: Can't add a masternode %s with invalid address", __func__, dmn->proTxHash.ToString()));
Copy link

@UdjinM6 UdjinM6 May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0f3519d: #6665 (comment), same for other new "invalid address" errors

Copy link
Collaborator Author

@kwvg kwvg May 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we need to register the underlying type in the unique properties map, this check has to be later expanded for DomainPort (see below, WIP code).

dash/src/evo/deterministicmns.cpp

Lines 474 to 491 in 4f716a5

if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) {
const CService& service{service_opt.value()};
if (!AddUniqueProperty(*dmn, service)) {
mnUniquePropertyMap = mnUniquePropertyMapSaved;
throw std::runtime_error(strprintf("%s: Can't add a masternode %s with a duplicate address=%s",
__func__, dmn->proTxHash.ToString(), service.ToStringAddrPort()));
}
} else if (const auto& domain_opt{entry.GetDomainPort()}; domain_opt.has_value()) {
const DomainPort& domain{domain_opt.value()};
if (!AddUniqueProperty(*dmn, domain)) {
mnUniquePropertyMap = mnUniquePropertyMapSaved;
throw std::runtime_error(strprintf("%s: Can't add a masternode %s with a duplicate address=%s",
__func__, dmn->proTxHash.ToString(), domain.ToStringAddrPort()));
}
} else {
throw std::runtime_error(
strprintf("%s: Can't add a masternode %s with invalid address", __func__, dmn->proTxHash.ToString()));
}

As for potentially invalid addresses, we try to prevent construction of an malformed NetInfoEntry (see below).

dash/src/evo/netinfo.h

Lines 68 to 73 in 66ee55c

NetInfoEntry(const CService& service)
{
if (!service.IsValid()) return;
m_type = NetInfoType::Service;
m_data = service;
}

But they can still be empty (i.e. invalid) and well-formed so I would still keep this check even if we have earlier checks against this (IsTriviallyValid())

Copy link
Collaborator

@knst knst May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, it should be validated much earlier; AddMN should return failure if and only if duplicated masternode (with same protx / address /etc are created); all other types of valiure (such as NetInfoEntry members) should be validated earlier.

Either, this code should be just removed (if it's already validated) or some extra validations for NetInfoEntry should be added. Maybe even just throw exception if you try to create NetInfoEntry with invalid object; but AddMN it's a bit too late to validate if MN is valid at all

}
}
if (!AddUniqueProperty(*dmn, dmn->pdmnState->keyIDOwner)) {
Expand Down Expand Up @@ -513,28 +520,40 @@ void CDeterministicMNList::UpdateMN(const CDeterministicMN& oldDmn, const std::s
// Using this temporary map as a checkpoint to roll back to in case of any issues.
decltype(mnUniquePropertyMap) mnUniquePropertyMapSaved = mnUniquePropertyMap;

const auto updateNetInfo = [&]() {
if (oldState->netInfo != pdmnState->netInfo) {
auto updateNetInfo = [this](const CDeterministicMN& dmn, const MnNetInfo& oldInfo,
const MnNetInfo& newInfo) -> std::string {
if (oldInfo != newInfo) {
// We track each individual entry in netInfo as opposed to netInfo itself (preventing us from
// using UpdateUniqueProperty()), so we need to successfully purge all old entries and insert
// new entries to successfully update.
for (const CService& old_entry : oldState->netInfo.GetEntries()) {
if (!DeleteUniqueProperty(*dmn, old_entry)) {
return strprintf("internal error"); // This shouldn't be possible
for (const NetInfoEntry& old_entry : oldInfo.GetEntries()) {
if (const auto& service_opt{old_entry.GetAddrPort()}) {
const CService& service{service_opt.value()};
if (!DeleteUniqueProperty(dmn, service)) {
return "internal error"; // This shouldn't be possible
}
} else {
return "invalid address";
}
}
for (const CService& new_entry : pdmnState->netInfo.GetEntries()) {
if (!AddUniqueProperty(*dmn, new_entry)) {
return strprintf("duplicate (%s)", new_entry.ToStringAddrPort());
for (const NetInfoEntry& new_entry : newInfo.GetEntries()) {
if (const auto& service_opt{new_entry.GetAddrPort()}) {
const CService& service{service_opt.value()};
if (!AddUniqueProperty(dmn, service)) {
return strprintf("duplicate (%s)", service.ToStringAddrPort());
}
} else {
return "invalid address";
}
}
}
return strprintf("");
}();
if (!updateNetInfo.empty()) {
return "";
};

if (auto err = updateNetInfo(*dmn, oldState->netInfo, pdmnState->netInfo); !err.empty()) {
mnUniquePropertyMap = mnUniquePropertyMapSaved;
throw(std::runtime_error(strprintf("%s: Can't update masternode %s with addresses, reason=%s", __func__,
oldDmn.proTxHash.ToString(), updateNetInfo)));
oldDmn.proTxHash.ToString(), err)));
}
if (!UpdateUniqueProperty(*dmn, oldState->keyIDOwner, pdmnState->keyIDOwner)) {
mnUniquePropertyMap = mnUniquePropertyMapSaved;
Expand Down Expand Up @@ -591,11 +610,18 @@ void CDeterministicMNList::RemoveMN(const uint256& proTxHash)
throw(std::runtime_error(strprintf("%s: Can't delete a masternode %s with a collateralOutpoint=%s", __func__,
proTxHash.ToString(), dmn->collateralOutpoint.ToStringShort())));
}
for (const CService& entry : dmn->pdmnState->netInfo.GetEntries()) {
if (!DeleteUniqueProperty(*dmn, entry)) {
for (const NetInfoEntry& entry : dmn->pdmnState->netInfo.GetEntries()) {
if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) {
const CService& service{service_opt.value()};
if (!DeleteUniqueProperty(*dmn, service)) {
mnUniquePropertyMap = mnUniquePropertyMapSaved;
throw std::runtime_error(strprintf("%s: Can't delete a masternode %s with an address=%s", __func__,
proTxHash.ToString(), service.ToStringAddrPort()));
}
} else {
mnUniquePropertyMap = mnUniquePropertyMapSaved;
throw(std::runtime_error(strprintf("%s: Can't delete a masternode %s with an address=%s", __func__,
proTxHash.ToString(), entry.ToStringAddrPort())));
throw std::runtime_error(strprintf("%s: Can't delete a masternode %s with invalid address", __func__,
dmn->proTxHash.ToString()));
}
}
if (!DeleteUniqueProperty(*dmn, dmn->pdmnState->keyIDOwner)) {
Expand Down Expand Up @@ -811,9 +837,14 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, gsl::no
}
}

for (const CService& entry : proTx.netInfo.GetEntries()) {
if (newList.HasUniqueProperty(entry)) {
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry");
for (const NetInfoEntry& entry : proTx.netInfo.GetEntries()) {
if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) {
const CService& service{service_opt.value()};
if (newList.HasUniqueProperty(service)) {
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry");
}
} else {
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-netinfo-entry");
}
}
if (newList.HasUniqueProperty(proTx.keyIDOwner) || newList.HasUniqueProperty(proTx.pubKeyOperator)) {
Expand Down Expand Up @@ -842,10 +873,15 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, gsl::no
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload");
}

for (const CService& entry : opt_proTx->netInfo.GetEntries()) {
if (newList.HasUniqueProperty(entry) &&
newList.GetUniquePropertyMN(entry)->proTxHash != opt_proTx->proTxHash) {
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry");
for (const NetInfoEntry& entry : opt_proTx->netInfo.GetEntries()) {
if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) {
const CService& service{service_opt.value()};
if (newList.HasUniqueProperty(service) &&
newList.GetUniquePropertyMN(service)->proTxHash != opt_proTx->proTxHash) {
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry");
}
} else {
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-netinfo-entry");
}
}

Expand Down Expand Up @@ -1205,12 +1241,22 @@ template <typename ProTx>
static bool CheckService(const ProTx& proTx, TxValidationState& state)
{
switch (proTx.netInfo.Validate()) {
case NetInfoStatus::BadInput:
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo");
case NetInfoStatus::BadAddress:
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-addr");
case NetInfoStatus::BadPort:
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-port");
case NetInfoStatus::BadType:
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-addr-type");
case NetInfoStatus::NotRoutable:
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-addr-unroutable");
case NetInfoStatus::Malformed:
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-bad");
case NetInfoStatus::Success:
return true;
// Shouldn't be possible during self-checks
case NetInfoStatus::BadInput:
case NetInfoStatus::MaxLimit:
assert(false);
} // no default case, so the compiler can warn about missing cases
assert(false);
}
Expand Down Expand Up @@ -1373,10 +1419,15 @@ bool CheckProRegTx(CDeterministicMNManager& dmnman, const CTransaction& tx, gsl:
auto mnList = dmnman.GetListForBlock(pindexPrev);

// only allow reusing of addresses when it's for the same collateral (which replaces the old MN)
for (const CService& entry : opt_ptx->netInfo.GetEntries()) {
if (mnList.HasUniqueProperty(entry) &&
mnList.GetUniquePropertyMN(entry)->collateralOutpoint != collateralOutpoint) {
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-netinfo-entry");
for (const NetInfoEntry& entry : opt_ptx->netInfo.GetEntries()) {
if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) {
const CService& service{service_opt.value()};
if (mnList.HasUniqueProperty(service) &&
mnList.GetUniquePropertyMN(service)->collateralOutpoint != collateralOutpoint) {
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-netinfo-entry");
}
} else {
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-entry");
}
}

Expand Down Expand Up @@ -1446,9 +1497,14 @@ bool CheckProUpServTx(CDeterministicMNManager& dmnman, const CTransaction& tx, g
}

// don't allow updating to addresses already used by other MNs
for (const CService& entry : opt_ptx->netInfo.GetEntries()) {
if (mnList.HasUniqueProperty(entry) && mnList.GetUniquePropertyMN(entry)->proTxHash != opt_ptx->proTxHash) {
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-netinfo-entry");
for (const NetInfoEntry& entry : opt_ptx->netInfo.GetEntries()) {
if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) {
const CService& service{service_opt.value()};
if (mnList.HasUniqueProperty(service) && mnList.GetUniquePropertyMN(service)->proTxHash != opt_ptx->proTxHash) {
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-dup-netinfo-entry");
}
} else {
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-entry");
}
}

Expand Down
1 change: 1 addition & 0 deletions src/evo/deterministicmns.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ class CDeterministicMNList
static_assert(!std::is_same_v<std::decay_t<T>, name>, "GetUniquePropertyHash cannot be templated against " #name)
DMNL_NO_TEMPLATE(CBLSPublicKey);
DMNL_NO_TEMPLATE(MnNetInfo);
DMNL_NO_TEMPLATE(NetInfoEntry);
#undef DMNL_NO_TEMPLATE
return ::SerializeHash(v);
}
Expand Down
Loading
Loading