ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-07-28 17:40:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

Improve invalid node handling and fee charging

Browse Source
This commit is contained in:
Bart
2026-07-27 21:27:25 -04:00
parent eb0176c1f4
commit 2f77e65e86
3 changed files with 29 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
include/xrpl/shamap/SHAMap.h
View File

@@ -96,14 +96,17 @@ enum class SHAMapState {
*/
/**
* Holds a SHAMap node's identity, serialized data, and leaf status. Used by
* Holds a SHAMap node's identity, leaf status, and serialized data. Used by
* getNodeFat to return node data for peer synchronization.
*/
struct SHAMapNodeData
{
SHAMapNodeID nodeID;
// The `data` field (a Blob, 8-byte aligned) needs 4 bytes of padding after the `nodeID` field
// (36 bytes, 4-byte aligned) regardless of what comes between them, so `isLeaf` costs nothing
// extra here. Moving it after `data` would add 8 bytes to the size of this struct instead.
bool isLeaf;
Blob data; // Placed last, so `isLeaf` can fit into the alignment padding of `nodeID`.
Blob data;
};
class SHAMap

30
src/xrpld/app/ledger/detail/InboundLedger.cpp
View File

@@ -894,16 +894,12 @@ InboundLedger::receiveNode(
return;
}
if (nodeID->isRoot())
{
san += map.addRootNode(rootHash, std::move(treeNode), f);
}
else
{
san += map.addKnownNode(*nodeID, std::move(treeNode), f);
}
auto const result = nodeID->isRoot()
? map.addRootNode(rootHash, std::move(treeNode), f)
: map.addKnownNode(*nodeID, std::move(treeNode), f);
san += result;
if (!san.isGood())
if (result.isInvalid())
{
JLOG(journal_.warn()) << "Got invalid node " << *nodeID << " for ledger " << hash_
<< " from peer " << peer->id();
@@ -970,9 +966,10 @@ InboundLedger::takeAsRootNode(std::string_view data, SHAMapAddNode& san)
}
AccountStateSF filter(ledger_->stateMap().family().db(), app_.getLedgerMaster());
san += ledger_->stateMap().addRootNode(
auto const result = ledger_->stateMap().addRootNode(
SHAMapHash{ledger_->header().accountHash}, std::move(treeNode), &filter);
return san.isGood();
san += result;
return !result.isInvalid();
}
/**
@@ -1005,9 +1002,10 @@ InboundLedger::takeTxRootNode(std::string_view data, SHAMapAddNode& san)
}
TransactionStateSF filter(ledger_->txMap().family().db(), app_.getLedgerMaster());
san += ledger_->txMap().addRootNode(
auto const result = ledger_->txMap().addRootNode(
SHAMapHash{ledger_->header().txHash}, std::move(treeNode), &filter);
return san.isGood();
san += result;
return !result.isInvalid();
}
std::vector<InboundLedger::neededHash_t>
@@ -1162,11 +1160,11 @@ InboundLedger::processData(std::shared_ptr<Peer> peer, protocol::TMLedgerData co
<< ((packet.type() == protocol::liTX_NODE) ? "TX" : "AS")
<< " node stats: " << san.get();
// `san` accumulates across the whole packet, so `isInvalid()` (bad_ > 0) does not mean the
// packet had no useful nodes: credit whatever good/useful nodes were sent rather than
// discarding everything because one node in an otherwise-good packet was bad.
// Note: Peer charges for invalid/malformed data are issued from within receiveNode at the
// exact failure site, so the peer is only charged for problems they are responsible for.
if (san.isInvalid())
return -1;
if (san.isUseful())
progress_ = true;

14
src/xrpld/overlay/detail/PeerImp.cpp
View File

@@ -1538,14 +1538,20 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMGetLedger> const& m)
}
}
// Charge once per request. Requesting too many node IDs is charged even for a relay
// response, since it's a distinct infraction from the "get ledger request" charge below,
// which is skipped for relay responses.
// These are two distinct infractions and are charged independently: requesting too many
// node IDs is charged even for a relay response, while the base "get ledger request" charge
// below is skipped for relay responses.
if (tooManyNodeIds)
{
peer->charge(Resource::kFeeModerateBurdenPeer, "TMGetLedger: too many node IDs");
// Truncate the request to what was actually parsed and charged for, so that if this
// request ends up being relayed to another peer, we don't forward the oversized list.
m->mutable_nodeids()->DeleteSubrange(
static_cast<int>(nodeIDs.size()),
m->nodeids_size() - static_cast<int>(nodeIDs.size()));
}
else if (!m->has_requestcookie())
if (!m->has_requestcookie())
{
peer->charge(Resource::kFeeModerateBurdenPeer, "TMGetLedger: get ledger request");
}