Skip to content

Commit

Permalink
PayChan and Escrow should ignore DisallowXRP (RIPD-1462)
Browse files Browse the repository at this point in the history
  • Loading branch information
@scottschurr
scottschurr committed Oct 3, 2017
1 parent c588d68 commit d72755d
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 45 deletions.
6 changes: 5 additions & 1 deletion src/ripple/app/tx/impl/Escrow.cpp
View file
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
16 changes: 12 additions & 4 deletions src/ripple/app/tx/impl/PayChan.cpp
View file
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,16 +461,20 @@ 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 dst set lsfDepositAuth and signer is not dst, then fail.
if (ctx_.view().rules().enabled(featureDepositAuth) &&
if (depositAuth &&
((sled->getFlags() & lsfDepositAuth) == lsfDepositAuth))
return tecNO_PERMISSION;
return tecNO_PERMISSION;
}

(*slep)[sfBalance] = ctx_.tx[sfBalance];
Expand Down
72 changes: 47 additions & 25 deletions src/test/app/Escrow_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,15 +207,15 @@ 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));
env(cancel("bob", "alice", 1), ter(temDISABLED));
}

{ // 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();
Expand All @@ -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");
Expand All @@ -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()
{
Expand All @@ -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();

Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -488,8 +512,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");
Expand Down Expand Up @@ -536,8 +559,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");
Expand All @@ -554,7 +576,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");
Expand All @@ -574,7 +596,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");
Expand Down Expand Up @@ -614,8 +636,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");
Expand Down Expand Up @@ -700,7 +721,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");
Expand Down Expand Up @@ -740,7 +761,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");
Expand Down Expand Up @@ -773,7 +794,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);

Expand All @@ -795,7 +816,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);
Expand Down Expand Up @@ -854,7 +875,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);
Expand Down Expand Up @@ -933,7 +954,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");
Expand Down Expand Up @@ -984,6 +1005,7 @@ struct Escrow_test : public beast::unit_test::suite
{
testEnablement();
testTags();
testDisallowXRP();
testFails();
testLockup();
testEscrowConditions();
Expand Down
47 changes: 32 additions & 15 deletions src/test/app/PayChan_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit d72755d

Please sign in to comment.