Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tune relaying of untrusted proposals & validations: #3391

Closed
wants to merge 10 commits into from
17 changes: 17 additions & 0 deletions cfg/rippled-example.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,23 @@
#
#
#
#
# [relay_proposals]
#
# Controls the relaying behavior for proposals received by this server that
# are issued by validators that are not on the server's UNL.
#
# Legal values are: "trusted" and "all". The default is "trusted".
#
#
# [relay_validations]
#
# Controls the relaying behavior for validations received by this server that
# are issued by validators that are not on the server's UNL.
#
# Legal values are: "trusted" and "all". The default is "all".
#
#
# [node_size]
#
# Tunes the servers based on the expected load and available memory. Legal
Expand Down
11 changes: 8 additions & 3 deletions src/ripple/app/consensus/RCLConsensus.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ RCLConsensus::Adaptor::propose(RCLCxPeerPos::Proposal const& proposal)

app_.getHashRouter().addSuppression(suppression);

app_.overlay().send(prop);
app_.overlay().broadcast(prop);
}

void
Expand Down Expand Up @@ -828,12 +828,17 @@ RCLConsensus::Adaptor::validate(
// suppress it if we receive it
app_.getHashRouter().addSuppression(
sha512Half(makeSlice(v->getSerialized())));

handleNewValidation(app_, v, "local");

// Broadcast to all our peers:
Blob validation = v->getSerialized();
protocol::TMValidation val;
val.set_validation(&validation[0], validation.size());
// Send signed validation to all of our directly connected peers
app_.overlay().send(val);
app_.overlay().broadcast(val);

// Publish to all our subscribers:
app_.getOPs().pubValidation(v);
}

void
Expand Down
40 changes: 7 additions & 33 deletions src/ripple/app/consensus/RCLValidations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ RCLValidationsAdaptor::acquire(LedgerHash const& hash)
return RCLValidatedLedger(std::move(ledger), j_);
}

bool
void
handleNewValidation(
Application& app,
std::shared_ptr<STValidation> const& val,
Expand All @@ -166,7 +166,6 @@ handleNewValidation(
if (!masterKey)
masterKey = app.validators().getListedKey(signingKey);

bool shouldRelay = false;
RCLValidations& validations = app.getValidations();
beast::Journal const j = validations.adaptor().journal();

Expand All @@ -182,67 +181,42 @@ handleNewValidation(
<< " [" << val->getSerializer().slice() << "]";
};

if (!val->isFieldPresent(sfLedgerSequence))
{
if (j.error())
dmp(j.error(), "missing ledger sequence field");
return false;
}

// masterKey is seated only if validator is trusted or listed
if (masterKey)
{
ValStatus const outcome = validations.add(calcNodeID(*masterKey), val);
auto const seq = val->getFieldU32(sfLedgerSequence);

if (j.debug())
dmp(j.debug(), to_string(outcome));

// One might think that we would not wish to relay validations that
// fail these checks. Somewhat counterintuitively, we actually want
// to do it for validations that we receive but deem suspicious, so
// that our peers will also observe them and realize they're bad.
if (outcome == ValStatus::conflicting && j.warn())
{
auto const seq = val->getFieldU32(sfLedgerSequence);
dmp(j.warn(),
"conflicting validations issued for " + to_string(seq) +
" (likely from a Byzantine validator)");
}

if (outcome == ValStatus::multiple && j.warn())
{
auto const seq = val->getFieldU32(sfLedgerSequence);
dmp(j.warn(),
"multiple validations issued for " + to_string(seq) +
" (multiple validators operating with the same key?)");
}

gregtatcam marked this conversation as resolved.
Show resolved Hide resolved
if (outcome == ValStatus::badSeq && j.warn())
{
auto const seq = val->getFieldU32(sfLedgerSequence);
dmp(j.debug(),
"already validated sequence at or past " + std::to_string(seq));
}

if (val->isTrusted() && outcome == ValStatus::current)
{
app.getLedgerMaster().checkAccept(
hash, val->getFieldU32(sfLedgerSequence));
shouldRelay = true;
}
app.getLedgerMaster().checkAccept(hash, seq);
}
else
{
JLOG(j.debug()) << "Val for " << hash << " from "
<< toBase58(TokenType::NodePublic, signingKey)
<< " not added UNlisted";
}

// This currently never forwards untrusted validations, though we may
// reconsider in the future. From @JoelKatz:
// The idea was that we would have a certain number of validation slots with
// priority going to validators we trusted. Remaining slots might be
// allocated to validators that were listed by publishers we trusted but
// that we didn't choose to trust. The shorter term plan was just to forward
// untrusted validations if peers wanted them or if we had the
// ability/bandwidth to. None of that was implemented.
return shouldRelay;
}

} // namespace ripple
4 changes: 1 addition & 3 deletions src/ripple/app/consensus/RCLValidations.h
Original file line number Diff line number Diff line change
Expand Up @@ -243,10 +243,8 @@ using RCLValidations = Validations<RCLValidationsAdaptor>;
@param app Application object containing validations and ledgerMaster
@param val The validation to add
@param source Name associated with validation used in logging

