From 259394029aa297f1c2618d898c70e7dc8c5b631f Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Fri, 25 Aug 2017 12:55:48 -0700 Subject: [PATCH] Support for lsfDepositAuth (RIPD-1487): MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 lsfDepositAuth protections are not extended to offers. If an account creates an offer it is in effect saying, “I will accept funds from anyone who takes this offer.” Therefore, the typical user of the lsfDepositAuth flag will choose never to create any offers. But they can if they so choose. 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. - adds Escrow and PayChan support for lsfDepositAuth along with as unit tests. --- Builds/VisualStudio2015/RippleD.vcxproj | 4 + .../VisualStudio2015/RippleD.vcxproj.filters | 3 + src/ripple/app/tx/impl/Escrow.cpp | 25 +- src/ripple/app/tx/impl/PayChan.cpp | 10 +- src/ripple/app/tx/impl/Payment.cpp | 126 +++++--- src/ripple/app/tx/impl/SetAccount.cpp | 88 +++--- src/ripple/protocol/Feature.h | 4 +- src/ripple/protocol/LedgerFormats.h | 1 + src/ripple/protocol/TxFlags.h | 1 + src/ripple/protocol/impl/Feature.cpp | 4 +- src/test/app/DepositAuth_test.cpp | 296 ++++++++++++++++++ src/test/app/Escrow_test.cpp | 80 ++++- src/test/app/Flow_test.cpp | 2 + src/test/app/PayChan_test.cpp | 70 +++++ src/test/jtx/flags.h | 13 +- src/test/rpc/AccountSet_test.cpp | 88 ++++-- src/test/unity/app_test_unity1.cpp | 1 + 17 files changed, 689 insertions(+), 127 deletions(-) create mode 100644 src/test/app/DepositAuth_test.cpp diff --git a/Builds/VisualStudio2015/RippleD.vcxproj b/Builds/VisualStudio2015/RippleD.vcxproj index 7f4e618378..657b35ba64 100644 --- a/Builds/VisualStudio2015/RippleD.vcxproj +++ b/Builds/VisualStudio2015/RippleD.vcxproj @@ -4642,6 +4642,10 @@ True True + + True + True + True True diff --git a/Builds/VisualStudio2015/RippleD.vcxproj.filters b/Builds/VisualStudio2015/RippleD.vcxproj.filters index 6c907fd41c..5f96dbdf2c 100644 --- a/Builds/VisualStudio2015/RippleD.vcxproj.filters +++ b/Builds/VisualStudio2015/RippleD.vcxproj.filters @@ -5598,6 +5598,9 @@ test\app + + test\app + test\app diff --git a/src/ripple/app/tx/impl/Escrow.cpp b/src/ripple/app/tx/impl/Escrow.cpp index 5600993bea..8b28be74f6 100644 --- a/src/ripple/app/tx/impl/Escrow.cpp +++ b/src/ripple/app/tx/impl/Escrow.cpp @@ -438,6 +438,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 @@ -460,14 +477,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); diff --git a/src/ripple/app/tx/impl/PayChan.cpp b/src/ripple/app/tx/impl/PayChan.cpp index 1a14cbe653..d8ee7f7e71 100644 --- a/src/ripple/app/tx/impl/PayChan.cpp +++ b/src/ripple/app/tx/impl/PayChan.cpp @@ -457,9 +457,17 @@ PayChanClaim::doApply() if (!sled) return terNO_ACCOUNT; - if (txAccount == src && ((*sled)[sfFlags] & lsfDisallowXRP)) + if (txAccount == src && (sled->getFlags() & lsfDisallowXRP)) return tecNO_TARGET; + // Check whether the destination account requires deposit authorization + if (txAccount != dst) + { + if (ctx_.view().rules().enabled(featureDepositAuth) && + ((sled->getFlags() & lsfDepositAuth) == lsfDepositAuth)) + return tecNO_PERMISSION; + } + (*slep)[sfBalance] = ctx_.tx[sfBalance]; XRPAmount const reqDelta = reqBalance - chanBalance; assert (reqDelta >= beast::zero); diff --git a/src/ripple/app/tx/impl/Payment.cpp b/src/ripple/app/tx/impl/Payment.cpp index c66b517a16..10a085ae92 100644 --- a/src/ripple/app/tx/impl/Payment.cpp +++ b/src/ripple/app/tx/impl/Payment.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -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 @@ -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, @@ -397,7 +406,7 @@ 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 @@ -405,56 +414,77 @@ Payment::doApply () // careful with their path spec next time. if (isTerRetry (terResult)) terResult = tecPATH_DRY; + return terResult; } - else + + assert (saDstAmount.native ()); + + // Direct XRP payment. + + // 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); + + // This is the total reserve in drops. + auto const reserve = view().fees().accountReserve(uOwnerCount); + + // 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) { - assert (saDstAmount.native ()); + // 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) << ")"; - // Direct XRP payment. - - // 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); - - // This is the total reserve in drops. - auto const reserve = view().fees().accountReserve(uOwnerCount); - - // 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) << ")"; - - 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; - } + return tecUNFUNDED_PAYMENT; } - return terResult; + // 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; + } + + // 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 diff --git a/src/ripple/app/tx/impl/SetAccount.cpp b/src/ripple/app/tx/impl/SetAccount.cpp index 83408704c9..200bb3eae8 100644 --- a/src/ripple/app/tx/impl/SetAccount.cpp +++ b/src/ripple/app/tx/impl/SetAccount.cpp @@ -201,38 +201,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 @@ -317,11 +316,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; } // @@ -331,7 +332,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; } @@ -361,22 +362,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) { @@ -393,9 +411,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) { @@ -412,9 +430,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 ()) { @@ -431,9 +449,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 ()) { @@ -450,9 +468,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) { @@ -469,9 +487,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"; diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 9794efe9ff..7f00426033 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -73,7 +73,8 @@ class FeatureCollections "fix1512", "fix1513", "fix1523", - "fix1528" + "fix1528", + "DepositAuth" }; std::vector features; @@ -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 diff --git a/src/ripple/protocol/LedgerFormats.h b/src/ripple/protocol/LedgerFormats.h index d7bb895ea0..14d43eb06b 100644 --- a/src/ripple/protocol/LedgerFormats.h +++ b/src/ripple/protocol/LedgerFormats.h @@ -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, diff --git a/src/ripple/protocol/TxFlags.h b/src/ripple/protocol/TxFlags.h index d2b2eeaf75..e09f5f0c6c 100644 --- a/src/ripple/protocol/TxFlags.h +++ b/src/ripple/protocol/TxFlags.h @@ -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; diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 2f202b2bc8..add7503879 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -106,7 +106,8 @@ detail::supportedAmendments () { "6C92211186613F9647A89DFFBAB8F94C99D4C7E956D495270789128569177DA1 fix1512" }, { "67A34F2CF55BFC0F93AACD5B281413176FEE195269FA6D95219A2DF738671172 fix1513" }, { "B9E739B8296B4A1BB29BE990B17D66E21B62A300A909F25AC55C22D6C72E1F9D fix1523" }, - { "1D3463A5891F9E589C5AE839FFAC4A917CE96197098A1EF22304E1BC5B98A454 fix1528" } + { "1D3463A5891F9E589C5AE839FFAC4A917CE96197098A1EF22304E1BC5B98A454 fix1528" }, + { "F64E1EABBE79D55B3BB82020516CEC2C582A98A6BFE20FBE9BB6A0D233418064 DepositAuth"} }; return supported; } @@ -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 diff --git a/src/test/app/DepositAuth_test.cpp b/src/test/app/DepositAuth_test.cpp new file mode 100644 index 0000000000..dbd44734f8 --- /dev/null +++ b/src/test/app/DepositAuth_test.cpp @@ -0,0 +1,296 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2017 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace ripple { +namespace test { + +struct DepositAuth_test : public beast::unit_test::suite +{ + // Helper function that returns the reserve on an account based on + // the passed in number of owners. + static XRPAmount reserve(jtx::Env& env, std::uint32_t count) + { + return env.current()->fees().accountReserve (count); + } + + // Helper function that returns true if acct has the lsfDepostAuth flag set. + static bool hasDepositAuth (jtx::Env const& env, jtx::Account const& acct) + { + return ((*env.le(acct))[sfFlags] & lsfDepositAuth) == lsfDepositAuth; + }; + + + void testEnable() + { + testcase ("Enable"); + + using namespace jtx; + Account const alice {"alice"}; + + { + // featureDepositAuth is disabled. + Env env (*this, supported_amendments() - featureDepositAuth); + env.fund (XRP (10000), alice); + + // Note that, to support old behavior, invalid flags are ignored. + env (fset (alice, asfDepositAuth)); + env.close(); + BEAST_EXPECT (! hasDepositAuth (env, alice)); + + env (fclear (alice, asfDepositAuth)); + env.close(); + BEAST_EXPECT (! hasDepositAuth (env, alice)); + } + { + // featureDepositAuth is enabled. + Env env (*this); + env.fund (XRP (10000), alice); + + env (fset (alice, asfDepositAuth)); + env.close(); + BEAST_EXPECT (hasDepositAuth (env, alice)); + + env (fclear (alice, asfDepositAuth)); + env.close(); + BEAST_EXPECT (! hasDepositAuth (env, alice)); + } + } + + void testPayIOU() + { + // Exercise IOU payments and non-direct XRP payments to an account + // that has the lsfDepositAuth flag set. + testcase ("Pay IOU"); + + using namespace jtx; + Account const alice {"alice"}; + Account const bob {"bob"}; + Account const carol {"carol"}; + Account const gw {"gw"}; + IOU const USD = gw["USD"]; + + Env env (*this); + + env.fund (XRP (10000), alice, bob, carol, gw); + env.trust (USD (1000), alice, bob); + env.close(); + + env (pay (gw, alice, USD (150))); + env (offer (carol, USD(100), XRP(100))); + env.close(); + + // Make sure bob's trust line is all set up so he can receive USD. + env (pay (alice, bob, USD (50))); + env.close(); + + // bob sets the lsfDepositAuth flag. + env (fset (bob, asfDepositAuth), require(flags (bob, asfDepositAuth))); + env.close(); + + // None of the following payments should succeed. + auto failedIouPayments = [this, &env, &alice, &bob, &USD] () + { + env.require (flags (bob, asfDepositAuth)); + + // Capture bob's balances before hand to confirm they don't change. + PrettyAmount const bobXrpBalance {env.balance (bob, XRP)}; + PrettyAmount const bobUsdBalance {env.balance (bob, USD)}; + + env (pay (alice, bob, USD (50)), ter (tecNO_PERMISSION)); + env.close(); + + // Note that even though alice is paying bob in XRP, the payment + // is still not allowed since the payment passes through an offer. + env (pay (alice, bob, drops(1)), + sendmax (USD (1)), ter (tecNO_PERMISSION)); + env.close(); + + BEAST_EXPECT (bobXrpBalance == env.balance (bob, XRP)); + BEAST_EXPECT (bobUsdBalance == env.balance (bob, USD)); + }; + + // Test when bob has an XRP balance > base reserve. + failedIouPayments(); + + // Set bob's XRP balance == base reserve. Also demonstrate that + // bob can make payments while his lsfDepositAuth flag is set. + env (pay (bob, alice, USD(25))); + env.close(); + + { + STAmount const bobPaysXRP { + env.balance (bob, XRP) - reserve (env, 1)}; + XRPAmount const bobPaysFee {reserve (env, 1) - reserve (env, 0)}; + env (pay (bob, alice, bobPaysXRP), fee (bobPaysFee)); + env.close(); + } + + // Test when bob's XRP balance == base reserve. + BEAST_EXPECT (env.balance (bob, XRP) == reserve (env, 0)); + BEAST_EXPECT (env.balance (bob, USD) == USD(25)); + failedIouPayments(); + + // Test when bob has an XRP balance == 0. + env (noop (bob), fee (reserve (env, 0))); + env.close (); + + BEAST_EXPECT (env.balance (bob, XRP) == XRP (0)); + failedIouPayments(); + + // Give bob enough XRP for the fee to clear the lsfDepositAuth flag. + env (pay (alice, bob, drops(env.current()->fees().base))); + + // bob clears the lsfDepositAuth and the next payment succeeds. + env (fclear (bob, asfDepositAuth)); + env.close(); + + env (pay (alice, bob, USD (50))); + env.close(); + + env (pay (alice, bob, drops(1)), sendmax (USD (1))); + env.close(); + } + + void testPayXRP() + { + // Exercise direct XRP payments to an account that has the + // lsfDepositAuth flag set. + testcase ("Pay XRP"); + + using namespace jtx; + Account const alice {"alice"}; + Account const bob {"bob"}; + + Env env (*this); + + env.fund (XRP (10000), alice, bob); + + // bob sets the lsfDepositAuth flag. + env (fset (bob, asfDepositAuth), fee (drops (10))); + env.close(); + BEAST_EXPECT (env.balance (bob, XRP) == XRP (10000) - drops(10)); + + // bob has more XRP than the base reserve. Any XRP payment should fail. + env (pay (alice, bob, drops(1)), ter (tecNO_PERMISSION)); + env.close(); + BEAST_EXPECT (env.balance (bob, XRP) == XRP (10000) - drops(10)); + + // Change bob's XRP balance to exactly the base reserve. + { + STAmount const bobPaysXRP { + env.balance (bob, XRP) - reserve (env, 1)}; + XRPAmount const bobPaysFee {reserve (env, 1) - reserve (env, 0)}; + env (pay (bob, alice, bobPaysXRP), fee (bobPaysFee)); + env.close(); + } + + // bob has exactly the base reserve. A small enough direct XRP + // payment should succeed. + BEAST_EXPECT (env.balance (bob, XRP) == reserve (env, 0)); + env (pay (alice, bob, drops(1))); + env.close(); + + // bob has exactly the base reserve + 1. No payment should succeed. + BEAST_EXPECT (env.balance (bob, XRP) == reserve (env, 0) + drops(1)); + env (pay (alice, bob, drops(1)), ter (tecNO_PERMISSION)); + env.close(); + + // Take bob down to a balance of 0 XRP. + env (noop (bob), fee (reserve (env, 0) + drops(1))); + env.close (); + BEAST_EXPECT (env.balance (bob, XRP) == drops(0)); + + // We should not be able to pay bob more than the base reserve. + env (pay (alice, bob, reserve (env, 0) + drops(1)), + ter (tecNO_PERMISSION)); + env.close(); + + // However a payment of exactly the base reserve should succeed. + env (pay (alice, bob, reserve (env, 0) + drops(0))); + env.close(); + BEAST_EXPECT (env.balance (bob, XRP) == reserve (env, 0)); + + // We should be able to pay bob the base reserve one more time. + env (pay (alice, bob, reserve (env, 0) + drops(0))); + env.close(); + BEAST_EXPECT (env.balance (bob, XRP) == + (reserve (env, 0) + reserve (env, 0))); + + // bob's above the threshold again. Any payment should fail. + env (pay (alice, bob, drops(1)), ter (tecNO_PERMISSION)); + env.close(); + BEAST_EXPECT (env.balance (bob, XRP) == + (reserve (env, 0) + reserve (env, 0))); + + // Take bob back down to a zero XRP balance. + env (noop (bob), fee (env.balance (bob, XRP))); + env.close(); + BEAST_EXPECT (env.balance (bob, XRP) == drops(0)); + + // bob should not be able to clear lsfDepositAuth. + env (fclear (bob, asfDepositAuth), ter (terINSUF_FEE_B)); + env.close(); + + // We should be able to pay bob 1 drop now. + env (pay (alice, bob, drops(1))); + env.close(); + BEAST_EXPECT (env.balance (bob, XRP) == drops(1)); + + // Pay bob enough so he can afford the fee to clear lsfDepositAuth. + env (pay (alice, bob, drops(9))); + env.close(); + + // Interestingly, at this point the terINSUF_FEE_B retry grabs the + // request to clear lsfDepositAuth. So the balance should be zero + // and lsfDepositAuth should be cleared. + BEAST_EXPECT (env.balance (bob, XRP) == drops(0)); + env.require (nflags (bob, asfDepositAuth)); + + // Since bob no longer has lsfDepositAuth set we should be able to + // pay him more than the base reserve. + env (pay (alice, bob, reserve (env, 0) + drops(1))); + env.close(); + BEAST_EXPECT (env.balance (bob, XRP) == reserve (env, 0) + drops(1)); + } + + void run() override + { + testEnable(); + testPayIOU(); + testPayXRP(); + } +}; + +BEAST_DEFINE_TESTSUITE(DepositAuth,app,ripple); + +} // test +} // ripple diff --git a/src/test/app/Escrow_test.cpp b/src/test/app/Escrow_test.cpp index 05d248fea3..154cb25483 100644 --- a/src/test/app/Escrow_test.cpp +++ b/src/test/app/Escrow_test.cpp @@ -377,8 +377,58 @@ struct Escrow_test : public beast::unit_test::suite env(cancel("bob", "alice", seq), ter(tecNO_PERMISSION)); env(finish("bob", "alice", seq)); } + { + // Unconditionally pay from alice to bob. jack (neither source nor + // destination) signs all cancels and finishes. This shows that + // Escrow will make a payment to bob with no intervention from bob. + Env env(*this); + env.fund(XRP(5000), "alice", "bob", "jack"); + auto const seq = env.seq("alice"); + env(lockup("alice", "bob", XRP(1000), env.now() + 1s)); + env.require(balance("alice", XRP(4000) - drops(10))); - { // Conditional + env(cancel("jack", "alice", seq), ter(tecNO_PERMISSION)); + env(finish("jack", "alice", seq), ter(tecNO_PERMISSION)); + env.close(); + + env(cancel("jack", "alice", seq), ter(tecNO_PERMISSION)); + env(finish("jack", "alice", seq)); + env.close(); + env.require(balance("alice", XRP(4000) - drops(10))); + env.require(balance("bob", XRP(6000))); + env.require(balance("jack", XRP(5000) - drops(40))); + } + { + // bob sets PaymentAuth so only bob can finish the escrow. + Env env(*this); + + env.fund(XRP(5000), "alice", "bob", "jack"); + env(fset ("bob", asfDepositAuth)); + env.close(); + + auto const seq = env.seq("alice"); + env(lockup("alice", "bob", XRP(1000), env.now() + 1s)); + env.require(balance("alice", XRP(4000) - drops(10))); + + env(cancel("jack", "alice", seq), ter(tecNO_PERMISSION)); + env(finish("jack", "alice", seq), ter(tecNO_PERMISSION)); + env(finish("alice", "alice", seq), ter(tecNO_PERMISSION)); + env(finish("bob", "alice", seq), ter(tecNO_PERMISSION)); + env.close(); + + env(cancel("jack", "alice", seq), ter(tecNO_PERMISSION)); + env(finish("jack", "alice", seq), ter(tecNO_PERMISSION)); + env(finish("alice", "alice", seq), ter(tecNO_PERMISSION)); + env.close(); + env(finish("bob", "alice", seq)); + env.close(); + auto const baseFee = env.current()->fees().base; + env.require(balance("alice", XRP(4000) - (baseFee * 3))); + env.require(balance("bob", XRP(6000) - (baseFee * 3))); + env.require(balance("jack", XRP(5000) - (baseFee * 4))); + } + { + // Conditional Env env(*this); env.fund(XRP(5000), "alice", "bob"); auto const seq = env.seq("alice"); @@ -393,12 +443,38 @@ struct Escrow_test : public beast::unit_test::suite env(cancel("bob", "alice", seq), ter(tecNO_PERMISSION)); env(finish("bob", "alice", seq), ter(tecCRYPTOCONDITION_ERROR)); - env(finish("bob", "alice", seq), ter(tecCRYPTOCONDITION_ERROR)); + env(finish("alice", "alice", seq), ter(tecCRYPTOCONDITION_ERROR)); env.close(); env(finish("bob", "alice", seq, makeSlice(cb2), makeSlice(fb2)), fee(1500)); } + { + // Self-escrowed conditional with PaymentAuth + Env env(*this); + + env.fund(XRP(5000), "alice", "bob"); + auto const seq = env.seq("alice"); + env(lockup("alice", "alice", XRP(1000), makeSlice(cb2), env.now() + 1s)); + env.require(balance("alice", XRP(4000) - drops(10))); + + env(cancel("bob", "alice", seq), ter(tecNO_PERMISSION)); + env(finish("bob", "alice", seq), ter(tecNO_PERMISSION)); + env(finish("bob", "alice", seq, + makeSlice(cb2), makeSlice(fb2)), fee(1500), ter(tecNO_PERMISSION)); + env.close(); + + env(cancel("bob", "alice", seq), ter(tecNO_PERMISSION)); + env(finish("bob", "alice", seq), ter(tecCRYPTOCONDITION_ERROR)); + env(finish("alice", "alice", seq), ter(tecCRYPTOCONDITION_ERROR)); + env(fset ("alice", asfDepositAuth)); + env.close(); + + env(finish("bob", "alice", seq, + makeSlice(cb2), makeSlice(fb2)), fee(1500), ter(tecNO_PERMISSION)); + env(finish("alice", "alice", seq, + makeSlice(cb2), makeSlice(fb2)), fee(1500)); + } } void diff --git a/src/test/app/Flow_test.cpp b/src/test/app/Flow_test.cpp index 2c8aed69c5..35bca09038 100644 --- a/src/test/app/Flow_test.cpp +++ b/src/test/app/Flow_test.cpp @@ -2,9 +2,11 @@ /* This file is part of rippled: https://github.com/ripple/rippled Copyright (c) 2012, 2013 Ripple Labs Inc. + Permission to use, copy, modify, and/or distribute this software for any purpose with or without fee is hereby granted, provided that the above copyright notice and this permission notice appear in all copies. + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index 86dd0e1ca9..a03ba33bc2 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -691,6 +691,75 @@ struct PayChan_test : public beast::unit_test::suite *env.current (), channel (*env.current (), alice, bob))); } + void + testDepositAuth () + { + testcase ("Deposit Authorization"); + using namespace jtx; + using namespace std::literals::chrono_literals; + + auto const alice = Account ("alice"); + auto const bob = Account ("bob"); + auto USDA = alice["USD"]; + { + Env env (*this); + env.fund (XRP (10000), alice, bob); + + env (fset (bob, asfDepositAuth)); + env.close(); + + auto const pk = alice.pk (); + auto const settleDelay = 100s; + env (create (alice, bob, XRP (1000), settleDelay, pk)); + env.close(); + + auto const chan = channel (*env.current (), alice, bob); + BEAST_EXPECT (channelBalance (*env.current (), chan) == XRP (0)); + BEAST_EXPECT (channelAmount (*env.current (), chan) == XRP (1000)); + + // alice can add more funds to the channel even though bob has + // asfDepositAuth set. + env (fund (alice, chan, XRP (1000))); + env.close(); + + // alice claims. Fails because bob's lsfDepositAuth flag is set. + env (claim (alice, chan, + XRP (500).value(), XRP (500).value()), ter (tecNO_PERMISSION)); + env.close(); + + // Claim with signature + auto const baseFee = env.current()->fees().base; + auto const preBob = env.balance (bob); + { + auto const delta = XRP (500).value(); + auto const sig = signClaimAuth (pk, alice.sk (), chan, delta); + + // alice claims with signature. Fails since bob has + // lsfDepositAuth flag set. + env (claim (alice, chan, + delta, delta, Slice (sig), pk), ter (tecNO_PERMISSION)); + env.close(); + BEAST_EXPECT (env.balance (bob) == preBob); + + // bob claims with signature. Succeeds even though bob's + // lsfDepositAuth flag is set since bob signed the transaction. + env (claim (bob, chan, delta, delta, Slice (sig), pk)); + env.close(); + BEAST_EXPECT (env.balance (bob) == preBob + delta - baseFee); + } + + // bob clears lsfDepositAuth. Now alice can use an unsigned claim. + env (fclear (bob, asfDepositAuth)); + env.close(); + + // alice claims successfully. + env (claim (alice, chan, XRP (800).value(), XRP (800).value())); + env.close(); + BEAST_EXPECT ( + env.balance (bob) == preBob + XRP (800) - (2 * baseFee)); + } + } + void testMultiple () { @@ -994,6 +1063,7 @@ struct PayChan_test : public beast::unit_test::suite testDefaultAmount (); testDisallowXRP (); testDstTag (); + testDepositAuth (); testMultiple (); testRPC (); testOptionalFields (); diff --git a/src/test/jtx/flags.h b/src/test/jtx/flags.h index 4d2a826632..388a67fb9f 100644 --- a/src/test/jtx/flags.h +++ b/src/test/jtx/flags.h @@ -65,13 +65,14 @@ private: switch(flag) { case asfRequireDest: mask_ |= lsfRequireDestTag; break; - case asfRequireAuth: mask_ |= lsfRequireAuth; break; - case asfDisallowXRP: mask_ |= lsfDisallowXRP; break; - case asfDisableMaster: mask_ |= lsfDisableMaster; break; + case asfRequireAuth: mask_ |= lsfRequireAuth; break; + case asfDisallowXRP: mask_ |= lsfDisallowXRP; break; + case asfDisableMaster: mask_ |= lsfDisableMaster; break; //case asfAccountTxnID: // ??? - case asfNoFreeze: mask_ |= lsfNoFreeze; break; - case asfGlobalFreeze: mask_ |= lsfGlobalFreeze; break; - case asfDefaultRipple: mask_ |= lsfDefaultRipple; break; + case asfNoFreeze: mask_ |= lsfNoFreeze; break; + case asfGlobalFreeze: mask_ |= lsfGlobalFreeze; break; + case asfDefaultRipple: mask_ |= lsfDefaultRipple; break; + case asfDepositAuth: mask_ |= lsfDepositAuth; break; default: Throw ( "unknown flag"); diff --git a/src/test/rpc/AccountSet_test.cpp b/src/test/rpc/AccountSet_test.cpp index 6bf06bb189..a524967f5e 100644 --- a/src/test/rpc/AccountSet_test.cpp +++ b/src/test/rpc/AccountSet_test.cpp @@ -42,24 +42,72 @@ public: BEAST_EXPECT((*env.le(alice))[ sfFlags ] == 0u); } - void testSetAndReset(unsigned int flag_val, std::string const& label) + void testMostFlags() { using namespace test::jtx; - Env env(*this); Account const alice ("alice"); + + // Test without DepositAuth enabled initially. + Env env(*this, supported_amendments() - featureDepositAuth); env.fund(XRP(10000), noripple(alice)); - env.memoize("eric"); - env(regkey(alice, "eric")); - unsigned int orig_flags = (*env.le(alice))[ sfFlags ]; + // Give alice a regular key so she can legally set and clear + // her asfDisableMaster flag. + Account const alie {"alie", KeyType::secp256k1}; + env(regkey (alice, alie)); + env.close(); - env.require(nflags(alice, flag_val)); - env(fset(alice, flag_val), sig(alice)); - env.require(flags(alice, flag_val)); - env(fclear(alice, flag_val)); - env.require(nflags(alice, flag_val)); - uint32 now_flags = (*env.le(alice))[ sfFlags ]; - BEAST_EXPECT(now_flags == orig_flags); + auto testFlags = [this, &alice, &alie, &env] + (std::initializer_list goodFlags) + { + std::uint32_t const orig_flags = (*env.le(alice))[ sfFlags ]; + for (std::uint32_t flag {1u}; + flag < std::numeric_limits::digits; ++flag) + { + if (flag == asfNoFreeze) + { + // The asfNoFreeze flag can't be cleared. It is tested + // elsewhere. + continue; + } + else if (std::find (goodFlags.begin(), goodFlags.end(), flag) != + goodFlags.end()) + { + // Good flag + env.require(nflags(alice, flag)); + env(fset(alice, flag), sig(alice)); + env.close(); + env.require(flags(alice, flag)); + env(fclear(alice, flag), sig(alie)); + env.close(); + env.require(nflags(alice, flag)); + std::uint32_t const now_flags = (*env.le(alice))[ sfFlags ]; + BEAST_EXPECT(now_flags == orig_flags); + } + else + { + // Bad flag + BEAST_EXPECT((*env.le(alice))[ sfFlags ] == orig_flags); + env(fset(alice, flag), sig(alice)); + env.close(); + BEAST_EXPECT((*env.le(alice))[ sfFlags ] == orig_flags); + env(fclear(alice, flag), sig(alie)); + env.close(); + BEAST_EXPECT((*env.le(alice))[ sfFlags ] == orig_flags); + } + } + }; + + // Test with featureDepositAuth disabled. + testFlags ({asfRequireDest, asfRequireAuth, asfDisallowXRP, + asfGlobalFreeze, asfDisableMaster, asfDefaultRipple}); + + // Enable featureDepositAuth and retest. + env.enableFeature (featureDepositAuth); + env.close(); + testFlags ({asfRequireDest, asfRequireAuth, asfDisallowXRP, + asfGlobalFreeze, asfDisableMaster, asfDefaultRipple, + asfDepositAuth}); } void testSetAndResetAccountTxnID() @@ -69,7 +117,7 @@ public: Account const alice ("alice"); env.fund(XRP(10000), noripple(alice)); - unsigned int orig_flags = (*env.le(alice))[ sfFlags ]; + std::uint32_t const orig_flags = (*env.le(alice))[ sfFlags ]; // asfAccountTxnID is special and not actually set as a flag, // so we check the field presence instead @@ -78,7 +126,7 @@ public: BEAST_EXPECT(env.le(alice)->isFieldPresent(sfAccountTxnID)); env(fclear(alice, asfAccountTxnID)); BEAST_EXPECT(! env.le(alice)->isFieldPresent(sfAccountTxnID)); - uint32 now_flags = (*env.le(alice))[ sfFlags ]; + std::uint32_t const now_flags = (*env.le(alice))[ sfFlags ]; BEAST_EXPECT(now_flags == orig_flags); } @@ -393,17 +441,7 @@ public: void run() { testNullAccountSet(); - for(auto const& flag_set : std::vector>({ - {asfRequireDest, "RequireDestTag"}, - {asfRequireAuth, "RequireAuth"}, - {asfDisallowXRP, "DisallowXRP"}, - {asfGlobalFreeze, "GlobalFreeze"}, - {asfDisableMaster, "DisableMaster"}, - {asfDefaultRipple, "DefaultRipple"} - })) - { - testSetAndReset(flag_set.first, flag_set.second); - } + testMostFlags(); testSetAndResetAccountTxnID(); testSetNoFreeze(); testDomain(); diff --git a/src/test/unity/app_test_unity1.cpp b/src/test/unity/app_test_unity1.cpp index f7bfb84d54..613a81202d 100644 --- a/src/test/unity/app_test_unity1.cpp +++ b/src/test/unity/app_test_unity1.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include