diff --git a/src/test/consensus/SteppingExtensions_test.cpp b/src/test/consensus/SteppingExtensions_test.cpp index fdcfa8bda2..87029bde48 100644 --- a/src/test/consensus/SteppingExtensions_test.cpp +++ b/src/test/consensus/SteppingExtensions_test.cpp @@ -1,12 +1,14 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include #include @@ -25,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -98,7 +101,6 @@ class SteppingExtensions_test : public beast::unit_test::suite std::string badShareBytes; Buffer badSignature; std::uint32_t badFramesReceived = 0; - std::uint32_t badShareJobs = 0; bool sawBadWhilePending = false; }; @@ -168,43 +170,6 @@ class SteppingExtensions_test : public beast::unit_test::suite return positions.size(); } - static bool - originSidecarHasSignature( - SHAMap const& map, - uint256 const& origin, - Slice signature) - { - bool found = false; - map.visitLeaves( - [&](boost::intrusive_ptr const& item) { - if (found) - return; - try - { - SerialIter sit{item->slice()}; - STObject const obj{sit, sfGeneric}; - if (obj.isFieldPresent(sfSidecarType) && - obj.getFieldU8(sfSidecarType) == sidecarExportSig && - obj.isFieldPresent(sfTransactionHash) && - obj.getFieldH256(sfTransactionHash) == origin && - obj.isFieldPresent(sfTxnSignature)) - { - auto const got = obj.getFieldVL(sfTxnSignature); - if (got.size() == signature.size() && - std::equal( - got.data(), - got.data() + got.size(), - signature.data())) - found = true; - } - } - catch (...) - { - } - }); - return found; - } - static bool authorizedAtEmission(Application& app, ExportShare const& share) { @@ -265,7 +230,8 @@ class SteppingExtensions_test : public beast::unit_test::suite directBatchId(batch)); for (auto const& blob : batch.shares()) { - if (stats->captureOrigin && !stats->honestShare) + if (id == observer && stats->captureOrigin && + !stats->honestShare) { auto const share = ExportShare::parse(makeSlice(blob)); @@ -273,7 +239,8 @@ class SteppingExtensions_test : public beast::unit_test::suite share->originTxn == *stats->captureOrigin) stats->honestShare = share; } - if (!stats->badShareBytes.empty() && + if (id == observer && + !stats->badShareBytes.empty() && blob == stats->badShareBytes) ++stats->badFramesReceived; } @@ -1453,8 +1420,7 @@ class SteppingExtensions_test : public beast::unit_test::suite if (!BEAST_EXPECT(net.minValidatedSeq() >= target)) return std::nullopt; if (net.controller().now() <= staleHorizon) - net.settle( - (staleHorizon + 1s) - net.controller().now(), 1'200'000); + net.settle((staleHorizon + 1s) - net.controller().now(), 1'200'000); BEAST_EXPECT(net.controller().now() > staleHorizon); BEAST_EXPECT(stats->newGenerationRan); BEAST_EXPECT(!stats->oldGenerationRan); @@ -1547,6 +1513,15 @@ class SteppingExtensions_test : public beast::unit_test::suite World world(net, false, true); if (!ready(world)) return std::nullopt; + + test::StreamSink sink{beast::severities::kTrace}; + auto& observerCE = net.node(observer).app().getConsensusExtensions(); + auto const previousJournal = observerCE.j_; + observerCE.j_ = beast::Journal{sink}; + scope_exit restoreJournal{[&observerCE, previousJournal]() { + observerCE.j_ = previousJournal; + }}; + auto const funding = world.submit( observer, jtx::pay(jtx::Account::master, world.owner, jtx::XRP(10'000)), @@ -1564,13 +1539,10 @@ class SteppingExtensions_test : public beast::unit_test::suite auto const origin = tx->getID(); stats->captureOrigin = origin; - auto collectorHasHonest = [&](std::uint32_t node) { + auto collectorHasHonest = [&]() { if (!stats->honestShare) return false; - auto const snap = net.node(node) - .app() - .getConsensusExtensions() - .postValidationExportSigCollector() + auto const snap = observerCE.postValidationExportSigCollector() .fullUnionSnapshot(); auto const it = snap.find(origin); if (it == snap.end()) @@ -1578,8 +1550,7 @@ class SteppingExtensions_test : public beast::unit_test::suite for (auto const& contrib : it->second) { if (contrib.signingKey == stats->honestShare->signingKey && - contrib.position == - stats->honestShare->committeePosition && + contrib.position == stats->honestShare->committeePosition && contrib.signature.size() == stats->honestShare->signature.size() && std::equal( @@ -1591,110 +1562,140 @@ class SteppingExtensions_test : public beast::unit_test::suite return false; }; - auto pendingOn = [&](std::uint32_t node) { - return net.node(node) - .app() - .getConsensusExtensions() - .hasEligiblePendingExports(); + auto observerOriginLive = [&]() { + if (!stats->honestShare) + return false; + auto const& share = *stats->honestShare; + if (net.validSeq(observer) < share.originLedgerSeq) + return false; + if (net.ledgerHash(observer, share.originLedgerSeq) != + share.originLedgerHash) + return false; + auto const ledger = net.ledger(observer, net.validSeq(observer)); + if (!ledger) + return false; + auto const latch = + ledger->read(keylet::exportLatch(share.owner, share.originTxn)); + return latch && latch->getType() == ltEXPORT_LATCH && + !latch->isFieldPresent(sfExportSignatureHash) && + latch->getFieldH256(sfTransactionHash) == share.originTxn && + ledger->seq() <= latch->getFieldU32(sfLastLedgerSequence); }; auto const junk = generateKeyPair( KeyType::secp256k1, generateSeed("dsf-b7-invalid-share")); + auto const invalidResult = std::to_string( + static_cast(ExportSigCollector::AdmitResult::invalid)); std::size_t sent = 0; bool injected = false; - bool honestKeptAfterBad = false; - bool conflictAfterBad = false; - net.controller().observeJobs( - [&](std::uint32_t id, JobType type, std::string const& name) { - if (injected) - { - if (id == observer && type == jtEXPORT_SHARES && - name == "recvExportShares") - ++stats->badShareJobs; - if (stats->badFramesReceived != 0) - { - if (collectorHasHonest(observer)) - honestKeptAfterBad = true; - auto const st = - net.node(observer) - .app() - .getConsensusExtensions() - .postValidationExportSigCollector() - .positionStatus( - origin, - stats->honestShare->committeePosition); - if (st == ExportSigCollector::PositionStatus::conflicted) - conflictAfterBad = true; - } - return; - } - if (!stats->honestShare) - return; - if (stats->honestShare->originLedgerSeq > net.validSeq(0)) - return; - if (!pendingOn(0)) - return; - auto bad = *stats->honestShare; - bad.signature = sign( - junk.first, junk.second, Slice{"dsf-b7-not-payload", 18}); - if (!bad.validShape()) - return; - stats->badSignature = bad.signature; - auto const framed = bad.serialize(); - stats->badShareBytes.assign( - reinterpret_cast(framed.data()), - framed.size()); - protocol::TMExportShares batch; - batch.add_shares(framed.data(), framed.size()); - auto const msg = std::make_shared( - batch, protocol::mtEXPORT_SHARES); - for (auto const& peer : - net.node(0).app().overlay().getActivePeers()) - { - peer->send(msg); - ++sent; - } - stats->sawBadWhilePending = pendingOn(0); - injected = true; - }); + std::size_t captureMark = 0; + uint256 badWire{}; + net.controller().observeJobs([&](std::uint32_t, + JobType, + std::string const&) { + if (injected) + return; + if (!stats->honestShare || !collectorHasHonest() || + !observerOriginLive()) + return; + auto bad = *stats->honestShare; + bad.signature = + sign(junk.first, junk.second, Slice{"dsf-b7-not-payload", 18}); + if (!bad.validShape()) + return; + stats->badSignature = bad.signature; + badWire = bad.wireHash(); + auto const framed = bad.serialize(); + stats->badShareBytes.assign( + reinterpret_cast(framed.data()), framed.size()); + protocol::TMExportShares batch; + batch.add_shares(framed.data(), framed.size()); + auto const msg = + std::make_shared(batch, protocol::mtEXPORT_SHARES); + auto const targetPub = + net.node(observer).app().nodeIdentity().first; + for (auto const& peer : + net.node(0).app().overlay().getActivePeers()) + { + if (peer->getNodePublic() != targetPub) + continue; + peer->send(msg); + ++sent; + break; + } + captureMark = sink.messages().str().size(); + stats->sawBadWhilePending = observerOriginLive(); + injected = true; + }); net.runTo(warmLedger + 8, SteppingNetwork::RunBudget{1600, 1'200'000}); net.controller().observeJobs({}); - if (!BEAST_EXPECT(injected && stats->honestShare && sent != 0)) + if (!BEAST_EXPECT(injected && stats->honestShare && sent == 1)) { - log << " no honest pending contribution before inject" + log << " no observer-admitted pending origin before inject" << " captured=" << static_cast(stats->honestShare) - << " pending0=" << pendingOn(0) - << " pendingObserver=" << pendingOn(observer) - << " injected=" << injected << " sent=" << sent << std::endl; + << " admitted=" << collectorHasHonest() + << " live=" << observerOriginLive() << " injected=" << injected + << " sent=" << sent << std::endl; + return std::nullopt; + } + if (!BEAST_EXPECT(stats->sawBadWhilePending)) + return std::nullopt; + if (!BEAST_EXPECT(stats->badFramesReceived == 1)) + { + log << " observer did not receive exactly one forged frame" + << " received=" << stats->badFramesReceived << " sent=" << sent + << std::endl; return std::nullopt; } - if (!BEAST_EXPECT(stats->badFramesReceived != 0)) + auto const captured = sink.messages().str().substr(captureMark); + auto const originText = "origin=" + to_string(origin); + auto const wireText = "wire=" + to_string(badWire); + auto const commitNeedle = std::string{"ExportShare: collector commit"}; + bool sawRejection = false; { - log << " bad share never arrived on the wire sent=" << sent - << std::endl; + std::istringstream in{captured}; + for (std::string line; std::getline(in, line);) + { + if (line.find(commitNeedle) == std::string::npos) + continue; + if (line.find(originText) == std::string::npos) + continue; + if (line.find(wireText) == std::string::npos) + continue; + if (line.find("signatureVerified=false") == std::string::npos) + continue; + if (line.find("result=" + invalidResult) == std::string::npos) + continue; + sawRejection = true; + break; + } + } + if (!BEAST_EXPECT(sawRejection)) + { + log << " no observer collector-commit rejection for forged wire" + << " " << originText << " " << wireText + << " result=" << invalidResult << std::endl; + std::istringstream in{captured}; + for (std::string line; std::getline(in, line);) + { + if (line.find("ExportShare:") == std::string::npos) + continue; + if (line.find(originText) == std::string::npos && + line.find(wireText) == std::string::npos) + continue; + log << " " << line << std::endl; + } return std::nullopt; } - if (!BEAST_EXPECT(stats->badShareJobs != 0)) - { - log << " recvExportShares did not run after inject jobs=" - << stats->badShareJobs << " " << net.jobDiagnostics() - << std::endl; - return std::nullopt; - } - BEAST_EXPECT(stats->sawBadWhilePending); - BEAST_EXPECT(honestKeptAfterBad); - BEAST_EXPECT(!conflictAfterBad); - BEAST_EXPECT(collectorHasHonest(observer)); - BEAST_EXPECT(collectorHasHonest(0)); - auto const& collector = - net.node(observer).app().getConsensusExtensions() - .postValidationExportSigCollector(); + + BEAST_EXPECT(collectorHasHonest()); BEAST_EXPECT( - collector.positionStatus( - origin, stats->honestShare->committeePosition) != - ExportSigCollector::PositionStatus::conflicted); - auto const snapAfter = collector.fullUnionSnapshot(); + observerCE.postValidationExportSigCollector().positionStatus( + origin, stats->honestShare->committeePosition) == + ExportSigCollector::PositionStatus::unique); + auto const snapAfter = + observerCE.postValidationExportSigCollector().fullUnionSnapshot(); auto const foundAfter = snapAfter.find(origin); if (!BEAST_EXPECT(foundAfter != snapAfter.end())) return std::nullopt; @@ -1705,23 +1706,6 @@ class SteppingExtensions_test : public beast::unit_test::suite contrib.signature.data(), contrib.signature.data() + contrib.signature.size(), stats->badSignature.data())); - if (auto const map = net.node(observer) - .app() - .getConsensusExtensions() - .exportSigSetMap_) - { - BEAST_EXPECT(!originSidecarHasSignature( - *map, - origin, - Slice{ - stats->badSignature.data(), stats->badSignature.size()})); - BEAST_EXPECT(originSidecarHasSignature( - *map, - origin, - Slice{ - stats->honestShare->signature.data(), - stats->honestShare->signature.size()})); - } auto const target = net.validSeq(0) + 4; net.runTo(target, SteppingNetwork::RunBudget{1600, 1'200'000}); @@ -1737,9 +1721,11 @@ class SteppingExtensions_test : public beast::unit_test::suite auto const seqW = witnessAt(net, origin, warmLedger); if (!BEAST_EXPECT(seqW != 0)) return std::nullopt; - BEAST_EXPECT(collectorHasHonest(0)); + BEAST_EXPECT(stats->builds[observer].contains( + {seqW, net.ledgerHash(0, seqW - 1), net.ledgerHash(0, seqW)})); + BEAST_EXPECT(collectorHasHonest()); BEAST_EXPECT( - collector.positionStatus( + observerCE.postValidationExportSigCollector().positionStatus( origin, stats->honestShare->committeePosition) != ExportSigCollector::PositionStatus::conflicted); @@ -1797,13 +1783,11 @@ class SteppingExtensions_test : public beast::unit_test::suite BEAST_EXPECT(unexpected.empty()); BEAST_EXPECT(hits[origin] == 1); log << " invalid-share: received=" << stats->badFramesReceived - << " jobs=" << stats->badShareJobs << " sent=" << sent + << " sent=" << sent << " pendingAtInject=" << stats->sawBadWhilePending - << " witness=" << seqW << std::endl; + << " rejected=" << sawRejection << " witness=" << seqW << std::endl; outcome.push_back(origin); - outcome.push_back(sha512Half( - static_cast(stats->badFramesReceived), - static_cast(stats->badShareJobs))); + outcome.push_back(badWire); return outcome; } @@ -1954,7 +1938,8 @@ public: return observerRestartAcrossExport(net); }); } - if (matches("invalid Export share does not poison an honest contribution")) + if (matches( + "invalid Export share does not poison an honest contribution")) { testcase( "invalid Export share does not poison an honest contribution");