diff --git a/src/ripple/app/tx/impl/Escrow.cpp b/src/ripple/app/tx/impl/Escrow.cpp index 91c4fcbfc35..a3045394e3d 100644 --- a/src/ripple/app/tx/impl/Escrow.cpp +++ b/src/ripple/app/tx/impl/Escrow.cpp @@ -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; } diff --git a/src/ripple/app/tx/impl/PayChan.cpp b/src/ripple/app/tx/impl/PayChan.cpp index d8ee7f7e710..c104a70846e 100644 --- a/src/ripple/app/tx/impl/PayChan.cpp +++ b/src/ripple/app/tx/impl/PayChan.cpp @@ -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; } @@ -457,15 +461,19 @@ PayChanClaim::doApply() if (!sled) return terNO_ACCOUNT; - if (txAccount == src && (sled->getFlags() & 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 (ctx_.view().rules().enabled(featureDepositAuth) && + if (depositAuth && ((sled->getFlags() & lsfDepositAuth) == lsfDepositAuth)) - return tecNO_PERMISSION; + return tecNO_PERMISSION; } (*slep)[sfBalance] = ctx_.tx[sfBalance]; diff --git a/src/test/app/Escrow_test.cpp b/src/test/app/Escrow_test.cpp index 4d75ce1a549..0d18f45da5e 100644 --- a/src/test/app/Escrow_test.cpp +++ b/src/test/app/Escrow_test.cpp @@ -207,7 +207,7 @@ struct Escrow_test : public beast::unit_test::suite using namespace std::chrono; { // Escrow not enabled - Env env(*this, no_features); + Env env(*this, all_features_except (featureEscrow)); env.fund(XRP(5000), "alice", "bob"); env(lockup("alice", "bob", XRP(1000), env.now() + 1s), ter(temDISABLED)); env(finish("bob", "alice", 1), ter(temDISABLED)); @@ -215,7 +215,7 @@ struct Escrow_test : public beast::unit_test::suite } { // Escrow enabled - Env env(*this, with_features(featureEscrow)); + Env env(*this); env.fund(XRP(5000), "alice", "bob"); env(lockup("alice", "bob", XRP(1000), env.now() + 1s)); env.close(); @@ -237,7 +237,7 @@ struct Escrow_test : public beast::unit_test::suite using namespace jtx; using namespace std::chrono; - Env env(*this, with_features(featureEscrow)); + Env env(*this); auto const alice = Account("alice"); env.fund(XRP(5000), alice, "bob"); @@ -253,6 +253,35 @@ struct Escrow_test : public beast::unit_test::suite BEAST_EXPECT((*sle)[sfDestinationTag] == 2); } + void + testDisallowXRP() + { + testcase ("Disallow XRP"); + + using namespace jtx; + using namespace std::chrono; + + { + // Respect the "asfDisallowXRP" account flag: + Env env(*this, all_features_except (featureDepositAuth)); + + env.fund(XRP(5000), "bob", "george"); + env(fset("george", asfDisallowXRP)); + env(lockup("bob", "george", XRP(10), env.now() + 1s), + ter (tecNO_TARGET)); + } + { + // Ignore the "asfDisallowXRP" account flag, which we should + // have been doing before. + Env env(*this); + + env.fund(XRP(5000), "bob", "george"); + env(fset("george", asfDisallowXRP)); + env(lockup("bob", "george", XRP(10), env.now() + 1s)); + } + + } + void testFails() { @@ -261,7 +290,7 @@ struct Escrow_test : public beast::unit_test::suite using namespace jtx; using namespace std::chrono; - Env env(*this, with_features(featureEscrow)); + Env env(*this); env.fund(XRP(5000), "alice", "bob"); env.close(); @@ -331,11 +360,6 @@ struct Escrow_test : public beast::unit_test::suite env.fund (accountReserve, "frank"); env(lockup("frank", "bob", XRP(1), env.now() + 1s), ter (tecINSUFFICIENT_RESERVE)); - // Respect the "asfDisallowXRP" account flag: - env.fund (accountReserve + accountIncrement, "george"); - env(fset("george", asfDisallowXRP)); - env(lockup("bob", "george", XRP(10), env.now() + 1s), ter (tecNO_TARGET)); - { // Specify incorrect sequence number env.fund (XRP(5000), "hannah"); auto const seq = env.seq("hannah"); @@ -364,7 +388,7 @@ struct Escrow_test : public beast::unit_test::suite using namespace std::chrono; { // Unconditional - Env env(*this, with_features(featureEscrow)); + Env env(*this); env.fund(XRP(5000), "alice", "bob"); auto const seq = env.seq("alice"); env(lockup("alice", "alice", XRP(1000), env.now() + 1s)); @@ -429,7 +453,7 @@ struct Escrow_test : public beast::unit_test::suite } { // Conditional - Env env(*this, with_features(featureEscrow)); + 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)); @@ -487,8 +511,7 @@ struct Escrow_test : public beast::unit_test::suite using S = seconds; { // Test cryptoconditions - Env env(*this, - with_features(featureEscrow)); + Env env(*this); auto T = [&env](NetClock::duration const& d) { return env.now() + d; }; env.fund(XRP(5000), "alice", "bob", "carol"); @@ -535,8 +558,7 @@ struct Escrow_test : public beast::unit_test::suite } { // Test cancel when condition is present - Env env(*this, - with_features(featureEscrow)); + Env env(*this); auto T = [&env](NetClock::duration const& d) { return env.now() + d; }; env.fund(XRP(5000), "alice", "bob", "carol"); @@ -553,7 +575,7 @@ struct Escrow_test : public beast::unit_test::suite } { - Env env(*this, with_features(featureEscrow)); + Env env(*this); auto T = [&env](NetClock::duration const& d) { return env.now() + d; }; env.fund(XRP(5000), "alice", "bob", "carol"); @@ -573,7 +595,7 @@ struct Escrow_test : public beast::unit_test::suite } { // Test long & short conditions during creation - Env env(*this, with_features(featureEscrow)); + Env env(*this); auto T = [&env](NetClock::duration const& d) { return env.now() + d; }; env.fund(XRP(5000), "alice", "bob", "carol"); @@ -613,8 +635,7 @@ struct Escrow_test : public beast::unit_test::suite } { // Test long and short conditions & fulfillments during finish - Env env(*this, - with_features(featureEscrow)); + Env env(*this); auto T = [&env](NetClock::duration const& d) { return env.now() + d; }; env.fund(XRP(5000), "alice", "bob", "carol"); @@ -699,7 +720,7 @@ struct Escrow_test : public beast::unit_test::suite { // Test empty condition during creation and // empty condition & fulfillment during finish - Env env(*this, with_features(featureEscrow)); + Env env(*this); auto T = [&env](NetClock::duration const& d) { return env.now() + d; }; env.fund(XRP(5000), "alice", "bob", "carol"); @@ -739,7 +760,7 @@ struct Escrow_test : public beast::unit_test::suite { // Test a condition other than PreimageSha256, which // would require a separate amendment - Env env(*this, with_features(featureEscrow)); + Env env(*this); auto T = [&env](NetClock::duration const& d) { return env.now() + d; }; env.fund(XRP(5000), "alice", "bob", "carol"); @@ -772,7 +793,7 @@ struct Escrow_test : public beast::unit_test::suite { testcase ("Metadata & Ownership (without fix1523)"); - Env env(*this, with_features(featureEscrow)); + Env env(*this, all_features_except (fix1523)); env.fund(XRP(5000), alice, bruce, carol); auto const seq = env.seq(alice); @@ -794,7 +815,7 @@ struct Escrow_test : public beast::unit_test::suite { testcase ("Metadata (with fix1523, to self)"); - Env env(*this, with_features(featureEscrow, fix1523)); + Env env(*this); env.fund(XRP(5000), alice, bruce, carol); auto const aseq = env.seq(alice); auto const bseq = env.seq(bruce); @@ -853,7 +874,7 @@ struct Escrow_test : public beast::unit_test::suite { testcase ("Metadata (with fix1523, to other)"); - Env env(*this, with_features(featureEscrow, fix1523)); + Env env(*this); env.fund(XRP(5000), alice, bruce, carol); auto const aseq = env.seq(alice); auto const bseq = env.seq(bruce); @@ -932,7 +953,7 @@ struct Escrow_test : public beast::unit_test::suite using namespace jtx; using namespace std::chrono; - Env env(*this, with_features(featureEscrow)); + Env env(*this); env.memoize("alice"); env.memoize("bob"); @@ -983,6 +1004,7 @@ struct Escrow_test : public beast::unit_test::suite { testEnablement(); testTags(); + testDisallowXRP(); testFails(); testLockup(); testEscrowConditions(); diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index 8df75572cae..30ee4381da5 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -629,40 +629,57 @@ struct PayChan_test : public beast::unit_test::suite testcase ("Disallow XRP"); using namespace jtx; using namespace std::literals::chrono_literals; + + auto const alice = Account ("alice"); + auto const bob = Account ("bob"); { // Create a channel where dst disallows XRP - Env env (*this); - auto const alice = Account ("alice"); - auto const bob = Account ("bob"); + Env env (*this, all_features_except (featureDepositAuth)); env.fund (XRP (10000), alice, bob); env (fset (bob, asfDisallowXRP)); - auto const pk = alice.pk (); - auto const settleDelay = 3600s; - auto const channelFunds = XRP (1000); - env (create (alice, bob, channelFunds, settleDelay, pk), + env (create (alice, bob, XRP (1000), 3600s, alice.pk()), ter (tecNO_TARGET)); auto const chan = channel (*env.current (), alice, bob); BEAST_EXPECT (!channelExists (*env.current (), chan)); } + { + // Create a channel where dst disallows XRP. Ignore that flag, + // since it's just advisory. + Env env (*this); + env.fund (XRP (10000), alice, bob); + env (fset (bob, asfDisallowXRP)); + env (create (alice, bob, XRP (1000), 3600s, alice.pk())); + auto const chan = channel (*env.current (), alice, bob); + BEAST_EXPECT (channelExists (*env.current (), chan)); + } + { // Claim to a channel where dst disallows XRP // (channel is created before disallow xrp is set) - Env env (*this); - auto const alice = Account ("alice"); - auto const bob = Account ("bob"); + Env env (*this, all_features_except (featureDepositAuth)); env.fund (XRP (10000), alice, bob); - auto const pk = alice.pk (); - auto const settleDelay = 3600s; - auto const channelFunds = XRP (1000); - env (create (alice, bob, channelFunds, settleDelay, pk)); + env (create (alice, bob, XRP (1000), 3600s, alice.pk())); auto const chan = channel (*env.current (), alice, bob); BEAST_EXPECT (channelExists (*env.current (), chan)); env (fset (bob, asfDisallowXRP)); - auto const preBob = env.balance (bob); auto const reqBal = XRP (500).value(); env (claim (alice, chan, reqBal, reqBal), ter(tecNO_TARGET)); } + { + // Claim to a channel where dst disallows XRP (channel is + // created before disallow xrp is set). Ignore that flag + // since it is just advisory. + Env env (*this); + env.fund (XRP (10000), alice, bob); + env (create (alice, bob, XRP (1000), 3600s, alice.pk())); + auto const chan = channel (*env.current (), alice, bob); + BEAST_EXPECT (channelExists (*env.current (), chan)); + + env (fset (bob, asfDisallowXRP)); + auto const reqBal = XRP (500).value(); + env (claim (alice, chan, reqBal, reqBal)); + } } void