Skip to content

Commit

Permalink
Add to allow users to opt-out of incoming Checks, Payment Channels an…
Browse files Browse the repository at this point in the history
…d NFTokenOffers
  • Loading branch information
RichardAH committed Nov 5, 2022
1 parent ebbf4b6 commit c2ce4c7
Show file tree
Hide file tree
Showing 11 changed files with 456 additions and 67 deletions.
9 changes: 8 additions & 1 deletion src/ripple/app/tx/impl/CreateCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,14 @@ CreateCheck::preclaim(PreclaimContext const& ctx)
return tecNO_DST;
}

if ((sleDst->getFlags() & lsfRequireDestTag) &&
uint32_t flags = sleDst->getFlags();

// Check if the destination has disallowed incoming checks
if (ctx.view.rules().enabled(featureDisallowIncoming) &&
(flags & lsfDisallowIncomingCheck))
return tecNO_PERMISSION;

if ((flags & lsfRequireDestTag) &&
!ctx.tx.isFieldPresent(sfDestinationTag))
{
// The tag is basically account-specific information we don't
Expand Down
45 changes: 40 additions & 5 deletions src/ripple/app/tx/impl/NFTokenCreateOffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,46 @@ NFTokenCreateOffer::preclaim(PreclaimContext const& ctx)
return tecUNFUNDED_OFFER;
}

// If a destination is specified, the destination must already be in
// the ledger.
if (auto const destination = ctx.tx[~sfDestination];
destination && !ctx.view.exists(keylet::account(*destination)))
return tecNO_DST;
auto const destination = ctx.tx[~sfDestination];
if (destination)
{
// If a destination is specified, the destination must already be in
// the ledger.
if (!ctx.view.exists(keylet::account(*destination)))
return tecNO_DST;

// check if the destination has disallowed incoming offers
if (ctx.view.rules().enabled(featureDisallowIncoming))
{
// flag cannot be set unless amendment is enabled but
// out of an abundance of caution check anyway

auto const sleDst =
ctx.view.read(keylet::account(*destination));

if (sleDst->getFlags() & lsfDisallowIncomingNFTOffer)
return tecNO_PERMISSION;
}
}

auto const owner = ctx.tx[~sfOwner];
if (owner)
{
// Check if the owner (buy offer) has disallowed incoming offers
if (ctx.view.rules().enabled(featureDisallowIncoming))
{
auto const sleOwner =
ctx.view.read(keylet::account(*owner));

// defensively check
// it should not be possible to specify owner that doesn't exist
if (!sleOwner)
return tecNO_TARGET;

if (sleOwner->getFlags() & lsfDisallowIncomingNFTOffer)
return tecNO_PERMISSION;
}
}

return tesSUCCESS;
}
Expand Down
11 changes: 9 additions & 2 deletions src/ripple/app/tx/impl/PayChan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -217,14 +217,21 @@ PayChanCreate::preclaim(PreclaimContext const& ctx)
auto const sled = ctx.view.read(keylet::account(dst));
if (!sled)
return tecNO_DST;
if (((*sled)[sfFlags] & lsfRequireDestTag) &&
uint32_t flags = sled->getFlags();

// Check if they have disallowed incoming payment channels
if (ctx.view.rules().enabled(featureDisallowIncoming) &&
(flags & lsfDisallowIncomingPayChan))
return tecNO_PERMISSION;

if ((flags & lsfRequireDestTag) &&
!ctx.tx[~sfDestinationTag])
return tecDST_TAG_NEEDED;

// Obeying the lsfDisallowXRP flag was a bug. Piggyback on
// featureDepositAuth to remove the bug.
if (!ctx.view.rules().enabled(featureDepositAuth) &&
((*sled)[sfFlags] & lsfDisallowXRP))
(flags & lsfDisallowXRP))
return tecNO_TARGET;
}

Expand Down
37 changes: 37 additions & 0 deletions src/ripple/app/tx/impl/SetAccount.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,21 @@ SetAccount::preflight(PreflightContext const& ctx)
tx.isFieldPresent(sfNFTokenMinter))
return temMALFORMED;
}

if (ctx.rules.enabled(featureDisallowIncoming))
{
if (
( uSetFlag == asfDisallowIncomingNFTOffer &&
uClearFlag == asfDisallowIncomingNFTOffer) ||
( uSetFlag == asfDisallowIncomingCheck &&
uClearFlag == asfDisallowIncomingCheck) ||
( uSetFlag == asfDisallowIncomingPayChan &&
uClearFlag == asfDisallowIncomingPayChan))
{
JLOG(j.trace()) << "Malformed transaction: Contradictory flags set.";
return temINVALID_FLAG;
}
}

