Skip to content

Commit

Permalink
[FOLD] Refactor Payment::doApply
Browse files Browse the repository at this point in the history
  • Loading branch information
@seelabs @scottschurr
seelabs authored and scottschurr committed Dec 2, 2017
1 parent 64643c9 commit fa20f12
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 89 deletions.
161 changes: 73 additions & 88 deletions src/ripple/app/tx/impl/Payment.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,30 +347,19 @@ Payment::doApply ()
view().update (sleDst);
}

TER terResult {tesSUCCESS};

// Determine whether the destination requires deposit authorization.
bool const reqDepositAuth = [] (ReadView& view, std::uint32_t dstFlags)
{
if (dstFlags & lsfDepositAuth &&
view.rules().enabled(featureDepositAuth))
return true;

return false;
} (view(), sleDst->getFlags());
bool const reqDepositAuth = sleDst->getFlags() & lsfDepositAuth &&
view().rules().enabled(featureDepositAuth);

bool const bRipple = paths || sendMax || !saDstAmount.native ();
// XXX Should sendMax be sufficient to imply ripple?

if (bRipple && reqDepositAuth)
{
// If
// 1. payment is not direct XRP to XRP and
// 2. the destination has lsfDepositAuth set,
// then the payment is not authorized.
terResult = tecNO_PERMISSION;
}
else if (bRipple)
// If the destination has lsfDepositAuth set, then only direct
// XRP payments (no intermediate steps) are allowed to the destination.
return tecNO_PERMISSION;