@return Whether the validation should be relayed
*/
bool
void
handleNewValidation(
Application& app,
std::shared_ptr<STValidation> const& val,
Expand Down
27 changes: 12 additions & 15 deletions src/ripple/app/misc/NetworkOPs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -356,10 +356,8 @@ class NetworkOPsImp final : public NetworkOPs
Json::Value& jvResult) override;

// Ledger proposal/close functions.
void
processTrustedProposal(
RCLCxPeerPos proposal,
std::shared_ptr<protocol::TMProposeSet> set) override;
bool
processTrustedProposal(RCLCxPeerPos proposal) override;

bool
recvValidation(
Expand Down Expand Up @@ -1780,17 +1778,10 @@ NetworkOPsImp::getConsensusLCL()
return mConsensus.prevLedgerID();
}

void
NetworkOPsImp::processTrustedProposal(
RCLCxPeerPos peerPos,
std::shared_ptr<protocol::TMProposeSet> set)
bool
NetworkOPsImp::processTrustedProposal(RCLCxPeerPos peerPos)
{
if (mConsensus.peerProposal(app_.timeKeeper().closeTime(), peerPos))
{
app_.overlay().relay(*set, peerPos.suppressionID());
}
else
JLOG(m_journal.info()) << "Not relaying trusted proposal";
return mConsensus.peerProposal(app_.timeKeeper().closeTime(), peerPos);
}

void
Expand Down Expand Up @@ -2481,8 +2472,14 @@ NetworkOPsImp::recvValidation(
{
JLOG(m_journal.debug())
<< "recvValidation " << val->getLedgerHash() << " from " << source;

handleNewValidation(app_, val, source);

pubValidation(val);
return handleNewValidation(app_, val, source);

// We will always relay trusted validations; if configured, we will
// also relay all untrusted validations.
return app_.config().RELAY_UNTRUSTED_VALIDATIONS || val->isTrusted();
}

Json::Value
Expand Down
6 changes: 2 additions & 4 deletions src/ripple/app/misc/NetworkOPs.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,8 @@ class NetworkOPs : public InfoSub::Source
//--------------------------------------------------------------------------

// ledger proposal/close functions
virtual void
processTrustedProposal(
RCLCxPeerPos peerPos,
std::shared_ptr<protocol::TMProposeSet> set) = 0;
virtual bool
processTrustedProposal(RCLCxPeerPos peerPos) = 0;

virtual bool
recvValidation(
Expand Down
2 changes: 2 additions & 0 deletions src/ripple/core/Config.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ class Config : public BasicConfig
std::size_t NETWORK_QUORUM = 1;

// Peer networking parameters
bool RELAY_UNTRUSTED_VALIDATIONS = true;
bool RELAY_UNTRUSTED_PROPOSALS = false;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bool RELAY_UNTRUSTED_PROPOSALS = false;
bool RELAY_UNTRUSTED_PROPOSALS = true;


// True to ask peers not to relay current IP.
bool PEER_PRIVATE = false;
Expand Down
2 changes: 2 additions & 0 deletions src/ripple/core/ConfigSections.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ struct ConfigSection
#define SECTION_PATH_SEARCH_MAX "path_search_max"
#define SECTION_PEER_PRIVATE "peer_private"
#define SECTION_PEERS_MAX "peers_max"
#define SECTION_RELAY_PROPOSALS "relay_proposals"
#define SECTION_RELAY_VALIDATIONS "relay_validations"
#define SECTION_RPC_STARTUP "rpc_startup"
#define SECTION_SIGNING_SUPPORT "signing_support"
#define SECTION_SNTP "sntp_servers"
Expand Down
24 changes: 24 additions & 0 deletions src/ripple/core/impl/Config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,30 @@ Config::loadFromString(std::string const& fileContents)
if (getSingleSection(secConfig, SECTION_SSL_VERIFY, strTemp, j_))
SSL_VERIFY = beast::lexicalCastThrow<bool>(strTemp);

if (getSingleSection(secConfig, SECTION_RELAY_VALIDATIONS, strTemp, j_))
{
if (boost::iequals(strTemp, "all"))
RELAY_UNTRUSTED_VALIDATIONS = true;
else if (boost::iequals(strTemp, "trusted"))
RELAY_UNTRUSTED_VALIDATIONS = false;
else
Throw<std::runtime_error>(
"Invalid value specified in [" SECTION_RELAY_VALIDATIONS
"] section");
}

if (getSingleSection(secConfig, SECTION_RELAY_PROPOSALS, strTemp, j_))
{
if (boost::iequals(strTemp, "all"))
RELAY_UNTRUSTED_PROPOSALS = true;
else if (boost::iequals(strTemp, "trusted"))
RELAY_UNTRUSTED_PROPOSALS = false;
else
Throw<std::runtime_error>(
"Invalid value specified in [" SECTION_RELAY_PROPOSALS
"] section");
}

if (exists(SECTION_VALIDATION_SEED) && exists(SECTION_VALIDATOR_TOKEN))
Throw<std::runtime_error>("Cannot have both [" SECTION_VALIDATION_SEED
"] "
Expand Down
4 changes: 2 additions & 2 deletions src/ripple/overlay/Overlay.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,11 @@ class Overlay : public Stoppable, public beast::PropertyStream::Source

/** Broadcast a proposal. */
virtual void
send(protocol::TMProposeSet& m) = 0;
broadcast(protocol::TMProposeSet& m) = 0;

/** Broadcast a validation. */
virtual void
send(protocol::TMValidation& m) = 0;
broadcast(protocol::TMValidation& m) = 0;

/** Relay a proposal. */
virtual void
Expand Down
24 changes: 8 additions & 16 deletions src/ripple/overlay/impl/OverlayImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1134,26 +1134,11 @@ OverlayImpl::findPeerByPublicKey(PublicKey const& pubKey)
}

void
OverlayImpl::send(protocol::TMProposeSet& m)
OverlayImpl::broadcast(protocol::TMProposeSet& m)
{
auto const sm = std::make_shared<Message>(m, protocol::mtPROPOSE_LEDGER);
for_each([&](std::shared_ptr<PeerImp>&& p) { p->send(sm); });
}
void
OverlayImpl::send(protocol::TMValidation& m)
{
auto const sm = std::make_shared<Message>(m, protocol::mtVALIDATION);
for_each([&](std::shared_ptr<PeerImp>&& p) { p->send(sm); });

SerialIter sit(m.validation().data(), m.validation().size());
auto val = std::make_shared<STValidation>(
std::ref(sit),
[this](PublicKey const& pk) {
return calcNodeID(app_.validatorManifests().getMasterKey(pk));
},
false);
app_.getOPs().pubValidation(val);
}

void
OverlayImpl::relay(protocol::TMProposeSet& m, uint256 const& uid)
Expand All @@ -1169,6 +1154,13 @@ OverlayImpl::relay(protocol::TMProposeSet& m, uint256 const& uid)
}
}

