diff --git a/src/ripple/app/tx/impl/Escrow.cpp b/src/ripple/app/tx/impl/Escrow.cpp index 5600993beab..91c4fcbfc35 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; } + auto const sled = ctx_.view().peek(keylet::account((*slep)[sfDestination])); + if (ctx_.view().rules().enabled(featureDepositAuth)) + { + // NOTE: Escrow payments cannot be used to fund accounts + if (! sled) + return tecNO_DST; + + // 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,11 +477,8 @@ 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])); + // Empty shared_ptr check is here (not earlier) to preserve transaction + // compatibility through the featureDepositAuth transition. if (! sled) return tecNO_DST; diff --git a/src/ripple/app/tx/impl/PayChan.cpp b/src/ripple/app/tx/impl/PayChan.cpp index 1a14cbe6539..d8ee7f7e710 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/test/app/Escrow_test.cpp b/src/test/app/Escrow_test.cpp index 05d248fea37..154cb25483c 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))); + + env(cancel("jack", "alice", seq), ter(tecNO_PERMISSION)); + env(finish("jack", "alice", seq), ter(tecNO_PERMISSION)); + env.close(); - { // Conditional + 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,10 +443,36 @@ 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("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)); } } diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index f2f5dfc7492..8df75572cae 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 () { @@ -886,6 +955,7 @@ struct PayChan_test : public beast::unit_test::suite testDefaultAmount (); testDisallowXRP (); testDstTag (); + testDepositAuth (); testMultiple (); testRPC (); testOptionalFields ();