Skip to content

Commit 025b02a

Browse files
marta-lokhovamarta-lokhova
authored
Remove support for old V1 survey (#4660)
Closes #4332. Now that all stellar-core nodes on the network support the V2 survey, and there are no node stellar-core nodes that will generate V1 survey messages, it's safe to cleanup the codebase a bit and remove support for the old V1 survey entirely. This has the benefit of greatly simplifying `SurveyManager`. # Checklist - [x] Reviewed the [contributing](https://github.com/stellar/stellar-core/blob/master/CONTRIBUTING.md#submitting-changes) document - [x] Rebased on top of master (no merge commits) - [x] Ran `clang-format` v8.0.0 (via `make format` or the Visual Studio extension) - [x] Compiles - [x] Ran all tests - [ ] If change impacts performance, include supporting evidence per the [performance document](https://github.com/stellar/stellar-core/blob/master/performance-eval/performance-eval.md)
2 parents adb6f3f + 4126cdd commit 025b02a

19 files changed

+261
-654
lines changed

Diff for: ‎docs/software/commands.md

-3
Original file line numberDiff line numberDiff line change
@@ -397,9 +397,6 @@ Most commands return their results in JSON format.
397397

398398
* **stopsurvey**
399399
`stopsurvey`<br>
400-
**This command is deprecated and will be removed in a future release. It is no
401-
longer necessary to explicitly stop a survey in the new time sliced survey
402-
interface as these surveys expire automatically.**
403400
Will stop the survey if one is running. Noop if no survey is running
404401

405402
* **startsurveycollecting**

Diff for: ‎src/main/CommandHandler.cpp

+3-45
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,6 @@ CommandHandler::CommandHandler(Application& app) : mApp(app)
101101
addRoute("stopsurvey", &CommandHandler::stopSurvey);
102102
#ifndef BUILD_TESTS
103103
addRoute("getsurveyresult", &CommandHandler::getSurveyResult);
104-
addRoute("surveytopology", &CommandHandler::surveyTopology);
105104
addRoute("startsurveycollecting",
106105
&CommandHandler::startSurveyCollecting);
107106
addRoute("stopsurveycollecting", &CommandHandler::stopSurveyCollecting);
@@ -128,7 +127,6 @@ CommandHandler::CommandHandler(Application& app) : mApp(app)
128127
addRoute("testacc", &CommandHandler::testAcc);
129128
addRoute("testtx", &CommandHandler::testTx);
130129
addRoute("getsurveyresult", &CommandHandler::getSurveyResult);
131-
addRoute("surveytopology", &CommandHandler::surveyTopology);
132130
addRoute("startsurveycollecting", &CommandHandler::startSurveyCollecting);
133131
addRoute("stopsurveycollecting", &CommandHandler::stopSurveyCollecting);
134132
addRoute("surveytopologytimesliced",
@@ -1051,46 +1049,10 @@ CommandHandler::checkBooted() const
10511049
}
10521050
}
10531051

