diff --git a/src/herder/HerderSCPDriver.cpp b/src/herder/HerderSCPDriver.cpp index d5551ccc56..4d93bbf8f5 100644 --- a/src/herder/HerderSCPDriver.cpp +++ b/src/herder/HerderSCPDriver.cpp @@ -1322,6 +1322,14 @@ HerderSCPDriver::recordBallotBlockedOnTxSet(uint64_t slotIndex, timing.mBallotBlockedOnTxSetStart.end()) { timing.mBallotBlockedOnTxSetStart[value] = mApp.getClock().now(); + + if (StellarValue sv; + isParallelTxSetDownloadEnabled() && toStellarValue(value, sv)) + { + // Remember that `value` is stalled waiting for the tx set + // with hash `sv.txSetHash`. + mStallingByTxSet[sv.txSetHash].emplace_back(slotIndex, value); + } } } @@ -1340,6 +1348,27 @@ HerderSCPDriver::measureAndRecordBallotBlockedOnTxSet(uint64_t slotIndex, std::chrono::duration_cast( mApp.getClock().now() - valueIt->second); mSCPMetrics.mBallotBlockedOnTxSet.Update(elapsed); + + if (StellarValue sv; toStellarValue(value, sv)) + { + // This value is no longer stalled. Remove it from + // `mStallingByTxSet` + auto sIt = mStallingByTxSet.find(sv.txSetHash); + if (sIt != mStallingByTxSet.end()) + { + auto& vec = sIt->second; + vec.erase(std::remove_if(vec.begin(), vec.end(), + [&](auto const& p) { + return p.first == slotIndex && + p.second == value; + }), + vec.end()); + if (vec.empty()) + { + mStallingByTxSet.erase(sIt); + } + } + } return; } } @@ -1712,6 +1741,23 @@ HerderSCPDriver::purgeSlotsOutsideRange(std::optional minSlotIndex, // Clean up expired weak_ptrs from the pending tx set registries. purgeExpiredWeakPtrs(mPendingTxSetWrappers); purgeExpiredWeakPtrs(mPendingTxSetEnvelopeWrappers); + + // Drop stalled-ballot resume entries whose slots fall outside the retained + // range. + for (auto it = mStallingByTxSet.begin(); it != mStallingByTxSet.end();) + { + auto& stalling = it->second; + stalling.erase( + std::remove_if(stalling.begin(), stalling.end(), + [&](auto const& slotAndValue) { + auto const slot = slotAndValue.first; + return slot != slotToKeep && + ((minSlotIndex && slot < *minSlotIndex) || + (maxSlotIndex && slot > *maxSlotIndex)); + }), + stalling.end()); + it = stalling.empty() ? mStallingByTxSet.erase(it) : std::next(it); + } } void @@ -1745,6 +1791,34 @@ HerderSCPDriver::onTxSetReceived(Hash const& txSetHash, } mPendingTxSetEnvelopeWrappers.erase(envIt); } + + // Resume any slot that stalled waiting for this tx set + maybeResumeBalloting(txSetHash); +} + +void +HerderSCPDriver::maybeResumeBalloting(Hash const& txSetHash) +{ + if (!isParallelTxSetDownloadEnabled()) + { + return; + } + + auto it = mStallingByTxSet.find(txSetHash); + if (it == mStallingByTxSet.end()) + { + return; + } + + // Remove entry from `mStallingByTxSet` and iterate over stalling slots + // Remove prior to iterating because the `receivedTxSet` flow may itself + // modify `mStallingByTxSet`. + auto const stalling = std::move(it->second); + mStallingByTxSet.erase(it); + for (auto const& slotAndValue : stalling) + { + mSCP.receivedTxSet(slotAndValue.first, slotAndValue.second); + } } void diff --git a/src/herder/HerderSCPDriver.h b/src/herder/HerderSCPDriver.h index 5c6a2c06ac..3a24d015f3 100644 --- a/src/herder/HerderSCPDriver.h +++ b/src/herder/HerderSCPDriver.h @@ -177,6 +177,10 @@ class HerderSCPDriver : public SCPDriver // downloading). void onTxSetReceived(Hash const& txSetHash, TxSetXDRFrameConstPtr txSet); + // If balloting is stalled waiting for txSetHash, then resume balloting from + // the stall point. Otherwise, do nothing. + void maybeResumeBalloting(Hash const& txSetHash); + double getExternalizeLag(NodeID const& id) const; Json::Value getQsetLagInfo(bool summary, bool fullKeys); @@ -309,6 +313,11 @@ class HerderSCPDriver : public SCPDriver // * first prepare to externalize std::map mSCPExecutionTimes; + // Values stalled at the ballot commit gate waiting for a tx set. + // Mapping from to pairs of (, ). + UnorderedMap>> + mStallingByTxSet; + uint32_t mLedgerSeqNominating; ValueWrapperPtr mCurrentValue; diff --git a/src/herder/test/HerderTests.cpp b/src/herder/test/HerderTests.cpp index bc4857c524..b9c7b29ebd 100644 --- a/src/herder/test/HerderTests.cpp +++ b/src/herder/test/HerderTests.cpp @@ -9048,6 +9048,120 @@ TEST_CASE_VERSIONS("Herder properly validates when tx set is missing", }); } +// Test that a stalled ballot resumes immediately on tx set arrival +TEST_CASE_VERSIONS("tx set arrival resumes stalled balloting", "[herder]") +{ + Config cfg(getTestConfig()); + cfg.MANUAL_CLOSE = false; + cfg.EXPERIMENTAL_PARALLEL_TX_SET_DOWNLOAD = true; + + VirtualClock clock; + + auto v1Key = SecretKey::pseudoRandomForTesting(); + auto v2Key = SecretKey::pseudoRandomForTesting(); + auto const& v1Pk = v1Key.getPublicKey(); + auto const& v2Pk = v2Key.getPublicKey(); + + // Local quorum set {self, v1, v2} with threshold 2 + cfg.QUORUM_SET.threshold = 2; + cfg.QUORUM_SET.validators.emplace_back(v1Pk); + cfg.QUORUM_SET.validators.emplace_back(v2Pk); + + Application::pointer app = createTestApplication(clock, cfg); + + for_versions_from( + static_cast(EMPTY_TX_SET_PROTOCOL_VERSION), *app, [&] { + auto const lcl = + app->getLedgerManager().getLastClosedLedgerHeader(); + uint64_t const slotIndex = lcl.header.ledgerSeq + 1; + auto& herder = dynamic_cast(app->getHerder()); + auto& pendingEnvelopes = herder.getPendingEnvelopes(); + + // Peers use the same 3-node qset; pre-cache it so envelopes don't + // block on a qset fetch. + SCPQuorumSet qSet; + qSet.threshold = 2; + qSet.validators.push_back(cfg.NODE_SEED.getPublicKey()); + qSet.validators.push_back(v1Pk); + qSet.validators.push_back(v2Pk); + auto qSetHash = sha256(xdr::xdr_to_opaque(qSet)); + pendingEnvelopes.addSCPQuorumSet(qSetHash, qSet); + + // Create a non-empty tx set that the node does not have + auto root = app->getRoot(); + std::vector txs = { + root->tx({payment(root->getPublicKey(), 1)})}; + auto txSet = makeTxSetFromTransactions(txs, *app, 0, 0).first; + auto txSetHash = txSet->getContentsHash(); + + auto sv = herder.makeStellarValue(txSetHash, + lcl.header.scpValue.closeTime + 1, + emptyUpgradeSteps, v1Key); + auto opaqueValue = xdr::xdr_to_opaque(sv); + + auto makePrepareFromPeer = [&](SecretKey const& peerKey) { + SCPEnvelope env; + env.statement.slotIndex = slotIndex; + env.statement.pledges.type(SCP_ST_PREPARE); + auto& prep = env.statement.pledges.prepare(); + prep.ballot.counter = 1; + prep.ballot.value = opaqueValue; + prep.prepared.activate() = prep.ballot; + prep.quorumSetHash = qSetHash; + env.statement.nodeID = peerKey.getPublicKey(); + herder.signEnvelope(peerKey, env); + return env; + }; + + // Both peers accept-prepared (1, v). The envelopes are + // ready without the tx set (parallel downloading), and processing + // them drives the local node to confirm-prepared and then stall + // because the tx set is still missing. + REQUIRE(herder.recvSCPEnvelope(makePrepareFromPeer(v1Key)) == + Herder::ENVELOPE_STATUS_READY); + REQUIRE(herder.recvSCPEnvelope(makePrepareFromPeer(v2Key)) == + Herder::ENVELOPE_STATUS_READY); + + auto localPrepare = [&]() { + auto const* env = herder.getSCP().getLatestMessage( + cfg.NODE_SEED.getPublicKey()); + REQUIRE(env); + REQUIRE(env->statement.pledges.type() == SCP_ST_PREPARE); + return env->statement.pledges.prepare(); + }; + + // Stalled: h is set but the commit is deferred (nC == 0). + { + auto const prep = localPrepare(); + REQUIRE(prep.ballot.counter == 1); + REQUIRE(prep.nH == 1); + REQUIRE(prep.nC == 0); + } + + // Deliver the tx set + REQUIRE(herder.recvTxSet(txSetHash, txSet)); + + // Resumed: the commit completed at the same counter, indicating + // the lack of a ballot timeout + { + auto const prep = localPrepare(); + REQUIRE(prep.ballot.counter == 1); + REQUIRE(prep.nH == 1); + REQUIRE(prep.nC == 1); + } + + // Repeat delivery is a no-op: the tx set is no longer being + // fetched, and balloting state does not change. + REQUIRE(!herder.recvTxSet(txSetHash, txSet)); + { + auto const prep = localPrepare(); + REQUIRE(prep.ballot.counter == 1); + REQUIRE(prep.nH == 1); + REQUIRE(prep.nC == 1); + } + }); +} + #ifdef CAP_0083 // This tests that the network externalizes an empty-tx-set value when a // voted-for value is not available on the network. diff --git a/src/scp/BallotProtocol.cpp b/src/scp/BallotProtocol.cpp index 2684c601d1..2f377aeac9 100644 --- a/src/scp/BallotProtocol.cpp +++ b/src/scp/BallotProtocol.cpp @@ -54,7 +54,7 @@ BallotProtocol::isNewerStatement(NodeID const& nodeID, SCPStatement const& st) bool BallotProtocol::isNewerStatement(SCPStatement const& oldst, - SCPStatement const& st) + SCPStatement const& st) const { bool res = false; @@ -98,7 +98,7 @@ BallotProtocol::isNewerStatement(SCPStatement const& oldst, else { // Lexicographical order between PREPARE statements: - // (b, p, p', h) + // (b, p, p', h, c) auto const& oldPrep = oldst.pledges.prepare(); auto const& prep = st.pledges.prepare(); @@ -124,7 +124,16 @@ BallotProtocol::isNewerStatement(SCPStatement const& oldst, } else if (compBallot == 0) { - res = (oldPrep.nH < prep.nH); + if (mSlot.getSCPDriver() + .protocolAllowsEmptyTxSetValues() && + oldPrep.nH == prep.nH) + { + res = (oldPrep.nC < prep.nC); + } + else + { + res = (oldPrep.nH < prep.nH); + } } } } @@ -675,7 +684,7 @@ BallotProtocol::createStatement(SCPStatementType const& type) return statement; } -void +SCPStatement BallotProtocol::emitCurrentStateStatement() { ZoneScoped; @@ -729,6 +738,11 @@ BallotProtocol::emitCurrentStateStatement() throw std::runtime_error("moved to a bad state (ballot protocol)"); } } + + // Return the statement this call generated. Intentionally does not return + // the statement generated by recursion so that the caller may reason about + // what this call specifically produced. + return envelope.statement; } void @@ -1144,6 +1158,7 @@ BallotProtocol::setConfirmPrepared(SCPBallot const& newC, SCPBallot const& newH) mSlot.getSlotIndex(), mSlot.getSCP().ballotToStr(newH)); bool didWork = false; + bool stalled = false; // remember newH's value mValueOverride = mSlot.getSCPDriver().wrapValue(newH.value); @@ -1179,6 +1194,8 @@ BallotProtocol::setConfirmPrepared(SCPBallot const& newC, SCPBallot const& newH) mSlot.getSCPDriver().recordBallotBlockedOnTxSet( mSlot.getSlotIndex(), newC.value); + stalled = true; + CLOG_TRACE( SCP, "BallotProtocol::setConfirmPrepared slot:{} " @@ -1217,12 +1234,53 @@ BallotProtocol::setConfirmPrepared(SCPBallot const& newC, SCPBallot const& newH) if (didWork) { - emitCurrentStateStatement(); + auto const emitted = emitCurrentStateStatement(); + + if (stalled) + { + // Stalled waiting for the tx set corresponding to newC.value. + // Remember the state that existed at the stall point so that we can + // evaluate whether it's safe to resume (skipping a ballot timeout) + // if the tx set arrives. + mStalledCommit = StalledCommit{newC, newH, emitted}; + } + else + { + mStalledCommit.reset(); + } } return didWork; } +void +BallotProtocol::receivedTxSet(Value const& value) +{ + ZoneScoped; + // Only act if this slot stalled waiting for exactly this value's tx set. + if (!mStalledCommit || !(mStalledCommit->mCommitBallot.value == value)) + { + return; + } + + // It should not be possible to end up here prior to the protocol supporting + // kStructurallyValidValue. + releaseAssert(mSlot.getSCPDriver().protocolAllowsEmptyTxSetValues()); + + auto const stalled = *mStalledCommit; + mStalledCommit.reset(); + + // Resume only if the node has done no balloting work since the stall. + auto const* selfEnv = getLatestMessage(mSlot.getSCP().getLocalNodeID()); + if (selfEnv == nullptr || !(selfEnv->statement == stalled.mStallStatement)) + { + return; + } + + // Re-run the commit step setConfirmPrepared deferred. + setConfirmPrepared(stalled.mCommitBallot, stalled.mHighBallot); +} + void BallotProtocol::findExtendedInterval(Interval& candidate, std::set const& boundaries, diff --git a/src/scp/BallotProtocol.h b/src/scp/BallotProtocol.h index 2ff2cb1207..1df9acd491 100644 --- a/src/scp/BallotProtocol.h +++ b/src/scp/BallotProtocol.h @@ -9,6 +9,7 @@ #include "util/GlobalChecks.h" #include #include +#include #include #include #include @@ -95,6 +96,16 @@ class BallotProtocol SCPEnvelopeWrapperPtr mLastEnvelopeEmit; // last envelope emitted by this node + // Information about the state of balloting upon stalling when attempting to + // set `c` to a value the node has not successfully fetched. + struct StalledCommit + { + SCPBallot mCommitBallot; // c (deferred; not in the emitted stmt) + SCPBallot mHighBallot; // h + SCPStatement mStallStatement; // self statement emitted at the stall + }; + std::optional mStalledCommit; + public: BallotProtocol(Slot& slot); @@ -119,6 +130,12 @@ class BallotProtocol // flavor that takes the actual desired counter value bool bumpState(Value const& value, uint32 n); + // Called when the tx set referenced by @p value arrives. + // If balloting stalled waiting for this tx set, AND the node's state has + // not changed since hitting the stall point, this function will resume + // balloting for this slot immediately. Otherwise, it does nothing. + void receivedTxSet(Value const& value); + // ** status methods // returns information about the local state in JSON format @@ -166,8 +183,8 @@ class BallotProtocol static std::set getStatementValues(SCPStatement const& st); // returns true if st is newer than oldst - static bool isNewerStatement(SCPStatement const& oldst, - SCPStatement const& st); + bool isNewerStatement(SCPStatement const& oldst, + SCPStatement const& st) const; private: // attempts to make progress using the latest statement as a hint @@ -305,9 +322,11 @@ class BallotProtocol // we have. bool updateCurrentValue(SCPBallot const& ballot); - // emits a statement reflecting the nodes' current state - // and attempts to make progress - void emitCurrentStateStatement(); + // Emits a statement reflecting the node's current state and attempts to + // make progress. Returns the statement it generated (built from current + // state, *before* the self-processing recursion that may advance the ballot + // further), so callers can capture exactly what this call produced. + SCPStatement emitCurrentStateStatement(); // verifies that the internal state is consistent void checkInvariants(); diff --git a/src/scp/SCP.cpp b/src/scp/SCP.cpp index 6bb7edce30..8b391896cc 100644 --- a/src/scp/SCP.cpp +++ b/src/scp/SCP.cpp @@ -51,6 +51,16 @@ SCP::stopNomination(uint64 slotIndex) } } +void +SCP::receivedTxSet(uint64 slotIndex, Value const& value) +{ + auto s = getSlot(slotIndex, false); + if (s) + { + s->receivedTxSet(value); + } +} + void SCP::updateLocalQuorumSet(SCPQuorumSet const& qSet) { diff --git a/src/scp/SCP.h b/src/scp/SCP.h index f06980937c..c99fb86be7 100644 --- a/src/scp/SCP.h +++ b/src/scp/SCP.h @@ -59,6 +59,10 @@ class SCP // stops nomination for a slot void stopNomination(uint64 slotIndex); + // Notify SCP that the tx set referenced by @p value has arrived so that it + // may resume balloting if stalled waiting for this tx set. + void receivedTxSet(uint64 slotIndex, Value const& value); + // Local QuorumSet interface (can be dynamically updated) void updateLocalQuorumSet(SCPQuorumSet const& qSet); SCPQuorumSet const& getLocalQuorumSet(); diff --git a/src/scp/Slot.cpp b/src/scp/Slot.cpp index bff0557579..ba15e214f1 100644 --- a/src/scp/Slot.cpp +++ b/src/scp/Slot.cpp @@ -131,7 +131,7 @@ Slot::isNewerNominationOrBallotSt(SCPStatement const& oldSt, } else { - if (BallotProtocol::isNewerStatement(oldSt, newSt)) + if (mBallotProtocol.isNewerStatement(oldSt, newSt)) { replace = true; } @@ -217,6 +217,12 @@ Slot::abandonBallot() return mBallotProtocol.abandonBallot(0); } +void +Slot::receivedTxSet(Value const& value) +{ + mBallotProtocol.receivedTxSet(value); +} + bool Slot::bumpState(Value const& value, bool force) { diff --git a/src/scp/Slot.h b/src/scp/Slot.h index ecf1e8fee5..e43f0b8573 100644 --- a/src/scp/Slot.h +++ b/src/scp/Slot.h @@ -117,6 +117,10 @@ class Slot : public std::enable_shared_from_this bool abandonBallot(); + // Notify this slot that the tx set referenced by @p value has arrived so + // that it may resume balloting if stalled waiting for this tx set. + void receivedTxSet(Value const& value); + // bumps the ballot based on the local state and the value passed in: // in prepare phase, attempts to take value // otherwise, no-ops diff --git a/src/scp/test/SCPTests.cpp b/src/scp/test/SCPTests.cpp index 9f605754d3..6efb0fc5ce 100644 --- a/src/scp/test/SCPTests.cpp +++ b/src/scp/test/SCPTests.cpp @@ -43,10 +43,12 @@ class TestSCP : public SCPDriver uint32_t mIncrementBallotTimeoutMS = 1000; uint32_t mInitialNominationTimeoutMS = 1000; uint32_t mIncrementNominationTimeoutMS = 1000; + bool const mProtocolAllowsEmptyTxSetValues; TestSCP(NodeID const& nodeID, SCPQuorumSet const& qSetLocal, - bool isValidator = true) + bool isValidator = true, bool protocolAllowsEmptyTxSetValues = true) : mSCP(*this, nodeID, isValidator, qSetLocal) + , mProtocolAllowsEmptyTxSetValues(protocolAllowsEmptyTxSetValues) { mPriorityLookup = [&](NodeID const& n) { return (n == mSCP.getLocalNodeID()) ? 1000 : 1; @@ -165,13 +167,17 @@ class TestSCP : public SCPDriver bool isParallelTxSetDownloadEnabled() const override { - return true; + // Leave unimplemented. A node's parallel downloading setting only + // affects higher level systems (such as PendingEnvelopes). + // NominationProtocol and BallotProtocol only reason about whether the + // protocol supports empty-tx-set values + releaseAssert(false); } bool protocolAllowsEmptyTxSetValues() const override { - return true; + return mProtocolAllowsEmptyTxSetValues; } void @@ -187,6 +193,12 @@ class TestSCP : public SCPDriver return mSCP.getSlot(slotIndex, true)->bumpState(v, true); } + void + receivedTxSet(uint64 slotIndex, Value const& v) + { + mSCP.receivedTxSet(slotIndex, v); + } + bool nominate(uint64 slotIndex, Value const& value, bool timedout) { @@ -855,7 +867,9 @@ TEST_CASE("ballot protocol core5", "[scp][ballotprotocol]") uint256 qSetHash = sha256(xdr::xdr_to_opaque(qSet)); - TestSCP scp(v0SecretKey.getPublicKey(), qSet); + bool const protocolAllowsEmptyTxSetValues = GENERATE(false, true); + TestSCP scp(v0SecretKey.getPublicKey(), qSet, /*isValidator*/ true, + protocolAllowsEmptyTxSetValues); auto test = [&](TestSCP& scp) { scp.storeQuorumSet(std::make_shared(qSet)); @@ -2645,7 +2659,8 @@ TEST_CASE("ballot protocol core5", "[scp][ballotprotocol]") SECTION("non validator watching the network") { SIMULATION_CREATE_NODE(NV); - TestSCP scpNV(vNVSecretKey.getPublicKey(), qSet, false); + TestSCP scpNV(vNVSecretKey.getPublicKey(), qSet, false, + protocolAllowsEmptyTxSetValues); scpNV.storeQuorumSet(std::make_shared(qSet)); uint256 qSetHashNV = scpNV.mSCP.getLocalNode()->getQuorumSetHash(); @@ -2674,7 +2689,8 @@ TEST_CASE("ballot protocol core5", "[scp][ballotprotocol]") SECTION("restore ballot protocol") { - TestSCP scp2(v0SecretKey.getPublicKey(), qSet); + TestSCP scp2(v0SecretKey.getPublicKey(), qSet, /*isValidator*/ true, + protocolAllowsEmptyTxSetValues); scp2.storeQuorumSet(std::make_shared(qSet)); SCPBallot b(2, xValue); SECTION("prepare") @@ -2719,7 +2735,9 @@ TEST_CASE("ballot protocol core3", "[scp][ballotprotocol]") uint256 qSetHash = sha256(xdr::xdr_to_opaque(qSet)); - TestSCP scp(v0SecretKey.getPublicKey(), qSet); + bool const protocolAllowsEmptyTxSetValues = GENERATE(false, true); + TestSCP scp(v0SecretKey.getPublicKey(), qSet, /*isValidator*/ true, + protocolAllowsEmptyTxSetValues); auto test = [&](TestSCP& scp) { scp.storeQuorumSet(std::make_shared(qSet)); @@ -2866,7 +2884,9 @@ TEST_CASE("ballot protocol core3", "[scp][ballotprotocol]") SECTION("node without self - quorum timeout") { SIMULATION_CREATE_NODE(NodeNS); - TestSCP scpNNS(vNodeNSSecretKey.getPublicKey(), qSet); + TestSCP scpNNS(vNodeNSSecretKey.getPublicKey(), qSet, + /*isValidator*/ true, + protocolAllowsEmptyTxSetValues); scpNNS.storeQuorumSet(std::make_shared(qSet)); uint256 qSetHashNodeNS = scpNNS.mSCP.getLocalNode()->getQuorumSetHash(); @@ -2923,9 +2943,11 @@ TEST_CASE("nomination tests core5", "[scp][nominationprotocol]") expectedLeaders.end())); }; + bool const protocolAllowsEmptyTxSetValues = GENERATE(false, true); SECTION("nomination - v0 is top") { - TestSCP scp(v0SecretKey.getPublicKey(), qSet); + TestSCP scp(v0SecretKey.getPublicKey(), qSet, /*isValidator*/ true, + protocolAllowsEmptyTxSetValues); auto test = [&](TestSCP& scp) { uint256 qSetHash0 = scp.mSCP.getLocalNode()->getQuorumSetHash(); @@ -3039,7 +3061,9 @@ TEST_CASE("nomination tests core5", "[scp][nominationprotocol]") } SECTION("nomination - restored state") { - TestSCP scp2(v0SecretKey.getPublicKey(), qSet); + TestSCP scp2(v0SecretKey.getPublicKey(), qSet, + /*isValidator*/ true, + protocolAllowsEmptyTxSetValues); scp2.storeQuorumSet( std::make_shared(qSet)); @@ -3276,7 +3300,8 @@ TEST_CASE("nomination tests core5", "[scp][nominationprotocol]") } SECTION("v1 is top node") { - TestSCP scp(v0SecretKey.getPublicKey(), qSet); + TestSCP scp(v0SecretKey.getPublicKey(), qSet, /*isValidator*/ true, + protocolAllowsEmptyTxSetValues); auto test = [&](TestSCP& scp) { uint256 qSetHash0 = scp.mSCP.getLocalNode()->getQuorumSetHash(); @@ -3485,7 +3510,7 @@ TEST_CASE("nomination tests core5", "[scp][nominationprotocol]") } } -#ifdef CAP_0087 +#ifdef CAP_0083 TEST_CASE("nomination times out structurally-valid value into empty tx set", "[scp][nomination]") { @@ -3867,6 +3892,114 @@ TEST_CASE("setConfirmPrepared stalls on kStructurallyValidValue value", } REQUIRE(foundC); } + + SECTION("resumes immediately on receivedTxSet") + { + auto const envsBeforeClear = scp.mEnvs.size(); + + // Simulate tx set arrival: the stalled value becomes fully validated. + scp.clearDownload(xValue); + + // Resume the deferred commit directly — no ballot bump, no new + // envelopes, no timer. + scp.receivedTxSet(0, xValue); + + // The commit now completes at the SAME counter it stalled on (1) — not + // a bumped counter — proving the resume, not a re-drive. + REQUIRE(scp.mEnvs.size() > envsBeforeClear); + auto const& lastPrep = scp.mEnvs.back().statement.pledges.prepare(); + REQUIRE(lastPrep.nC == 1); + REQUIRE(lastPrep.nH == 1); + + // A repeat delivery no-ops. + auto const envsAfterResume = scp.mEnvs.size(); + scp.receivedTxSet(0, xValue); + REQUIRE(scp.mEnvs.size() == envsAfterResume); + } + + SECTION("receivedTxSet no-ops for a value the slot is not stalled on") + { + auto const envsBefore = scp.mEnvs.size(); + + scp.clearDownload(xValue); + // yValue is not what balloting stalled on, so the stash does not match. + scp.receivedTxSet(0, yValue); + + REQUIRE(scp.mEnvs.size() == envsBefore); + } + + SECTION("receivedTxSet with bad tx set stays stalled") + { + auto const envsBefore = scp.mEnvs.size(); + + // No clearDownload: xValue is still only kStructurallyValidValue, so + // re-running the commit step must re-stall rather than commit. + // Simulates a bad (only structurally valid) tx set arriving. + scp.receivedTxSet(0, xValue); + + REQUIRE(scp.mEnvs.size() == envsBefore); + } + + SECTION("receivedTxSet declines after a local ballot bump (state changed)") + { + // The race the guard exists for: the ballot timer fires (bumping the + // ballot) just before the tx set arrives, so the stash is stale. + REQUIRE(scp.bumpState(0, xValue)); + auto const envsAfterBump = scp.mEnvs.size(); + { + auto const& prep = scp.mEnvs.back().statement.pledges.prepare(); + REQUIRE(prep.ballot.counter == 2); + REQUIRE(prep.nC == 0); + REQUIRE(prep.nH == 1); + } + + scp.clearDownload(xValue); + scp.receivedTxSet(0, xValue); + + // The self statement changed since the stall (b bumped 1 -> 2), so + // the resume declines: no emission, no commit. + REQUIRE(scp.mEnvs.size() == envsAfterBump); + + // The normal path still completes once the network confirms prepared + // at the bumped counter (tx set present -> no re-stall). + SCPBallot xB2(2, xValue); + REQUIRE(scp.receiveEnvelope( + makePrepare(v1SecretKey, qSetHash, 0, xB2, &xB2)) == + SCP::EnvelopeState::VALID); + REQUIRE(scp.receiveEnvelope( + makePrepare(v2SecretKey, qSetHash, 0, xB2, &xB2)) == + SCP::EnvelopeState::VALID); + auto const& lastPrep = scp.mEnvs.back().statement.pledges.prepare(); + REQUIRE(lastPrep.nC == 2); + REQUIRE(lastPrep.nH == 2); + } + + SECTION("receivedTxSet declines after accepting a higher incompatible " + "prepared") + { + // {v1, v2} is a quorum voting prepare (2, y): the node accepts it as + // prepared (p = (2,y), p' = (1,x)), which invalidates the resume path. + SCPBallot yB2(2, yValue); + REQUIRE( + scp.receiveEnvelope(makePrepare(v1SecretKey, qSetHash, 0, yB2)) == + SCP::EnvelopeState::VALID); + REQUIRE( + scp.receiveEnvelope(makePrepare(v2SecretKey, qSetHash, 0, yB2)) == + SCP::EnvelopeState::VALID); + + auto const envsAfterPrepared = scp.mEnvs.size(); + auto const& prep = scp.mEnvs.back().statement.pledges.prepare(); + REQUIRE(prep.prepared); + REQUIRE(*prep.prepared == yB2); + REQUIRE(prep.nC == 0); + REQUIRE(prep.nH == 1); + + scp.clearDownload(xValue); + scp.receivedTxSet(0, xValue); + + // Statement changed since the stall -> resume declines, no emission. + REQUIRE(scp.mEnvs.size() == envsAfterPrepared); + } } TEST_CASE("incoming PREPARE with structurally valid prepared value is accepted", @@ -3939,6 +4072,6 @@ TEST_CASE("incoming PREPARE with non-tx-set-invalid value is dropped", // No local emit triggered. REQUIRE(scp.mEnvs.empty()); } -#endif // CAP_0087 +#endif // CAP_0083 }