From 570be15028847f4df745e0c8f347c260152a8548 Mon Sep 17 00:00:00 2001 From: Denis Angell Date: Fri, 6 Oct 2023 01:25:16 +0200 Subject: [PATCH] `fixDisallowIncomingV1`: allow issuers to authorize trust lines (#4721) Context: The `DisallowIncoming` amendment provides an option to block incoming trust lines from reaching your account. The asfDisallowIncomingTrustline AccountSet Flag, when enabled, prevents any incoming trust line from being created. However, it was too restrictive: it would block an issuer from authorizing a trust line, even if the trust line already exists. Consider: 1. Issuer sets asfRequireAuth on their account. 2. User sets asfDisallowIncomingTrustline on their account. 3. User submits tx to SetTrust to Issuer. At this point, without `fixDisallowIncomingV1` active, the issuer would not be able to authorize the trust line. The `fixDisallowIncomingV1` amendment, once activated, allows an issuer to authorize a trust line even after the user sets the asfDisallowIncomingTrustline flag, as long as the trust line already exists. --- src/ripple/app/tx/impl/SetTrust.cpp | 15 +++++++++- src/ripple/protocol/Feature.h | 3 +- src/ripple/protocol/impl/Feature.cpp | 1 + src/test/app/SetTrust_test.cpp | 42 ++++++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/ripple/app/tx/impl/SetTrust.cpp b/src/ripple/app/tx/impl/SetTrust.cpp index 7869cc7027d..00a5165221e 100644 --- a/src/ripple/app/tx/impl/SetTrust.cpp +++ b/src/ripple/app/tx/impl/SetTrust.cpp @@ -140,7 +140,20 @@ SetTrust::preclaim(PreclaimContext const& ctx) return tecNO_DST; if (sleDst->getFlags() & lsfDisallowIncomingTrustline) - return tecNO_PERMISSION; + { + // The original implementation of featureDisallowIncoming was + // too restrictive. If + // o fixDisallowIncomingV1 is enabled and + // o The trust line already exists + // Then allow the TrustSet. + if (ctx.view.rules().enabled(fixDisallowIncomingV1) && + ctx.view.exists(keylet::line(id, uDstAccountID, currency))) + { + // pass + } + else + return tecNO_PERMISSION; + } } // If destination is AMM and the trustline doesn't exist then only diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index df0570d3a52..17aca813f71 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 = 62; +static constexpr std::size_t numFeatures = 63; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated @@ -349,6 +349,7 @@ extern uint256 const fixNFTokenRemint; extern uint256 const fixReducedOffersV1; extern uint256 const featureClawback; extern uint256 const featureXChainBridge; +extern uint256 const fixDisallowIncomingV1; } // namespace ripple diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index aa41b7d5b6e..6a3430f4f50 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -456,6 +456,7 @@ REGISTER_FIX (fixReducedOffersV1, Supported::yes, VoteBehavior::De REGISTER_FEATURE(Clawback, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FEATURE(AMM, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FEATURE(XChainBridge, Supported::yes, VoteBehavior::DefaultNo); +REGISTER_FIX(fixDisallowIncomingV1, Supported::yes, VoteBehavior::DefaultNo); // The following amendments are obsolete, but must remain supported // because they could potentially get enabled. diff --git a/src/test/app/SetTrust_test.cpp b/src/test/app/SetTrust_test.cpp index fce9c4295c2..599ac1917f9 100644 --- a/src/test/app/SetTrust_test.cpp +++ b/src/test/app/SetTrust_test.cpp @@ -275,6 +275,48 @@ class SetTrust_test : public beast::unit_test::suite BEAST_EXPECT(!(flags & lsfDisallowIncomingTrustline)); } + // fixDisallowIncomingV1 + { + for (bool const withFix : {true, false}) + { + auto const amend = withFix + ? features | disallowIncoming + : (features | disallowIncoming) - fixDisallowIncomingV1; + + Env env{*this, amend}; + auto const dist = Account("dist"); + auto const gw = Account("gw"); + auto const USD = gw["USD"]; + auto const distUSD = dist["USD"]; + + env.fund(XRP(1000), gw, dist); + env.close(); + + env(fset(gw, asfRequireAuth)); + env.close(); + + env(fset(dist, asfDisallowIncomingTrustline)); + env.close(); + + env(trust(dist, USD(10000))); + env.close(); + + // withFix: can set trustline + // withOutFix: cannot set trustline + auto const trustResult = + withFix ? ter(tesSUCCESS) : ter(tecNO_PERMISSION); + env(trust(gw, distUSD(10000)), + txflags(tfSetfAuth), + trustResult); + env.close(); + + auto const txResult = + withFix ? ter(tesSUCCESS) : ter(tecPATH_DRY); + env(pay(gw, dist, USD(1000)), txResult); + env.close(); + } + } + Env env{*this, features | disallowIncoming}; auto const gw = Account{"gateway"};