Skip to content

Commit

Permalink
address PR comments, fixes after rebase
Browse files Browse the repository at this point in the history
  • Loading branch information
@pwang200
pwang200 committed May 8, 2020
1 parent 54cfdfb commit e764ec6
Show file tree
Hide file tree
Showing 19 changed files with 391 additions and 365 deletions.
12 changes: 7 additions & 5 deletions src/ripple/app/consensus/RCLConsensus.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,9 @@ RCLConsensus::Adaptor::Adaptor(
, nodeID_{validatorKeys.nodeID}
, valPublic_{validatorKeys.publicKey}
, valSecret_{validatorKeys.secretKey}
, valCookie_{
rand_int<std::uint64_t>(1, std::numeric_limits<std::uint64_t>::max())}
, valCookie_{rand_int<std::uint64_t>(
1,
std::numeric_limits<std::uint64_t>::max())}
, nUnlVote_(nodeID_, j_)
{
assert(valCookie_ != 0);
Expand Down Expand Up @@ -326,9 +327,10 @@ RCLConsensus::Adaptor::onClose(
{
// previous ledger was flag ledger, add fee and amendment
// pseudo-transactions
auto validations = app_.getValidations().getTrustedForLedger(
prevLedger->info().parentHash);
filterValsWithnUnl(validations, app_.validators().getnUnlNodeIDs());
auto validations = negativeUNLFilter(
app_.getValidations().getTrustedForLedger(
prevLedger->info().parentHash),
app_.validators().getNegativeUnlNodeIDs());
if (validations.size() >= app_.validators().quorum())
{
feeVote_->doVoting(prevLedger, validations, initialSet);
Expand Down
25 changes: 12 additions & 13 deletions src/ripple/app/consensus/RCLValidations.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,23 +245,22 @@ handleNewValidation(
return shouldRelay;
}

void
filterValsWithnUnl(
std::vector<std::shared_ptr<STValidation>>& validations,
hash_set<NodeID> const& nUnl)
std::vector<std::shared_ptr<STValidation>>
negativeUNLFilter(
std::vector<std::shared_ptr<STValidation>> const& validations,
hash_set<NodeID> const& negUnl)
{
/* Remove validations that are from validators on the negative UNL. */
if (!nUnl.empty())
if (negUnl.empty())
return validations;

std::vector<std::shared_ptr<STValidation>> res;
for (auto const& v : validations)
{
validations.erase(
std::remove_if(
validations.begin(),
validations.end(),
[&nUnl](auto const& v) {
return nUnl.find(v->getNodeID()) != nUnl.end();
}),
validations.end());
if (negUnl.find(v->getNodeID()) == negUnl.end())
res.push_back(v);
}
return res;
}

} // namespace ripple
15 changes: 8 additions & 7 deletions src/ripple/app/consensus/RCLValidations.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,15 +253,16 @@ handleNewValidation(
std::string const& source);

/**
* Remove validations that are from Negative UNL validators
* Remove validations that are from validators on the negative UNL.
*
* @param validations the validations to filter
* @param nUnl the Negative UNL
* @param validations the validations to filter
* @param negUnl the Negative UNL
* @return a filtered copy of the validations
*/
void
filterValsWithnUnl(
std::vector<std::shared_ptr<STValidation>>& validations,
hash_set<NodeID> const& nUnl);
std::vector<std::shared_ptr<STValidation>>
negativeUNLFilter(
std::vector<std::shared_ptr<STValidation>> const& validations,
hash_set<NodeID> const& negUnl);

} // namespace ripple

Expand Down
144 changes: 70 additions & 74 deletions src/ripple/app/ledger/Ledger.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -606,62 +606,56 @@ Ledger::peek(Keylet const& k) const
}

hash_set<PublicKey>
Ledger::nUnl() const
Ledger::negativeUnl() const
{
hash_set<PublicKey> nUnl;
if (auto sle = read(keylet::negativeUNL()))
hash_set<PublicKey> negUnl;
if (auto sle = read(keylet::negativeUNL());
sle && sle->isFieldPresent(sfNegativeUNL))
{
if (sle->isFieldPresent(sfNegativeUNL))
auto const& nUnlData = sle->getFieldArray(sfNegativeUNL);
for (auto const& n : nUnlData)
{
auto const& nUnlData = sle->getFieldArray(sfNegativeUNL);
for (auto const& n : nUnlData)
if (n.isFieldPresent(sfPublicKey))
{
if (n.isFieldPresent(sfPublicKey))
auto d = n.getFieldVL(sfPublicKey);
auto s = makeSlice(d);
if (!publicKeyType(s))
{
auto d = n.getFieldVL(sfPublicKey);
auto s = makeSlice(d);
if (!publicKeyType(s))
{
continue;
}
nUnl.emplace(s);
continue;
}
negUnl.emplace(s);
}
}
}

return nUnl;
return negUnl;
}

boost::optional<PublicKey>
Ledger::nUnlToDisable() const
Ledger::negativeUnlToDisable() const
{
if (auto sle = read(keylet::negativeUNL()))
if (auto sle = read(keylet::negativeUNL());
sle && sle->isFieldPresent(sfNegativeUNLToDisable))
{
if (sle->isFieldPresent(sfNegativeUNLToDisable))
{
auto d = sle->getFieldVL(sfNegativeUNLToDisable);
auto s = makeSlice(d);
if (publicKeyType(s))
return PublicKey(s);
}
auto d = sle->getFieldVL(sfNegativeUNLToDisable);
auto s = makeSlice(d);
if (publicKeyType(s))
return PublicKey(s);
}

return boost::none;
}

boost::optional<PublicKey>
Ledger::nUnlToReEnable() const
Ledger::negativeUnlToReEnable() const
{
if (auto sle = read(keylet::negativeUNL()))
if (auto sle = read(keylet::negativeUNL());
sle && sle->isFieldPresent(sfNegativeUNLToReEnable))
{
if (sle->isFieldPresent(sfNegativeUNLToReEnable))
{
auto d = sle->getFieldVL(sfNegativeUNLToReEnable);
auto s = makeSlice(d);
if (publicKeyType(s))
return PublicKey(s);
}
auto d = sle->getFieldVL(sfNegativeUNLToReEnable);
auto s = makeSlice(d);
if (publicKeyType(s))
return PublicKey(s);
}

return boost::none;
Expand All @@ -670,51 +664,53 @@ Ledger::nUnlToReEnable() const
void
Ledger::updateNegativeUNL()
{
if (info_.seq % 256 == 0)
if (info_.seq % 256 != 0)
return;

auto sle = peek(keylet::negativeUNL());
if (!sle)
return;

bool const hasToDisable = sle->isFieldPresent(sfNegativeUNLToDisable);
bool const hasToReEnable = sle->isFieldPresent(sfNegativeUNLToReEnable);

if (!hasToDisable && !hasToReEnable)
return;

STArray newNUnl;
if (sle->isFieldPresent(sfNegativeUNL))
{
if (auto sle = peek(keylet::negativeUNL()))
auto const& oldNUnl = sle->getFieldArray(sfNegativeUNL);
for (auto v : oldNUnl)
{
bool hasToDisable = sle->isFieldPresent(sfNegativeUNLToDisable);
bool hasToReEnable = sle->isFieldPresent(sfNegativeUNLToReEnable);
if (hasToDisable || hasToReEnable)
{
STArray newNUnl;
if (sle->isFieldPresent(sfNegativeUNL))
{
auto const& oldNUnl = sle->getFieldArray(sfNegativeUNL);
for (auto v : oldNUnl)
{
if (hasToReEnable && v.isFieldPresent(sfPublicKey) &&
v.getFieldVL(sfPublicKey) ==
sle->getFieldVL(sfNegativeUNLToReEnable))
continue;
newNUnl.push_back(v);
}
}
if (hasToReEnable && v.isFieldPresent(sfPublicKey) &&
v.getFieldVL(sfPublicKey) ==
sle->getFieldVL(sfNegativeUNLToReEnable))
continue;
newNUnl.push_back(v);
}
}

if (hasToDisable)
{
newNUnl.emplace_back(sfNegativeUNLEntry);
newNUnl.back().setFieldVL(
sfPublicKey, sle->getFieldVL(sfNegativeUNLToDisable));
newNUnl.back().setFieldU32(sfFirstLedgerSequence, seq());
}
if (hasToDisable)
{
newNUnl.emplace_back(sfNegativeUNLEntry);
newNUnl.back().setFieldVL(
sfPublicKey, sle->getFieldVL(sfNegativeUNLToDisable));
newNUnl.back().setFieldU32(sfFirstLedgerSequence, seq());
}

if (!newNUnl.empty())
{
sle->setFieldArray(sfNegativeUNL, newNUnl);
if (hasToReEnable)
sle->delField(sfNegativeUNLToReEnable);
if (hasToDisable)
sle->delField(sfNegativeUNLToDisable);
rawReplace(sle);
}
else
{
rawErase(sle);
}
}
}
if (!newNUnl.empty())
{
sle->setFieldArray(sfNegativeUNL, newNUnl);
if (hasToReEnable)
sle->makeFieldAbsent(sfNegativeUNLToReEnable);
if (hasToDisable)
sle->makeFieldAbsent(sfNegativeUNLToDisable);
rawReplace(sle);
}
else
{
rawErase(sle);
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/ripple/app/ledger/Ledger.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,23 +335,23 @@ class Ledger final : public std::enable_shared_from_this<Ledger>,
* @return the public keys
*/
hash_set<PublicKey>
nUnl() const;
negativeUnl() const;

/**
* get the to be disabled validator's master public key if any
*
* @return the public key if any
*/
boost::optional<PublicKey>
nUnlToDisable() const;
negativeUnlToDisable() const;

/**
* get the to be re-enabled validator's master public key if any
*
* @return the public key if any
*/
boost::optional<PublicKey>
nUnlToReEnable() const;
negativeUnlToReEnable() const;

private:
class sles_iter_impl;
Expand Down
31 changes: 17 additions & 14 deletions src/ripple/app/ledger/impl/LedgerMaster.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,14 +330,15 @@ LedgerMaster::setValidLedger(std::shared_ptr<Ledger const> const& l)

if (!standalone_)
{
auto vals = app_.getValidations().getTrustedForLedger(l->info().hash);
filterValsWithnUnl(vals, app_.validators().getnUnlNodeIDs());
times.reserve(vals.size());
for (auto const& val : vals)
auto validations = negativeUNLFilter(
app_.getValidations().getTrustedForLedger(l->info().hash),
app_.validators().getNegativeUnlNodeIDs());
times.reserve(validations.size());
for (auto const& val : validations)
times.push_back(val->getSignTime());

if (!vals.empty())
consensusHash = vals.front()->getConsensusHash();
if (!validations.empty())
consensusHash = validations.front()->getConsensusHash();
}

NetClock::time_point signTime;
Expand Down Expand Up @@ -945,8 +946,9 @@ LedgerMaster::checkAccept(uint256 const& hash, std::uint32_t seq)
if (seq < mValidLedgerSeq)
return;

auto validations = app_.getValidations().getTrustedForLedger(hash);
filterValsWithnUnl(validations, app_.validators().getnUnlNodeIDs());
auto validations = negativeUNLFilter(
app_.getValidations().getTrustedForLedger(hash),
app_.validators().getNegativeUnlNodeIDs());
valCount = validations.size();
if (valCount >= app_.validators().quorum())
{
Expand Down Expand Up @@ -1011,9 +1013,9 @@ LedgerMaster::checkAccept(std::shared_ptr<Ledger const> const& ledger)
return;

auto const minVal = getNeededValidations();
auto validations =
app_.getValidations().getTrustedForLedger(ledger->info().hash);
filterValsWithnUnl(validations, app_.validators().getnUnlNodeIDs());
auto validations = negativeUNLFilter(
app_.getValidations().getTrustedForLedger(ledger->info().hash),
app_.validators().getNegativeUnlNodeIDs());
auto const tvc = validations.size();
if (tvc < minVal) // nothing we can do
{
Expand Down Expand Up @@ -1097,8 +1099,9 @@ LedgerMaster::consensusBuilt(
// This ledger cannot be the new fully-validated ledger, but
// maybe we saved up validations for some other ledger that can be

auto val = app_.getValidations().currentTrusted();
filterValsWithnUnl(val, app_.validators().getnUnlNodeIDs());
auto validations = negativeUNLFilter(
app_.getValidations().currentTrusted(),
app_.validators().getNegativeUnlNodeIDs());

// Track validation counts with sequence numbers
class valSeq
Expand All @@ -1125,7 +1128,7 @@ LedgerMaster::consensusBuilt(

// Count the number of current, trusted validations
hash_map<uint256, valSeq> count;
for (auto const& v : val)
for (auto const& v : validations)
{
valSeq& vs = count[v->getLedgerHash()];
vs.mergeValidation(v->getFieldU32(sfLedgerSequence));
Expand Down
Loading

0 comments on commit e764ec6

Please sign in to comment.