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 Dec 18, 2017
1 parent 9388f98 commit 733422c
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 25 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,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];
Expand Down
35 changes: 30 additions & 5 deletions src/test/app/Escrow_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, supported_amendments() - 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 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 @@ -980,6 +1004,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, supported_amendments() - 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, supported_amendments() - 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 733422c

Please sign in to comment.