Skip to content

Commit

Permalink
[FOLD] Make prevLedgerID a private detail of Validations
Browse files Browse the repository at this point in the history
  • Loading branch information
@bachase
bachase committed Jul 21, 2017
1 parent 1ef6605 commit 7982f1d
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 99 deletions.
21 changes: 0 additions & 21 deletions src/ripple/app/consensus/RCLValidations.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,27 +99,6 @@ class RCLValidation
return val_->isTrusted();
}

/// Set the prior ledger hash this validation is following
void
setPreviousLedgerID(uint256 const& hash)
{
val_->setPreviousHash(hash);
}

/// Get the prior ledger hash this validation is following
uint256
getPreviousLedgerID() const
{
return val_->getPreviousHash();
}

/// Check whether the given hash matches this validation's prior hash
bool
isPreviousLedgerID(uint256 const& hash) const
{
return val_->isPreviousHash(hash);
}

/// Get the load fee of the validation if it exists
boost::optional<std::uint32_t>
loadFee() const
Expand Down
75 changes: 39 additions & 36 deletions src/ripple/consensus/Validations.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,21 +152,6 @@ isCurrent(
// arrived
bool trusted() const;
// The PreviousLedger functions below assume instances corresponding
// to the same validation (ID/hash/key etc.) share the same previous
// ledger ID storage.
// Set the previous validation ledger from this publishing node that
// this validation replaced
void setPreviousLedgerID(LedgerID &);
// Get the previous validation ledger from this publishing node that
// this validation replaced
LedgerID getPreviousLedgerID() const;
// Check if this validation had the given ledger ID as its prior ledger
bool isPreviousLedgerID(LedgerID const& ) const;
implementation_specific_t
unwrap() -> return the implementation-specific type being wrapped
Expand Down Expand Up @@ -207,20 +192,32 @@ class Validations
decay_result_t<decltype (&Validation::ledgerID)(Validation)>;
using NodeKey = decay_result_t<decltype (&Validation::key)(Validation)>;
using NodeID = decay_result_t<decltype (&Validation::nodeID)(Validation)>;
using ValidationMap = hash_map<NodeKey, Validation>;


using ScopedLock = std::lock_guard<MutexType>;

// Manages concurrent access to current_ and byLedger_
MutexType mutex_;

//! For the most recent validation, we also want to store the ID
//! of the ledger it replaces
struct ValidationAndPrevID
{
ValidationAndPrevID(Validation const& v) : val{v}, prevLedgerID{0}
{
}

Validation val;
LedgerID prevLedgerID;
};

//! The latest validation from each node
ValidationMap current_;
hash_map<NodeKey, ValidationAndPrevID> current_;

//! Recent validations from nodes, indexed by ledger identifier
beast::aged_unordered_map<
LedgerID,
ValidationMap,
hash_map<NodeKey, Validation>,
std::chrono::steady_clock,
beast::uhash<>>
byLedger_;
Expand Down Expand Up @@ -264,15 +261,15 @@ class Validations
// Check for staleness, if time specified
if (t &&
!isCurrent(
parms_, *t, it->second.signTime(), it->second.seenTime()))
parms_, *t, it->second.val.signTime(), it->second.val.seenTime()))
{
// contains a stale record
stalePolicy_.onStale(std::move(it->second));
stalePolicy_.onStale(std::move(it->second.val));
it = current_.erase(it);
}
else
{
auto cit = typename ValidationMap::const_iterator{it};
auto cit = typename decltype(current_)::const_iterator{it};
// contains a live record
f(cit->first, cit->second);
++it;
Expand Down Expand Up @@ -397,7 +394,8 @@ class Validations
if (!ins.second)
{
// Had a previous validation from the node, consider updating
Validation& oldVal = ins.first->second;
Validation& oldVal = ins.first->second.val;
LedgerID const previousLedgerID = ins.first->second.prevLedgerID;

std::uint32_t const oldSeq{oldVal.seq()};
std::uint32_t const newSeq{val.seq()};
Expand All @@ -414,7 +412,7 @@ class Validations
auto const mapIt = byLedger_.find(oldVal.ledgerID());
if (mapIt != byLedger_.end())
{
ValidationMap& validationMap = mapIt->second;
auto& validationMap = mapIt->second;
// If a new validation with the same ID was
// reissued we simply replace.
if(oldVal.ledgerID() == val.ledgerID())
Expand Down Expand Up @@ -449,14 +447,14 @@ class Validations
// In the case the key was revoked and a new validation
// for the same ledger ID was sent, the previous ledger
// is still the one the now revoked validation had
return oldVal.getPreviousLedgerID();
return previousLedgerID;
}();

// Allow impl to take over oldVal
maybeStaleValidation.emplace(std::move(oldVal));
// Replace old val in the map and set the previous ledger ID
ins.first->second = val;
ins.first->second.setPreviousLedgerID(prevID);
ins.first->second.val = val;
ins.first->second.prevLedgerID = prevID;
}
else
{
Expand Down Expand Up @@ -514,8 +512,10 @@ class Validations
// The number of validations does not correspond to the number of
// distinct ledgerIDs so we do not call reserve on ret.
[&](std::size_t) {},
[&](NodeKey const&, Validation const& v) {
[&](NodeKey const&, ValidationAndPrevID const& vp) {

Validation const& v = vp.val;
LedgerID const& prevLedgerID = vp.prevLedgerID;
if (!v.trusted())
return;

Expand All @@ -529,7 +529,7 @@ class Validations
if (!countPreferred && // allow up to one ledger slip in
// either direction
((valCurrentLedger &&
v.isPreviousLedgerID(currentLedger)) ||
(prevLedgerID == currentLedger)) ||
(valPriorLedger && (v.ledgerID() == priorLedger))))
{
countPreferred = true;
Expand Down Expand Up @@ -567,8 +567,8 @@ class Validations
current(
boost::none,
[&](std::size_t) {}, // nothing to reserve
[&](NodeKey const&, Validation const& v) {
if (v.trusted() && v.isPreviousLedgerID(ledgerID))
[&](NodeKey const&, ValidationAndPrevID const& v) {
if (v.val.trusted() && v.prevLedgerID == ledgerID)
++count;
});
return count;
Expand All @@ -586,9 +586,9 @@ class Validations
current(
stalePolicy_.now(),
[&](std::size_t numValidations) { ret.reserve(numValidations); },
[&](NodeKey const&, Validation const& v) {
if (v.trusted())
ret.push_back(v.unwrap());
[&](NodeKey const&, ValidationAndPrevID const& v) {
if (v.val.trusted())
ret.push_back(v.val.unwrap());
});
return ret;
}
Expand All @@ -605,7 +605,7 @@ class Validations
current(
stalePolicy_.now(),
[&](std::size_t numValidations) { ret.reserve(numValidations); },
[&](NodeKey const& k, Validation const&) { ret.insert(k); });
[&](NodeKey const& k, ValidationAndPrevID const&) { ret.insert(k); });

return ret;
}
Expand Down Expand Up @@ -702,10 +702,13 @@ class Validations
JLOG(j_.info()) << "Flushing validations";

hash_map<NodeKey, Validation> flushed;
using std::swap;
{
ScopedLock lock{mutex_};
swap(flushed, current_);
for (auto it : current_)
{
flushed.emplace(it.first, std::move(it.second.val));
}
current_.clear();
}

stalePolicy_.flush(std::move(flushed));
Expand Down
15 changes: 0 additions & 15 deletions src/ripple/protocol/STValidation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,26 +100,11 @@ class STValidation final
// Signs the validation and returns the signing hash
uint256 sign (SecretKey const& secretKey);

// The validation this replaced
uint256 const& getPreviousHash ()
{
return mPreviousHash;
}
bool isPreviousHash (uint256 const& h) const
{
return mPreviousHash == h;
}
void setPreviousHash (uint256 const& h)
{
mPreviousHash = h;
}

private:
static SOTemplate const& getFormat ();

void setNode ();

uint256 mPreviousHash;
NodeID mNodeID;
bool mTrusted = false;
NetClock::time_point mSeen = {};
Expand Down
38 changes: 11 additions & 27 deletions src/test/consensus/Validations_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,9 @@ class Validations_test : public beast::unit_test::suite
std::size_t nodeID_ = 0;
bool trusted_ = true;
boost::optional<std::uint32_t> loadFee_;
// Shared prevID to ensure value is shared amongst instances
std::shared_ptr<ID> prevID_;

public:
Validation() : prevID_(std::make_shared<ID>(0))
Validation()
{
}

Expand Down Expand Up @@ -153,24 +151,6 @@ class Validations_test : public beast::unit_test::suite
return trusted_;
}

void
setPreviousLedgerID(ID const& prevID)
{
*prevID_ = prevID;
}

bool
isPreviousLedgerID(ID const& prevID) const
{
return *prevID_ == prevID;
}

ID
getPreviousLedgerID() const
{
return *prevID_;
}

boost::optional<std::uint32_t>
loadFee() const
{
Expand All @@ -194,7 +174,6 @@ class Validations_test : public beast::unit_test::suite
key_,
nodeID_,
trusted_,
*prevID_,
loadFee_);
}
bool
Expand Down Expand Up @@ -488,19 +467,24 @@ class Validations_test : public beast::unit_test::suite
// new ID but the same sequence number
a.advanceKey();

// No validations following ID{2}
BEAST_EXPECT(harness.vals().getNodesAfter(ID{2}) == 0);

BEAST_EXPECT(
AddOutcome::sameSeq == harness.add(a, Seq{2}, ID{20}));

// Old ID should be gone ...
BEAST_EXPECT(harness.vals().numTrustedForLedger(ID{2}) == 0);
BEAST_EXPECT(harness.vals().numTrustedForLedger(ID{20}) == 1);
// ... and the previous ID set to the old ID
{
// Should the the only trusted for ID{20}
auto trustedVals =
harness.vals().getTrustedForLedger(ID{20});
BEAST_EXPECT(trustedVals.size() == 1);
BEAST_EXPECT(trustedVals[0].key() == a.currKey());
BEAST_EXPECT(trustedVals[0].getPreviousLedgerID() == ID{2});
// ... and should be the only node after ID{2}
BEAST_EXPECT(harness.vals().getNodesAfter(ID{2}) == 1);

}

// A new key, but re-issue a validation with the same ID and
Expand All @@ -509,13 +493,14 @@ class Validations_test : public beast::unit_test::suite

BEAST_EXPECT(
AddOutcome::sameSeq == harness.add(a, Seq{2}, ID{20}));
// Ensure the old prevLedgerID was retained
{
// Still the only trusted validation for ID{20}
auto trustedVals =
harness.vals().getTrustedForLedger(ID{20});
BEAST_EXPECT(trustedVals.size() == 1);
BEAST_EXPECT(trustedVals[0].key() == a.currKey());
BEAST_EXPECT(trustedVals[0].getPreviousLedgerID() == ID{2});
// and still follows ID{2} since it was a re-issue
BEAST_EXPECT(harness.vals().getNodesAfter(ID{2}) == 1);
}
}

Expand Down Expand Up @@ -943,7 +928,6 @@ class Validations_test : public beast::unit_test::suite
harness.clock().advance(1s);
auto newVal = a.validation(Seq{2}, ID{2});
BEAST_EXPECT(AddOutcome::current == harness.add(a, newVal));
newVal.setPreviousLedgerID(staleA.ledgerID());
expected[a.masterKey()] = newVal;

// Now flush
Expand Down

0 comments on commit 7982f1d

Please sign in to comment.