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.

Sign up for GitHub

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

Unconditionalize 2017 amendments #3292

Merged
merged 13 commits into from
Apr 15, 2020
+447 −1,168
Merged

Unconditionalize 2017 amendments #3292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
1661c84
Remove unused featureCompareFlowV1V2
scottschurr Feb 28, 2020
6bb9dd2
Remove conditionals for featureCryptoConditions enabled 03Jan2017
scottschurr Feb 27, 2020
78d771a
Remove conditionals for featureTickSize enabled 21Feb2017
scottschurr Feb 27, 2020
3e87086
Remove conditionals for featurePayChan enabled 31Mar2017
scottschurr Feb 27, 2020
e93a44f
Remove conditionals for fix1368 enabled 31Mar2017
scottschurr Feb 27, 2020
c828279
Remove conditionals for featureEscrow enabled 31Mar2017
scottschurr Feb 27, 2020
a6246b0
Remove conditionals for fix1373 enabled 07Jul2017
scottschurr Feb 28, 2020
46a76fb
Remove conditionals for featureEnforceInvariants enabled 07Jul2017
scottschurr Feb 28, 2020
323dbc7
Remove conditionals for featureSortedDirectories enabled 14Nov2017
scottschurr Feb 28, 2020
8cf7c95
Remove conditionals for fix1528 enabled 14Nov2017
scottschurr Feb 28, 2020
3d3b6d8
Remove conditionals for fix1523 enabled 14Nov2017
scottschurr Feb 29, 2020
9f8d648
Remove conditionals for fix1512 enabled 14Nov2017
scottschurr Feb 29, 2020
0d83223
Remove conditionals for fix1201 enabled 14Nov2017
scottschurr Mar 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions src/ripple/app/consensus/RCLConsensus.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,6 @@ RCLConsensus::Adaptor::acquireLedger(LedgerHash const& hash)
// Notify inbound transactions of the new ledger sequence number
inboundTransactions_.newRound(built->info().seq);

// Use the ledger timing rules of the acquired ledger
parms_.useRoundedCloseTime = built->rules().enabled(fix1528);

return RCLCxLedger(built);
}

Expand Down Expand Up @@ -913,9 +910,6 @@ RCLConsensus::Adaptor::preStartRound(RCLCxLedger const & prevLgr)
// Notify inbound ledgers that we are starting a new round
inboundTransactions_.newRound(prevLgr.seq());

// Use parent ledger's rules to determine whether to use rounded close time
parms_.useRoundedCloseTime = prevLgr.ledger_->rules().enabled(fix1528);

// propose only if we're in sync with the network (and validating)
return validating_ && synced;
}
Expand Down
3 changes: 1 addition & 2 deletions src/ripple/app/paths/impl/BookStep.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -999,8 +999,7 @@ BookStep<TIn, TOut, TDerived>::check(StrandContext const& ctx) const
return temBAD_PATH_LOOP;
}

