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

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
+581 −69
Open

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
18ac2ab
evo: expand error codes for `netInfo` validation
kwvg May 10, 2025
d73aa0a
evo: introduce type-flexible `NetInfoEntry` to allow non-`CService` data
kwvg May 16, 2025
8a9d249
evo: ensure the ADDRV2 serialization is always used in `NetInfoEntry`
kwvg May 9, 2025
34687b1
evo: change internal type of `MnNetInfo` to `NetInfoEntry`
kwvg May 15, 2025
b9d5319
evo: return `MnNetInfo::GetEntries()` with `NetInfoEntry`
kwvg May 10, 2025
ee6cf6f
evo: utilize `NetInfoEntry::IsTriviallyValid()` in ProTx trivial checks
kwvg May 3, 2025
e49f8e3
evo: prohibit overwriting entry in `MnNetInfo`
kwvg May 9, 2025
c3e48ab
evo: fast-fail `MnNetInfo::AddEntry()` if invalid characters found
kwvg May 15, 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
116 changes: 84 additions & 32 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,17 @@ 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)) {
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())));
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 {
throw std::runtime_error(
strprintf("%s: Can't add a masternode %s with invalid address", __func__, dmn->proTxHash.ToString()));
Copy link

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

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())

}
}
if (!AddUniqueProperty(*dmn, dmn->pdmnState->keyIDOwner)) {
Expand Down Expand Up @@ -518,14 +524,24 @@ void CDeterministicMNList::UpdateMN(const CDeterministicMN& oldDmn, const std::s
// 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 : oldState->netInfo.GetEntries()) {
if (const auto& service_opt{old_entry.GetAddrPort()}; service_opt.has_value()) {
const CService& service{service_opt.value()};
if (!DeleteUniqueProperty(*dmn, service)) {
return strprintf("internal error"); // This shouldn't be possible
}
} else {
return strprintf("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 : pdmnState->netInfo.GetEntries()) {
if (const auto& service_opt{new_entry.GetAddrPort()}; service_opt.has_value()) {
const CService& service{service_opt.value()};
if (!AddUniqueProperty(*dmn, service)) {
return strprintf("duplicate (%s)", service.ToStringAddrPort());
}
} else {
return strprintf("invalid address");
}
}
}
Expand Down Expand Up @@ -591,11 +607,17 @@ 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)) {
mnUniquePropertyMap = mnUniquePropertyMapSaved;
throw(std::runtime_error(strprintf("%s: Can't delete a masternode %s with an address=%s", __func__,
proTxHash.ToString(), entry.ToStringAddrPort())));
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 {
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 +833,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 +869,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 +1237,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:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I just noticed ASSERT_IF_DEBUG here. Does it mean if I use debug version and someone will send me a broken p2p message with invalid protx transaction, I will get here assert here?

ASSERT_IF_DEBUG(false);
} // no default case, so the compiler can warn about missing cases
assert(false);
}
Expand Down Expand Up @@ -1373,10 +1415,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 +1493,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