Skip to content

Commit

Permalink
Support for lsfDepositAuth (RIPD-1487):
Browse files Browse the repository at this point in the history 
The DepositAuth feature allows an account to require that
it signs for any funds that are deposited to the account.
For the time being this limits the account to accepting
only XRP, although there are plans to allow IOU payments
in the future.

The DepositAuth feature leaves a small gap in its
protections.  An XRP payment is allowed to a destination
account with the lsfDepositAuth flag set if:

- The Destination XRP balance is less than or equal to
  the base reserve and

- The value of the XRP Payment is less than or equal to
  the base reserve.

This exception is intended to make it impossible for an
account to wedge itself by spending all of its XRP on fees
and leave itself unable to pay the fee to get more XRP.

This commit

- adds featureDepositAuth,

- adds the lsfDepositAuth flag,

- adds support for lsfDepositAuth in SetAccount.cpp

- adds support in Payment.cpp for rejecting payments that
  don't meet the lsfDepositAuth requirements,

- adds unit tests for Payment transactions to an an account
  with lsfDepositAuth set.
  • Loading branch information
@scottschurr
scottschurr committed Dec 2, 2017
1 parent 090d813 commit 64643c9
Show file tree
Hide file tree
Showing 13 changed files with 442 additions and 59 deletions.
4 changes: 4 additions & 0 deletions Builds/VisualStudio2015/RippleD.vcxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4253,6 +4253,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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5076,6 +5076,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
75 changes: 60 additions & 15 deletions src/ripple/app/tx/impl/Payment.cpp
View file
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,12 +347,30 @@ Payment::doApply ()
view().update (sleDst);
}

TER terResult;
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 bRipple = paths || sendMax || !saDstAmount.native ();
// XXX Should sendMax be sufficient to imply ripple?

if (bRipple)
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)
{
// Ripple payment with at least one intermediate step and uses
// transitive balances.
Expand Down Expand Up @@ -412,7 +431,7 @@ Payment::doApply ()

// Direct XRP payment.

// uOwnerCount is the number of entries in this legder for this
// 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);
Expand All @@ -439,18 +458,44 @@ Payment::doApply ()
}
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;
// 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);
}
}
}

Expand Down
95 changes: 60 additions & 35 deletions src/ripple/app/tx/impl/SetAccount.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,13 @@ SetAccount::preflight (PreflightContext const& ctx)
return telBAD_DOMAIN;
}

// DepositAuth
if (uSetFlag == asfDepositAuth || uClearFlag == asfDepositAuth)
{
if (!ctx.rules.enabled(featureDepositAuth))
return temDISABLED;
}

return preflight2(ctx);
}

Expand Down Expand Up @@ -201,38 +208,37 @@ SetAccount::preclaim(PreclaimContext const& ctx)
TER
SetAccount::doApply ()
{
std::uint32_t const uTxFlags = ctx_.tx.getFlags ();

auto const sle = view().peek(
keylet::account(account_));
auto const sle = view().peek(keylet::account(account_));

std::uint32_t const uFlagsIn = sle->getFieldU32 (sfFlags);
std::uint32_t uFlagsOut = uFlagsIn;

std::uint32_t const uSetFlag = ctx_.tx.getFieldU32 (sfSetFlag);
std::uint32_t const uClearFlag = ctx_.tx.getFieldU32 (sfClearFlag);
STTx const& tx {ctx_.tx};
std::uint32_t const uSetFlag {tx.getFieldU32 (sfSetFlag)};
std::uint32_t const uClearFlag {tx.getFieldU32 (sfClearFlag)};

// legacy AccountSet flags
bool bSetRequireDest = (uTxFlags & TxFlag::requireDestTag) || (uSetFlag == asfRequireDest);
bool bClearRequireDest = (uTxFlags & tfOptionalDestTag) || (uClearFlag == asfRequireDest);
bool bSetRequireAuth = (uTxFlags & tfRequireAuth) || (uSetFlag == asfRequireAuth);
bool bClearRequireAuth = (uTxFlags & tfOptionalAuth) || (uClearFlag == asfRequireAuth);
bool bSetDisallowXRP = (uTxFlags & tfDisallowXRP) || (uSetFlag == asfDisallowXRP);
bool bClearDisallowXRP = (uTxFlags & tfAllowXRP) || (uClearFlag == asfDisallowXRP);

bool sigWithMaster = false;
std::uint32_t const uTxFlags {tx.getFlags ()};
bool const bSetRequireDest {(uTxFlags & TxFlag::requireDestTag) || (uSetFlag == asfRequireDest)};
bool const bClearRequireDest {(uTxFlags & tfOptionalDestTag) || (uClearFlag == asfRequireDest)};
bool const bSetRequireAuth {(uTxFlags & tfRequireAuth) || (uSetFlag == asfRequireAuth)};
bool const bClearRequireAuth {(uTxFlags & tfOptionalAuth) || (uClearFlag == asfRequireAuth)};
bool const bSetDisallowXRP {(uTxFlags & tfDisallowXRP) || (uSetFlag == asfDisallowXRP)};
bool const bClearDisallowXRP {(uTxFlags & tfAllowXRP) || (uClearFlag == asfDisallowXRP)};

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

if (publicKeyType (makeSlice (spk)))
{
PublicKey const signingPubKey (makeSlice (spk));

if (calcAccountID(signingPubKey) == account_)
sigWithMaster = true;
if (calcAccountID(signingPubKey) == acct)
return true;
}
}
return false;
}()};

