From 9b865b86835bdd6eb284e148d380b1d3f90e1a76 Mon Sep 17 00:00:00 2001 From: RichardAH Date: Tue, 20 Dec 2022 02:35:35 +0100 Subject: [PATCH] `featureDisallowIncoming`: Opt-out of incoming Checks, PayChans, NFTokenOffers and Trustlines (#4336) featureDisallowIncoming is a new amendment that would allow users to opt-out of incoming Checks, Payment Channels, NFTokenOffers, and trust lines. This commit includes tests. Adds four new AccountSet Flags: 1. asfDisallowIncomingNFTOffer 2. asfDisallowIncomingCheck 3. asfDisallowIncomingPayChan 4. asfDisallowIncomingTrustline --- src/ripple/app/tx/impl/CreateCheck.cpp | 10 +- src/ripple/app/tx/impl/NFTokenCreateOffer.cpp | 41 +++- src/ripple/app/tx/impl/PayChan.cpp | 13 +- src/ripple/app/tx/impl/SetAccount.cpp | 24 ++ src/ripple/app/tx/impl/SetTrust.cpp | 14 ++ src/ripple/protocol/Feature.h | 3 +- src/ripple/protocol/LedgerFormats.h | 11 + src/ripple/protocol/TxFlags.h | 7 + src/ripple/protocol/impl/Feature.cpp | 1 + src/test/app/Check_test.cpp | 98 ++++++++ src/test/app/NFToken_test.cpp | 133 ++++++++++ src/test/app/PayChan_test.cpp | 230 ++++++++++++------ src/test/app/SetTrust_test.cpp | 143 +++++++++-- src/test/rpc/AccountSet_test.cpp | 15 +- 14 files changed, 644 insertions(+), 99 deletions(-) diff --git a/src/ripple/app/tx/impl/CreateCheck.cpp b/src/ripple/app/tx/impl/CreateCheck.cpp index a59a7c12eba..f5c2cbfbfd9 100644 --- a/src/ripple/app/tx/impl/CreateCheck.cpp +++ b/src/ripple/app/tx/impl/CreateCheck.cpp @@ -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. diff --git a/src/ripple/app/tx/impl/NFTokenCreateOffer.cpp b/src/ripple/app/tx/impl/NFTokenCreateOffer.cpp index 80e4c3964a7..695efdd0aa4 100644 --- a/src/ripple/app/tx/impl/NFTokenCreateOffer.cpp +++ b/src/ripple/app/tx/impl/NFTokenCreateOffer.cpp @@ -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) + 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; } diff --git a/src/ripple/app/tx/impl/PayChan.cpp b/src/ripple/app/tx/impl/PayChan.cpp index aab3dcc5a6b..1667bddcdb1 100644 --- a/src/ripple/app/tx/impl/PayChan.cpp +++ b/src/ripple/app/tx/impl/PayChan.cpp @@ -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)) return tecNO_TARGET; } diff --git a/src/ripple/app/tx/impl/SetAccount.cpp b/src/ripple/app/tx/impl/SetAccount.cpp index 85fe290ca55..5c7d4369a76 100644 --- a/src/ripple/app/tx/impl/SetAccount.cpp +++ b/src/ripple/app/tx/impl/SetAccount.cpp @@ -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); diff --git a/src/ripple/app/tx/impl/SetTrust.cpp b/src/ripple/app/tx/impl/SetTrust.cpp index 23af19c7b15..acbbedabf10 100644 --- a/src/ripple/app/tx/impl/SetTrust.cpp +++ b/src/ripple/app/tx/impl/SetTrust.cpp @@ -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; } diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 949d7d49578..34cc52bb1e0 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -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 = 54; +static constexpr std::size_t numFeatures = 55; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated @@ -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; extern uint256 const featureXChainBridge; } // namespace ripple diff --git a/src/ripple/protocol/LedgerFormats.h b/src/ripple/protocol/LedgerFormats.h index 697ebcaf8e3..84b4b698d1c 100644 --- a/src/ripple/protocol/LedgerFormats.h +++ b/src/ripple/protocol/LedgerFormats.h @@ -251,6 +251,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, diff --git a/src/ripple/protocol/TxFlags.h b/src/ripple/protocol/TxFlags.h index dd1378c6466..58ef0cf62ff 100644 --- a/src/ripple/protocol/TxFlags.h +++ b/src/ripple/protocol/TxFlags.h @@ -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; diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index ea7f26527b6..8a5c5a1bc4c 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -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); REGISTER_FEATURE(XChainBridge, Supported::no, DefaultVote::no); // The following amendments have been active for at least two years. Their diff --git a/src/test/app/Check_test.cpp b/src/test/app/Check_test.cpp index 31a2e572e70..8f0c0ec46b8 100644 --- a/src/test/app/Check_test.cpp +++ b/src/test/app/Check_test.cpp @@ -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) { @@ -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); + } + void testCreateInvalid(FeatureBitset features) { @@ -2602,6 +2698,7 @@ class Check_test : public beast::unit_test::suite { testEnabled(features); testCreateValid(features); + testCreateDisallowIncoming(features); testCreateInvalid(features); testCashXRP(features); testCashIOU(features); @@ -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); testWithFeats(sa); testTrustLineCreation(sa); // Test with featureCheckCashMakesTrustLine diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 2fb27f8a352..42a6eb4d3ce 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -29,6 +29,8 @@ namespace ripple { class NFToken_test : public beast::unit_test::suite { + FeatureBitset const disallowIncoming{featureDisallowIncoming}; + // Helper function that returns the owner count of an account root. static std::uint32_t ownerCount(test::jtx::Env const& env, test::jtx::Account const& acct) @@ -2975,6 +2977,135 @@ class NFToken_test : public beast::unit_test::suite } } + void + testCreateOfferDestinationDisallowIncoming(FeatureBitset features) + { + testcase("Create offer destination 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, asfDisallowIncomingNFTOffer)); + env.close(); + auto const sle = env.le(alice); + uint32_t flags = sle->getFlags(); + BEAST_EXPECT(!(flags & lsfDisallowIncomingNFTOffer)); + } + + Env env{*this, features | disallowIncoming}; + + Account const issuer{"issuer"}; + Account const minter{"minter"}; + Account const buyer{"buyer"}; + Account const alice{"alice"}; + + env.fund(XRP(1000), issuer, minter, buyer, alice); + + env(token::setMinter(issuer, minter)); + env.close(); + + uint256 const nftokenID = + token::getNextID(env, issuer, 0, tfTransferable); + env(token::mint(minter, 0), + token::issuer(issuer), + txflags(tfTransferable)); + env.close(); + + // enable flag + env(fset(buyer, asfDisallowIncomingNFTOffer)); + env.close(); + + // a sell offer from the minter to the buyer should be rejected + { + env(token::createOffer(minter, nftokenID, drops(1)), + token::destination(buyer), + txflags(tfSellNFToken), + ter(tecNO_PERMISSION)); + env.close(); + BEAST_EXPECT(ownerCount(env, issuer) == 0); + BEAST_EXPECT(ownerCount(env, minter) == 1); + BEAST_EXPECT(ownerCount(env, buyer) == 0); + } + + // disable the flag + env(fclear(buyer, asfDisallowIncomingNFTOffer)); + env.close(); + + // create offer (allowed now) then cancel + { + uint256 const offerIndex = + keylet::nftoffer(minter, env.seq(minter)).key; + + env(token::createOffer(minter, nftokenID, drops(1)), + token::destination(buyer), + txflags(tfSellNFToken)); + env.close(); + + env(token::cancelOffer(minter, {offerIndex})); + env.close(); + } + + // create offer, enable flag, then cancel + { + uint256 const offerIndex = + keylet::nftoffer(minter, env.seq(minter)).key; + + env(token::createOffer(minter, nftokenID, drops(1)), + token::destination(buyer), + txflags(tfSellNFToken)); + env.close(); + + env(fset(buyer, asfDisallowIncomingNFTOffer)); + env.close(); + + env(token::cancelOffer(minter, {offerIndex})); + env.close(); + + env(fclear(buyer, asfDisallowIncomingNFTOffer)); + env.close(); + } + + // create offer then transfer + { + uint256 const offerIndex = + keylet::nftoffer(minter, env.seq(minter)).key; + + env(token::createOffer(minter, nftokenID, drops(1)), + token::destination(buyer), + txflags(tfSellNFToken)); + env.close(); + + env(token::acceptSellOffer(buyer, offerIndex)); + env.close(); + } + + // buyer now owns the token + + // enable flag again + env(fset(buyer, asfDisallowIncomingNFTOffer)); + env.close(); + + // a random offer to buy the token + { + env(token::createOffer(alice, nftokenID, drops(1)), + token::owner(buyer), + ter(tecNO_PERMISSION)); + env.close(); + } + + // minter offer to buy the token + { + env(token::createOffer(minter, nftokenID, drops(1)), + token::owner(buyer), + ter(tecNO_PERMISSION)); + env.close(); + } + } + void testCreateOfferExpiration(FeatureBitset features) { @@ -4929,6 +5060,7 @@ class NFToken_test : public beast::unit_test::suite testMintTaxon(features); testMintURI(features); testCreateOfferDestination(features); + testCreateOfferDestinationDisallowIncoming(features); testCreateOfferExpiration(features); testCancelOffers(features); testCancelTooManyOffers(features); @@ -4949,6 +5081,7 @@ class NFToken_test : public beast::unit_test::suite FeatureBitset const fixNFTDir{fixNFTokenDirV1}; testWithFeats(all - fixNFTDir); + testWithFeats(all - disallowIncoming); testWithFeats(all); } }; diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index cf600a9fc87..2a8ea360e6c 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -32,6 +32,8 @@ namespace ripple { namespace test { struct PayChan_test : public beast::unit_test::suite { + FeatureBitset const disallowIncoming{featureDisallowIncoming}; + static uint256 channel( jtx::Account const& account, @@ -175,12 +177,12 @@ struct PayChan_test : public beast::unit_test::suite } void - testSimple() + testSimple(FeatureBitset features) { testcase("simple"); using namespace jtx; using namespace std::literals::chrono_literals; - Env env(*this); + Env env{*this, features}; auto const alice = Account("alice"); auto const bob = Account("bob"); auto USDA = alice["USD"]; @@ -350,7 +352,91 @@ struct PayChan_test : public beast::unit_test::suite } void - testCancelAfter() + testDisallowIncoming(FeatureBitset features) + { + testcase("Disallow Incoming Flag"); + using namespace 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, asfDisallowIncomingPayChan)); + env.close(); + auto const sle = env.le(alice); + uint32_t flags = sle->getFlags(); + BEAST_EXPECT(!(flags & lsfDisallowIncomingPayChan)); + } + + using namespace std::literals::chrono_literals; + Env env{*this, features | disallowIncoming}; + auto const alice = Account("alice"); + auto const bob = Account("bob"); + auto const cho = Account("cho"); + env.fund(XRP(10000), alice, bob, cho); + auto const pk = alice.pk(); + auto const settleDelay = 100s; + + // set flag on bob only + env(fset(bob, asfDisallowIncomingPayChan)); + env.close(); + + // channel creation from alice to bob is disallowed + { + auto const chan = channel(alice, bob, env.seq(alice)); + env(create(alice, bob, XRP(1000), settleDelay, pk), + ter(tecNO_PERMISSION)); + BEAST_EXPECT(!channelExists(*env.current(), chan)); + } + + // set flag on alice also + env(fset(alice, asfDisallowIncomingPayChan)); + env.close(); + + // channel creation from bob to alice is now disallowed + { + auto const chan = channel(bob, alice, env.seq(bob)); + env(create(bob, alice, XRP(1000), settleDelay, pk), + ter(tecNO_PERMISSION)); + BEAST_EXPECT(!channelExists(*env.current(), chan)); + } + + // remove flag from bob + env(fclear(bob, asfDisallowIncomingPayChan)); + env.close(); + + // now the channel between alice and bob can exist + { + auto const chan = channel(alice, bob, env.seq(alice)); + env(create(alice, bob, XRP(1000), settleDelay, pk), + ter(tesSUCCESS)); + BEAST_EXPECT(channelExists(*env.current(), chan)); + } + + // a channel from cho to alice isn't allowed + { + auto const chan = channel(cho, alice, env.seq(cho)); + env(create(cho, alice, XRP(1000), settleDelay, pk), + ter(tecNO_PERMISSION)); + BEAST_EXPECT(!channelExists(*env.current(), chan)); + } + + // remove flag from alice + env(fclear(alice, asfDisallowIncomingPayChan)); + env.close(); + + // now a channel from cho to alice is allowed + { + auto const chan = channel(cho, alice, env.seq(cho)); + env(create(cho, alice, XRP(1000), settleDelay, pk), + ter(tesSUCCESS)); + BEAST_EXPECT(channelExists(*env.current(), chan)); + } + } + + void + testCancelAfter(FeatureBitset features) { testcase("cancel after"); using namespace jtx; @@ -360,7 +446,7 @@ struct PayChan_test : public beast::unit_test::suite auto const carol = Account("carol"); { // If dst claims after cancel after, channel closes - Env env(*this); + Env env{*this, features}; env.fund(XRP(10000), alice, bob); auto const pk = alice.pk(); auto const settleDelay = 100s; @@ -392,7 +478,7 @@ struct PayChan_test : public beast::unit_test::suite } { // Third party can close after cancel after - Env env(*this); + Env env{*this, features}; env.fund(XRP(10000), alice, bob, carol); auto const pk = alice.pk(); auto const settleDelay = 100s; @@ -415,12 +501,12 @@ struct PayChan_test : public beast::unit_test::suite } void - testExpiration() + testExpiration(FeatureBitset features) { testcase("expiration"); using namespace jtx; using namespace std::literals::chrono_literals; - Env env(*this); + Env env{*this, features}; auto const alice = Account("alice"); auto const bob = Account("bob"); auto const carol = Account("carol"); @@ -481,12 +567,12 @@ struct PayChan_test : public beast::unit_test::suite } void - testSettleDelay() + testSettleDelay(FeatureBitset features) { testcase("settle delay"); using namespace jtx; using namespace std::literals::chrono_literals; - Env env(*this); + Env env{*this, features}; auto const alice = Account("alice"); auto const bob = Account("bob"); env.fund(XRP(10000), alice, bob); @@ -541,12 +627,12 @@ struct PayChan_test : public beast::unit_test::suite } void - testCloseDry() + testCloseDry(FeatureBitset features) { testcase("close dry"); using namespace jtx; using namespace std::literals::chrono_literals; - Env env(*this); + Env env{*this, features}; auto const alice = Account("alice"); auto const bob = Account("bob"); env.fund(XRP(10000), alice, bob); @@ -575,13 +661,13 @@ struct PayChan_test : public beast::unit_test::suite } void - testDefaultAmount() + testDefaultAmount(FeatureBitset features) { // auth amount defaults to balance if not present testcase("default amount"); using namespace jtx; using namespace std::literals::chrono_literals; - Env env(*this); + Env env{*this, features}; auto const alice = Account("alice"); auto const bob = Account("bob"); env.fund(XRP(10000), alice, bob); @@ -630,7 +716,7 @@ struct PayChan_test : public beast::unit_test::suite } void - testDisallowXRP() + testDisallowXRP(FeatureBitset features) { // auth amount defaults to balance if not present testcase("Disallow XRP"); @@ -641,7 +727,7 @@ struct PayChan_test : public beast::unit_test::suite auto const bob = Account("bob"); { // Create a channel where dst disallows XRP - Env env(*this, supported_amendments() - featureDepositAuth); + Env env(*this, features - featureDepositAuth); env.fund(XRP(10000), alice, bob); env(fset(bob, asfDisallowXRP)); auto const chan = channel(alice, bob, env.seq(alice)); @@ -652,7 +738,7 @@ struct PayChan_test : public beast::unit_test::suite { // Create a channel where dst disallows XRP. Ignore that flag, // since it's just advisory. - Env env(*this); + Env env{*this, features}; env.fund(XRP(10000), alice, bob); env(fset(bob, asfDisallowXRP)); auto const chan = channel(alice, bob, env.seq(alice)); @@ -663,7 +749,7 @@ struct PayChan_test : public beast::unit_test::suite { // Claim to a channel where dst disallows XRP // (channel is created before disallow xrp is set) - Env env(*this, supported_amendments() - featureDepositAuth); + Env env(*this, features - featureDepositAuth); env.fund(XRP(10000), alice, bob); auto const chan = channel(alice, bob, env.seq(alice)); env(create(alice, bob, XRP(1000), 3600s, alice.pk())); @@ -677,7 +763,7 @@ struct PayChan_test : public beast::unit_test::suite // 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 env{*this, features}; env.fund(XRP(10000), alice, bob); auto const chan = channel(alice, bob, env.seq(alice)); env(create(alice, bob, XRP(1000), 3600s, alice.pk())); @@ -690,14 +776,14 @@ struct PayChan_test : public beast::unit_test::suite } void - testDstTag() + testDstTag(FeatureBitset features) { // auth amount defaults to balance if not present testcase("Dst Tag"); using namespace jtx; using namespace std::literals::chrono_literals; // Create a channel where dst disallows XRP - Env env(*this); + Env env{*this, features}; auto const alice = Account("alice"); auto const bob = Account("bob"); env.fund(XRP(10000), alice, bob); @@ -720,7 +806,7 @@ struct PayChan_test : public beast::unit_test::suite } void - testDepositAuth() + testDepositAuth(FeatureBitset features) { testcase("Deposit Authorization"); using namespace jtx; @@ -731,7 +817,7 @@ struct PayChan_test : public beast::unit_test::suite auto const carol = Account("carol"); auto USDA = alice["USD"]; { - Env env(*this); + Env env{*this, features}; env.fund(XRP(10000), alice, bob, carol); env(fset(bob, asfDepositAuth)); @@ -844,13 +930,13 @@ struct PayChan_test : public beast::unit_test::suite } void - testMultiple() + testMultiple(FeatureBitset features) { // auth amount defaults to balance if not present testcase("Multiple channels to the same account"); using namespace jtx; using namespace std::literals::chrono_literals; - Env env(*this); + Env env{*this, features}; auto const alice = Account("alice"); auto const bob = Account("bob"); env.fund(XRP(10000), alice, bob); @@ -867,13 +953,13 @@ struct PayChan_test : public beast::unit_test::suite } void - testAccountChannelsRPC() + testAccountChannelsRPC(FeatureBitset features) { testcase("AccountChannels RPC"); using namespace jtx; using namespace std::literals::chrono_literals; - Env env(*this); + Env env{*this, features}; auto const alice = Account("alice"); auto const bob = Account("bob"); auto const charlie = Account("charlie", KeyType::ed25519); @@ -922,7 +1008,7 @@ struct PayChan_test : public beast::unit_test::suite } void - testAccountChannelsRPCMarkers() + testAccountChannelsRPCMarkers(FeatureBitset features) { testcase("Account channels RPC markers"); @@ -941,7 +1027,7 @@ struct PayChan_test : public beast::unit_test::suite return r; }(); - Env env(*this); + Env env{*this, features}; env.fund(XRP(10000), alice); for (auto const& a : bobs) { @@ -1038,7 +1124,7 @@ struct PayChan_test : public beast::unit_test::suite } void - testAccountChannelsRPCSenderOnly() + testAccountChannelsRPCSenderOnly(FeatureBitset features) { // Check that the account_channels command only returns channels owned // by the account @@ -1049,7 +1135,7 @@ struct PayChan_test : public beast::unit_test::suite auto const alice = Account("alice"); auto const bob = Account("bob"); - Env env(*this); + Env env{*this, features}; env.fund(XRP(10000), alice, bob); // Create a channel from alice to bob and from bob to alice @@ -1075,12 +1161,12 @@ struct PayChan_test : public beast::unit_test::suite } void - testAuthVerifyRPC() + testAuthVerifyRPC(FeatureBitset features) { testcase("PayChan Auth/Verify RPC"); using namespace jtx; using namespace std::literals::chrono_literals; - Env env(*this); + Env env{*this, features}; auto const alice = Account("alice"); auto const bob = Account("bob"); auto const charlie = Account("charlie", KeyType::ed25519); @@ -1415,12 +1501,12 @@ struct PayChan_test : public beast::unit_test::suite } void - testOptionalFields() + testOptionalFields(FeatureBitset features) { testcase("Optional Fields"); using namespace jtx; using namespace std::literals::chrono_literals; - Env env(*this); + Env env{*this, features}; auto const alice = Account("alice"); auto const bob = Account("bob"); auto const carol = Account("carol"); @@ -1466,12 +1552,12 @@ struct PayChan_test : public beast::unit_test::suite } void - testMalformedPK() + testMalformedPK(FeatureBitset features) { testcase("malformed pk"); using namespace jtx; using namespace std::literals::chrono_literals; - Env env(*this); + Env env{*this, features}; auto const alice = Account("alice"); auto const bob = Account("bob"); auto USDA = alice["USD"]; @@ -1536,7 +1622,7 @@ struct PayChan_test : public beast::unit_test::suite } void - testMetaAndOwnership() + testMetaAndOwnership(FeatureBitset features) { testcase("Metadata & Ownership"); @@ -1565,8 +1651,7 @@ struct PayChan_test : public beast::unit_test::suite { // Test without adding the paychan to the recipient's owner // directory - Env env( - *this, supported_amendments() - fixPayChanRecipientOwnerDir); + Env env(*this, features - fixPayChanRecipientOwnerDir); env.fund(XRP(10000), alice, bob); env(create(alice, bob, XRP(1000), settleDelay, pk)); env.close(); @@ -1587,7 +1672,7 @@ struct PayChan_test : public beast::unit_test::suite { // Test with adding the paychan to the recipient's owner directory - Env env(*this); + Env env{*this, features}; env.fund(XRP(10000), alice, bob); env(create(alice, bob, XRP(1000), settleDelay, pk)); env.close(); @@ -1609,8 +1694,7 @@ struct PayChan_test : public beast::unit_test::suite { // Test removing paychans created before adding to the recipient's // owner directory - Env env( - *this, supported_amendments() - fixPayChanRecipientOwnerDir); + Env env(*this, features - fixPayChanRecipientOwnerDir); env.fund(XRP(10000), alice, bob); // create the channel before the amendment activates env(create(alice, bob, XRP(1000), settleDelay, pk)); @@ -1644,7 +1728,7 @@ struct PayChan_test : public beast::unit_test::suite } void - testAccountDelete() + testAccountDelete(FeatureBitset features) { testcase("Account Delete"); using namespace test::jtx; @@ -1678,8 +1762,8 @@ struct PayChan_test : public beast::unit_test::suite for (bool const withOwnerDirFix : {false, true}) { auto const amd = withOwnerDirFix - ? supported_amendments() - : supported_amendments() - fixPayChanRecipientOwnerDir; + ? features + : features - fixPayChanRecipientOwnerDir; Env env{*this, amd}; env.fund(XRP(10000), alice, bob, carol); env.close(); @@ -1771,8 +1855,7 @@ struct PayChan_test : public beast::unit_test::suite { // test resurrected account - Env env{ - *this, supported_amendments() - fixPayChanRecipientOwnerDir}; + Env env{*this, features - fixPayChanRecipientOwnerDir}; env.fund(XRP(10000), alice, bob, carol); env.close(); auto const feeDrops = env.current()->fees().base; @@ -1878,12 +1961,12 @@ struct PayChan_test : public beast::unit_test::suite } void - testUsingTickets() + testUsingTickets(FeatureBitset features) { testcase("using tickets"); using namespace jtx; using namespace std::literals::chrono_literals; - Env env(*this); + Env env{*this, features}; auto const alice = Account("alice"); auto const bob = Account("bob"); auto USDA = alice["USD"]; @@ -2039,28 +2122,39 @@ struct PayChan_test : public beast::unit_test::suite BEAST_EXPECT(env.seq(bob) == bobSeq); } + void + testWithFeats(FeatureBitset features) + { + testSimple(features); + testDisallowIncoming(features); + testCancelAfter(features); + testSettleDelay(features); + testExpiration(features); + testCloseDry(features); + testDefaultAmount(features); + testDisallowXRP(features); + testDstTag(features); + testDepositAuth(features); + testMultiple(features); + testAccountChannelsRPC(features); + testAccountChannelsRPCMarkers(features); + testAccountChannelsRPCSenderOnly(features); + testAuthVerifyRPC(features); + testOptionalFields(features); + testMalformedPK(features); + testMetaAndOwnership(features); + testAccountDelete(features); + testUsingTickets(features); + } + +public: void run() override { - testSimple(); - testCancelAfter(); - testSettleDelay(); - testExpiration(); - testCloseDry(); - testDefaultAmount(); - testDisallowXRP(); - testDstTag(); - testDepositAuth(); - testMultiple(); - testAccountChannelsRPC(); - testAccountChannelsRPCMarkers(); - testAccountChannelsRPCSenderOnly(); - testAuthVerifyRPC(); - testOptionalFields(); - testMalformedPK(); - testMetaAndOwnership(); - testAccountDelete(); - testUsingTickets(); + using namespace test::jtx; + FeatureBitset const all{supported_amendments()}; + testWithFeats(all - disallowIncoming); + testWithFeats(all); } }; diff --git a/src/test/app/SetTrust_test.cpp b/src/test/app/SetTrust_test.cpp index 45a9e5c767e..fce9c4295c2 100644 --- a/src/test/app/SetTrust_test.cpp +++ b/src/test/app/SetTrust_test.cpp @@ -26,9 +26,14 @@ namespace test { class SetTrust_test : public beast::unit_test::suite { + FeatureBitset const disallowIncoming{featureDisallowIncoming}; + public: void - testFreeTrustlines(bool thirdLineCreatesLE, bool createOnHighAcct) + testFreeTrustlines( + FeatureBitset features, + bool thirdLineCreatesLE, + bool createOnHighAcct) { if (thirdLineCreatesLE) testcase("Allow two free trustlines"); @@ -36,7 +41,7 @@ class SetTrust_test : public beast::unit_test::suite testcase("Dynamic reserve for trustline"); using namespace jtx; - Env env(*this); + Env env(*this, features); auto const gwA = Account{"gwA"}; auto const gwB = Account{"gwB"}; @@ -107,14 +112,14 @@ class SetTrust_test : public beast::unit_test::suite } void - testTicketSetTrust() + testTicketSetTrust(FeatureBitset features) { testcase("SetTrust using a ticket"); using namespace jtx; // Verify that TrustSet transactions can use tickets. - Env env{*this}; + Env env{*this, features}; auto const gw = Account{"gateway"}; auto const alice = Account{"alice"}; auto const USD = gw["USD"]; @@ -152,12 +157,12 @@ class SetTrust_test : public beast::unit_test::suite } void - testMalformedTransaction() + testMalformedTransaction(FeatureBitset features) { testcase("SetTrust checks for malformed transactions"); using namespace jtx; - Env env{*this}; + Env env{*this, features}; auto const gw = Account{"gateway"}; auto const alice = Account{"alice"}; @@ -199,14 +204,17 @@ class SetTrust_test : public beast::unit_test::suite } void - testModifyQualityOfTrustline(bool createQuality, bool createOnHighAcct) + testModifyQualityOfTrustline( + FeatureBitset features, + bool createQuality, + bool createOnHighAcct) { testcase << "SetTrust " << (createQuality ? "creates" : "removes") << " quality of trustline for " << (createOnHighAcct ? "high" : "low") << " account"; using namespace jtx; - Env env{*this}; + Env env{*this, features}; auto const alice = Account{"alice"}; auto const bob = Account{"bob"}; @@ -249,20 +257,119 @@ class SetTrust_test : public beast::unit_test::suite } void - run() override + testDisallowIncoming(FeatureBitset features) + { + testcase("Create trustline 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, asfDisallowIncomingTrustline)); + env.close(); + auto const sle = env.le(alice); + uint32_t flags = sle->getFlags(); + BEAST_EXPECT(!(flags & lsfDisallowIncomingTrustline)); + } + + Env env{*this, features | disallowIncoming}; + + auto const gw = Account{"gateway"}; + auto const alice = Account{"alice"}; + auto const bob = Account{"bob"}; + auto const USD = gw["USD"]; + + env.fund(XRP(10000), gw, alice, bob); + env.close(); + + // Set flag on gateway + env(fset(gw, asfDisallowIncomingTrustline)); + env.close(); + + // Create a trustline which will fail + env(trust(alice, USD(1000)), ter(tecNO_PERMISSION)); + env.close(); + + // Unset the flag + env(fclear(gw, asfDisallowIncomingTrustline)); + env.close(); + + // Create a trustline which will now succeed + env(trust(alice, USD(1000))); + env.close(); + + // Now the payment succeeds. + env(pay(gw, alice, USD(200))); + env.close(); + + // Set flag on gateway again + env(fset(gw, asfDisallowIncomingTrustline)); + env.close(); + + // Destroy the balance by sending it back + env(pay(gw, alice, USD(200))); + env.close(); + + // The trustline still exists in default state + // So a further payment should work + env(pay(gw, alice, USD(200))); + env.close(); + + // Also set the flag on bob + env(fset(bob, asfDisallowIncomingTrustline)); + env.close(); + + // But now bob can't open a trustline because he didn't already have one + env(trust(bob, USD(1000)), ter(tecNO_PERMISSION)); + env.close(); + + // The gateway also can't open this trustline because bob has the flag + // set + env(trust(gw, bob["USD"](1000)), ter(tecNO_PERMISSION)); + env.close(); + + // Unset the flag only on the gateway + env(fclear(gw, asfDisallowIncomingTrustline)); + env.close(); + + // Now bob can open a trustline + env(trust(bob, USD(1000))); + env.close(); + + // And the gateway can send bob a balance + env(pay(gw, bob, USD(200))); + env.close(); + } + + void + testWithFeats(FeatureBitset features) { - testFreeTrustlines(true, false); - testFreeTrustlines(false, true); - testFreeTrustlines(false, true); + testFreeTrustlines(features, true, false); + testFreeTrustlines(features, false, true); + testFreeTrustlines(features, false, true); // true, true case doesn't matter since creating a trustline ledger // entry requires reserve from the creator // independent of hi/low account ids for endpoints - testTicketSetTrust(); - testMalformedTransaction(); - testModifyQualityOfTrustline(false, false); - testModifyQualityOfTrustline(false, true); - testModifyQualityOfTrustline(true, false); - testModifyQualityOfTrustline(true, true); + testTicketSetTrust(features); + testMalformedTransaction(features); + testModifyQualityOfTrustline(features, false, false); + testModifyQualityOfTrustline(features, false, true); + testModifyQualityOfTrustline(features, true, false); + testModifyQualityOfTrustline(features, true, true); + testDisallowIncoming(features); + } + +public: + void + run() override + { + using namespace test::jtx; + auto const sa = supported_amendments(); + testWithFeats(sa - disallowIncoming); + testWithFeats(sa); } }; BEAST_DEFINE_TESTSUITE(SetTrust, app, ripple); diff --git a/src/test/rpc/AccountSet_test.cpp b/src/test/rpc/AccountSet_test.cpp index 8e1ec790b12..b3ca4c9f017 100644 --- a/src/test/rpc/AccountSet_test.cpp +++ b/src/test/rpc/AccountSet_test.cpp @@ -75,6 +75,7 @@ class AccountSet_test : public beast::unit_test::suite // elsewhere. continue; } + if (flag == asfAuthorizedNFTokenMinter) { // The asfAuthorizedNFTokenMinter flag requires the @@ -82,8 +83,18 @@ class AccountSet_test : public beast::unit_test::suite // the transaction. It is tested elsewhere. continue; } - else if ( - std::find(goodFlags.begin(), goodFlags.end(), flag) != + + if (flag == asfDisallowIncomingCheck || + flag == asfDisallowIncomingPayChan || + flag == asfDisallowIncomingNFTOffer || + flag == asfDisallowIncomingTrustline) + { + // These flags are part of the DisallowIncoming amendment + // and are tested elsewhere + continue; + } + + if (std::find(goodFlags.begin(), goodFlags.end(), flag) != goodFlags.end()) { // Good flag