void
OverlayImpl::broadcast(protocol::TMValidation& m)
{
auto const sm = std::make_shared<Message>(m, protocol::mtVALIDATION);
for_each([sm](std::shared_ptr<PeerImp>&& p) { p->send(sm); });
}

void
OverlayImpl::relay(protocol::TMValidation& m, uint256 const& uid)
{
Expand Down
4 changes: 2 additions & 2 deletions src/ripple/overlay/impl/OverlayImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,10 @@ class OverlayImpl : public Overlay
findPeerByPublicKey(PublicKey const& pubKey) override;

void
send(protocol::TMProposeSet& m) override;
broadcast(protocol::TMProposeSet& m) override;

void
send(protocol::TMValidation& m) override;
broadcast(protocol::TMValidation& m) override;

void
relay(protocol::TMProposeSet& m, uint256 const& uid) override;
Expand Down
27 changes: 9 additions & 18 deletions src/ripple/overlay/impl/PeerImp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2173,7 +2173,8 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMValidation> const& m)
}
catch (std::exception const& e)
{
JLOG(p_journal_.warn()) << "Validation: Exception, " << e.what();
JLOG(p_journal_.warn())
<< "Exception processing validation: " << e.what();
fee_ = Resource::feeInvalidRequest;
}
}
Expand Down Expand Up @@ -2460,25 +2461,15 @@ PeerImp::checkPropose(
return;
}

bool relay;

if (isTrusted)
{
app_.getOPs().processTrustedProposal(peerPos, packet);
}
relay = app_.getOPs().processTrustedProposal(peerPos);
pwang200 marked this conversation as resolved.
Show resolved Hide resolved
else
{
if (cluster() ||
(app_.getOPs().getConsensusLCL() ==
peerPos.proposal().prevLedger()))
{
// relay untrusted proposal
JLOG(p_journal_.trace()) << "relaying UNTRUSTED proposal";
overlay_.relay(*packet, peerPos.suppressionID());
}
else
{
JLOG(p_journal_.debug()) << "Not relaying UNTRUSTED proposal";
}
}
relay = app_.config().RELAY_UNTRUSTED_PROPOSALS || cluster();
gregtatcam marked this conversation as resolved.
Show resolved Hide resolved

if (relay)
app_.overlay().relay(*packet, peerPos.suppressionID());
}

void
Expand Down
5 changes: 5 additions & 0 deletions src/ripple/protocol/STValidation.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,11 @@ class STValidation final : public STObject, public CountedObject<STValidation>
// Perform additional initialization
f(*this);

// This is a logic error because we should never assemble a
// validation without a ledger sequence number.
if (!isFieldPresent(sfLedgerSequence))
LogicError("Missing ledger sequence in validation");

// Finally, sign the validation and mark it as trusted:
setFlag(vfFullyCanonicalSig);
setFieldVL(sfSignature, signDigest(pk, sk, getSigningHash()));
Expand Down
Loading