diff --git a/src/test/consensus/ConsensusExtensions_test.cpp b/src/test/consensus/ConsensusExtensions_test.cpp index eb8a855f8..5e94f7bd1 100644 --- a/src/test/consensus/ConsensusExtensions_test.cpp +++ b/src/test/consensus/ConsensusExtensions_test.cpp @@ -157,6 +157,20 @@ makeExportTx(STObject const& inner, AccountID const& account) return std::make_shared(makeSTTx(exportObj)); } +std::shared_ptr +makeCancelExportTx(AccountID const& account, std::uint32_t sequence) +{ + STObject exportObj(sfGeneric); + exportObj.setFieldU16(sfTransactionType, ttEXPORT); + exportObj.setAccountID(sfAccount, account); + exportObj.setFieldU32(sfSequence, sequence); + exportObj.setFieldU32(sfCancelTicketSequence, 1000 + sequence); + exportObj.setFieldVL(sfSigningPubKey, Blob{}); + exportObj.setFieldAmount(sfFee, XRPAmount{0}); + + return std::make_shared(makeSTTx(exportObj)); +} + RCLTxSet makeRCLTxSet(Application& app, std::vector> txns) { @@ -1920,6 +1934,77 @@ class ConsensusExtensions_test : public beast::unit_test::suite BEAST_EXPECT(fetched.buildExportSigSet(seq) == exportSigSetHash); } + void + testExportSidecarIgnoresCancelOnlyExports() + { + testcase("Export sidecar ignores cancel-only exports"); + + using namespace jtx; + Env env{ + *this, envconfig(validator, ""), supported_amendments(), nullptr}; + auto const ledger = env.app().getLedgerMaster().getClosedLedger(); + auto const& valKeys = env.app().getValidatorKeys(); + BEAST_EXPECT(valKeys.keys); + if (!valKeys.keys) + return; + + auto const& valPK = valKeys.keys->publicKey; + auto const& valSK = valKeys.keys->secretKey; + auto const signerAccount = calcAccountID(valPK); + auto const seq = ledger->seq() + 1; + + auto const dst = calcAccountID(randomKeyPair(KeyType::secp256k1).first); + auto const innerObj = makeExportedPayment(signerAccount, dst); + auto const innerTx = makeSTTx(innerObj); + auto const exportTx = makeExportTx(innerObj, signerAccount); + auto const txHash = exportTx->getTransactionID(); + + std::vector> txs; + for (std::uint32_t sequence = 1; + txs.size() < ExportLimits::maxPendingExports && sequence < 1000; + ++sequence) + { + auto cancelTx = makeCancelExportTx(signerAccount, sequence); + if (cancelTx->getTransactionID() < txHash) + txs.push_back(std::move(cancelTx)); + } + BEAST_EXPECT(txs.size() == ExportLimits::maxPendingExports); + if (txs.size() != ExportLimits::maxPendingExports) + return; + txs.push_back(exportTx); + + auto const txSet = makeRCLTxSet(env.app(), txs); + + ConsensusExtensions source{env.app(), activeNoopJournal()}; + source.setExportEnabledThisRound(true); + source.cacheUNLReport(ledger); + source.cacheConsensusTxSet(txSet); + + auto const sigData = buildMultiSigningData(innerTx, signerAccount); + auto const sig = sign(valPK, valSK, sigData.slice()); + Buffer sigBuf(sig.data(), sig.size()); + source.exportSigCollector().addVerifiedSignature( + txHash, valPK, sigBuf, seq); + + auto const exportSigSetHash = source.buildExportSigSet(seq); + auto const exportedSet = + env.app().getInboundTransactions().getSet(exportSigSetHash, false); + BEAST_EXPECT(exportedSet); + if (exportedSet) + BEAST_EXPECT(sidecarLeafCount(*exportedSet) == 1); + + ConsensusExtensions fetched{env.app(), activeNoopJournal()}; + fetched.setExportEnabledThisRound(true); + fetched.cacheUNLReport(ledger); + fetched.cacheConsensusTxSet(txSet); + fetched.fetchSidecarSetIfNeeded( + exportSigSetHash, ConsensusExtensions::SidecarKind::exportSigSet); + + BEAST_EXPECT( + fetched.exportSigCollector().hasVerifiedSignature(txHash, valPK)); + BEAST_EXPECT(fetched.buildExportSigSet(seq) == exportSigSetHash); + } + void testExportSidecarBuildCapsConsensusCandidates() { @@ -3487,6 +3572,7 @@ class ConsensusExtensions_test : public beast::unit_test::suite FakeExtensions ext; ext.localExportSigs = false; + ext.consensusExportTxns = true; ExtensionTickHarness harness; auto const peerHash = makeHash("peer-export-sig-set"); @@ -3506,6 +3592,26 @@ class ConsensusExtensions_test : public beast::unit_test::suite BEAST_EXPECT(ext.exportSigConvergenceFailed_); } + void + testExportSigGateIgnoresAdvertisedSetsWithoutExportTxns() + { + testcase("Export sig gate ignores advertised sets without export txns"); + + FakeExtensions ext; + ext.localExportSigs = false; + ext.consensusExportTxns = false; + ExtensionTickHarness harness; + auto const peerHash = makeHash("empty-round-export-sig-set"); + + harness.addPeer(1, peerHash); + + auto result = harness.tick(ext); + BEAST_EXPECT(result.readyForAccept); + BEAST_EXPECT(!ext.exportSigGateStarted_); + BEAST_EXPECT(ext.fetchedExportSets.empty()); + BEAST_EXPECT(!ext.exportSigConvergenceFailed_); + } + void testExportSigGateObservingModeDoesNotPropose() { @@ -3986,6 +4092,7 @@ public: testProposalProofRoundTrip(); testHarvestRngDataReplacementAndRejection(); testExportSidecarBuildFetchAndMerge(); + testExportSidecarIgnoresCancelOnlyExports(); testExportSidecarBuildCapsConsensusCandidates(); testExportSidecarRejectsInvalidFetchedEntries(); testExportSidecarRejectsOversizedFetchedSet(); @@ -4019,6 +4126,7 @@ public: testExportSigGateAllowsAlignedQuorumDespiteMinorityConflict(); testExportSigGateAllowsQuorumDespiteMissingObservation(); testExportSigGateFetchesAdvertisedPeerSets(); + testExportSigGateIgnoresAdvertisedSetsWithoutExportTxns(); testExportSigGateObservingModeDoesNotPropose(); testExportSigGateRefreshesHashBeforeWaiting(); testExportSigGateBoundsCandidateObservationWindow(); diff --git a/src/xrpld/app/consensus/ConsensusExtensions.cpp b/src/xrpld/app/consensus/ConsensusExtensions.cpp index a2109dd94..ca17bbd0c 100644 --- a/src/xrpld/app/consensus/ConsensusExtensions.cpp +++ b/src/xrpld/app/consensus/ConsensusExtensions.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -246,8 +247,9 @@ addExportSidecarCandidate( std::shared_ptr stx) { // Export signature sidecars are bounded by maxPendingExports. Overflow - // ttEXPORTs may still be present in the base consensus tx set; they are - // just not sidecar-signature candidates for this round. + // export-work txns may still be present in the base consensus tx set; they + // are just not sidecar-signature candidates for this round. Cancel-only + // ttEXPORTs are not export work and must not consume scarce signing slots. if (exportTxns.size() >= ExportLimits::maxPendingExports) return false; @@ -264,7 +266,7 @@ buildExportTxnLookup(SHAMap const& txns, beast::Journal j) { SerialIter sit(item->slice()); auto stx = std::make_shared(sit); - if (stx->getTxnType() == ttEXPORT) + if (ExportLedgerOps::isPendingExportWorkTxn(*stx)) addExportSidecarCandidate(exportTxns, std::move(stx)); } catch (std::exception const& e) @@ -289,7 +291,7 @@ buildOpenLedgerExportTxnLookup(Application& app) for (auto const& entry : openLedger->txs) { auto const& stx = entry.first; - if (stx && stx->getTxnType() == ttEXPORT) + if (stx && ExportLedgerOps::isPendingExportWorkTxn(*stx)) addExportSidecarCandidate(exportTxns, stx); } return exportTxns; @@ -2999,7 +3001,7 @@ ConsensusExtensions::attachExportSignatures( for (auto const& [stx, meta] : openLedger->txs) { - if (!stx || stx->getTxnType() != ttEXPORT) + if (!stx || !ExportLedgerOps::isPendingExportWorkTxn(*stx)) continue; if (attached >= ExportLimits::maxPendingExports) diff --git a/src/xrpld/consensus/ConsensusExtensionsTick.h b/src/xrpld/consensus/ConsensusExtensionsTick.h index 1baabb493..52df5b717 100644 --- a/src/xrpld/consensus/ConsensusExtensionsTick.h +++ b/src/xrpld/consensus/ConsensusExtensionsTick.h @@ -994,7 +994,7 @@ extensionsTick(Ext& ext, Ctx const& ctx) }; bool hasLocalExportSigs = ext.hasPendingExportSigs(); - if (!hasLocalExportSigs) + if (!hasLocalExportSigs && ext.hasConsensusExportTxns()) { auto const peerSets = fetchPeerExportSigSets(ctx.getPosition()); if (peerSets > 0) @@ -1030,7 +1030,7 @@ extensionsTick(Ext& ext, Ctx const& ctx) << " action=retry-or-expire"; } } - else if (ext.hasConsensusExportTxns()) + else { // A candidate ttEXPORT with no local sig material gets one // short observation window so an already-reachable peer