//
// RequireAuth
Expand Down Expand Up @@ -317,11 +323,13 @@ SetAccount::doApply ()
//
if (uSetFlag == asfDefaultRipple)
{
uFlagsOut |= lsfDefaultRipple;
JLOG(j_.trace()) << "Set lsfDefaultRipple.";
uFlagsOut |= lsfDefaultRipple;
}
else if (uClearFlag == asfDefaultRipple)
{
uFlagsOut &= ~lsfDefaultRipple;
JLOG(j_.trace()) << "Clear lsfDefaultRipple.";
uFlagsOut &= ~lsfDefaultRipple;
}

//
Expand All @@ -331,7 +339,7 @@ SetAccount::doApply ()
{
if (!sigWithMaster && !(uFlagsIn & lsfDisableMaster))
{
JLOG(j_.trace()) << "Can't use regular key to set NoFreeze.";
JLOG(j_.trace()) << "Must use master key to set NoFreeze.";
return tecNEED_MASTER_KEY;
}

Expand Down Expand Up @@ -361,22 +369,39 @@ SetAccount::doApply ()
//
if ((uSetFlag == asfAccountTxnID) && !sle->isFieldPresent (sfAccountTxnID))
{
JLOG(j_.trace()) << "Set AccountTxnID";
JLOG(j_.trace()) << "Set AccountTxnID.";
sle->makeFieldPresent (sfAccountTxnID);
}

if ((uClearFlag == asfAccountTxnID) && sle->isFieldPresent (sfAccountTxnID))
{
JLOG(j_.trace()) << "Clear AccountTxnID";
JLOG(j_.trace()) << "Clear AccountTxnID.";
sle->makeFieldAbsent (sfAccountTxnID);
}

//
// DepositAuth
//
if (view().rules().enabled(featureDepositAuth))
{
if (uSetFlag == asfDepositAuth)
{
JLOG(j_.trace()) << "Set lsfDepositAuth.";
uFlagsOut |= lsfDepositAuth;
}
else if (uClearFlag == asfDepositAuth)
{
JLOG(j_.trace()) << "Clear lsfDepositAuth.";
uFlagsOut &= ~lsfDepositAuth;
}
}

//
// EmailHash
//
if (ctx_.tx.isFieldPresent (sfEmailHash))
if (tx.isFieldPresent (sfEmailHash))
{
uint128 const uHash = ctx_.tx.getFieldH128 (sfEmailHash);
uint128 const uHash = tx.getFieldH128 (sfEmailHash);

if (!uHash)
{
Expand All @@ -393,9 +418,9 @@ SetAccount::doApply ()
//
// WalletLocator
//
if (ctx_.tx.isFieldPresent (sfWalletLocator))
if (tx.isFieldPresent (sfWalletLocator))
{
uint256 const uHash = ctx_.tx.getFieldH256 (sfWalletLocator);
uint256 const uHash = tx.getFieldH256 (sfWalletLocator);

if (!uHash)
{
Expand All @@ -412,9 +437,9 @@ SetAccount::doApply ()
//
// MessageKey
//
if (ctx_.tx.isFieldPresent (sfMessageKey))
if (tx.isFieldPresent (sfMessageKey))
{
Blob const messageKey = ctx_.tx.getFieldVL (sfMessageKey);
Blob const messageKey = tx.getFieldVL (sfMessageKey);

if (messageKey.empty ())
{
Expand All @@ -431,9 +456,9 @@ SetAccount::doApply ()
//
// Domain
//
if (ctx_.tx.isFieldPresent (sfDomain))
if (tx.isFieldPresent (sfDomain))
{
Blob const domain = ctx_.tx.getFieldVL (sfDomain);
Blob const domain = tx.getFieldVL (sfDomain);

if (domain.empty ())
{
Expand All @@ -450,9 +475,9 @@ SetAccount::doApply ()
//
// TransferRate
//
if (ctx_.tx.isFieldPresent (sfTransferRate))
if (tx.isFieldPresent (sfTransferRate))
{
std::uint32_t uRate = ctx_.tx.getFieldU32 (sfTransferRate);
std::uint32_t uRate = tx.getFieldU32 (sfTransferRate);

if (uRate == 0 || uRate == QUALITY_ONE)
{
Expand All @@ -469,9 +494,9 @@ SetAccount::doApply ()
//
// TickSize
//
if (ctx_.tx.isFieldPresent (sfTickSize))
if (tx.isFieldPresent (sfTickSize))
{
auto uTickSize = ctx_.tx[sfTickSize];
auto uTickSize = tx[sfTickSize];
if ((uTickSize == 0) || (uTickSize == Quality::maxTickSize))
{
JLOG(j_.trace()) << "unset tick size";
Expand Down
4 changes: 3 additions & 1 deletion src/ripple/protocol/Feature.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ class FeatureCollections
"fix1512",
"fix1513",
"fix1523",
"fix1528"
"fix1528",
"DepositAuth"
};

std::vector<uint256> features;
Expand Down Expand Up @@ -353,6 +354,7 @@ extern uint256 const fix1512;
extern uint256 const fix1513;
extern uint256 const fix1523;
extern uint256 const fix1528;
extern uint256 const featureDepositAuth;

} // ripple

Expand Down
1 change: 1 addition & 0 deletions src/ripple/protocol/LedgerFormats.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ enum LedgerSpecificFlags
lsfNoFreeze = 0x00200000, // True, cannot freeze ripple states
lsfGlobalFreeze = 0x00400000, // True, all assets frozen
lsfDefaultRipple = 0x00800000, // True, trust lines allow rippling by default
lsfDepositAuth = 0x01000000, // True, all deposits require authorization

// ltOFFER
lsfPassive = 0x00010000,
Expand Down
1 change: 1 addition & 0 deletions src/ripple/protocol/TxFlags.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ const std::uint32_t asfAccountTxnID = 5;
const std::uint32_t asfNoFreeze = 6;
const std::uint32_t asfGlobalFreeze = 7;
const std::uint32_t asfDefaultRipple = 8;
const std::uint32_t asfDepositAuth = 9;

// OfferCreate flags:
const std::uint32_t tfPassive = 0x00010000;
Expand Down
4 changes: 3 additions & 1 deletion src/ripple/protocol/impl/Feature.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ detail::supportedAmendments ()
{ "6C92211186613F9647A89DFFBAB8F94C99D4C7E956D495270789128569177DA1 fix1512" },
{ "67A34F2CF55BFC0F93AACD5B281413176FEE195269FA6D95219A2DF738671172 fix1513" },
{ "B9E739B8296B4A1BB29BE990B17D66E21B62A300A909F25AC55C22D6C72E1F9D fix1523" },
{ "1D3463A5891F9E589C5AE839FFAC4A917CE96197098A1EF22304E1BC5B98A454 fix1528" }
{ "1D3463A5891F9E589C5AE839FFAC4A917CE96197098A1EF22304E1BC5B98A454 fix1528" },
{ "F64E1EABBE79D55B3BB82020516CEC2C582A98A6BFE20FBE9BB6A0D233418064 DepositAuth"}
};
return supported;
}
Expand Down Expand Up @@ -154,5 +155,6 @@ uint256 const fix1512 = *getRegisteredFeature("fix1512");
uint256 const fix1513 = *getRegisteredFeature("fix1513");
uint256 const fix1523 = *getRegisteredFeature("fix1523");
uint256 const fix1528 = *getRegisteredFeature("fix1528");
uint256 const featureDepositAuth = *getRegisteredFeature("DepositAuth");

} // ripple
Loading

0 comments on commit 64643c9

Please sign in to comment.