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.

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

Support for the Deposit Authorization account root flag #2239

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions Builds/VisualStudio2015/RippleD.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -4642,6 +4642,10 @@
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>
</ClCompile>
<ClCompile Include="..\..\src\test\app\DepositAuth_test.cpp">
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>
</ClCompile>
<ClCompile Include="..\..\src\test\app\Discrepancy_test.cpp">
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>
Expand Down
3 changes: 3 additions & 0 deletions Builds/VisualStudio2015/RippleD.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -5598,6 +5598,9 @@
<ClCompile Include="..\..\src\test\app\DeliverMin_test.cpp">
<Filter>test\app</Filter>
</ClCompile>
<ClCompile Include="..\..\src\test\app\DepositAuth_test.cpp">
<Filter>test\app</Filter>
</ClCompile>
<ClCompile Include="..\..\src\test\app\Discrepancy_test.cpp">
<Filter>test\app</Filter>
</ClCompile>
Expand Down
31 changes: 22 additions & 9 deletions src/ripple/app/tx/impl/Escrow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,11 @@ EscrowCreate::doApply()
if (((*sled)[sfFlags] & lsfRequireDestTag) &&
! ctx_.tx[~sfDestinationTag])
return tecDST_TAG_NEEDED;
if ((*sled)[sfFlags] & lsfDisallowXRP)

// Obeying the lsfDissalowXRP flag was a bug. Piggyback on
// featureDepositAuth to remove the bug.
if (! ctx_.view().rules().enabled(featureDepositAuth) &&
((*sled)[sfFlags] & lsfDisallowXRP))
return tecNO_TARGET;
}

Expand Down Expand Up @@ -438,6 +442,23 @@ EscrowFinish::doApply()
return tecCRYPTOCONDITION_ERROR;
}

// NOTE: Escrow payments cannot be used to fund accounts
auto const sled = ctx_.view().peek(keylet::account((*slep)[sfDestination]));
if (! sled)
return tecNO_DST;

if (ctx_.view().rules().enabled(featureDepositAuth))
{
// Is EscrowFinished authorized?
if (sled->getFlags() & lsfDepositAuth)
{
// Authorized if Destination == Account, otherwise no permission.
AccountID const destID = (*slep)[sfDestination];
if (ctx_.tx[sfAccount] != destID)
return tecNO_PERMISSION;
}
}

AccountID const account = (*slep)[sfAccount];

// Remove escrow from owner directory
Expand All @@ -460,14 +481,6 @@ EscrowFinish::doApply()
return ter;
}

// NOTE: These payments cannot be used to fund accounts

// Fetch Destination SLE
auto const sled = ctx_.view().peek(
keylet::account((*slep)[sfDestination]));
if (! sled)
return tecNO_DST;

// Transfer amount to destination
(*sled)[sfBalance] = (*sled)[sfBalance] + (*slep)[sfAmount];
ctx_.view().update(sled);
Expand Down
20 changes: 18 additions & 2 deletions src/ripple/app/tx/impl/PayChan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,11 @@ PayChanCreate::preclaim(PreclaimContext const &ctx)
if (((*sled)[sfFlags] & lsfRequireDestTag) &&
!ctx.tx[~sfDestinationTag])
return tecDST_TAG_NEEDED;
if ((*sled)[sfFlags] & lsfDisallowXRP)

// Obeying the lsfDisallowXRP flag was a bug. Piggyback on
// featureDepositAuth to remove the bug.
if (!ctx.view.rules().enabled(featureDepositAuth) &&
((*sled)[sfFlags] & lsfDisallowXRP))
return tecNO_TARGET;
}

Expand Down Expand Up @@ -457,9 +461,21 @@ PayChanClaim::doApply()
if (!sled)
return terNO_ACCOUNT;

if (txAccount == src && ((*sled)[sfFlags] & lsfDisallowXRP))
// Obeying the lsfDisallowXRP flag was a bug. Piggyback on
// featureDepositAuth to remove the bug.
bool const depositAuth {ctx_.view().rules().enabled(featureDepositAuth)};
if (!depositAuth &&
(txAccount == src && (sled->getFlags() & lsfDisallowXRP)))
return tecNO_TARGET;

// Check whether the destination account requires deposit authorization
if (txAccount != dst)
{
if (depositAuth &&
((sled->getFlags() & lsfDepositAuth) == lsfDepositAuth))
return tecNO_PERMISSION;
}

(*slep)[sfBalance] = ctx_.tx[sfBalance];
XRPAmount const reqDelta = reqBalance - chanBalance;
assert (reqDelta >= beast::zero);
Expand Down
120 changes: 75 additions & 45 deletions src/ripple/app/tx/impl/Payment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <ripple/app/paths/RippleCalc.h>
#include <ripple/basics/Log.h>
#include <ripple/core/Config.h>
#include <ripple/protocol/Feature.h>
#include <ripple/protocol/st.h>
#include <ripple/protocol/TxFlags.h>
#include <ripple/protocol/JsonFields.h>
Expand Down Expand Up @@ -346,11 +347,18 @@ Payment::doApply ()
view().update (sleDst);
}

TER terResult;
// Determine whether the destination requires deposit authorization.
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 the destination has lsfDepositAuth set, then only direct XRP
// payments (no intermediate steps) are allowed to the destination.
if (bRipple && reqDepositAuth)
return tecNO_PERMISSION;

if (bRipple)
{
// Ripple payment with at least one intermediate step and uses
Expand All @@ -369,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 @@ -397,64 +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 legder 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, so 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);

terResult = tesSUCCESS;
}
// 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
Loading