return preflight2(ctx);
}
Expand Down Expand Up @@ -538,6 +553,28 @@ SetAccount::doApply()
sle->makeFieldAbsent(sfNFTokenMinter);
}

// Set or clear flags for disallowing various incoming instruments
if (ctx_.view().rules().enabled(featureDisallowIncoming))
{
if (uSetFlag == asfDisallowIncomingNFTOffer)
uFlagsOut |= lsfDisallowIncomingNFTOffer;
else
if (uClearFlag == asfDisallowIncomingNFTOffer)
uFlagsOut &= ~lsfDisallowIncomingNFTOffer;

if (uSetFlag == asfDisallowIncomingCheck)
uFlagsOut |= lsfDisallowIncomingCheck;
else
if (uClearFlag == asfDisallowIncomingCheck)
uFlagsOut &= ~lsfDisallowIncomingCheck;

if (uSetFlag == asfDisallowIncomingPayChan)
uFlagsOut |= lsfDisallowIncomingPayChan;
else
if (uClearFlag == asfDisallowIncomingPayChan)
uFlagsOut &= ~lsfDisallowIncomingPayChan;
}

if (uFlagsIn != uFlagsOut)
sle->setFieldU32(sfFlags, uFlagsOut);

Expand Down
3 changes: 2 additions & 1 deletion src/ripple/protocol/Feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ namespace detail {
// Feature.cpp. Because it's only used to reserve storage, and determine how
// large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
// the actual number of amendments. A LogicError on startup will verify this.
static constexpr std::size_t numFeatures = 53;
static constexpr std::size_t numFeatures = 54;

/** Amendments that this server supports and the default voting behavior.
Whether they are enabled depends on the Rules defined in the validated
Expand Down Expand Up @@ -340,6 +340,7 @@ extern uint256 const featureNonFungibleTokensV1_1;
extern uint256 const fixTrustLinesToSelf;
extern uint256 const fixRemoveNFTokenAutoTrustLine;
extern uint256 const featureImmediateOfferKilled;
extern uint256 const featureDisallowIncoming;

} // namespace ripple

Expand Down
9 changes: 9 additions & 0 deletions src/ripple/protocol/LedgerFormats.h
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,15 @@ enum LedgerSpecificFlags {
lsfDefaultRipple =
0x00800000, // True, trust lines allow rippling by default
lsfDepositAuth = 0x01000000, // True, all deposits require authorization
/* // reserved for Hooks amendment
lsfTshCollect = 0x02000000, // True, allow TSH collect-calls to acc hooks
*/
lsfDisallowIncomingNFTOffer =
0x04000000, // True, reject new incoming NFT offers
lsfDisallowIncomingCheck =
0x08000000, // True, reject new checks
lsfDisallowIncomingPayChan =
0x10000000, // True, reject new paychans

// ltOFFER
lsfPassive = 0x00010000,
Expand Down
6 changes: 6 additions & 0 deletions src/ripple/protocol/TxFlags.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@ constexpr std::uint32_t asfGlobalFreeze = 7;
constexpr std::uint32_t asfDefaultRipple = 8;
constexpr std::uint32_t asfDepositAuth = 9;
constexpr std::uint32_t asfAuthorizedNFTokenMinter = 10;
/* // reserved for Hooks amendment
constexpr std::uint32_t asfTshCollect = 11;
*/
constexpr std::uint32_t asfDisallowIncomingNFTOffer = 12;
constexpr std::uint32_t asfDisallowIncomingCheck = 13;
constexpr std::uint32_t asfDisallowIncomingPayChan = 14;

// OfferCreate flags:
constexpr std::uint32_t tfPassive = 0x00010000;
Expand Down
1 change: 1 addition & 0 deletions src/ripple/protocol/impl/Feature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,7 @@ REGISTER_FEATURE(NonFungibleTokensV1_1, Supported::yes, DefaultVote::no)
REGISTER_FIX (fixTrustLinesToSelf, Supported::yes, DefaultVote::no);
REGISTER_FIX (fixRemoveNFTokenAutoTrustLine, Supported::yes, DefaultVote::yes);
REGISTER_FEATURE(ImmediateOfferKilled, Supported::yes, DefaultVote::no);
REGISTER_FEATURE(DisallowIncoming, Supported::yes, DefaultVote::no);

// The following amendments have been active for at least two years. Their
// pre-amendment code has been removed and the identifiers are deprecated.
Expand Down
85 changes: 85 additions & 0 deletions src/test/app/Check_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ class dest_tag

class Check_test : public beast::unit_test::suite
{
FeatureBitset const disallowIncoming{featureDisallowIncoming};

static uint256
getCheckIndex(AccountID const& account, std::uint32_t uSequence)
{
Expand Down Expand Up @@ -292,6 +294,87 @@ class Check_test : public beast::unit_test::suite
BEAST_EXPECT(checksOnAccount(env, alice).size() == aliceCount + 7);
BEAST_EXPECT(checksOnAccount(env, bob).size() == bobCount + 7);
}

void
testCreateDisallowIncoming(FeatureBitset features)
{
// Explore many of the valid ways to create a check.
testcase("Create valid with disallow incoming");

using namespace test::jtx;

Account const gw{"gateway"};
Account const alice{"alice"};
Account const bob{"bob"};
IOU const USD{gw["USD"]};

Env env{*this, features | disallowIncoming};

STAmount const startBalance{XRP(1000).value()};
env.fund(startBalance, gw, alice, bob);

// Note that no trust line has been set up for alice, but alice can
// still write a check for USD. You don't have to have the funds
// necessary to cover a check in order to write a check.
auto writeTwoChecksDI = [&env, &USD, this](
Account const& from, Account const& to, TER expected) {
std::uint32_t const fromOwnerCount{ownerCount(env, from)};
std::uint32_t const toOwnerCount{ownerCount(env, to)};

std::size_t const fromCkCount{checksOnAccount(env, from).size()};
std::size_t const toCkCount{checksOnAccount(env, to).size()};

env(check::create(from, to, XRP(2000)), ter(expected));
env.close();

env(check::create(from, to, USD(50)), ter(expected));
env.close();

if (expected == tesSUCCESS)
{
BEAST_EXPECT(checksOnAccount(env, from).size() == fromCkCount + 2);
BEAST_EXPECT(checksOnAccount(env, to).size() == toCkCount + 2);

env.require(owners(from, fromOwnerCount + 2));
env.require(
owners(to, to == from ? fromOwnerCount + 2 : toOwnerCount));
return;
}

BEAST_EXPECT(checksOnAccount(env, from).size() == fromCkCount);
BEAST_EXPECT(checksOnAccount(env, to).size() == toCkCount);

env.require(owners(from, fromOwnerCount));
env.require(
owners(to, to == from ? fromOwnerCount : toOwnerCount));
};
// from to

env(fset(bob, asfDisallowIncomingCheck));
env(fset(alice, asfDisallowIncomingCheck));
env.close();

// both alice and bob can't receive checks
writeTwoChecksDI(alice, bob, tecNO_PERMISSION);
writeTwoChecksDI(gw, alice, tecNO_PERMISSION);


// remove the flag from alice but not from bob
env(fclear(alice, asfDisallowIncomingCheck));
env.close();

// now bob can send alice a cheque but not visa-versa
writeTwoChecksDI(bob, alice, tesSUCCESS);
writeTwoChecksDI(alice, bob, tecNO_PERMISSION);

// remove bob's flag too
env(fclear(bob, asfDisallowIncomingCheck));
env.close();

// now they can send checks freely
writeTwoChecksDI(bob, alice, tesSUCCESS);
writeTwoChecksDI(alice, bob, tesSUCCESS);
}

void
testCreateInvalid(FeatureBitset features)
Expand Down Expand Up @@ -2602,6 +2685,7 @@ class Check_test : public beast::unit_test::suite
{
testEnabled(features);
testCreateValid(features);
testCreateDisallowIncoming(features);
testCreateInvalid(features);
testCashXRP(features);
testCashIOU(features);
Expand All @@ -2621,6 +2705,7 @@ class Check_test : public beast::unit_test::suite
using namespace test::jtx;
auto const sa = supported_amendments();
testWithFeats(sa - featureCheckCashMakesTrustLine);
testWithFeats(sa - disallowIncoming);
testWithFeats(sa);

testTrustLineCreation(sa); // Test with featureCheckCashMakesTrustLine
Expand Down
Loading

0 comments on commit c2ce4c7

Please sign in to comment.