if (bRipple)
{
// Ripple payment with at least one intermediate step and uses
// transitive balances.
Expand All @@ -388,7 +377,8 @@ Payment::doApply ()
{
PaymentSandbox pv(&view());
JLOG(j_.debug())
<< "Entering RippleCalc in payment: " << ctx_.tx.getTransactionID();
<< "Entering RippleCalc in payment: "
<< ctx_.tx.getTransactionID();
rc = path::RippleCalc::rippleCalculate (
pv,
maxSourceAmount,
Expand Down Expand Up @@ -416,90 +406,85 @@ Payment::doApply ()
ctx_.deliver (rc.actualAmountOut);
}

terResult = rc.result ();
auto terResult = rc.result ();

// Because of its overhead, if RippleCalc
// fails with a retry code, claim a fee
// instead. Maybe the user will be more
// careful with their path spec next time.
if (isTerRetry (terResult))
terResult = tecPATH_DRY;
return terResult;
}
else
{
assert (saDstAmount.native ());

// Direct XRP payment.
assert (saDstAmount.native ());

// uOwnerCount is the number of entries in this ledger for this
// account that require a reserve.
auto const uOwnerCount = view().read(
keylet::account(account_))->getFieldU32 (sfOwnerCount);
// Direct XRP payment.

// This is the total reserve in drops.
auto const reserve = view().fees().accountReserve(uOwnerCount);
// uOwnerCount is the number of entries in this ledger for this
// account that require a reserve.
auto const uOwnerCount = view().read(
keylet::account(account_))->getFieldU32 (sfOwnerCount);

// mPriorBalance is the balance on the sending account BEFORE the
// fees were charged. We want to make sure we have enough reserve
// to send. Allow final spend to use reserve for fee.
auto const mmm = std::max(reserve,
ctx_.tx.getFieldAmount (sfFee).xrp ());
// This is the total reserve in drops.
auto const reserve = view().fees().accountReserve(uOwnerCount);

if (mPriorBalance < saDstAmount.xrp () + mmm)
{
// Vote no. However the transaction might succeed, if applied in
// a different order.
JLOG(j_.trace()) << "Delay transaction: Insufficient funds: " <<
" " << to_string (mPriorBalance) <<
" / " << to_string (saDstAmount.xrp () + mmm) <<
" (" << to_string (reserve) << ")";

terResult = tecUNFUNDED_PAYMENT;
}
else
{
// The source account does have enough money. Make sure the
// source account has authority to deposit to the destination.
if (reqDepositAuth)
{
// Get the base reserve.
XRPAmount const dstReserve {view().fees().accountReserve (0)};

// If the destination's XRP balance is
// 1. below the base reserve and
// 2. the deposit amount is also below the base reserve,
// then we allow the deposit.
//
// This rule is designed to keep an account from getting wedged
// in an unusable state if it sets the lsfDepositAuth flag and
// then consumes all of its XRP. Without the rule if an
// account with lsfDepositAuth set spent all of its XRP, it
// would be unable to acquire more XRP required to pay fees.
//
// We choose the base reserve as our bound because it is
// a small number that seldom changes but is always sufficient
// to get the account un-wedged.
if (saDstAmount > dstReserve ||
sleDst->getFieldAmount (sfBalance) > dstReserve)
terResult = tecNO_PERMISSION;
}

if (terResult == tesSUCCESS)
{
// Do the arithmetic for the transfer and make the ledger change.
view().peek(keylet::account(account_))->setFieldAmount (
sfBalance, mSourceBalance - saDstAmount);
sleDst->setFieldAmount (sfBalance,
sleDst->getFieldAmount (sfBalance) + saDstAmount);

// Re-arm the password change fee if we can and need to.
if ((sleDst->getFlags() & lsfPasswordSpent))
sleDst->clearFlag (lsfPasswordSpent);
}
}
// mPriorBalance is the balance on the sending account BEFORE the
// fees were charged. We want to make sure we have enough reserve
// to send. Allow final spend to use reserve for fee.
auto const mmm = std::max(reserve,
ctx_.tx.getFieldAmount (sfFee).xrp ());

if (mPriorBalance < saDstAmount.xrp () + mmm)
{
// Vote no. However the transaction might succeed, if applied in
// a different order.
JLOG(j_.trace()) << "Delay transaction: Insufficient funds: " <<
" " << to_string (mPriorBalance) <<
" / " << to_string (saDstAmount.xrp () + mmm) <<
" (" << to_string (reserve) << ")";

return tecUNFUNDED_PAYMENT;
}

// The source account does have enough money. Make sure the
// source account has authority to deposit to the destination.
if (reqDepositAuth)
{
// Get the base reserve.
XRPAmount const dstReserve {view().fees().accountReserve (0)};

// If the destination's XRP balance is
// 1. below the base reserve and
// 2. the deposit amount is also below the base reserve,
// then we allow the deposit.
//
// This rule is designed to keep an account from getting wedged
// in an unusable state if it sets the lsfDepositAuth flag and
// then consumes all of its XRP. Without the rule if an
// account with lsfDepositAuth set spent all of its XRP, it
// would be unable to acquire more XRP required to pay fees.
//
// We choose the base reserve as our bound because it is
// a small number that seldom changes but is always sufficient
// to get the account un-wedged.
if (saDstAmount > dstReserve ||
sleDst->getFieldAmount (sfBalance) > dstReserve)
return tecNO_PERMISSION;
}

return terResult;
// Do the arithmetic for the transfer and make the ledger change.
view()
.peek(keylet::account(account_))
->setFieldAmount(sfBalance, mSourceBalance - saDstAmount);
sleDst->setFieldAmount(
sfBalance, sleDst->getFieldAmount(sfBalance) + saDstAmount);

// Re-arm the password change fee if we can and need to.
if ((sleDst->getFlags() & lsfPasswordSpent))
sleDst->clearFlag(lsfPasswordSpent);

return tesSUCCESS;
}

} // ripple
2 changes: 1 addition & 1 deletion src/ripple/app/tx/impl/SetAccount.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ SetAccount::doApply ()
bool const bSetDisallowXRP {(uTxFlags & tfDisallowXRP) || (uSetFlag == asfDisallowXRP)};
bool const bClearDisallowXRP {(uTxFlags & tfAllowXRP) || (uClearFlag == asfDisallowXRP)};

bool const sigWithMaster {[tx, acct = account_] ()
bool const sigWithMaster {[&tx, &acct = account_] ()
{
auto const spk = tx.getSigningPubKey();

Expand Down

0 comments on commit fa20f12

Please sign in to comment.