Skip to content

Commit

Permalink
ledger_request should confirm ledger is present (RIPD-1365):
Browse files Browse the repository at this point in the history
The ledger_request RPC call, under some conditions, did not
actually check that the entire ledger was present in the
database, making it unsuitable for use in cases where the
database was believed to be incorrect or incomplete.
With this change, the full ledger will be checked for
integrity unless it has already recently been checked
(according to the InboundLedgers cache).
  • Loading branch information
JoelKatz authored and nbougalis committed Apr 19, 2017
1 parent c6b6d82 commit 10a7f5b
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 20 deletions.
2 changes: 1 addition & 1 deletion src/ripple/app/ledger/InboundLedger.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class InboundLedger
// Called when another attempt is made to fetch this same ledger
void update (std::uint32_t seq);

std::shared_ptr<Ledger> const&
std::shared_ptr<Ledger const>
getLedger() const
{
return mLedger;
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/ledger/InboundLedgers.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class InboundLedgers
// VFALCO TODO Should this be called findOrAdd ?
//
virtual
std::shared_ptr<Ledger>
std::shared_ptr<Ledger const>
acquire (uint256 const& hash,
std::uint32_t seq, InboundLedger::fcReason) = 0;

Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/ledger/Ledger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ bool Ledger::walkLedger (beast::Journal j) const
return missingNodes1.empty () && missingNodes2.empty ();
}

bool Ledger::assertSane (beast::Journal ledgerJ)
bool Ledger::assertSane (beast::Journal ledgerJ) const
{
if (info_.hash.isNonZero () &&
info_.accountHash.isNonZero () &&
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/ledger/Ledger.h
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ class Ledger final

bool walkLedger (beast::Journal j) const;

bool assertSane (beast::Journal ledgerJ);
bool assertSane (beast::Journal ledgerJ) const;

void make_v2();
void invariants() const;
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/ledger/impl/InboundLedgers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class InboundLedgersImp
{
}

std::shared_ptr<Ledger>
std::shared_ptr<Ledger const>
acquire (
uint256 const& hash,
std::uint32_t seq,
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/main/Application.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1543,7 +1543,7 @@ bool ApplicationImp::loadOldLedger (
{
try
{
std::shared_ptr<Ledger> loadLedger, replayLedger;
std::shared_ptr<Ledger const> loadLedger, replayLedger;

if (isFileName)
{
Expand Down
33 changes: 19 additions & 14 deletions src/ripple/rpc/handlers/LedgerRequest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <ripple/app/ledger/LedgerMaster.h>
#include <ripple/app/main/Application.h>
#include <ripple/net/RPCErr.h>
#include <ripple/resource/Fees.h>
#include <ripple/rpc/Context.h>
#include <ripple/protocol/ErrorCodes.h>
#include <ripple/protocol/JsonFields.h>
Expand All @@ -38,6 +39,7 @@ Json::Value doLedgerRequest (RPC::Context& context)
{
auto const hasHash = context.params.isMember (jss::ledger_hash);
auto const hasIndex = context.params.isMember (jss::ledger_index);
std::uint32_t ledgerIndex = 0;

auto& ledgerMaster = context.app.getLedgerMaster();
LedgerHash ledgerHash;
Expand All @@ -48,6 +50,8 @@ Json::Value doLedgerRequest (RPC::Context& context)
"Exactly one of ledger_hash and ledger_index can be set.");
}

context.loadType = Resource::feeHighBurdenRPC;

if (hasHash)
{
auto const& jsonHash = context.params[jss::ledger_hash];
Expand All @@ -65,7 +69,7 @@ Json::Value doLedgerRequest (RPC::Context& context)
RPC::Tuning::maxValidatedLedgerAge)
return rpcError (rpcNO_CURRENT);

auto ledgerIndex = jsonIndex.asInt();
ledgerIndex = jsonIndex.asInt();
auto ledger = ledgerMaster.getValidatedLedger();

if (ledgerIndex >= ledger->info().seq)
Expand Down Expand Up @@ -118,28 +122,29 @@ Json::Value doLedgerRequest (RPC::Context& context)
ledgerHash = neededHash ? *neededHash : zero; // kludge
}

auto ledger = ledgerMaster.getLedgerByHash (ledgerHash);
// Try to get the desired ledger
// Verify all nodes even if we think we have it
auto ledger = context.app.getInboundLedgers().acquire (
ledgerHash, ledgerIndex, InboundLedger::fcGENERIC);

// In standalone mode, accept the ledger from the ledger cache
if (! ledger && context.app.config().standalone())
ledger = ledgerMaster.getLedgerByHash (ledgerHash);

if (ledger)
{
// We already have the ledger they want
// We already had the entire ledger verified/acquired
Json::Value jvResult;
jvResult[jss::ledger_index] = ledger->info().seq;
addJson (jvResult, {*ledger, 0});
return jvResult;
}
else
{
// Try to get the desired ledger
if (auto il = context.app.getInboundLedgers ().acquire (
ledgerHash, 0, InboundLedger::fcGENERIC))
return getJson (LedgerFill (*il));

if (auto il = context.app.getInboundLedgers().find (ledgerHash))
return il->getJson (0);
if (auto il = context.app.getInboundLedgers().find (ledgerHash))
return il->getJson (0);

return RPC::make_error (
rpcNOT_READY, "findCreate failed to return an inbound ledger");
}
return RPC::make_error (
rpcNOT_READY, "findCreate failed to return an inbound ledger");
}

} // ripple

0 comments on commit 10a7f5b

Please sign in to comment.