From 9286edc57ff830b224197a5b97de3be433fd534d Mon Sep 17 00:00:00 2001 From: Nicholas Dudfield Date: Tue, 14 Jul 2026 18:27:56 +0700 Subject: [PATCH] fix(export): defer validation-skewed relay shares --- .../consensus/ConsensusExtensions_test.cpp | 70 ++++- .../app/consensus/ConsensusExtensions.cpp | 274 +++++++++++++++--- src/xrpld/app/consensus/ConsensusExtensions.h | 44 +++ src/xrpld/app/main/Application.cpp | 16 +- src/xrpld/overlay/ExportShareAdmission.h | 56 ++++ src/xrpld/overlay/Overlay.h | 16 +- src/xrpld/overlay/detail/OverlayImpl.cpp | 12 +- src/xrpld/overlay/detail/OverlayImpl.h | 6 +- src/xrpld/overlay/detail/PeerImp.cpp | 33 ++- 9 files changed, 469 insertions(+), 58 deletions(-) create mode 100644 src/xrpld/overlay/ExportShareAdmission.h diff --git a/src/test/consensus/ConsensusExtensions_test.cpp b/src/test/consensus/ConsensusExtensions_test.cpp index 87ea0077b..4b6711b56 100644 --- a/src/test/consensus/ConsensusExtensions_test.cpp +++ b/src/test/consensus/ConsensusExtensions_test.cpp @@ -3714,7 +3714,6 @@ class ConsensusExtensions_test : public beast::unit_test::suite pendingLatch && pendingLatch->isFieldPresent(sfExportNode) && !pendingLatch->isFieldPresent(sfExportSignatureHash))) return; - installValidated(originLedger); auto release = ExportOriginMemo::releaseForm( innerTx, @@ -3782,6 +3781,44 @@ class ConsensusExtensions_test : public beast::unit_test::suite expectNoShareEvent(); }; + auto deferredCount = [&] { + std::lock_guard lock(ce.deferredExportSharesMutex_); + return ce.deferredExportShares_.size(); + }; + std::atomic deferredCharges{0}; + std::atomic lastDeferredCharge{ + ExportShareCharge::none}; + auto const deferredCharge = [&](ExportShareCharge const charge) { + lastDeferredCharge.store(charge, std::memory_order_relaxed); + deferredCharges.fetch_add(1, std::memory_order_relaxed); + }; + + auto admission = ce.onExportShare(share, deferredCharge); + BEAST_EXPECT(admission.disposition == ExportShareDisposition::deferred); + BEAST_EXPECT(admission.charge == ExportShareCharge::none); + BEAST_EXPECT(deferredCount() == 1); + BEAST_EXPECT(!hasRetainedContribution()); + + admission = ce.onExportShare(share, deferredCharge); + BEAST_EXPECT( + admission.disposition == ExportShareDisposition::duplicate); + BEAST_EXPECT(deferredCount() == 1); + + auto wrongBranch = share; + wrongBranch.originLedgerHash = makeHash("wrong-export-origin"); + admission = ce.onExportShare(wrongBranch, deferredCharge); + BEAST_EXPECT(admission.disposition == ExportShareDisposition::deferred); + BEAST_EXPECT(deferredCount() == 2); + + auto beyondHorizon = share; + beyondHorizon.originLedgerSeq = universe->info().seq + + ConsensusExtensions::maxDeferredExportShareFutureLedgers_ + 1; + admission = ce.onExportShare(beyondHorizon, deferredCharge); + BEAST_EXPECT(admission.disposition == ExportShareDisposition::deferred); + BEAST_EXPECT(deferredCount() == 2); + + installValidated(originLedger); + if (replayWins) { std::unique_lock streamLock{ce.exportStreamMutex_}; @@ -3839,6 +3876,29 @@ class ConsensusExtensions_test : public beast::unit_test::suite } BEAST_EXPECT(hasRetainedContribution()); + admission = ce.onExportShare(share, {}); + BEAST_EXPECT( + admission.disposition == ExportShareDisposition::duplicate); + BEAST_EXPECT(admission.charge == ExportShareCharge::none); + + admission = ce.onExportShare(wrongBranch, {}); + BEAST_EXPECT(admission.disposition == ExportShareDisposition::invalid); + BEAST_EXPECT(admission.charge == ExportShareCharge::invalidData); + + auto invalidSignature = share; + invalidSignature.signature = sign( + valKeys.keys->publicKey, + valKeys.keys->secretKey, + Slice{"invalid-export-share", 20}); + admission = ce.onExportShare(invalidSignature, {}); + BEAST_EXPECT(admission.disposition == ExportShareDisposition::invalid); + BEAST_EXPECT(admission.charge == ExportShareCharge::invalidSignature); + + BEAST_EXPECT(deferredCount() == 0); + BEAST_EXPECT(deferredCharges.load(std::memory_order_relaxed) == 1); + BEAST_EXPECT( + lastDeferredCharge.load(std::memory_order_relaxed) == + ExportShareCharge::invalidData); BEAST_EXPECT( ce.lastExportReplaySeq_.load(std::memory_order_relaxed) == originLedger->info().seq); @@ -3882,7 +3942,15 @@ class ConsensusExtensions_test : public beast::unit_test::suite BEAST_EXPECT(hasRetainedContribution()); expectNoShareEvent(); + auto pendingAtStop = share; + pendingAtStop.originLedgerSeq = witnessed->info().seq + 1; + pendingAtStop.originLedgerHash = makeHash("pending-at-stop"); + admission = ce.onExportShare(pendingAtStop, deferredCharge); + BEAST_EXPECT(admission.disposition == ExportShareDisposition::deferred); + BEAST_EXPECT(deferredCount() == 1); + ce.stopExportShareService(); + BEAST_EXPECT(deferredCount() == 0); BEAST_EXPECT( wsc->invoke("unsubscribe", stream)[jss::status] == "success"); } diff --git a/src/xrpld/app/consensus/ConsensusExtensions.cpp b/src/xrpld/app/consensus/ConsensusExtensions.cpp index 186b6ba91..538bf7a40 100644 --- a/src/xrpld/app/consensus/ConsensusExtensions.cpp +++ b/src/xrpld/app/consensus/ConsensusExtensions.cpp @@ -74,6 +74,19 @@ struct ResolvedExportShare STTx releaseTarget; }; +enum class ExportShareResolutionStatus { + resolved, + deferred, + duplicate, + invalid +}; + +struct ExportShareResolution +{ + ExportShareResolutionStatus status; + std::optional value; +}; + bool isPendingExportShare( ReadView const& view, @@ -104,7 +117,7 @@ isPendingExportShare( view.info().seq <= latch->getFieldU32(sfLastLedgerSequence); } -std::optional +ExportShareResolution resolveExportShare( Application& app, ConsensusExtensions const& extensions, @@ -112,26 +125,55 @@ resolveExportShare( std::shared_ptr const& validated, beast::Journal j) { - if (!validated || !validated->rules().enabled(featureExport) || - share.triggerTxn != share.originTxn || - !isPendingExportShare(*validated, share, j)) - return std::nullopt; + if (!validated) + return {ExportShareResolutionStatus::deferred, std::nullopt}; + if (!validated->rules().enabled(featureExport) || + share.triggerTxn != share.originTxn) + return {ExportShareResolutionStatus::invalid, std::nullopt}; + if (share.originLedgerSeq > validated->info().seq) + return {ExportShareResolutionStatus::deferred, std::nullopt}; + + auto const canonicalOriginHash = + share.originLedgerSeq == validated->info().seq + ? std::optional{validated->info().hash} + : hashOfSeq(*validated, share.originLedgerSeq, j); + if (!canonicalOriginHash) + return {ExportShareResolutionStatus::deferred, std::nullopt}; + if (*canonicalOriginHash != share.originLedgerHash) + return {ExportShareResolutionStatus::invalid, std::nullopt}; auto const latchKey = keylet::shadowTicket(share.owner, share.originTxn); auto const latch = validated->read(latchKey); if (!latch) - return std::nullopt; + return {ExportShareResolutionStatus::invalid, std::nullopt}; + if (latch->getType() != ltSHADOW_TICKET || + !latch->isFieldPresent(sfTransactionHash) || + !latch->isFieldPresent(sfExportUniverseHash) || + !latch->isFieldPresent(sfExportCommittee) || + !latch->isFieldPresent(sfLastLedgerSequence) || + !latch->isFieldPresent(sfAccount) || + !latch->isFieldPresent(sfLedgerSequence) || + latch->getAccountID(sfAccount) != share.owner || + latch->getFieldH256(sfTransactionHash) != share.originTxn || + latch->getFieldU32(sfLedgerSequence) != share.originLedgerSeq) + return {ExportShareResolutionStatus::invalid, std::nullopt}; + if (!latch->isFieldPresent(sfExportNode) || + latch->isFieldPresent(sfExportSignatureHash) || + validated->info().seq > latch->getFieldU32(sfLastLedgerSequence)) + return {ExportShareResolutionStatus::duplicate, std::nullopt}; auto const originLedger = app.getLedgerMaster().getLedgerByHash(share.originLedgerHash); - if (!originLedger || originLedger->info().seq != share.originLedgerSeq) - return std::nullopt; + if (!originLedger) + return {ExportShareResolutionStatus::deferred, std::nullopt}; + if (originLedger->info().seq != share.originLedgerSeq) + return {ExportShareResolutionStatus::invalid, std::nullopt}; auto const [outer, _] = originLedger->txRead(share.originTxn); if (!outer || outer->getTxnType() != ttEXPORT || !outer->isFieldPresent(sfExportedTxn) || outer->getAccountID(sfAccount) != share.owner) - return std::nullopt; + return {ExportShareResolutionStatus::invalid, std::nullopt}; bool const hasUniverse = outer->isFieldPresent(sfExportUniverseHash); bool const hasCommittee = outer->isFieldPresent(sfExportCommittee); @@ -142,15 +184,15 @@ resolveExportShare( latch->getFieldH256(sfExportUniverseHash) || outer->getFieldVL(sfExportCommittee) != latch->getFieldVL(sfExportCommittee)))) - return std::nullopt; + return {ExportShareResolutionStatus::invalid, std::nullopt}; auto const universeHash = latch->getFieldH256(sfExportUniverseHash); if (originLedger->info().parentHash != universeHash) - return std::nullopt; + return {ExportShareResolutionStatus::invalid, std::nullopt}; auto const universeLedger = app.getLedgerMaster().getLedgerByHash(universeHash); if (!universeLedger) - return std::nullopt; + return {ExportShareResolutionStatus::deferred, std::nullopt}; auto const validatorView = extensions.makeActiveValidatorView(universeLedger); @@ -158,23 +200,23 @@ resolveExportShare( *validatorView->sourceLedgerHash != universeHash || share.universePosition >= validatorView->orderedOriginalMasterKeys.size()) - return std::nullopt; + return {ExportShareResolutionStatus::invalid, std::nullopt}; auto const committee = resolveExportCommittee( makeSlice(latch->getFieldVL(sfExportCommittee)), validatorView->orderedOriginalMasterKeys.size()); if (!committee || !committee->members.contains(share.universePosition)) - return std::nullopt; + return {ExportShareResolutionStatus::invalid, std::nullopt}; auto const& expectedMaster = validatorView->orderedOriginalMasterKeys[share.universePosition]; if (app.validatorManifests().getMasterKey(share.signingKey) != expectedMaster) - return std::nullopt; + return {ExportShareResolutionStatus::duplicate, std::nullopt}; auto const inner = ExportLedgerOps::innerExportedTx(*outer); if (!inner) - return std::nullopt; + return {ExportShareResolutionStatus::invalid, std::nullopt}; auto const targetNetworkID = inner->isFieldPresent(sfNetworkID) ? inner->getFieldU32(sfNetworkID) : std::uint32_t{0}; @@ -189,9 +231,11 @@ resolveExportShare( if (!identity || !release || ExportResultBuilder::exportIntentHash(identity.value()) != latch->getFieldH256(sfDigest)) - return std::nullopt; + return {ExportShareResolutionStatus::invalid, std::nullopt}; - return ResolvedExportShare{latch, std::move(release.value())}; + return { + ExportShareResolutionStatus::resolved, + ResolvedExportShare{latch, std::move(release.value())}}; } ExportShare @@ -261,17 +305,144 @@ ConsensusExtensions::publishExportShareLocked( bool ConsensusExtensions::onExportShare(ExportShare const& share) { + return admitExportShare(share, {}, true).isAccepted(); +} + +ExportShareAdmission +ConsensusExtensions::onExportShare( + ExportShare const& share, + ExportShareChargeHandler deferredCharge) +{ + return admitExportShare(share, std::move(deferredCharge), true); +} + +ExportShareAdmission +ConsensusExtensions::deferExportShare( + ExportShare const& share, + ExportShareChargeHandler deferredCharge, + LedgerIndex const validatedLedgerSeq) +{ + if (share.originLedgerSeq <= validatedLedgerSeq || + share.originLedgerSeq - validatedLedgerSeq > + maxDeferredExportShareFutureLedgers_) + return {ExportShareDisposition::deferred, ExportShareCharge::none}; + + std::size_t const serializedBytes = share.serialize().size(); + auto const wireHash = share.wireHash(); + + std::lock_guard lock(deferredExportSharesMutex_); if (!exportShareServiceStarted_.load(std::memory_order_acquire)) - return false; + return {ExportShareDisposition::duplicate, ExportShareCharge::none}; + + if (auto const it = deferredExportShares_.find(wireHash); + it != deferredExportShares_.end()) + { + if (!it->second.charge && deferredCharge) + it->second.charge = std::move(deferredCharge); + return {ExportShareDisposition::duplicate, ExportShareCharge::none}; + } + + bool const newOrigin = + !deferredExportShareOrigins_.contains(share.originTxn); + if (deferredExportShares_.size() >= maxDeferredExportShares_ || + serializedBytes > maxDeferredExportShareBytes_ || + deferredExportShareBytes_ > + maxDeferredExportShareBytes_ - serializedBytes || + (newOrigin && + deferredExportShareOrigins_.size() >= maxDeferredExportShareOrigins_)) + return {ExportShareDisposition::deferred, ExportShareCharge::none}; + + deferredExportShareBytes_ += serializedBytes; + ++deferredExportShareOrigins_[share.originTxn]; + deferredExportShares_.emplace( + wireHash, + DeferredExportShare{share, std::move(deferredCharge), serializedBytes}); + return {ExportShareDisposition::deferred, ExportShareCharge::none}; +} + +void +ConsensusExtensions::retryDeferredExportShares( + LedgerIndex const validatedLedgerSeq) +{ + std::vector retry; + { + std::lock_guard lock(deferredExportSharesMutex_); + for (auto it = deferredExportShares_.begin(); + it != deferredExportShares_.end();) + { + if (it->second.share.originLedgerSeq > validatedLedgerSeq) + { + ++it; + continue; + } + + auto const origin = it->second.share.originTxn; + deferredExportShareBytes_ -= it->second.serializedBytes; + auto const originIt = deferredExportShareOrigins_.find(origin); + if (originIt != deferredExportShareOrigins_.end() && + --originIt->second == 0) + deferredExportShareOrigins_.erase(originIt); + retry.push_back(std::move(it->second)); + it = deferredExportShares_.erase(it); + } + } + + for (auto& deferred : retry) + { + auto const result = admitExportShare(deferred.share, {}, false); + if (result.isAccepted()) + { + auto const frame = deferred.share.serialize(); + protocol::TMExportShares message; + message.add_shares(frame.data(), frame.size()); + app_.overlay().broadcast(message); + } + else if ( + result.disposition == ExportShareDisposition::invalid && + result.charge != ExportShareCharge::none && deferred.charge) + { + deferred.charge(result.charge); + } + } +} + +void +ConsensusExtensions::clearDeferredExportShares() +{ + std::lock_guard lock(deferredExportSharesMutex_); + deferredExportShares_.clear(); + deferredExportShareOrigins_.clear(); + deferredExportShareBytes_ = 0; +} + +ExportShareAdmission +ConsensusExtensions::admitExportShare( + ExportShare const& share, + ExportShareChargeHandler deferredCharge, + bool const allowDeferral) +{ + if (!exportShareServiceStarted_.load(std::memory_order_acquire)) + return {ExportShareDisposition::duplicate, ExportShareCharge::none}; auto const validated = app_.getLedgerMaster().getValidatedLedger(); - auto const resolved = resolveExportShare(app_, *this, share, validated, j_); - if (!resolved) - return false; + if (allowDeferral && validated && + share.originLedgerSeq > validated->info().seq) + return deferExportShare( + share, std::move(deferredCharge), validated->info().seq); + + auto resolved = resolveExportShare(app_, *this, share, validated, j_); + if (resolved.status == ExportShareResolutionStatus::deferred) + return {ExportShareDisposition::deferred, ExportShareCharge::none}; + if (resolved.status == ExportShareResolutionStatus::duplicate) + return {ExportShareDisposition::duplicate, ExportShareCharge::none}; + if (resolved.status == ExportShareResolutionStatus::invalid || + !resolved.value || !validated) + return { + ExportShareDisposition::invalid, ExportShareCharge::invalidData}; if (!postValidationExportSigCollector_.reopenPublication( share.originTxn, share.triggerTxn, validated->info().seq)) - return false; + return {ExportShareDisposition::deferred, ExportShareCharge::none}; ExportSigCollectorV2::Contribution contribution{ share.universePosition, share.signingKey, share.signature}; @@ -279,10 +450,31 @@ ConsensusExtensions::onExportShare(ExportShare const& share) share.originTxn, std::move(contribution), validated->info().seq); if (admission.result != ExportSigCollectorV2::BeginResult::verify || !admission.ticket) - return false; + { + switch (admission.result) + { + case ExportSigCollectorV2::BeginResult::duplicate: + case ExportSigCollectorV2::BeginResult::conflicted: + return { + ExportShareDisposition::duplicate, ExportShareCharge::none}; + case ExportSigCollectorV2::BeginResult::unknownOrigin: + case ExportSigCollectorV2::BeginResult::capacity: + return { + ExportShareDisposition::deferred, ExportShareCharge::none}; + case ExportSigCollectorV2::BeginResult::malformed: + return { + ExportShareDisposition::invalid, + ExportShareCharge::invalidData}; + case ExportSigCollectorV2::BeginResult::verify: + break; + } + return { + ExportShareDisposition::invalid, ExportShareCharge::invalidData}; + } auto const signer = calcAccountID(share.signingKey); - auto const data = buildMultiSigningData(resolved->releaseTarget, signer); + auto const data = + buildMultiSigningData(resolved.value->releaseTarget, signer); auto const signatureVerified = verify( share.signingKey, data.slice(), @@ -293,17 +485,21 @@ ConsensusExtensions::onExportShare(ExportShare const& share) { std::lock_guard streamLock(exportStreamMutex_); if (!exportShareServiceStarted_.load(std::memory_order_acquire)) - return false; + return {ExportShareDisposition::duplicate, ExportShareCharge::none}; auto const latest = app_.getLedgerMaster().getValidatedLedger(); if (!latest || !isPendingExportShare(*latest, share, j_)) - return false; + return {ExportShareDisposition::duplicate, ExportShareCharge::none}; publishExportShareLocked( share, latest->info().seq, latest->info().hash); - return true; + return {ExportShareDisposition::accepted, ExportShareCharge::none}; } + if (outcome.result == ExportSigCollectorV2::AdmitResult::invalid) + return { + ExportShareDisposition::invalid, + ExportShareCharge::invalidSignature}; if (outcome.result != ExportSigCollectorV2::AdmitResult::conflicted || !outcome.priorContribution || !outcome.conflictingContribution) - return false; + return {ExportShareDisposition::duplicate, ExportShareCharge::none}; // Propagate both valid encodings so every honest collector can observe the // same absorbing conflict even if it missed the first frame. @@ -315,7 +511,7 @@ ConsensusExtensions::onExportShare(ExportShare const& share) conflict.add_shares(frame.data(), frame.size()); } app_.overlay().broadcast(conflict); - return false; + return {ExportShareDisposition::duplicate, ExportShareCharge::none}; } void @@ -395,6 +591,11 @@ ConsensusExtensions::onValidatedLedger( } } + // Retained replay owns the start of this cursor. Deferred relay shares + // are reconsidered afterward, so newly accepted entries publish as + // edge events before this node authors its own shares below. + retryDeferredExportShares(validated->info().seq); + std::vector shares; shares.reserve(ExportLimits::maxLiveExportLatches); auto const& keys = app_.getValidatorKeys(); @@ -551,6 +752,7 @@ ConsensusExtensions::onValidatedLedger( void ConsensusExtensions::startExportShareService() { + clearDeferredExportShares(); std::lock_guard streamLock(exportStreamMutex_); exportStreamEmissionSeq_ = 0; exportStreamEmittedShares_.clear(); @@ -562,9 +764,12 @@ void ConsensusExtensions::stopExportShareService() noexcept { exportShareServiceStarted_.store(false, std::memory_order_release); - std::lock_guard streamLock(exportStreamMutex_); - exportStreamEmissionSeq_ = 0; - exportStreamEmittedShares_.clear(); + { + std::lock_guard streamLock(exportStreamMutex_); + exportStreamEmissionSeq_ = 0; + exportStreamEmittedShares_.clear(); + } + clearDeferredExportShares(); } //------------------------------------------------------------------------------ @@ -3180,7 +3385,8 @@ ConsensusExtensions::attachExportSignatures( contribution.position, contribution.signingKey, contribution.signature}; - if (!resolveExportShare(app_, *this, share, validated, j_)) + if (resolveExportShare(app_, *this, share, validated, j_).status != + ExportShareResolutionStatus::resolved) continue; auto const frame = share.serialize(); diff --git a/src/xrpld/app/consensus/ConsensusExtensions.h b/src/xrpld/app/consensus/ConsensusExtensions.h index 91683a68d..28966f15a 100644 --- a/src/xrpld/app/consensus/ConsensusExtensions.h +++ b/src/xrpld/app/consensus/ConsensusExtensions.h @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -61,12 +62,50 @@ class ConsensusExtensions std::atomic exportShareServiceStarted_{false}; std::atomic lastExportReplaySeq_{0}; + struct DeferredExportShare + { + ExportShare share; + ExportShareChargeHandler charge; + std::size_t serializedBytes; + }; + + static constexpr std::size_t maxDeferredExportShareOrigins_ = + ExportLimits::maxLiveExportLatches; + static constexpr std::size_t maxDeferredExportShares_ = + ExportLimits::maxLiveExportLatches * ExportLimits::maxCommitteeMembers; + static constexpr std::size_t maxDeferredExportShareBytes_ = + maxDeferredExportShares_ * ExportLimits::maxSerializedExportShareBytes; + static constexpr LedgerIndex maxDeferredExportShareFutureLedgers_ = 8; + + std::mutex deferredExportSharesMutex_; + std::map deferredExportShares_; + std::map deferredExportShareOrigins_; + std::size_t deferredExportShareBytes_{0}; + bool publishExportShareLocked( ExportShare const& share, LedgerIndex validatedLedgerSeq, uint256 const& validatedLedgerHash); + ExportShareAdmission + admitExportShare( + ExportShare const& share, + ExportShareChargeHandler deferredCharge, + bool allowDeferral); + + ExportShareAdmission + deferExportShare( + ExportShare const& share, + ExportShareChargeHandler deferredCharge, + LedgerIndex validatedLedgerSeq); + + void + retryDeferredExportShares(LedgerIndex validatedLedgerSeq); + + void + clearDeferredExportShares(); + public: beast::Journal j_; // public: accessed by extensionsTick template @@ -203,6 +242,11 @@ public: bool onExportShare(ExportShare const& share); + ExportShareAdmission + onExportShare( + ExportShare const& share, + ExportShareChargeHandler deferredCharge); + /** Release local shares unlocked by an exact network-validated ledger. */ void onValidatedLedger(LedgerIndex seq, uint256 const& hash) noexcept; diff --git a/src/xrpld/app/main/Application.cpp b/src/xrpld/app/main/Application.cpp index 98bd958d7..d754458a5 100644 --- a/src/xrpld/app/main/Application.cpp +++ b/src/xrpld/app/main/Application.cpp @@ -1612,10 +1612,18 @@ ApplicationImp::start(bool withTimers) { auto const weak = std::weak_ptr{consensusExtensions_}; - overlay_->setExportShareHandler([weak](ExportShare const& share) { - auto const extensions = weak.lock(); - return extensions && extensions->onExportShare(share); - }); + overlay_->setExportShareHandler( + [weak]( + ExportShare const& share, + ExportShareChargeHandler deferredCharge) { + auto const extensions = weak.lock(); + if (!extensions) + return ExportShareAdmission{ + ExportShareDisposition::deferred, + ExportShareCharge::none}; + return extensions->onExportShare( + share, std::move(deferredCharge)); + }); consensusExtensions_->startExportShareService(); overlay_->start(); } diff --git a/src/xrpld/overlay/ExportShareAdmission.h b/src/xrpld/overlay/ExportShareAdmission.h new file mode 100644 index 000000000..5df8f3323 --- /dev/null +++ b/src/xrpld/overlay/ExportShareAdmission.h @@ -0,0 +1,56 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES WHATSOEVER + RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF + CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#ifndef RIPPLE_OVERLAY_EXPORTSHAREADMISSION_H_INCLUDED +#define RIPPLE_OVERLAY_EXPORTSHAREADMISSION_H_INCLUDED + +#include +#include + +namespace ripple { + +enum class ExportShareDisposition : std::uint8_t { + accepted, + deferred, + duplicate, + invalid +}; + +enum class ExportShareCharge : std::uint8_t { + none, + invalidData, + invalidSignature +}; + +struct ExportShareAdmission +{ + ExportShareDisposition disposition{ExportShareDisposition::invalid}; + ExportShareCharge charge{ExportShareCharge::none}; + + bool + isAccepted() const + { + return disposition == ExportShareDisposition::accepted; + } +}; + +using ExportShareChargeHandler = std::function; + +} // namespace ripple + +#endif diff --git a/src/xrpld/overlay/Overlay.h b/src/xrpld/overlay/Overlay.h index 2767cc98a..8bd9649ba 100644 --- a/src/xrpld/overlay/Overlay.h +++ b/src/xrpld/overlay/Overlay.h @@ -20,6 +20,7 @@ #ifndef RIPPLE_OVERLAY_OVERLAY_H_INCLUDED #define RIPPLE_OVERLAY_OVERLAY_H_INCLUDED +#include #include #include #include @@ -65,12 +66,9 @@ protected: public: enum class Promote { automatic, never, always }; - /** Application admission for a structurally valid ExportShare. - - Returning true means the application has verified the share against - its origin context, committee position/key, and target multisignature. - */ - using ExportShareHandler = std::function; + /** Application admission for a structurally valid ExportShare. */ + using ExportShareHandler = std::function< + ExportShareAdmission(ExportShare const&, ExportShareChargeHandler)>; struct Setup { @@ -176,8 +174,10 @@ public: With no callback installed, admission fails closed. */ - virtual bool - acceptExportShare(ExportShare const& share) = 0; + virtual ExportShareAdmission + acceptExportShare( + ExportShare const& share, + ExportShareChargeHandler deferredCharge) = 0; /** Relay a proposal. * @param m the serialized proposal diff --git a/src/xrpld/overlay/detail/OverlayImpl.cpp b/src/xrpld/overlay/detail/OverlayImpl.cpp index cf99067ce..e7287a7f1 100644 --- a/src/xrpld/overlay/detail/OverlayImpl.cpp +++ b/src/xrpld/overlay/detail/OverlayImpl.cpp @@ -1226,8 +1226,10 @@ OverlayImpl::setExportShareHandler(ExportShareHandler handler) exportShareHandler_ = std::move(handler); } -bool -OverlayImpl::acceptExportShare(ExportShare const& share) +ExportShareAdmission +OverlayImpl::acceptExportShare( + ExportShare const& share, + ExportShareChargeHandler deferredCharge) { ExportShareHandler handler; { @@ -1236,11 +1238,11 @@ OverlayImpl::acceptExportShare(ExportShare const& share) } if (!handler) - return false; + return {ExportShareDisposition::deferred, ExportShareCharge::none}; try { - return handler(share); + return handler(share, std::move(deferredCharge)); } catch (std::exception const& e) { @@ -1251,7 +1253,7 @@ OverlayImpl::acceptExportShare(ExportShare const& share) { JLOG(journal_.error()) << "ExportShare admission callback failed"; } - return false; + return {ExportShareDisposition::deferred, ExportShareCharge::none}; } std::set diff --git a/src/xrpld/overlay/detail/OverlayImpl.h b/src/xrpld/overlay/detail/OverlayImpl.h index 754397bed..bf67d823d 100644 --- a/src/xrpld/overlay/detail/OverlayImpl.h +++ b/src/xrpld/overlay/detail/OverlayImpl.h @@ -235,8 +235,10 @@ public: void setExportShareHandler(ExportShareHandler handler) override; - bool - acceptExportShare(ExportShare const& share) override; + ExportShareAdmission + acceptExportShare( + ExportShare const& share, + ExportShareChargeHandler deferredCharge) override; std::set relay( diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index 59f63b8fe..07379b176 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -64,6 +64,21 @@ std::chrono::milliseconds constexpr peerHighLatency{300}; /** How often we PING the peer to check for latency and sendq probe */ std::chrono::seconds constexpr peerTimerInterval{60}; + +Resource::Charge const* +exportShareFee(ExportShareCharge const charge) +{ + switch (charge) + { + case ExportShareCharge::none: + return nullptr; + case ExportShareCharge::invalidData: + return &Resource::feeInvalidData; + case ExportShareCharge::invalidSignature: + return &Resource::feeInvalidSignature; + } + return nullptr; +} } // namespace // TODO: Remove this exclusion once unit tests are added after the hotfix @@ -1151,10 +1166,7 @@ PeerImp::onMessage(std::shared_ptr const& m) } if (fresh.empty()) - { - fee_.update(Resource::feeUselessData, "duplicate export shares"); return; - } std::weak_ptr weak = shared_from_this(); app_.getJobQueue().addJob( @@ -1169,7 +1181,16 @@ PeerImp::onMessage(std::shared_ptr const& m) accepted.mutable_shares()->Reserve(fresh.size()); for (auto const index : fresh) { - if (peer->overlay_.acceptExportShare(shares[index])) + auto const chargeDeferred = + [weak](ExportShareCharge const charge) { + auto const peer = weak.lock(); + auto const fee = exportShareFee(charge); + if (peer && fee) + peer->charge(*fee, "deferred export share"); + }; + auto const admission = peer->overlay_.acceptExportShare( + shares[index], chargeDeferred); + if (admission.isAccepted()) { // Stable raw-wire routing begins only after semantic // admission; an early state-relative rejection must not @@ -1178,6 +1199,10 @@ PeerImp::onMessage(std::shared_ptr const& m) shares[index].wireHash(), peer->id_); accepted.add_shares(m->shares(index)); } + else if (auto const fee = exportShareFee(admission.charge)) + { + peer->charge(*fee, "export share"); + } } // Structural validity is insufficient: only application-admitted