1054-
void
1055-
CommandHandler::surveyTopology(std::string const& params, std::string& retStr)
1056-
{
1057-
ZoneScoped;
1058-
1059-
CLOG_WARNING(
1060-
Overlay,
1061-
"`surveytopology` is deprecated and will be removed in a future "
1062-
"release. Please use the new time sliced survey interface.");
1063-
1064-
checkBooted();
1065-
1066-
std::map<std::string, std::string> map;
1067-
http::server::server::parseParams(params, map);
1068-
1069-
auto duration =
1070-
std::chrono::seconds(parseRequiredParam<uint32>(map, "duration"));
1071-
auto idString = parseRequiredParam<std::string>(map, "node");
1072-
NodeID id = KeyUtils::fromStrKey<NodeID>(idString);
1073-
1074-
auto& surveyManager = mApp.getOverlayManager().getSurveyManager();
1075-
1076-
bool success = surveyManager.startSurveyReporting(
1077-
SurveyMessageCommandType::SURVEY_TOPOLOGY, duration);
1078-
1079-
surveyManager.addNodeToRunningSurveyBacklog(
1080-
SurveyMessageCommandType::SURVEY_TOPOLOGY, duration, id, std::nullopt,
1081-
std::nullopt);
1082-
retStr = "Adding node.";
1083-
1084-
retStr += success ? "Survey started " : "Survey already running!";
1085-
}
1086-
10871052
void
10881053
CommandHandler::stopSurvey(std::string const&, std::string& retStr)
10891054
{
10901055
ZoneScoped;
1091-
CLOG_WARNING(Overlay,
1092-
"`stopsurvey` is deprecated and will be removed in a future "
1093-
"release. Please use the new time sliced survey interface.");
10941056
auto& surveyManager = mApp.getOverlayManager().getSurveyManager();
10951057
surveyManager.stopSurveyReporting();
10961058
retStr = "survey stopped";
@@ -1164,14 +1126,10 @@ CommandHandler::surveyTopologyTimeSliced(std::string const& params,
11641126

11651127
auto& surveyManager = mApp.getOverlayManager().getSurveyManager();
11661128

1167-
bool success = surveyManager.startSurveyReporting(
1168-
SurveyMessageCommandType::TIME_SLICED_SURVEY_TOPOLOGY,
1169-
/*surveyDuration*/ std::nullopt);
1129+
bool success = surveyManager.startSurveyReporting();
11701130

1171-
surveyManager.addNodeToRunningSurveyBacklog(
1172-
SurveyMessageCommandType::TIME_SLICED_SURVEY_TOPOLOGY,
1173-
/*surveyDuration*/ std::nullopt, id, inboundPeerIndex,
1174-
outboundPeerIndex);
1131+
surveyManager.addNodeToRunningSurveyBacklog(id, inboundPeerIndex,
1132+
outboundPeerIndex);
11751133
retStr = "Adding node.";
11761134

11771135
retStr += success ? "Survey started " : "Survey already running!";

Diff for: ‎src/main/Config.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ Config::Config() : NODE_SEED(SecretKey::random())
149149
LEDGER_PROTOCOL_MIN_VERSION_INTERNAL_ERROR_REPORT = 18;
150150

151151
OVERLAY_PROTOCOL_MIN_VERSION = 35;
152-
OVERLAY_PROTOCOL_VERSION = 36;
152+
OVERLAY_PROTOCOL_VERSION = 37;
153153

154154
VERSION_STR = STELLAR_CORE_VERSION;
155155

Diff for: ‎src/main/main.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,11 @@ main(int argc, char* const* argv)
350350
checkStellarCoreMajorVersionProtocolIdentity();
351351
rust_bridge::check_sensible_soroban_config_for_protocol(
352352
Config::CURRENT_LEDGER_PROTOCOL_VERSION);
353+
354+
// Disable XDR hash checking in vnext builds
355+
#ifndef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION
353356
checkXDRFileIdentity();
357+
#endif
354358

355359
int res = handleCommandLine(argc, argv);
356360
return res;

Diff for: ‎src/overlay/Floodgate.cpp

+1-8
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,7 @@ Floodgate::addRecord(StellarMessage const& msg, Peer::pointer peer,
8484
// send message to anyone you haven't gotten it from
8585
bool
8686
Floodgate::broadcast(std::shared_ptr<StellarMessage const> msg,
87-
std::optional<Hash> const& hash,
88-
uint32_t minOverlayVersion)
87+
std::optional<Hash> const& hash)
8988
{
9089
ZoneScoped;
9190
if (mShuttingDown)
@@ -124,12 +123,6 @@ Floodgate::broadcast(std::shared_ptr<StellarMessage const> msg,
124123
// Assert must hold since only main thread is allowed to modify
125124
// authenticated peers and peer state during drop
126125
peer.second->assertAuthenticated();
127-
if (peer.second->getRemoteOverlayVersion() < minOverlayVersion)
128-
{
129-
// Skip peers running overlay versions that are older than
130-
// `minOverlayVersion`.
131-
continue;
132-
}
133126

134127
bool pullMode = msg->type() == TRANSACTION;
135128

Diff for: ‎src/overlay/Floodgate.h

+1-4
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,8 @@ class Floodgate
6161

6262
// returns true if msg was sent to at least one peer
6363
// The hash required for transactions
64-
// `minOverlayVersion` is the minimum overlay version a peer must have in
65-
// order to be sent the message.
6664
bool broadcast(std::shared_ptr<StellarMessage const> msg,
67-
std::optional<Hash> const& hash = std::nullopt,
68-
uint32_t minOverlayVersion = 0);
65+
std::optional<Hash> const& hash = std::nullopt);
6966

7067
// returns the list of peers that sent us the item with hash `msgID`
7168
// NB: `msgID` is the hash of a `StellarMessage`

Diff for: ‎src/overlay/FlowControl.cpp

+17-14
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,7 @@ FlowControl::endMessageProcessing(StellarMessage const& msg)
245245
mFloodDataProcessed += mFlowControlCapacity.releaseLocalCapacity(msg);
246246
mFloodDataProcessedBytes +=
247247
mFlowControlBytesCapacity.releaseLocalCapacity(msg);
248+
mTotalMsgsProcessed++;
248249

249250
releaseAssert(mFloodDataProcessed <=
250251
mAppConnector.getConfig().FLOW_CONTROL_SEND_MORE_BATCH_SIZE);
@@ -256,12 +257,18 @@ FlowControl::endMessageProcessing(StellarMessage const& msg)
256257
shouldSendMore =
257258
shouldSendMore || mFloodDataProcessedBytes >= byteBatchSize;
258259

259-
SendMoreCapacity res{0, 0};
260+
SendMoreCapacity res{0, 0, 0};
261+
if (mTotalMsgsProcessed == mAppConnector.getConfig().PEER_READING_CAPACITY)
262+
{
263+
res.numTotalMessages = mTotalMsgsProcessed;
264+
mTotalMsgsProcessed = 0;
265+
}
266+
260267
if (shouldSendMore)
261268
{
262269
// First save result to return
263-
res.first = mFloodDataProcessed;
264-
res.second = mFloodDataProcessedBytes;
270+
res.numFloodMessages = mFloodDataProcessed;
271+
res.numFloodBytes = mFloodDataProcessedBytes;
265272

266273
// Reset counters
267274
mFloodDataProcessed = 0;
@@ -554,20 +561,16 @@ FlowControl::maybeThrottleRead()
554561
return false;
555562
}
556563

557-
bool
564+
void
558565
FlowControl::stopThrottling()
559566
{
560567
std::lock_guard<std::mutex> guard(mFlowControlMutex);
561-
if (mLastThrottle)
562-
{
563-
CLOG_DEBUG(Overlay, "Stop throttling reading from peer {}",
564-
mAppConnector.getConfig().toShortString(mNodeID));
565-
mOverlayMetrics.mConnectionReadThrottle.Update(mAppConnector.now() -
566-
*mLastThrottle);
567-
mLastThrottle.reset();
568-
return true;
569-
}
570-
return false;
568+
releaseAssert(mLastThrottle);
569+
CLOG_DEBUG(Overlay, "Stop throttling reading from peer {}",
570+
mAppConnector.getConfig().toShortString(mNodeID));
571+
mOverlayMetrics.mConnectionReadThrottle.Update(mAppConnector.now() -
572+
*mLastThrottle);
573+
mLastThrottle.reset();
571574
}
572575

573576
bool

Diff for: ‎src/overlay/FlowControl.h

+10-3
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,12 @@ namespace stellar
1616
class AppConnector;
1717
struct OverlayMetrics;
1818

19-
// num messages, bytes
20-
using SendMoreCapacity = std::pair<uint64_t, uint64_t>;
19+
struct SendMoreCapacity
20+
{
21+
uint64_t numFloodMessages{0};
22+
uint64_t numFloodBytes{0};
23+
uint32_t numTotalMessages{0};
24+
};
2125

2226
// The FlowControl class allows core to throttle flood traffic among its
2327
// connections. If a connections wants to use flow control, it should maintain
@@ -82,6 +86,9 @@ class FlowControl
8286
// How many bytes we received and processed since sending
8387
// SEND_MORE to this peer
8488
uint64_t mFloodDataProcessedBytes{0};
89+
// How many total messages we received and processed so far (used to track
90+
// throttling)
91+
uint64_t mTotalMsgsProcessed{0};
8592
std::optional<VirtualClock::time_point> mNoOutboundCapacity;
8693
FlowControlMetrics mMetrics;
8794

@@ -178,7 +185,7 @@ class FlowControl
178185
bool maybeThrottleRead();
179186
// After releasing capacity, check if throttling was applied, and if so,
180187
// reset it. Returns true if peer was throttled, and false otherwise
181-
bool stopThrottling();
188+
void stopThrottling();
182189
bool isThrottled() const;
183190
};
184191

Diff for: ‎src/overlay/OverlayManager.h

+3-5
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,9 @@ class OverlayManager
7373
// returns true if message was sent to at least one peer
7474
// When passing a transaction message,
7575
// the hash of TransactionEnvelope must be passed also for pull mode.
76-
// `minOverlayVersion` is the minimum overlay version a peer must have in
77-
// order to be sent the message.
78-
virtual bool broadcastMessage(std::shared_ptr<StellarMessage const> msg,
79-
std::optional<Hash> const hash = std::nullopt,
80-
uint32_t minOverlayVersion = 0) = 0;
76+
virtual bool
77+
broadcastMessage(std::shared_ptr<StellarMessage const> msg,
78+
std::optional<Hash> const hash = std::nullopt) = 0;
8179

8280
// Make a note in the FloodGate that a given peer has provided us with a
8381
// given broadcast message, so that it is inhibited from being resent to

Diff for: ‎src/overlay/OverlayManagerImpl.cpp

+3-5
Original file line numberDiff line numberDiff line change
@@ -1249,11 +1249,10 @@ OverlayManagerImpl::recvTxDemand(FloodDemand const& dmd, Peer::pointer peer)
12491249

12501250
bool
12511251
OverlayManagerImpl::broadcastMessage(std::shared_ptr<StellarMessage const> msg,
1252-
std::optional<Hash> const hash,
1253-
uint32_t minOverlayVersion)
1252+
std::optional<Hash> const hash)
12541253
{
12551254
ZoneScoped;
1256-
auto res = mFloodGate.broadcast(msg, hash, minOverlayVersion);
1255+
auto res = mFloodGate.broadcast(msg, hash);
12571256
if (res)
12581257
{
12591258
mOverlayMetrics.mMessagesBroadcast.Mark();
@@ -1339,8 +1338,7 @@ OverlayManagerImpl::recordMessageMetric(StellarMessage const& stellarMsg,
13391338
};
13401339

13411340
bool flood = false;
1342-
if (isFloodMessage(stellarMsg) || stellarMsg.type() == SURVEY_REQUEST ||
1343-
stellarMsg.type() == SURVEY_RESPONSE ||
1341+
if (isFloodMessage(stellarMsg) ||
13441342
stellarMsg.type() == TIME_SLICED_SURVEY_START_COLLECTING ||
13451343
stellarMsg.type() == TIME_SLICED_SURVEY_STOP_COLLECTING ||
13461344
stellarMsg.type() == TIME_SLICED_SURVEY_REQUEST ||

Diff for: ‎src/overlay/OverlayManagerImpl.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,9 @@ class OverlayManagerImpl : public OverlayManager
118118
Hash const& index) override;
119119
void forgetFloodedMsg(Hash const& msgID) override;
120120
void recvTxDemand(FloodDemand const& dmd, Peer::pointer peer) override;
121-
bool broadcastMessage(std::shared_ptr<StellarMessage const> msg,
122-
std::optional<Hash> const hash = std::nullopt,
123-
uint32_t minOverlayVersion = 0) override;
121+
bool
122+
broadcastMessage(std::shared_ptr<StellarMessage const> msg,
123+
std::optional<Hash> const hash = std::nullopt) override;
124124
void connectTo(PeerBareAddress const& address) override;
125125

126126
void maybeAddInboundConnection(Peer::pointer peer) override;

Diff for: ‎src/overlay/Peer.cpp

+22-14
Original file line numberDiff line numberDiff line change
@@ -167,18 +167,32 @@ Peer::endMessageProcessing(StellarMessage const& msg)
167167
// We may release reading capacity, which gets taken by the background
168168
// thread immediately, so we can't assert `canRead` here
169169
auto res = mFlowControl->endMessageProcessing(msg);
170-
if (res.first > 0 || res.second > 0)
170+
if (res.numFloodMessages > 0 || res.numFloodBytes > 0)
171171
{
172-
sendSendMore(static_cast<uint32>(res.first),
173-
static_cast<uint32>(res.second));
172+
sendSendMore(static_cast<uint32>(res.numFloodMessages),
173+
static_cast<uint32>(res.numFloodBytes));
174174
}
175175

176-
// Now that we've released some capacity, maybe schedule more reads
177-
if (mFlowControl->stopThrottling())
176+
// If throttled, schedule read as soon as a full batch is processed
177+
if (mFlowControl->isThrottled() && res.numTotalMessages > 0)
178178
{
179-
maybeExecuteInBackground(
180-
"Peer::stopThrottling scheduleRead",
181-
[](std::shared_ptr<Peer> self) { self->scheduleRead(); });
179+
mFlowControl->stopThrottling();
180+
#ifdef BUILD_TESTS
181+
// For LoopbackPeer tests, do so asynchronously to ensure
182+
// LoopbackPeer::processInQueue function completes.
183+
if (!useBackgroundThread() && threadIsMain())
184+
{
185+
mAppConnector.postOnMainThread(
186+
[self = shared_from_this()]() { self->scheduleRead(); },
187+
"Peer::stopThrottling scheduleRead");
188+
}
189+
else
190+
#endif
191+
{
192+
maybeExecuteInBackground(
193+
"Peer::stopThrottling scheduleRead",
194+
[](std::shared_ptr<Peer> self) { self->scheduleRead(); });
195+
}
182196
}
183197
}
184198

@@ -660,8 +674,6 @@ Peer::msgSummary(StellarMessage const& msg)
660674
return fmt::format(FMT_STRING("GET_SCP_STATE {:d}"),
661675
msg.getSCPLedgerSeq());
662676

663-
case SURVEY_REQUEST:
664-
case SURVEY_RESPONSE:
665677
case TIME_SLICED_SURVEY_REQUEST:
666678
case TIME_SLICED_SURVEY_RESPONSE:
667679
case TIME_SLICED_SURVEY_START_COLLECTING:
@@ -726,11 +738,9 @@ Peer::sendMessage(std::shared_ptr<StellarMessage const> msg, bool log)
726738
case GET_SCP_STATE:
727739
mOverlayMetrics.mSendGetSCPStateMeter.Mark();
728740
break;
729-
case SURVEY_REQUEST:
730741
case TIME_SLICED_SURVEY_REQUEST:
731742
mOverlayMetrics.mSendSurveyRequestMeter.Mark();
732743
break;
733-
case SURVEY_RESPONSE:
734744
case TIME_SLICED_SURVEY_RESPONSE:
735745
mOverlayMetrics.mSendSurveyResponseMeter.Mark();
736746
break;
@@ -1128,15 +1138,13 @@ Peer::recvRawMessage(std::shared_ptr<CapacityTrackedMessage> msgTracker)
11281138
}
11291139
break;
11301140

1131-
case SURVEY_REQUEST:
11321141
case TIME_SLICED_SURVEY_REQUEST:
11331142
{
11341143
auto t = mOverlayMetrics.mRecvSurveyRequestTimer.TimeScope();
11351144
recvSurveyRequestMessage(stellarMsg);
11361145
}
11371146
break;
11381147

1139-
case SURVEY_RESPONSE:
11401148
case TIME_SLICED_SURVEY_RESPONSE:
11411149
{
11421150
auto t = mOverlayMetrics.mRecvSurveyResponseTimer.TimeScope();

0 commit comments

Comments
 (0)