if (ctx.view.rules().enabled(fix1373) &&
ctx.seenDirectIssues[1].count(book_.out))
if (ctx.seenDirectIssues[1].count(book_.out))
{
JLOG(j_.debug()) << "BookStep: loop detected: " << *this;
return temBAD_PATH_LOOP;
Expand Down
275 changes: 2 additions & 273 deletions src/ripple/app/paths/impl/PaySteps.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,7 @@ toStep (
JLOG (j.error())
<< "Found offer/account payment step. Aborting payment strand.";
assert (0);
if (ctx.view.rules().enabled(fix1373))
return {temBAD_PATH, std::unique_ptr<Step>{}};
Throw<FlowException> (tefEXCEPTION, "Found offer/account payment step.");
return {temBAD_PATH, std::unique_ptr<Step>{}};
}

assert ((e2->getNodeType () & STPathElement::typeCurrency) ||
Expand Down Expand Up @@ -133,255 +131,7 @@ toStep (
}

std::pair<TER, Strand>
toStrandV1 (
ReadView const& view,
AccountID const& src,
AccountID const& dst,
Issue const& deliver,
boost::optional<Quality> const& limitQuality,
boost::optional<Issue> const& sendMaxIssue,
STPath const& path,
bool ownerPaysTransferFee,
bool offerCrossing,
beast::Journal j)
{
if (isXRP (src))
{
JLOG (j.debug()) << "toStrand with xrpAccount as src";
return {temBAD_PATH, Strand{}};
}
if (isXRP (dst))
{
JLOG (j.debug()) << "toStrand with xrpAccount as dst";
return {temBAD_PATH, Strand{}};
}
if (!isConsistent (deliver))
{
JLOG (j.debug()) << "toStrand inconsistent deliver issue";
return {temBAD_PATH, Strand{}};
}
if (sendMaxIssue && !isConsistent (*sendMaxIssue))
{
JLOG (j.debug()) << "toStrand inconsistent sendMax issue";
return {temBAD_PATH, Strand{}};
}

Issue curIssue = [&]
{
auto& currency =
sendMaxIssue ? sendMaxIssue->currency : deliver.currency;
if (isXRP (currency))
return xrpIssue ();
return Issue{currency, src};
}();

STPathElement const firstNode (
STPathElement::typeAll, src, curIssue.currency, curIssue.account);

boost::optional<STPathElement> sendMaxPE;
if (sendMaxIssue && sendMaxIssue->account != src &&
(path.empty () || !path[0].isAccount () ||
path[0].getAccountID () != sendMaxIssue->account))
sendMaxPE.emplace (sendMaxIssue->account, boost::none, boost::none);

STPathElement const lastNode (dst, boost::none, boost::none);

auto hasCurrency = [](STPathElement const* pe)
{
return pe->getNodeType () & STPathElement::typeCurrency;
};

boost::optional<STPathElement> deliverOfferNode;
boost::optional<STPathElement> deliverAccountNode;

std::vector<STPathElement const*> pes;
// reserve enough for the path, the implied source, destination,
// sendmax and deliver.
pes.reserve (4 + path.size ());
pes.push_back (&firstNode);
if (sendMaxPE)
pes.push_back (&*sendMaxPE);
for (auto& i : path)
pes.push_back (&i);

// Note that for offer crossing (only) we do use an offer book even if
// all that is changing is the Issue.account.
STPathElement const* const lastCurrency =
*std::find_if (pes.rbegin(), pes.rend(), hasCurrency);
if ((lastCurrency->getCurrency() != deliver.currency) ||
(offerCrossing && lastCurrency->getIssuerID() != deliver.account))
{
deliverOfferNode.emplace (boost::none, deliver.currency, deliver.account);
pes.push_back (&*deliverOfferNode);
}
if (!((pes.back ()->isAccount() && pes.back ()->getAccountID () == deliver.account) ||
(lastNode.isAccount() && lastNode.getAccountID () == deliver.account)))
{
deliverAccountNode.emplace (deliver.account, boost::none, boost::none);
pes.push_back (&*deliverAccountNode);
}
if (*pes.back() != lastNode)
pes.push_back (&lastNode);

auto const strandSrc = firstNode.getAccountID ();
auto const strandDst = lastNode.getAccountID ();
bool const isDefaultPath = path.empty();

Strand result;
result.reserve (2 * pes.size ());

/* A strand may not include the same account node more than once
in the same currency. In a direct step, an account will show up
at most twice: once as a src and once as a dst (hence the two element array).
The strandSrc and strandDst will only show up once each.
*/
std::array<boost::container::flat_set<Issue>, 2> seenDirectIssues;
// A strand may not include the same offer book more than once
boost::container::flat_set<Issue> seenBookOuts;
seenDirectIssues[0].reserve (pes.size());
seenDirectIssues[1].reserve (pes.size());
seenBookOuts.reserve (pes.size());
auto ctx = [&](bool isLast = false)
{
return StrandContext{view, result, strandSrc, strandDst, deliver,
limitQuality, isLast, ownerPaysTransferFee, offerCrossing,
isDefaultPath, seenDirectIssues, seenBookOuts, j};
};

for (int i = 0; i < pes.size () - 1; ++i)
{
/* Iterate through the path elements considering them in pairs.
The first element of the pair is `cur` and the second element is
`next`. When an offer is one of the pairs, the step created will be for
`next`. This means when `cur` is an offer and `next` is an
account then no step is created, as a step has already been created for
that offer.
*/
boost::optional<STPathElement> impliedPE;
auto cur = pes[i];
auto next = pes[i + 1];

if (cur->isNone() || next->isNone())
return {temBAD_PATH, Strand{}};

/* If both account and issuer are set, use the account
(and don't insert an implied node for the issuer).
This matches the behavior of the previous generation payment code
*/
if (cur->isAccount())
curIssue.account = cur->getAccountID ();
else if (cur->hasIssuer())
curIssue.account = cur->getIssuerID ();

if (cur->hasCurrency())
curIssue.currency = cur->getCurrency ();

if (cur->isAccount() && next->isAccount())
{
if (!isXRP (curIssue.currency) &&
curIssue.account != cur->getAccountID () &&
curIssue.account != next->getAccountID ())
{
JLOG (j.trace()) << "Inserting implied account";
auto msr = make_DirectStepI (ctx(), cur->getAccountID (),
curIssue.account, curIssue.currency);
if (msr.first != tesSUCCESS)
return {msr.first, Strand{}};
result.push_back (std::move (msr.second));
Currency dummy;
impliedPE.emplace (STPathElement::typeAccount,
curIssue.account, dummy, curIssue.account);
cur = &*impliedPE;
}
}
else if (cur->isAccount() && next->isOffer())
{
if (curIssue.account != cur->getAccountID ())
{
JLOG (j.trace()) << "Inserting implied account before offer";
auto msr = make_DirectStepI (ctx(), cur->getAccountID (),
curIssue.account, curIssue.currency);
if (msr.first != tesSUCCESS)
return {msr.first, Strand{}};
result.push_back (std::move (msr.second));
Currency dummy;
impliedPE.emplace (STPathElement::typeAccount,
curIssue.account, dummy, curIssue.account);
cur = &*impliedPE;
}
}
else if (cur->isOffer() && next->isAccount())
{
if (curIssue.account != next->getAccountID () &&
!isXRP (next->getAccountID ()))
{
JLOG (j.trace()) << "Inserting implied account after offer";
auto msr = make_DirectStepI (ctx(), curIssue.account,
next->getAccountID (), curIssue.currency);
if (msr.first != tesSUCCESS)
return {msr.first, Strand{}};
result.push_back (std::move (msr.second));
}
continue;
}

if (!next->isOffer() &&
next->hasCurrency() && next->getCurrency () != curIssue.currency)
{
auto const& nextCurrency = next->getCurrency ();
auto const& nextIssuer =
next->hasIssuer () ? next->getIssuerID () : curIssue.account;

if (isXRP (curIssue.currency))
{
JLOG (j.trace()) << "Inserting implied XI offer";
auto msr = make_BookStepXI (
ctx(), {nextCurrency, nextIssuer});
if (msr.first != tesSUCCESS)
return {msr.first, Strand{}};
result.push_back (std::move (msr.second));
}
else if (isXRP (nextCurrency))
{
JLOG (j.trace()) << "Inserting implied IX offer";
auto msr = make_BookStepIX (ctx(), curIssue);
if (msr.first != tesSUCCESS)
return {msr.first, Strand{}};
result.push_back (std::move (msr.second));
}
else
{
JLOG (j.trace()) << "Inserting implied II offer";
auto msr = make_BookStepII (
ctx(), curIssue, {nextCurrency, nextIssuer});
if (msr.first != tesSUCCESS)
return {msr.first, Strand{}};
result.push_back (std::move (msr.second));
}
impliedPE.emplace (
boost::none, nextCurrency, nextIssuer);
cur = &*impliedPE;
curIssue.currency = nextCurrency;
curIssue.account = nextIssuer;
}

auto s =
toStep (ctx (/*isLast*/ i == pes.size () - 2), cur, next, curIssue);
if (s.first == tesSUCCESS)
result.emplace_back (std::move (s.second));
else
{
JLOG (j.debug()) << "toStep failed: " << s.first;
return {s.first, Strand{}};
}
}

return {tesSUCCESS, std::move (result)};
}


std::pair<TER, Strand>
toStrandV2 (
toStrand (
Copy link
Contributor

Choose a reason for hiding this comment

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

Great reduction in unnecessary code!

ReadView const& view,
AccountID const& src,
AccountID const& dst,
Expand Down Expand Up @@ -689,27 +439,6 @@ toStrandV2 (
return {tesSUCCESS, std::move (result)};
}

std::pair<TER, Strand>
toStrand (
ReadView const& view,
AccountID const& src,
AccountID const& dst,
Issue const& deliver,
boost::optional<Quality> const& limitQuality,
boost::optional<Issue> const& sendMaxIssue,
STPath const& path,
bool ownerPaysTransferFee,
bool offerCrossing,
beast::Journal j)
{
if (view.rules().enabled(fix1373))
return toStrandV2(view, src, dst, deliver, limitQuality,
sendMaxIssue, path, ownerPaysTransferFee, offerCrossing, j);
else
return toStrandV1(view, src, dst, deliver, limitQuality,
sendMaxIssue, path, ownerPaysTransferFee, offerCrossing, j);
}

std::pair<TER, std::vector<Strand>>
toStrands (
ReadView const& view,
Expand Down
Loading