Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

featureDisallowIncoming: Opt-out of incoming Checks, PayChans, NFTokenOffers and Trustlines #4336

Merged
merged 5 commits into from
Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/ripple/app/tx/impl/CreateCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,14 @@ CreateCheck::preclaim(PreclaimContext const& ctx)
return tecNO_DST;
}

if ((sleDst->getFlags() & lsfRequireDestTag) &&
!ctx.tx.isFieldPresent(sfDestinationTag))
auto const 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
// understand, but we can require someone to fill it in.
Expand Down
41 changes: 36 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,42 @@ 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;
if (auto const destination = ctx.tx[~sfDestination])
{
// If a destination is specified, the destination must already be in
// the ledger.
auto const sleDst = ctx.view.read(keylet::account(*destination));

if (!sleDst)
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

if (sleDst->getFlags() & lsfDisallowIncomingNFTOffer)
intelliot marked this conversation as resolved.
Show resolved Hide resolved
return tecNO_PERMISSION;
}
}

if (auto const owner = ctx.tx[~sfOwner])
{
// 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
13 changes: 10 additions & 3 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) &&
!ctx.tx[~sfDestinationTag])

auto const 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))
RichardAH marked this conversation as resolved.
Show resolved Hide resolved
return tecNO_TARGET;
}

Expand Down
24 changes: 24 additions & 0 deletions src/ripple/app/tx/impl/SetAccount.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,30 @@ 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 (uSetFlag == asfDisallowIncomingTrustline)
uFlagsOut |= lsfDisallowIncomingTrustline;
else if (uClearFlag == asfDisallowIncomingTrustline)
uFlagsOut &= ~lsfDisallowIncomingTrustline;
}

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

Expand Down
14 changes: 14 additions & 0 deletions src/ripple/app/tx/impl/SetTrust.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,20 @@ SetTrust::preclaim(PreclaimContext const& ctx)
}
}

// If the destination has opted to disallow incoming trustlines
// then honour that flag
if (ctx.view.rules().enabled(featureDisallowIncoming))
{
auto const sleDst = ctx.view.read(keylet::account(uDstAccountID));

if (!sleDst)
return tecNO_DST;

auto const dstFlags = sleDst->getFlags();
if (dstFlags & lsfDisallowIncomingTrustline)
return tecNO_PERMISSION;
}

return tesSUCCESS;
}

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
11 changes: 11 additions & 0 deletions src/ripple/protocol/LedgerFormats.h
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,17 @@ 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
lsfDisallowIncomingTrustline =
0x20000000, // True, reject new trustlines (only if no issued assets)

// ltOFFER
lsfPassive = 0x00010000,
Expand Down
7 changes: 7 additions & 0 deletions src/ripple/protocol/TxFlags.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,13 @@ 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;
constexpr std::uint32_t asfDisallowIncomingTrustline = 15;

// 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
98 changes: 98 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 @@ -293,6 +295,100 @@ class Check_test : public beast::unit_test::suite
BEAST_EXPECT(checksOnAccount(env, bob).size() == bobCount + 7);
}

void
testCreateDisallowIncoming(FeatureBitset features)
{
testcase("Create valid with disallow incoming");

using namespace test::jtx;

// test flag doesn't set unless amendment enabled
{
Env env{*this, features - disallowIncoming};
Account const alice{"alice"};
env.fund(XRP(10000), alice);
env(fset(alice, asfDisallowIncomingCheck));
env.close();
auto const sle = env.le(alice);
uint32_t flags = sle->getFlags();
BEAST_EXPECT(!(flags & lsfDisallowIncomingCheck));
}

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);

/*
* Attempt to create two checks from `from` to `to` and
* require they both result in error/success code `expected`
*/
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));
};

// enable the DisallowIncoming flag on both bob and alice
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);
}
RichardAH marked this conversation as resolved.
Show resolved Hide resolved

void
testCreateInvalid(FeatureBitset features)
{
Expand Down Expand Up @@ -2602,6 +2698,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 +2718,7 @@ class Check_test : public beast::unit_test::suite
using namespace test::jtx;
auto const sa = supported_amendments();
testWithFeats(sa - featureCheckCashMakesTrustLine);
testWithFeats(sa - disallowIncoming);
RichardAH marked this conversation as resolved.
Show resolved Hide resolved
testWithFeats(sa);

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