From 9754309c942f38715557c050944fb29221075390 Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Thu, 5 Sep 2024 16:37:32 -0700 Subject: [PATCH 1/6] Permissioned Domains: https://github.com/XRPLF/XRPL-Standards/discussions/228 --- include/xrpl/protocol/Feature.h | 3 +- include/xrpl/protocol/Indexes.h | 3 + include/xrpl/protocol/LedgerFormats.h | 5 + include/xrpl/protocol/SField.h | 4 + include/xrpl/protocol/TxFormats.h | 6 + include/xrpl/protocol/jss.h | 4 + src/libxrpl/protocol/Feature.cpp | 1 + src/libxrpl/protocol/Indexes.cpp | 9 + src/libxrpl/protocol/LedgerFormats.cpp | 12 + src/libxrpl/protocol/SField.cpp | 5 + src/libxrpl/protocol/TxFormats.cpp | 15 + src/test/app/PermissionedDomains_test.cpp | 478 ++++++++++++++++++ src/test/ledger/Invariants_test.cpp | 41 ++ src/xrpld/app/tx/detail/InvariantCheck.cpp | 48 ++ src/xrpld/app/tx/detail/InvariantCheck.h | 31 +- .../tx/detail/PermissionedDomainDelete.cpp | 75 +++ .../app/tx/detail/PermissionedDomainDelete.h | 49 ++ .../app/tx/detail/PermissionedDomainSet.cpp | 147 ++++++ .../app/tx/detail/PermissionedDomainSet.h | 54 ++ src/xrpld/app/tx/detail/applySteps.cpp | 6 + src/xrpld/rpc/handlers/AccountObjects.cpp | 3 +- 21 files changed, 996 insertions(+), 3 deletions(-) create mode 100644 src/test/app/PermissionedDomains_test.cpp create mode 100644 src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp create mode 100644 src/xrpld/app/tx/detail/PermissionedDomainDelete.h create mode 100644 src/xrpld/app/tx/detail/PermissionedDomainSet.cpp create mode 100644 src/xrpld/app/tx/detail/PermissionedDomainSet.h diff --git a/include/xrpl/protocol/Feature.h b/include/xrpl/protocol/Feature.h index a00d6b85c1b..e7b4aaaf1d0 100644 --- a/include/xrpl/protocol/Feature.h +++ b/include/xrpl/protocol/Feature.h @@ -80,7 +80,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 = 79; +static constexpr std::size_t numFeatures = 80; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated @@ -372,6 +372,7 @@ extern uint256 const fixEnforceNFTokenTrustline; extern uint256 const fixInnerObjTemplate2; extern uint256 const featureInvariantsV1_1; extern uint256 const fixNFTokenPageLinks; +extern uint256 const featurePermissionedDomains; } // namespace ripple diff --git a/include/xrpl/protocol/Indexes.h b/include/xrpl/protocol/Indexes.h index f179bbacfab..dcd7fd4b76b 100644 --- a/include/xrpl/protocol/Indexes.h +++ b/include/xrpl/protocol/Indexes.h @@ -287,6 +287,9 @@ did(AccountID const& account) noexcept; Keylet oracle(AccountID const& account, std::uint32_t const& documentID) noexcept; +Keylet +permissionedDomain(AccountID const& account, std::uint32_t seq) noexcept; + } // namespace keylet // Everything below is deprecated and should be removed in favor of keylets: diff --git a/include/xrpl/protocol/LedgerFormats.h b/include/xrpl/protocol/LedgerFormats.h index 0ee6c992d8d..bdbe8d27638 100644 --- a/include/xrpl/protocol/LedgerFormats.h +++ b/include/xrpl/protocol/LedgerFormats.h @@ -197,6 +197,11 @@ enum LedgerEntryType : std::uint16_t */ ltORACLE = 0x0080, + /** A ledger object which tracks Permissioned Domain + \sa keylet::permissionedDomain + */ + ltPERMISSIONED_DOMAIN = 0x0077, + //--------------------------------------------------------------------------- /** A special type, matching any ledger entry type. diff --git a/include/xrpl/protocol/SField.h b/include/xrpl/protocol/SField.h index 7f54201a4b8..6aa34a5869f 100644 --- a/include/xrpl/protocol/SField.h +++ b/include/xrpl/protocol/SField.h @@ -511,6 +511,7 @@ extern SF_UINT256 const sfHookStateKey; extern SF_UINT256 const sfHookHash; extern SF_UINT256 const sfHookNamespace; extern SF_UINT256 const sfHookSetTxnID; +extern SF_UINT256 const sfDomainID; // currency amount (common) extern SF_AMOUNT const sfAmount; @@ -564,6 +565,7 @@ extern SF_VL const sfDIDDocument; extern SF_VL const sfData; extern SF_VL const sfAssetClass; extern SF_VL const sfProvider; +extern SF_VL const sfCredentialType; // variable length (uncommon) extern SF_VL const sfFulfillment; @@ -638,6 +640,7 @@ extern SField const sfVoteEntry; extern SField const sfAuctionSlot; extern SField const sfAuthAccount; extern SField const sfPriceData; +extern SField const sfAcceptedCredential; extern SField const sfSigner; extern SField const sfMajority; @@ -667,6 +670,7 @@ extern SField const sfHooks; extern SField const sfVoteSlots; extern SField const sfAuthAccounts; extern SField const sfPriceDataSeries; +extern SField const sfAcceptedCredentials; // array of objects (uncommon) extern SField const sfMajorities; diff --git a/include/xrpl/protocol/TxFormats.h b/include/xrpl/protocol/TxFormats.h index a3f5cca108c..52d87f8ee4f 100644 --- a/include/xrpl/protocol/TxFormats.h +++ b/include/xrpl/protocol/TxFormats.h @@ -199,6 +199,12 @@ enum TxType : std::uint16_t /** This transaction type fixes a problem in the ledger state */ ttLEDGER_STATE_FIX = 53, + /** This transaction type creates or modifies a Permissioned Domain */ + ttPERMISSIONED_DOMAIN_SET = 54, + + /** This transaction type deletes a Permissioned Domain */ + ttPERMISSIONED_DOMAIN_DELETE = 55, + /** This system-generated transaction type is used to update the status of the various amendments. diff --git a/include/xrpl/protocol/jss.h b/include/xrpl/protocol/jss.h index e3eda80b44f..a6f9bfc96a1 100644 --- a/include/xrpl/protocol/jss.h +++ b/include/xrpl/protocol/jss.h @@ -123,6 +123,9 @@ JSS(Payment); // transaction type. JSS(PaymentChannelClaim); // transaction type. JSS(PaymentChannelCreate); // transaction type. JSS(PaymentChannelFund); // transaction type. +JSS(PermissionedDomain); // ledger type. +JSS(PermissionedDomainSet); // transaction type. +JSS(PermissionedDomainDelete); // transaction type. JSS(PriceDataSeries); // field. JSS(PriceData); // field. JSS(Provider); // field. @@ -551,6 +554,7 @@ JSS(peers); // out: InboundLedger, handlers/Peers, Overlay JSS(peer_disconnects); // Severed peer connection counter. JSS(peer_disconnects_resources); // Severed peer connections because of // excess resource consumption. +JSS(permissioned_domain); // out: AccountObjects JSS(port); // in: Connect, out: NetworkOPs JSS(ports); // out: NetworkOPs JSS(previous); // out: Reservations diff --git a/src/libxrpl/protocol/Feature.cpp b/src/libxrpl/protocol/Feature.cpp index 078369bf20c..713973daf7f 100644 --- a/src/libxrpl/protocol/Feature.cpp +++ b/src/libxrpl/protocol/Feature.cpp @@ -501,6 +501,7 @@ REGISTER_FIX (fixNFTokenPageLinks, Supported::yes, VoteBehavior::De // InvariantsV1_1 will be changes to Supported::yes when all the // invariants expected to be included under it are complete. REGISTER_FEATURE(InvariantsV1_1, Supported::no, VoteBehavior::DefaultNo); +REGISTER_FEATURE(PermissionedDomains, Supported::no, VoteBehavior::DefaultNo); // The following amendments are obsolete, but must remain supported // because they could potentially get enabled. diff --git a/src/libxrpl/protocol/Indexes.cpp b/src/libxrpl/protocol/Indexes.cpp index 30d97416cfa..0eddfdc6e2c 100644 --- a/src/libxrpl/protocol/Indexes.cpp +++ b/src/libxrpl/protocol/Indexes.cpp @@ -73,6 +73,7 @@ enum class LedgerNameSpace : std::uint16_t { XCHAIN_CREATE_ACCOUNT_CLAIM_ID = 'K', DID = 'I', ORACLE = 'R', + PERMISSIONED_DOMAIN = 'P', // No longer used or supported. Left here to reserve the space // to avoid accidental reuse. @@ -451,6 +452,14 @@ oracle(AccountID const& account, std::uint32_t const& documentID) noexcept return {ltORACLE, indexHash(LedgerNameSpace::ORACLE, account, documentID)}; } +Keylet +permissionedDomain(AccountID const& account, std::uint32_t seq) noexcept +{ + return { + ltPERMISSIONED_DOMAIN, + indexHash(LedgerNameSpace::PERMISSIONED_DOMAIN, account, seq)}; +} + } // namespace keylet } // namespace ripple diff --git a/src/libxrpl/protocol/LedgerFormats.cpp b/src/libxrpl/protocol/LedgerFormats.cpp index 9401c00278b..e89c54c7662 100644 --- a/src/libxrpl/protocol/LedgerFormats.cpp +++ b/src/libxrpl/protocol/LedgerFormats.cpp @@ -365,6 +365,18 @@ LedgerFormats::LedgerFormats() }, commonFields); + add(jss::PermissionedDomain, + ltPERMISSIONED_DOMAIN, + { + {sfOwner, soeREQUIRED}, + {sfSequence, soeREQUIRED}, + {sfAcceptedCredentials, soeREQUIRED}, + {sfOwnerNode, soeREQUIRED}, + {sfPreviousTxnID, soeREQUIRED}, + {sfPreviousTxnLgrSeq, soeREQUIRED} + }, + commonFields); + // clang-format on } diff --git a/src/libxrpl/protocol/SField.cpp b/src/libxrpl/protocol/SField.cpp index f8eb2d6f877..492679a95c1 100644 --- a/src/libxrpl/protocol/SField.cpp +++ b/src/libxrpl/protocol/SField.cpp @@ -238,6 +238,7 @@ CONSTRUCT_TYPED_SFIELD(sfHookStateKey, "HookStateKey", UINT256, CONSTRUCT_TYPED_SFIELD(sfHookHash, "HookHash", UINT256, 31); CONSTRUCT_TYPED_SFIELD(sfHookNamespace, "HookNamespace", UINT256, 32); CONSTRUCT_TYPED_SFIELD(sfHookSetTxnID, "HookSetTxnID", UINT256, 33); +CONSTRUCT_TYPED_SFIELD(sfDomainID, "DomainID", UINT256, 34); // currency amount (common) CONSTRUCT_TYPED_SFIELD(sfAmount, "Amount", AMOUNT, 1); @@ -307,6 +308,7 @@ CONSTRUCT_TYPED_SFIELD(sfDIDDocument, "DIDDocument", VL, CONSTRUCT_TYPED_SFIELD(sfData, "Data", VL, 27); CONSTRUCT_TYPED_SFIELD(sfAssetClass, "AssetClass", VL, 28); CONSTRUCT_TYPED_SFIELD(sfProvider, "Provider", VL, 29); +CONSTRUCT_TYPED_SFIELD(sfCredentialType, "CredentialType", VL, 30); // account CONSTRUCT_TYPED_SFIELD(sfAccount, "Account", ACCOUNT, 1); @@ -391,6 +393,8 @@ CONSTRUCT_UNTYPED_SFIELD(sfXChainCreateAccountAttestationCollectionElement, "XChainCreateAccountAttestationCollectionElement", OBJECT, 31); CONSTRUCT_UNTYPED_SFIELD(sfPriceData, "PriceData", OBJECT, 32); +// TODO perhaps this should be a typed field once actual credentials are merged. +CONSTRUCT_UNTYPED_SFIELD(sfAcceptedCredential, "AcceptedCredential", OBJECT, 33); // array of objects // ARRAY/1 is reserved for end of array @@ -405,6 +409,7 @@ CONSTRUCT_UNTYPED_SFIELD(sfMemos, "Memos", ARRAY, CONSTRUCT_UNTYPED_SFIELD(sfNFTokens, "NFTokens", ARRAY, 10); CONSTRUCT_UNTYPED_SFIELD(sfHooks, "Hooks", ARRAY, 11); CONSTRUCT_UNTYPED_SFIELD(sfVoteSlots, "VoteSlots", ARRAY, 12); +CONSTRUCT_UNTYPED_SFIELD(sfAcceptedCredentials, "AcceptedCredentials", ARRAY, 13); // array of objects (uncommon) CONSTRUCT_UNTYPED_SFIELD(sfMajorities, "Majorities", ARRAY, 16); diff --git a/src/libxrpl/protocol/TxFormats.cpp b/src/libxrpl/protocol/TxFormats.cpp index 8a93232604e..2d7430b3027 100644 --- a/src/libxrpl/protocol/TxFormats.cpp +++ b/src/libxrpl/protocol/TxFormats.cpp @@ -513,6 +513,21 @@ TxFormats::TxFormats() {sfOwner, soeOPTIONAL}, }, commonFields); + + add(jss::PermissionedDomainSet, + ttPERMISSIONED_DOMAIN_SET, + { + {sfDomainID, soeOPTIONAL}, + {sfAcceptedCredentials, soeREQUIRED}, + }, + commonFields); + + add(jss::PermissionedDomainDelete, + ttPERMISSIONED_DOMAIN_DELETE, + { + {sfDomainID, soeREQUIRED}, + }, + commonFields); } TxFormats const& diff --git a/src/test/app/PermissionedDomains_test.cpp b/src/test/app/PermissionedDomains_test.cpp new file mode 100644 index 00000000000..cacbf0b46a7 --- /dev/null +++ b/src/test/app/PermissionedDomains_test.cpp @@ -0,0 +1,478 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace ripple { +namespace test { +namespace jtx { + +class PermissionedDomains_test : public beast::unit_test::suite +{ + FeatureBitset withFeature_{ + supported_amendments() | featurePermissionedDomains}; + FeatureBitset withoutFeature_{supported_amendments()}; + + using Credential = std::pair; + using Credentials = std::vector; + + // helpers + // Make json for PermissionedDomainSet transaction + static Json::Value + setTx( + AccountID const& account, + Credentials const& credentials, + std::optional domain = std::nullopt) + { + Json::Value jv; + jv[sfTransactionType.jsonName] = jss::PermissionedDomainSet; + jv[sfAccount.jsonName] = to_string(account); + if (domain) + jv[sfDomainID.jsonName] = to_string(*domain); + Json::Value a(Json::arrayValue); + for (auto const& credential : credentials) + { + Json::Value obj(Json::objectValue); + obj[sfIssuer.jsonName] = to_string(credential.first); + obj[sfCredentialType.jsonName] = strHex( + Slice{credential.second.data(), credential.second.size()}); + Json::Value o2(Json::objectValue); + o2[sfAcceptedCredential.jsonName] = obj; + a.append(o2); + } + jv[sfAcceptedCredentials.jsonName] = a; + return jv; + } + + // Make json for PermissionedDomainDelete transaction + static Json::Value + deleteTx(AccountID const& account, uint256 const& domain) + { + Json::Value jv{Json::objectValue}; + jv[sfTransactionType.jsonName] = jss::PermissionedDomainDelete; + jv[sfAccount.jsonName] = to_string(account); + jv[sfDomainID.jsonName] = to_string(domain); + return jv; + } + + // Get PermissionedDomain objects from account_objects rpc call + static std::map + getObjects(Account const& account, Env& env) + { + std::map ret; + Json::Value params; + params[jss::account] = account.human(); + auto const& resp = + env.rpc("json", "account_objects", to_string(params)); + Json::Value a(Json::arrayValue); + a = resp[jss::result][jss::account_objects]; + for (auto const& object : a) + { + if (object["LedgerEntryType"] != "PermissionedDomain") + continue; + uint256 index; + std::ignore = index.parseHex(object[jss::index].asString()); + ret[index] = object; + } + return ret; + } + + // Convert string to Blob + static Blob + toBlob(std::string const& input) + { + Blob ret; + for (auto const& c : input) + ret.push_back(c); + return ret; + } + + // Extract credentials from account_object object + static Credentials + credentialsFromJson(Json::Value const& object) + { + Credentials ret; + Json::Value a(Json::arrayValue); + a = object["AcceptedCredentials"]; + for (auto const& credential : a) + { + Json::Value obj(Json::objectValue); + obj = credential["AcceptedCredential"]; + auto const issuer = obj["Issuer"]; + auto const credentialType = obj["CredentialType"]; + auto aid = parseBase58(issuer.asString()); + auto ct = strUnHex(credentialType.asString()); + ret.emplace_back( + *parseBase58(issuer.asString()), + strUnHex(credentialType.asString()).value()); + } + return ret; + } + + // Sort credentials the same way as PermissionedDomainSet + static Credentials + sortCredentials(Credentials const& input) + { + Credentials ret = input; + std::sort( + ret.begin(), + ret.end(), + [](Credential const& left, Credential const& right) -> bool { + return left.first < right.first; + }); + return ret; + } + + // Get account_info + static Json::Value + ownerInfo(Account const& account, Env& env) + { + Json::Value params; + params[jss::account] = account.human(); + auto const& resp = env.rpc("json", "account_info", to_string(params)); + return env.rpc( + "json", + "account_info", + to_string(params))["result"]["account_data"]; + } + + // tests + // Verify that each tx type can execute if the feature is enabled. + void + testEnabled() + { + testcase("Enabled"); + Account const alice("alice"); + Env env(*this, withFeature_); + env.fund(XRP(1000), alice); + auto const setFee(drops(env.current()->fees().increment)); + Credentials credentials{{alice, toBlob("first credential")}}; + env(setTx(alice, credentials), fee(setFee)); + BEAST_EXPECT(ownerInfo(alice, env)["OwnerCount"].asUInt() == 1); + auto objects = getObjects(alice, env); + BEAST_EXPECT(objects.size() == 1); + auto const domain = objects.begin()->first; + env(deleteTx(alice, domain)); + } + + // Verify that each tx does not execute if feature is disabled + void + testDisabled() + { + testcase("Disabled"); + Account const alice("alice"); + Env env(*this, withoutFeature_); + env.fund(XRP(1000), alice); + auto const setFee(drops(env.current()->fees().increment)); + Credentials credentials{{alice, toBlob("first credential")}}; + env(setTx(alice, credentials), fee(setFee), ter(temDISABLED)); + env(deleteTx(alice, uint256(75)), ter(temDISABLED)); + } + + // Verify that bad inputs fail for each of create new and update + // behaviors of PermissionedDomainSet + void + testBadData( + Account const& account, + Env& env, + std::optional domain = std::nullopt) + { + Account const alice2("alice2"); + Account const alice3("alice3"); + Account const alice4("alice4"); + Account const alice5("alice5"); + Account const alice6("alice6"); + Account const alice7("alice7"); + Account const alice8("alice8"); + Account const alice9("alice9"); + Account const alice10("alice10"); + Account const alice11("alice11"); + Account const alice12("alice12"); + auto const setFee(drops(env.current()->fees().increment)); + + // Test empty credentials. + env(setTx(account, Credentials(), domain), + fee(setFee), + ter(temMALFORMED)); + + // Test 11 credentials. + Credentials const credentials11{ + {alice2, toBlob("credential1")}, + {alice3, toBlob("credential2")}, + {alice4, toBlob("credential3")}, + {alice5, toBlob("credential4")}, + {alice6, toBlob("credential5")}, + {alice7, toBlob("credential6")}, + {alice8, toBlob("credential7")}, + {alice9, toBlob("credential8")}, + {alice10, toBlob("credential9")}, + {alice11, toBlob("credential10")}, + {alice12, toBlob("credential11")}}; + BEAST_EXPECT( + credentials11.size() == PermissionedDomainSet::PD_ARRAY_MAX + 1); + env(setTx(account, credentials11, domain), + fee(setFee), + ter(temMALFORMED)); + + // Test credentials including non-existent issuer. + Account const nobody("nobody"); + Credentials const credentialsNon{ + {alice2, toBlob("credential1")}, + {alice3, toBlob("credential2")}, + {alice4, toBlob("credential3")}, + {nobody, toBlob("credential4")}, + {alice5, toBlob("credential5")}, + {alice6, toBlob("credential6")}, + {alice7, toBlob("credential7")}}; + env(setTx(account, credentialsNon, domain), + fee(setFee), + ter(temBAD_ISSUER)); + + Credentials const credentials4{ + {alice2, toBlob("credential1")}, + {alice3, toBlob("credential2")}, + {alice4, toBlob("credential3")}, + {alice5, toBlob("credential4")}, + }; + auto txJsonMutable = setTx(account, credentials4, domain); + auto const credentialOrig = txJsonMutable["AcceptedCredentials"][2u]; + + // Remove Issuer from the 3rd credential and apply. + txJsonMutable["AcceptedCredentials"][2u]["AcceptedCredential"] + .removeMember("Issuer"); + env(txJsonMutable, fee(setFee), ter(temMALFORMED)); + + txJsonMutable["AcceptedCredentials"][2u] = credentialOrig; + // Remove Credentialtype from the 3rd credential and apply. + txJsonMutable["AcceptedCredentials"][2u]["AcceptedCredential"] + .removeMember("CredentialType"); + env(txJsonMutable, fee(setFee), ter(temMALFORMED)); + + // Remove both + txJsonMutable["AcceptedCredentials"][2u]["AcceptedCredential"] + .removeMember("Issuer"); + env(txJsonMutable, fee(setFee), ter(temMALFORMED)); + } + + // Test PermissionedDomainSet + void + testSet() + { + testcase("Set"); + Env env(*this, withFeature_); + Account const alice("alice"); + env.fund(XRP(1000), alice); + Account const alice2("alice2"); + env.fund(XRP(1000), alice2); + Account const alice3("alice3"); + env.fund(XRP(1000), alice3); + Account const alice4("alice4"); + env.fund(XRP(1000), alice4); + Account const alice5("alice5"); + env.fund(XRP(1000), alice5); + Account const alice6("alice6"); + env.fund(XRP(1000), alice6); + Account const alice7("alice7"); + env.fund(XRP(1000), alice7); + Account const alice8("alice8"); + env.fund(XRP(1000), alice8); + Account const alice9("alice9"); + env.fund(XRP(1000), alice9); + Account const alice10("alice10"); + env.fund(XRP(1000), alice10); + Account const alice11("alice11"); + env.fund(XRP(1000), alice11); + Account const alice12("alice12"); + env.fund(XRP(1000), alice12); + auto const dropsFee = env.current()->fees().increment; + auto const setFee(drops(dropsFee)); + + // Create new from existing account with a single credential. + Credentials const credentials1{{alice2, toBlob("credential1")}}; + { + env(setTx(alice, credentials1), fee(setFee)); + BEAST_EXPECT(ownerInfo(alice, env)["OwnerCount"].asUInt() == 1); + auto tx = env.tx()->getJson(JsonOptions::none); + BEAST_EXPECT(tx[jss::TransactionType] == "PermissionedDomainSet"); + BEAST_EXPECT(tx["Account"] == alice.human()); + auto objects = getObjects(alice, env); + auto domain = objects.begin()->first; + auto object = objects.begin()->second; + BEAST_EXPECT(object["LedgerEntryType"] == "PermissionedDomain"); + BEAST_EXPECT(object["Owner"] == alice.human()); + BEAST_EXPECT(object["Sequence"] == tx["Sequence"]); + BEAST_EXPECT(credentialsFromJson(object) == credentials1); + } + + // Create new from existing account with 10 credentials. + Credentials const credentials10{ + {alice2, toBlob("credential1")}, + {alice3, toBlob("credential2")}, + {alice4, toBlob("credential3")}, + {alice5, toBlob("credential4")}, + {alice6, toBlob("credential5")}, + {alice7, toBlob("credential6")}, + {alice8, toBlob("credential7")}, + {alice9, toBlob("credential8")}, + {alice10, toBlob("credential9")}, + {alice11, toBlob("credential10")}, + }; + uint256 domain2; + { + BEAST_EXPECT( + credentials10.size() == PermissionedDomainSet::PD_ARRAY_MAX); + BEAST_EXPECT(credentials10 != sortCredentials(credentials10)); + env(setTx(alice, credentials10), fee(setFee)); + auto tx = env.tx()->getJson(JsonOptions::none); + auto meta = env.meta()->getJson(JsonOptions::none); + Json::Value a(Json::arrayValue); + a = meta["AffectedNodes"]; + + for (auto const& node : a) + { + if (!node.isMember("CreatedNode") || + node["CreatedNode"]["LedgerEntryType"] != + "PermissionedDomain") + { + continue; + } + std::ignore = domain2.parseHex( + node["CreatedNode"]["LedgerIndex"].asString()); + } + auto objects = getObjects(alice, env); + auto object = objects[domain2]; + BEAST_EXPECT( + credentialsFromJson(object) == sortCredentials(credentials10)); + } + + // Make a new domain with insufficient fee. + env(setTx(alice, credentials10), + fee(drops(dropsFee - 1)), + ter(telINSUF_FEE_P)); + + // Update with 1 credential. + env(setTx(alice, credentials1, domain2)); + BEAST_EXPECT( + credentialsFromJson(getObjects(alice, env)[domain2]) == + credentials1); + + // Update with 10 credentials. + env(setTx(alice, credentials10, domain2)); + env.close(); + BEAST_EXPECT( + credentialsFromJson(getObjects(alice, env)[domain2]) == + sortCredentials(credentials10)); + + // Test bad data when creating a domain. + testBadData(alice, env); + // Test bad data when updating a domain. + testBadData(alice, env, domain2); + + // Try to delete the account with domains. + auto const acctDelFee(drops(env.current()->fees().increment)); + constexpr std::size_t deleteDelta = 255; + { + // Close enough ledgers to make it potentially deletable if empty. + std::size_t ownerSeq = ownerInfo(alice, env)["Sequence"].asUInt(); + while (deleteDelta + ownerSeq > env.current()->seq()) + env.close(); + env(acctdelete(alice, alice2), + fee(acctDelFee), + ter(tecHAS_OBLIGATIONS)); + } + + { + // Delete the domains and then the owner account. + for (auto const& objs : getObjects(alice, env)) + env(deleteTx(alice, objs.first)); + env.close(); + std::size_t ownerSeq = ownerInfo(alice, env)["Sequence"].asUInt(); + while (deleteDelta + ownerSeq > env.current()->seq()) + env.close(); + env(acctdelete(alice, alice2), fee(acctDelFee)); + } + } + + // Test PermissionedDomainDelete + void + testDelete() + { + testcase("Delete"); + Env env(*this, withFeature_); + Account const alice("alice"); + + env.fund(XRP(1000), alice); + auto const setFee(drops(env.current()->fees().increment)); + Credentials credentials{{alice, toBlob("first credential")}}; + env(setTx(alice, credentials), fee(setFee)); + env.close(); + auto objects = getObjects(alice, env); + BEAST_EXPECT(objects.size() == 1); + auto const domain = objects.begin()->first; + + // Delete a domain that doesn't belong to the account. + Account const bob("bob"); + env.fund(XRP(1000), bob); + env(deleteTx(bob, domain), ter(temINVALID_ACCOUNT_ID)); + + // Delete a non-existent domain. + env(deleteTx(alice, uint256(75)), ter(tecNO_ENTRY)); + + // Delete a zero domain. + env(deleteTx(alice, uint256(0)), ter(temMALFORMED)); + + // Make sure owner count reflects the existing domain. + BEAST_EXPECT(ownerInfo(alice, env)["OwnerCount"].asUInt() == 1); + // Delete domain that belongs to user. + env(deleteTx(alice, domain), ter(tesSUCCESS)); + auto const tx = env.tx()->getJson(JsonOptions::none); + BEAST_EXPECT(tx[jss::TransactionType] == "PermissionedDomainDelete"); + // Make sure the owner count goes back to 0. + BEAST_EXPECT(ownerInfo(alice, env)["OwnerCount"].asUInt() == 0); + } + +public: + void + run() override + { + testEnabled(); + testDisabled(); + testSet(); + testDelete(); + } +}; + +BEAST_DEFINE_TESTSUITE_PRIO(PermissionedDomains, app, ripple, 2); + +} // namespace jtx +} // namespace test +} // namespace ripple diff --git a/src/test/ledger/Invariants_test.cpp b/src/test/ledger/Invariants_test.cpp index 8d7b08fa1ab..765b47e9796 100644 --- a/src/test/ledger/Invariants_test.cpp +++ b/src/test/ledger/Invariants_test.cpp @@ -798,6 +798,46 @@ class Invariants_test : public beast::unit_test::suite }); } + void + testPermissionedDomainInvariants() + { + using namespace test::jtx; + testcase << "PermissionedDomain"; + doInvariantCheck( + {{"permissioned domain with no rules"}}, + [](Account const& A1, Account const&, ApplyContext& ac) { + Keylet const pdKeylet = keylet::permissionedDomain(A1.id(), 10); + auto slePd = std::make_shared(pdKeylet); + slePd->setAccountID(sfOwner, A1); + slePd->setFieldU32(sfSequence, 10); + + ac.view().insert(slePd); + return true; + }, + XRPAmount{}, + STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject& tx) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + + doInvariantCheck( + {{"permissioned domain bad credentials size 11"}}, + [](Account const& A1, Account const&, ApplyContext& ac) { + Keylet const pdKeylet = keylet::permissionedDomain(A1.id(), 10); + auto slePd = std::make_shared(pdKeylet); + slePd->setAccountID(sfOwner, A1); + slePd->setFieldU32(sfSequence, 10); + + STArray credentials(sfAcceptedCredentials); + for (std::size_t n = 0; n < 11; ++n) + credentials.push_back(STObject(sfSequence)); + slePd->setFieldArray(sfAcceptedCredentials, credentials); + ac.view().insert(slePd); + return true; + }, + XRPAmount{}, + STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject& tx) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}); + } + public: void run() override @@ -813,6 +853,7 @@ class Invariants_test : public beast::unit_test::suite testNoZeroEscrow(); testValidNewAccountRoot(); testNFTokenPageInvariants(); + testPermissionedDomainInvariants(); } }; diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index f855ad8578c..197c7727520 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -20,6 +20,7 @@ #include #include +#include #include #include #include @@ -86,6 +87,8 @@ XRPNotCreated::visitEntry( std::shared_ptr const& before, std::shared_ptr const& after) { + std::stringstream ss; + ss << "XRPNotCreated::visitEntry start drops: " << drops_ << ". "; /* We go through all modified ledger entries, looking only at account roots, * escrow payments, and payment channels. We remove from the total any * previous XRP values and add to the total any new XRP values. The net @@ -95,6 +98,7 @@ XRPNotCreated::visitEntry( */ if (before) { + ss << "before type: " << before->getType() << ". "; switch (before->getType()) { case ltACCOUNT_ROOT: @@ -114,6 +118,7 @@ XRPNotCreated::visitEntry( if (after) { + ss << "after type: " << after->getType() << ". "; switch (after->getType()) { case ltACCOUNT_ROOT: @@ -478,6 +483,7 @@ LedgerEntryTypesMatch::visitEntry( case ltXCHAIN_OWNED_CREATE_ACCOUNT_CLAIM_ID: case ltDID: case ltORACLE: + case ltPERMISSIONED_DOMAIN: break; default: invalidTypeAdded_ = true; @@ -930,4 +936,46 @@ ValidClawback::finalize( return true; } +//------------------------------------------------------------------------------ + +void +ValidPermissionedDomain::visitEntry( + bool, + std::shared_ptr const& before, + std::shared_ptr const& after) +{ + if (after->getType() != ltPERMISSIONED_DOMAIN) + return; + credentialsSize_ = after->getFieldArray(sfAcceptedCredentials).size(); +} + +bool +ValidPermissionedDomain::finalize( + STTx const& tx, + TER const result, + XRPAmount const, + ReadView const& view, + beast::Journal const& j) +{ + if (tx.getTxnType() != ttPERMISSIONED_DOMAIN_SET || result != tesSUCCESS) + return true; + + if (!credentialsSize_) + { + JLOG(j.fatal()) << "Invariant failed: permissioned domain with " + "no rules."; + return false; + } + + if (credentialsSize_ > PermissionedDomainSet::PD_ARRAY_MAX) + { + JLOG(j.fatal()) << "Invariant failed: permissioned domain bad " + "credentials size " + << credentialsSize_; + return false; + } + + return true; +} + } // namespace ripple diff --git a/src/xrpld/app/tx/detail/InvariantCheck.h b/src/xrpld/app/tx/detail/InvariantCheck.h index 1b3234bae69..ad298d56bbc 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.h +++ b/src/xrpld/app/tx/detail/InvariantCheck.h @@ -28,6 +28,7 @@ #include #include +#include #include #include @@ -450,6 +451,33 @@ class ValidClawback beast::Journal const&); }; +/** + * @brief Invariants: Permissioned Domains must have some rules and + * AcceptedCredentials must have length between 1 and 10 inclusive. + * + * Since only permissions constitute rules, an empty credentials list + * means that there are no rules and the invariant is violated. + */ +class ValidPermissionedDomain +{ + std::size_t credentialsSize_{0}; + +public: + void + visitEntry( + bool, + std::shared_ptr const&, + std::shared_ptr const&); + + bool + finalize( + STTx const&, + TER const, + XRPAmount const, + ReadView const&, + beast::Journal const&); +}; + // additional invariant checks can be declared above and then added to this // tuple using InvariantChecks = std::tuple< @@ -465,7 +493,8 @@ using InvariantChecks = std::tuple< ValidNewAccountRoot, ValidNFTokenPage, NFTokenCountTracking, - ValidClawback>; + ValidClawback, + ValidPermissionedDomain>; /** * @brief get a tuple of all invariant checks diff --git a/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp b/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp new file mode 100644 index 00000000000..b737af5e570 --- /dev/null +++ b/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp @@ -0,0 +1,75 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include +#include + +namespace ripple { + +NotTEC +PermissionedDomainDelete::preflight(PreflightContext const& ctx) +{ + if (!ctx.rules.enabled(featurePermissionedDomains)) + return temDISABLED; + if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + return ret; + if (!ctx.tx.isFieldPresent(sfDomainID)) + return temMALFORMED; + return preflight2(ctx); +} + +TER +PermissionedDomainDelete::preclaim(PreclaimContext const& ctx) +{ + assert(ctx.tx.isFieldPresent(sfDomainID)); + auto const domain = ctx.tx.getFieldH256(sfDomainID); + if (domain == beast::zero) + return temMALFORMED; + auto const sleDomain = ctx.view.read({ltPERMISSIONED_DOMAIN, domain}); + if (!sleDomain) + return tecNO_ENTRY; + assert( + sleDomain->isFieldPresent(sfOwner) && ctx.tx.isFieldPresent(sfAccount)); + if (sleDomain->getAccountID(sfOwner) != ctx.tx.getAccountID(sfAccount)) + return temINVALID_ACCOUNT_ID; + return tesSUCCESS; +} + +/** Attempt to delete the Permissioned Domain. */ +TER +PermissionedDomainDelete::doApply() +{ + assert(ctx_.tx.isFieldPresent(sfDomainID)); + auto const slePd = + view().peek({ltPERMISSIONED_DOMAIN, ctx_.tx.at(sfDomainID)}); + auto const page{(*slePd)[sfOwnerNode]}; + if (!view().dirRemove(keylet::ownerDir(account_), page, slePd->key(), true)) + { + JLOG(j_.fatal()) + << "Unable to delete permissioned domain directory entry."; + return tefBAD_LEDGER; + } + auto const ownerSle = view().peek(keylet::account(account_)); + assert(ownerSle && ownerSle->getFieldU32(sfOwnerCount) > 0); + adjustOwnerCount(view(), ownerSle, -1, ctx_.journal); + view().erase(slePd); + return tesSUCCESS; +} + +} // namespace ripple diff --git a/src/xrpld/app/tx/detail/PermissionedDomainDelete.h b/src/xrpld/app/tx/detail/PermissionedDomainDelete.h new file mode 100644 index 00000000000..3fdf50ed298 --- /dev/null +++ b/src/xrpld/app/tx/detail/PermissionedDomainDelete.h @@ -0,0 +1,49 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#ifndef RIPPLE_TX_PERMISSIONEDDOMAINDELETE_H_INCLUDED +#define RIPPLE_TX_PERMISSIONEDDOMAINDELETE_H_INCLUDED + +#include + +namespace ripple { + +class PermissionedDomainDelete : public Transactor +{ +public: + static constexpr ConsequencesFactoryType ConsequencesFactory{Normal}; + + explicit PermissionedDomainDelete(ApplyContext& ctx) : Transactor(ctx) + { + } + + static NotTEC + preflight(PreflightContext const& ctx); + + static TER + preclaim(PreclaimContext const& ctx); + + /** Attempt to create the Permissioned Domain. */ + TER + doApply() override; +}; + +} // namespace ripple + +#endif // RIPPLE_TX_PERMISSIONEDDOMAINDELETE_H_INCLUDED diff --git a/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp new file mode 100644 index 00000000000..e7cf94f0b64 --- /dev/null +++ b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp @@ -0,0 +1,147 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include +#include +#include +#include + +#include + +namespace ripple { + +NotTEC +PermissionedDomainSet::preflight(PreflightContext const& ctx) +{ + if (!ctx.rules.enabled(featurePermissionedDomains)) + return temDISABLED; + if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + return ret; + + if (!ctx.tx.isFieldPresent(sfAcceptedCredentials)) + return temMALFORMED; + auto const credentials = ctx.tx.getFieldArray(sfAcceptedCredentials); + // TODO check to see if we should disallow duplicate issuers. + // If so, it probably means sorting on the CredentialType field + // for identical issuers. + if (credentials.empty() || credentials.size() > PD_ARRAY_MAX) + return temMALFORMED; + + return preflight2(ctx); +} + +XRPAmount +PermissionedDomainSet::calculateBaseFee(ReadView const& view, STTx const& tx) +{ + if (tx.isFieldPresent(sfDomainID)) + return Transactor::calculateBaseFee(view, tx); + // The fee required for a new PermissionedDomain is one owner reserve. + return view.fees().increment; +} + +TER +PermissionedDomainSet::preclaim(PreclaimContext const& ctx) +{ + if (!ctx.view.read(keylet::account(ctx.tx.getAccountID(sfAccount)))) + return tefINTERNAL; + + assert(ctx.tx.isFieldPresent(sfAcceptedCredentials)); + auto const credentials = ctx.tx.getFieldArray(sfAcceptedCredentials); + for (auto const& credential : credentials) + { + if (!credential.isFieldPresent(sfIssuer) || + !credential.isFieldPresent(sfCredentialType)) + { + return temMALFORMED; + } + if (!ctx.view.read(keylet::account(credential.getAccountID(sfIssuer)))) + return temBAD_ISSUER; + } + + if (!ctx.tx.isFieldPresent(sfDomainID)) + return tesSUCCESS; + auto const domain = ctx.tx.getFieldH256(sfDomainID); + if (domain == beast::zero) + return temMALFORMED; + auto const sleDomain = ctx.view.read({ltPERMISSIONED_DOMAIN, domain}); + if (!sleDomain) + return tecNO_ENTRY; + auto const owner = sleDomain->getAccountID(sfOwner); + auto account = ctx.tx.getAccountID(sfAccount); + if (owner != account) + return temINVALID_ACCOUNT_ID; + + return tesSUCCESS; +} + +/** Attempt to create the Permissioned Domain. */ +TER +PermissionedDomainSet::doApply() +{ + auto const ownerSle = view().peek(keylet::account(account_)); + + // All checks have already been done. Just update credentials. Same logic + // for either new domain or updating existing. + auto updateSle = [this](std::shared_ptr const& sle) { + auto credentials = ctx_.tx.getFieldArray(sfAcceptedCredentials); + // TODO when credentials are merged, it is possible that this will + // also need to sort on the CredentialType field in case there + // are multiple issuers in each set of credentials. The spec + // needs to be ironed out. + credentials.sort( + [](STObject const& left, STObject const& right) -> bool { + return dynamic_cast(&left)->getAccountID( + sfIssuer) < + dynamic_cast(&right)->getAccountID( + sfIssuer); + }); + sle->setFieldArray(sfAcceptedCredentials, credentials); + }; + + if (ctx_.tx.isFieldPresent(sfDomainID)) + { + // Modify existing permissioned domain. + auto sleUpdate = view().peek( + {ltPERMISSIONED_DOMAIN, ctx_.tx.getFieldH256(sfDomainID)}); + updateSle(sleUpdate); + view().update(sleUpdate); + } + else + { + // Create new permissioned domain. + Keylet const pdKeylet = keylet::permissionedDomain( + account_, ctx_.tx.getFieldU32(sfSequence)); + auto slePd = std::make_shared(pdKeylet); + slePd->setAccountID(sfOwner, account_); + slePd->setFieldU32(sfSequence, ctx_.tx.getFieldU32(sfSequence)); + updateSle(slePd); + view().insert(slePd); + auto const page = view().dirInsert( + keylet::ownerDir(account_), pdKeylet, describeOwnerDir(account_)); + if (!page) + return tecDIR_FULL; + slePd->setFieldU64(sfOwnerNode, *page); + // If we succeeded, the new entry counts against the creator's reserve. + adjustOwnerCount(view(), ownerSle, 1, ctx_.journal); + } + + return tesSUCCESS; +} + +} // namespace ripple diff --git a/src/xrpld/app/tx/detail/PermissionedDomainSet.h b/src/xrpld/app/tx/detail/PermissionedDomainSet.h new file mode 100644 index 00000000000..e1f105b1d15 --- /dev/null +++ b/src/xrpld/app/tx/detail/PermissionedDomainSet.h @@ -0,0 +1,54 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#ifndef RIPPLE_TX_PERMISSIONEDDOMAINSET_H_INCLUDED +#define RIPPLE_TX_PERMISSIONEDDOMAINSET_H_INCLUDED + +#include +#include + +namespace ripple { + +class PermissionedDomainSet : public Transactor +{ +public: + static constexpr std::size_t PD_ARRAY_MAX = 10; + static constexpr ConsequencesFactoryType ConsequencesFactory{Normal}; + + explicit PermissionedDomainSet(ApplyContext& ctx) : Transactor(ctx) + { + } + + static NotTEC + preflight(PreflightContext const& ctx); + + static XRPAmount + calculateBaseFee(ReadView const& view, STTx const& tx); + + static TER + preclaim(PreclaimContext const& ctx); + + /** Attempt to create the Permissioned Domain. */ + TER + doApply() override; +}; + +} // namespace ripple + +#endif // RIPPLE_TX_PERMISSIONEDDOMAINSET_H_INCLUDED diff --git a/src/xrpld/app/tx/detail/applySteps.cpp b/src/xrpld/app/tx/detail/applySteps.cpp index cbeabb6fc9c..db4a5c0405e 100644 --- a/src/xrpld/app/tx/detail/applySteps.cpp +++ b/src/xrpld/app/tx/detail/applySteps.cpp @@ -46,6 +46,8 @@ #include #include #include +#include +#include #include #include #include @@ -168,6 +170,10 @@ with_txn_type(TxType txnType, F&& f) return f.template operator()(); case ttORACLE_DELETE: return f.template operator()(); + case ttPERMISSIONED_DOMAIN_SET: + return f.template operator()(); + case ttPERMISSIONED_DOMAIN_DELETE: + return f.template operator()(); default: throw UnknownTxnType(txnType); } diff --git a/src/xrpld/rpc/handlers/AccountObjects.cpp b/src/xrpld/rpc/handlers/AccountObjects.cpp index c192fbf9071..0ed2ac1e835 100644 --- a/src/xrpld/rpc/handlers/AccountObjects.cpp +++ b/src/xrpld/rpc/handlers/AccountObjects.cpp @@ -222,7 +222,8 @@ doAccountObjects(RPC::JsonContext& context) {jss::xchain_owned_claim_id, ltXCHAIN_OWNED_CLAIM_ID}, {jss::xchain_owned_create_account_claim_id, ltXCHAIN_OWNED_CREATE_ACCOUNT_CLAIM_ID}, - {jss::bridge, ltBRIDGE}}; + {jss::bridge, ltBRIDGE}, + {jss::permissioned_domain, ltPERMISSIONED_DOMAIN}}; typeFilter.emplace(); typeFilter->reserve(std::size(deletionBlockers)); From 61e785b5b1256ef216a9ef870c2d94b1c3a17a5a Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Tue, 1 Oct 2024 15:10:24 -0700 Subject: [PATCH 2/6] Review fixes from Olek. --- src/test/app/PermissionedDomains_test.cpp | 352 +++++++----------- src/test/jtx/PermissionedDomains.h | 78 ++++ src/test/jtx/impl/PermissionedDomains.cpp | 151 ++++++++ src/xrpld/app/tx/detail/InvariantCheck.cpp | 4 - .../tx/detail/PermissionedDomainDelete.cpp | 2 - .../app/tx/detail/PermissionedDomainSet.cpp | 46 ++- 6 files changed, 390 insertions(+), 243 deletions(-) create mode 100644 src/test/jtx/PermissionedDomains.h create mode 100644 src/test/jtx/impl/PermissionedDomains.cpp diff --git a/src/test/app/PermissionedDomains_test.cpp b/src/test/app/PermissionedDomains_test.cpp index cacbf0b46a7..e96c88d55e9 100644 --- a/src/test/app/PermissionedDomains_test.cpp +++ b/src/test/app/PermissionedDomains_test.cpp @@ -18,6 +18,7 @@ //============================================================================== #include +#include #include #include #include @@ -26,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -42,130 +44,6 @@ class PermissionedDomains_test : public beast::unit_test::suite supported_amendments() | featurePermissionedDomains}; FeatureBitset withoutFeature_{supported_amendments()}; - using Credential = std::pair; - using Credentials = std::vector; - - // helpers - // Make json for PermissionedDomainSet transaction - static Json::Value - setTx( - AccountID const& account, - Credentials const& credentials, - std::optional domain = std::nullopt) - { - Json::Value jv; - jv[sfTransactionType.jsonName] = jss::PermissionedDomainSet; - jv[sfAccount.jsonName] = to_string(account); - if (domain) - jv[sfDomainID.jsonName] = to_string(*domain); - Json::Value a(Json::arrayValue); - for (auto const& credential : credentials) - { - Json::Value obj(Json::objectValue); - obj[sfIssuer.jsonName] = to_string(credential.first); - obj[sfCredentialType.jsonName] = strHex( - Slice{credential.second.data(), credential.second.size()}); - Json::Value o2(Json::objectValue); - o2[sfAcceptedCredential.jsonName] = obj; - a.append(o2); - } - jv[sfAcceptedCredentials.jsonName] = a; - return jv; - } - - // Make json for PermissionedDomainDelete transaction - static Json::Value - deleteTx(AccountID const& account, uint256 const& domain) - { - Json::Value jv{Json::objectValue}; - jv[sfTransactionType.jsonName] = jss::PermissionedDomainDelete; - jv[sfAccount.jsonName] = to_string(account); - jv[sfDomainID.jsonName] = to_string(domain); - return jv; - } - - // Get PermissionedDomain objects from account_objects rpc call - static std::map - getObjects(Account const& account, Env& env) - { - std::map ret; - Json::Value params; - params[jss::account] = account.human(); - auto const& resp = - env.rpc("json", "account_objects", to_string(params)); - Json::Value a(Json::arrayValue); - a = resp[jss::result][jss::account_objects]; - for (auto const& object : a) - { - if (object["LedgerEntryType"] != "PermissionedDomain") - continue; - uint256 index; - std::ignore = index.parseHex(object[jss::index].asString()); - ret[index] = object; - } - return ret; - } - - // Convert string to Blob - static Blob - toBlob(std::string const& input) - { - Blob ret; - for (auto const& c : input) - ret.push_back(c); - return ret; - } - - // Extract credentials from account_object object - static Credentials - credentialsFromJson(Json::Value const& object) - { - Credentials ret; - Json::Value a(Json::arrayValue); - a = object["AcceptedCredentials"]; - for (auto const& credential : a) - { - Json::Value obj(Json::objectValue); - obj = credential["AcceptedCredential"]; - auto const issuer = obj["Issuer"]; - auto const credentialType = obj["CredentialType"]; - auto aid = parseBase58(issuer.asString()); - auto ct = strUnHex(credentialType.asString()); - ret.emplace_back( - *parseBase58(issuer.asString()), - strUnHex(credentialType.asString()).value()); - } - return ret; - } - - // Sort credentials the same way as PermissionedDomainSet - static Credentials - sortCredentials(Credentials const& input) - { - Credentials ret = input; - std::sort( - ret.begin(), - ret.end(), - [](Credential const& left, Credential const& right) -> bool { - return left.first < right.first; - }); - return ret; - } - - // Get account_info - static Json::Value - ownerInfo(Account const& account, Env& env) - { - Json::Value params; - params[jss::account] = account.human(); - auto const& resp = env.rpc("json", "account_info", to_string(params)); - return env.rpc( - "json", - "account_info", - to_string(params))["result"]["account_data"]; - } - - // tests // Verify that each tx type can execute if the feature is enabled. void testEnabled() @@ -175,13 +53,13 @@ class PermissionedDomains_test : public beast::unit_test::suite Env env(*this, withFeature_); env.fund(XRP(1000), alice); auto const setFee(drops(env.current()->fees().increment)); - Credentials credentials{{alice, toBlob("first credential")}}; - env(setTx(alice, credentials), fee(setFee)); - BEAST_EXPECT(ownerInfo(alice, env)["OwnerCount"].asUInt() == 1); - auto objects = getObjects(alice, env); + pd::Credentials credentials{{alice, pd::toBlob("first credential")}}; + env(pd::setTx(alice, credentials), fee(setFee)); + BEAST_EXPECT(pd::ownerInfo(alice, env)["OwnerCount"].asUInt() == 1); + auto objects = pd::getObjects(alice, env); BEAST_EXPECT(objects.size() == 1); auto const domain = objects.begin()->first; - env(deleteTx(alice, domain)); + env(pd::deleteTx(alice, domain)); } // Verify that each tx does not execute if feature is disabled @@ -193,9 +71,9 @@ class PermissionedDomains_test : public beast::unit_test::suite Env env(*this, withoutFeature_); env.fund(XRP(1000), alice); auto const setFee(drops(env.current()->fees().increment)); - Credentials credentials{{alice, toBlob("first credential")}}; - env(setTx(alice, credentials), fee(setFee), ter(temDISABLED)); - env(deleteTx(alice, uint256(75)), ter(temDISABLED)); + pd::Credentials credentials{{alice, pd::toBlob("first credential")}}; + env(pd::setTx(alice, credentials), fee(setFee), ter(temDISABLED)); + env(pd::deleteTx(alice, uint256(75)), ter(temDISABLED)); } // Verify that bad inputs fail for each of create new and update @@ -220,59 +98,64 @@ class PermissionedDomains_test : public beast::unit_test::suite auto const setFee(drops(env.current()->fees().increment)); // Test empty credentials. - env(setTx(account, Credentials(), domain), + env(pd::setTx(account, pd::Credentials(), domain), fee(setFee), ter(temMALFORMED)); // Test 11 credentials. - Credentials const credentials11{ - {alice2, toBlob("credential1")}, - {alice3, toBlob("credential2")}, - {alice4, toBlob("credential3")}, - {alice5, toBlob("credential4")}, - {alice6, toBlob("credential5")}, - {alice7, toBlob("credential6")}, - {alice8, toBlob("credential7")}, - {alice9, toBlob("credential8")}, - {alice10, toBlob("credential9")}, - {alice11, toBlob("credential10")}, - {alice12, toBlob("credential11")}}; + pd::Credentials const credentials11{ + {alice2, pd::toBlob("credential1")}, + {alice3, pd::toBlob("credential2")}, + {alice4, pd::toBlob("credential3")}, + {alice5, pd::toBlob("credential4")}, + {alice6, pd::toBlob("credential5")}, + {alice7, pd::toBlob("credential6")}, + {alice8, pd::toBlob("credential7")}, + {alice9, pd::toBlob("credential8")}, + {alice10, pd::toBlob("credential9")}, + {alice11, pd::toBlob("credential10")}, + {alice12, pd::toBlob("credential11")}}; BEAST_EXPECT( credentials11.size() == PermissionedDomainSet::PD_ARRAY_MAX + 1); - env(setTx(account, credentials11, domain), + env(pd::setTx(account, credentials11, domain), fee(setFee), ter(temMALFORMED)); // Test credentials including non-existent issuer. Account const nobody("nobody"); - Credentials const credentialsNon{ - {alice2, toBlob("credential1")}, - {alice3, toBlob("credential2")}, - {alice4, toBlob("credential3")}, - {nobody, toBlob("credential4")}, - {alice5, toBlob("credential5")}, - {alice6, toBlob("credential6")}, - {alice7, toBlob("credential7")}}; - env(setTx(account, credentialsNon, domain), + pd::Credentials const credentialsNon{ + {alice2, pd::toBlob("credential1")}, + {alice3, pd::toBlob("credential2")}, + {alice4, pd::toBlob("credential3")}, + {nobody, pd::toBlob("credential4")}, + {alice5, pd::toBlob("credential5")}, + {alice6, pd::toBlob("credential6")}, + {alice7, pd::toBlob("credential7")}}; + env(pd::setTx(account, credentialsNon, domain), fee(setFee), ter(temBAD_ISSUER)); - Credentials const credentials4{ - {alice2, toBlob("credential1")}, - {alice3, toBlob("credential2")}, - {alice4, toBlob("credential3")}, - {alice5, toBlob("credential4")}, + pd::Credentials const credentials4{ + {alice2, pd::toBlob("credential1")}, + {alice3, pd::toBlob("credential2")}, + {alice4, pd::toBlob("credential3")}, + {alice5, pd::toBlob("credential4")}, }; - auto txJsonMutable = setTx(account, credentials4, domain); + auto txJsonMutable = pd::setTx(account, credentials4, domain); auto const credentialOrig = txJsonMutable["AcceptedCredentials"][2u]; - // Remove Issuer from the 3rd credential and apply. + // Remove Issuer from a credential and apply. txJsonMutable["AcceptedCredentials"][2u]["AcceptedCredential"] .removeMember("Issuer"); env(txJsonMutable, fee(setFee), ter(temMALFORMED)); txJsonMutable["AcceptedCredentials"][2u] = credentialOrig; - // Remove Credentialtype from the 3rd credential and apply. + // Make an empty CredentialType. + txJsonMutable["AcceptedCredentials"][2u]["AcceptedCredential"] + ["CredentialType"] = ""; + env(txJsonMutable, fee(setFee), ter(temMALFORMED)); + + // Remove Credentialtype from a credential and apply. txJsonMutable["AcceptedCredentials"][2u]["AcceptedCredential"] .removeMember("CredentialType"); env(txJsonMutable, fee(setFee), ter(temMALFORMED)); @@ -281,6 +164,17 @@ class PermissionedDomains_test : public beast::unit_test::suite txJsonMutable["AcceptedCredentials"][2u]["AcceptedCredential"] .removeMember("Issuer"); env(txJsonMutable, fee(setFee), ter(temMALFORMED)); + + // Make 2 identical credentials. + pd::Credentials const credentialsDup{ + {alice2, pd::toBlob("credential1")}, + {alice3, pd::toBlob("credential2")}, + {alice2, pd::toBlob("credential1")}, + {alice5, pd::toBlob("credential4")}, + }; + env(pd::setTx(account, credentialsDup, domain), + fee(setFee), + ter(temMALFORMED)); } // Test PermissionedDomainSet @@ -289,69 +183,63 @@ class PermissionedDomains_test : public beast::unit_test::suite { testcase("Set"); Env env(*this, withFeature_); - Account const alice("alice"); - env.fund(XRP(1000), alice); - Account const alice2("alice2"); - env.fund(XRP(1000), alice2); - Account const alice3("alice3"); - env.fund(XRP(1000), alice3); - Account const alice4("alice4"); - env.fund(XRP(1000), alice4); - Account const alice5("alice5"); - env.fund(XRP(1000), alice5); - Account const alice6("alice6"); - env.fund(XRP(1000), alice6); - Account const alice7("alice7"); - env.fund(XRP(1000), alice7); - Account const alice8("alice8"); - env.fund(XRP(1000), alice8); - Account const alice9("alice9"); - env.fund(XRP(1000), alice9); - Account const alice10("alice10"); - env.fund(XRP(1000), alice10); - Account const alice11("alice11"); - env.fund(XRP(1000), alice11); - Account const alice12("alice12"); - env.fund(XRP(1000), alice12); + Account const alice("alice"), alice2("alice2"), alice3("alice3"), + alice4("alice4"), alice5("alice5"), alice6("alice6"), + alice7("alice7"), alice8("alice8"), alice9("alice9"), + alice10("alice10"), alice11("alice11"), alice12("alice12"); + env.fund( + XRP(1000), + alice, + alice2, + alice3, + alice4, + alice5, + alice6, + alice7, + alice8, + alice9, + alice10, + alice11, + alice12); auto const dropsFee = env.current()->fees().increment; auto const setFee(drops(dropsFee)); // Create new from existing account with a single credential. - Credentials const credentials1{{alice2, toBlob("credential1")}}; + pd::Credentials const credentials1{{alice2, pd::toBlob("credential1")}}; { - env(setTx(alice, credentials1), fee(setFee)); - BEAST_EXPECT(ownerInfo(alice, env)["OwnerCount"].asUInt() == 1); + env(pd::setTx(alice, credentials1), fee(setFee)); + BEAST_EXPECT(pd::ownerInfo(alice, env)["OwnerCount"].asUInt() == 1); auto tx = env.tx()->getJson(JsonOptions::none); BEAST_EXPECT(tx[jss::TransactionType] == "PermissionedDomainSet"); BEAST_EXPECT(tx["Account"] == alice.human()); - auto objects = getObjects(alice, env); + auto objects = pd::getObjects(alice, env); auto domain = objects.begin()->first; auto object = objects.begin()->second; BEAST_EXPECT(object["LedgerEntryType"] == "PermissionedDomain"); BEAST_EXPECT(object["Owner"] == alice.human()); BEAST_EXPECT(object["Sequence"] == tx["Sequence"]); - BEAST_EXPECT(credentialsFromJson(object) == credentials1); + BEAST_EXPECT(pd::credentialsFromJson(object) == credentials1); } // Create new from existing account with 10 credentials. - Credentials const credentials10{ - {alice2, toBlob("credential1")}, - {alice3, toBlob("credential2")}, - {alice4, toBlob("credential3")}, - {alice5, toBlob("credential4")}, - {alice6, toBlob("credential5")}, - {alice7, toBlob("credential6")}, - {alice8, toBlob("credential7")}, - {alice9, toBlob("credential8")}, - {alice10, toBlob("credential9")}, - {alice11, toBlob("credential10")}, + pd::Credentials const credentials10{ + {alice2, pd::toBlob("credential1")}, + {alice3, pd::toBlob("credential2")}, + {alice4, pd::toBlob("credential3")}, + {alice5, pd::toBlob("credential4")}, + {alice6, pd::toBlob("credential5")}, + {alice7, pd::toBlob("credential6")}, + {alice8, pd::toBlob("credential7")}, + {alice9, pd::toBlob("credential8")}, + {alice10, pd::toBlob("credential9")}, + {alice11, pd::toBlob("credential10")}, }; uint256 domain2; { BEAST_EXPECT( credentials10.size() == PermissionedDomainSet::PD_ARRAY_MAX); - BEAST_EXPECT(credentials10 != sortCredentials(credentials10)); - env(setTx(alice, credentials10), fee(setFee)); + BEAST_EXPECT(credentials10 != pd::sortCredentials(credentials10)); + env(pd::setTx(alice, credentials10), fee(setFee)); auto tx = env.tx()->getJson(JsonOptions::none); auto meta = env.meta()->getJson(JsonOptions::none); Json::Value a(Json::arrayValue); @@ -368,29 +256,34 @@ class PermissionedDomains_test : public beast::unit_test::suite std::ignore = domain2.parseHex( node["CreatedNode"]["LedgerIndex"].asString()); } - auto objects = getObjects(alice, env); + auto objects = pd::getObjects(alice, env); auto object = objects[domain2]; BEAST_EXPECT( - credentialsFromJson(object) == sortCredentials(credentials10)); + pd::credentialsFromJson(object) == + pd::sortCredentials(credentials10)); } // Make a new domain with insufficient fee. - env(setTx(alice, credentials10), + env(pd::setTx(alice, credentials10), fee(drops(dropsFee - 1)), ter(telINSUF_FEE_P)); // Update with 1 credential. - env(setTx(alice, credentials1, domain2)); + env(pd::setTx(alice, credentials1, domain2)); BEAST_EXPECT( - credentialsFromJson(getObjects(alice, env)[domain2]) == + pd::credentialsFromJson(pd::getObjects(alice, env)[domain2]) == credentials1); // Update with 10 credentials. - env(setTx(alice, credentials10, domain2)); + env(pd::setTx(alice, credentials10, domain2)); env.close(); BEAST_EXPECT( - credentialsFromJson(getObjects(alice, env)[domain2]) == - sortCredentials(credentials10)); + pd::credentialsFromJson(pd::getObjects(alice, env)[domain2]) == + pd::sortCredentials(credentials10)); + + // Update from the wrong owner. + env(pd::setTx(alice2, credentials1, domain2), + ter(temINVALID_ACCOUNT_ID)); // Test bad data when creating a domain. testBadData(alice, env); @@ -402,7 +295,8 @@ class PermissionedDomains_test : public beast::unit_test::suite constexpr std::size_t deleteDelta = 255; { // Close enough ledgers to make it potentially deletable if empty. - std::size_t ownerSeq = ownerInfo(alice, env)["Sequence"].asUInt(); + std::size_t ownerSeq = + pd::ownerInfo(alice, env)["Sequence"].asUInt(); while (deleteDelta + ownerSeq > env.current()->seq()) env.close(); env(acctdelete(alice, alice2), @@ -412,10 +306,11 @@ class PermissionedDomains_test : public beast::unit_test::suite { // Delete the domains and then the owner account. - for (auto const& objs : getObjects(alice, env)) - env(deleteTx(alice, objs.first)); + for (auto const& objs : pd::getObjects(alice, env)) + env(pd::deleteTx(alice, objs.first)); env.close(); - std::size_t ownerSeq = ownerInfo(alice, env)["Sequence"].asUInt(); + std::size_t ownerSeq = + pd::ownerInfo(alice, env)["Sequence"].asUInt(); while (deleteDelta + ownerSeq > env.current()->seq()) env.close(); env(acctdelete(alice, alice2), fee(acctDelFee)); @@ -432,32 +327,37 @@ class PermissionedDomains_test : public beast::unit_test::suite env.fund(XRP(1000), alice); auto const setFee(drops(env.current()->fees().increment)); - Credentials credentials{{alice, toBlob("first credential")}}; - env(setTx(alice, credentials), fee(setFee)); + pd::Credentials credentials{{alice, pd::toBlob("first credential")}}; + env(pd::setTx(alice, credentials), fee(setFee)); env.close(); - auto objects = getObjects(alice, env); + auto objects = pd::getObjects(alice, env); BEAST_EXPECT(objects.size() == 1); auto const domain = objects.begin()->first; // Delete a domain that doesn't belong to the account. Account const bob("bob"); env.fund(XRP(1000), bob); - env(deleteTx(bob, domain), ter(temINVALID_ACCOUNT_ID)); + env(pd::deleteTx(bob, domain), ter(temINVALID_ACCOUNT_ID)); // Delete a non-existent domain. - env(deleteTx(alice, uint256(75)), ter(tecNO_ENTRY)); + env(pd::deleteTx(alice, uint256(75)), ter(tecNO_ENTRY)); // Delete a zero domain. - env(deleteTx(alice, uint256(0)), ter(temMALFORMED)); + env(pd::deleteTx(alice, uint256(0)), ter(temMALFORMED)); // Make sure owner count reflects the existing domain. - BEAST_EXPECT(ownerInfo(alice, env)["OwnerCount"].asUInt() == 1); + BEAST_EXPECT(pd::ownerInfo(alice, env)["OwnerCount"].asUInt() == 1); + auto const objID = pd::getObjects(alice, env).begin()->first; + BEAST_EXPECT(pd::objectExists(objID, env)); // Delete domain that belongs to user. - env(deleteTx(alice, domain), ter(tesSUCCESS)); + env(pd::deleteTx(alice, domain), ter(tesSUCCESS)); auto const tx = env.tx()->getJson(JsonOptions::none); BEAST_EXPECT(tx[jss::TransactionType] == "PermissionedDomainDelete"); // Make sure the owner count goes back to 0. - BEAST_EXPECT(ownerInfo(alice, env)["OwnerCount"].asUInt() == 0); + BEAST_EXPECT(pd::ownerInfo(alice, env)["OwnerCount"].asUInt() == 0); + // The object needs to be gone. + BEAST_EXPECT(pd::getObjects(alice, env).empty()); + BEAST_EXPECT(!pd::objectExists(objID, env)); } public: diff --git a/src/test/jtx/PermissionedDomains.h b/src/test/jtx/PermissionedDomains.h new file mode 100644 index 00000000000..ae7b673a946 --- /dev/null +++ b/src/test/jtx/PermissionedDomains.h @@ -0,0 +1,78 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#ifndef RIPPLE_TEST_JTX_PERMISSIONEDDOMAINS_H_INCLUDED +#define RIPPLE_TEST_JTX_PERMISSIONEDDOMAINS_H_INCLUDED + +#include + +namespace ripple { +namespace test { +namespace jtx { +namespace pd { + +// Helpers for PermissionedDomains testing +using Credential = std::pair; +using Credentials = std::vector; + +// helpers +// Make json for PermissionedDomainSet transaction +Json::Value +setTx( + AccountID const& account, + Credentials const& credentials, + std::optional domain = std::nullopt); + +// Make json for PermissionedDomainDelete transaction +Json::Value +deleteTx(AccountID const& account, uint256 const& domain); + +// Get PermissionedDomain objects from account_objects rpc call +std::map +getObjects(Account const& account, Env& env); + +// Check if ledger object is there +bool +objectExists(uint256 const& objID, Env& env); + +// Convert string to Blob +inline Blob +toBlob(std::string const& input) +{ + return Blob(input.begin(), input.end()); +} + +// Extract credentials from account_object object +Credentials +credentialsFromJson(Json::Value const& object); + +// Sort credentials the same way as PermissionedDomainSet +Credentials +sortCredentials(Credentials const& input); + +// Get account_info +Json::Value +ownerInfo(Account const& account, Env& env); + +} // namespace pd +} // namespace jtx +} // namespace test +} // namespace ripple + +#endif // RIPPLE_TEST_JTX_PERMISSIONEDDOMAINS_H_INCLUDED diff --git a/src/test/jtx/impl/PermissionedDomains.cpp b/src/test/jtx/impl/PermissionedDomains.cpp new file mode 100644 index 00000000000..7b27263e990 --- /dev/null +++ b/src/test/jtx/impl/PermissionedDomains.cpp @@ -0,0 +1,151 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include + +namespace ripple { +namespace test { +namespace jtx { +namespace pd { + +// helpers +// Make json for PermissionedDomainSet transaction +Json::Value +setTx( + AccountID const& account, + Credentials const& credentials, + std::optional domain) +{ + Json::Value jv; + jv[sfTransactionType.jsonName] = jss::PermissionedDomainSet; + jv[sfAccount.jsonName] = to_string(account); + if (domain) + jv[sfDomainID.jsonName] = to_string(*domain); + Json::Value a(Json::arrayValue); + for (auto const& credential : credentials) + { + Json::Value obj(Json::objectValue); + obj[sfIssuer.jsonName] = to_string(credential.first); + obj[sfCredentialType.jsonName] = + strHex(Slice{credential.second.data(), credential.second.size()}); + Json::Value o2(Json::objectValue); + o2[sfAcceptedCredential.jsonName] = obj; + a.append(o2); + } + jv[sfAcceptedCredentials.jsonName] = a; + return jv; +} + +// Make json for PermissionedDomainDelete transaction +Json::Value +deleteTx(AccountID const& account, uint256 const& domain) +{ + Json::Value jv{Json::objectValue}; + jv[sfTransactionType.jsonName] = jss::PermissionedDomainDelete; + jv[sfAccount.jsonName] = to_string(account); + jv[sfDomainID.jsonName] = to_string(domain); + return jv; +} + +// Get PermissionedDomain objects from account_objects rpc call +std::map +getObjects(Account const& account, Env& env) +{ + std::map ret; + Json::Value params; + params[jss::account] = account.human(); + auto const& resp = env.rpc("json", "account_objects", to_string(params)); + Json::Value a(Json::arrayValue); + a = resp[jss::result][jss::account_objects]; + for (auto const& object : a) + { + if (object["LedgerEntryType"] != "PermissionedDomain") + continue; + uint256 index; + std::ignore = index.parseHex(object[jss::index].asString()); + ret[index] = object; + } + return ret; +} + +// Check if ledger object is there +bool +objectExists(uint256 const& objID, Env& env) +{ + Json::Value params; + params[jss::index] = to_string(objID); + auto const& resp = + env.rpc("json", "ledger_entry", to_string(params))["result"]["status"] + .asString(); + if (resp == "success") + return true; + if (resp == "error") + return false; + throw std::runtime_error("Error getting ledger_entry RPC result."); +} + +// Extract credentials from account_object object +Credentials +credentialsFromJson(Json::Value const& object) +{ + Credentials ret; + Json::Value a(Json::arrayValue); + a = object["AcceptedCredentials"]; + for (auto const& credential : a) + { + Json::Value obj(Json::objectValue); + obj = credential["AcceptedCredential"]; + auto const& issuer = obj["Issuer"]; + auto const& credentialType = obj["CredentialType"]; + ret.emplace_back( + *parseBase58(issuer.asString()), + strUnHex(credentialType.asString()).value()); + } + return ret; +} + +// Sort credentials the same way as PermissionedDomainSet +Credentials +sortCredentials(Credentials const& input) +{ + Credentials ret = input; + std::sort( + ret.begin(), + ret.end(), + [](Credential const& left, Credential const& right) -> bool { + return left.first < right.first; + }); + return ret; +} + +// Get account_info +Json::Value +ownerInfo(Account const& account, Env& env) +{ + Json::Value params; + params[jss::account] = account.human(); + auto const& resp = env.rpc("json", "account_info", to_string(params)); + return env.rpc( + "json", "account_info", to_string(params))["result"]["account_data"]; +} + +} // namespace pd +} // namespace jtx +} // namespace test +} // namespace ripple diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index 197c7727520..28ec63a374b 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -87,8 +87,6 @@ XRPNotCreated::visitEntry( std::shared_ptr const& before, std::shared_ptr const& after) { - std::stringstream ss; - ss << "XRPNotCreated::visitEntry start drops: " << drops_ << ". "; /* We go through all modified ledger entries, looking only at account roots, * escrow payments, and payment channels. We remove from the total any * previous XRP values and add to the total any new XRP values. The net @@ -98,7 +96,6 @@ XRPNotCreated::visitEntry( */ if (before) { - ss << "before type: " << before->getType() << ". "; switch (before->getType()) { case ltACCOUNT_ROOT: @@ -118,7 +115,6 @@ XRPNotCreated::visitEntry( if (after) { - ss << "after type: " << after->getType() << ". "; switch (after->getType()) { case ltACCOUNT_ROOT: diff --git a/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp b/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp index b737af5e570..a1a5418f5c9 100644 --- a/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp +++ b/src/xrpld/app/tx/detail/PermissionedDomainDelete.cpp @@ -29,8 +29,6 @@ PermissionedDomainDelete::preflight(PreflightContext const& ctx) return temDISABLED; if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) return ret; - if (!ctx.tx.isFieldPresent(sfDomainID)) - return temMALFORMED; return preflight2(ctx); } diff --git a/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp index e7cf94f0b64..42a91bb0c33 100644 --- a/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp +++ b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp @@ -34,12 +34,7 @@ PermissionedDomainSet::preflight(PreflightContext const& ctx) if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) return ret; - if (!ctx.tx.isFieldPresent(sfAcceptedCredentials)) - return temMALFORMED; auto const credentials = ctx.tx.getFieldArray(sfAcceptedCredentials); - // TODO check to see if we should disallow duplicate issuers. - // If so, it probably means sorting on the CredentialType field - // for identical issuers. if (credentials.empty() || credentials.size() > PD_ARRAY_MAX) return temMALFORMED; @@ -70,6 +65,8 @@ PermissionedDomainSet::preclaim(PreclaimContext const& ctx) { return temMALFORMED; } + if (credential.getFieldVL(sfCredentialType).empty()) + return temMALFORMED; if (!ctx.view.read(keylet::account(credential.getAccountID(sfIssuer)))) return temBAD_ISSUER; } @@ -106,10 +103,23 @@ PermissionedDomainSet::doApply() // needs to be ironed out. credentials.sort( [](STObject const& left, STObject const& right) -> bool { - return dynamic_cast(&left)->getAccountID( - sfIssuer) < - dynamic_cast(&right)->getAccountID( - sfIssuer); + if (left.getAccountID(sfIssuer) < right.getAccountID(sfIssuer)) + return true; + if (left.getAccountID(sfIssuer) == right.getAccountID(sfIssuer)) + { + if (left.getFieldVL(sfCredentialType) < + right.getFieldVL(sfCredentialType)) + { + return true; + } + if (left.getFieldVL(sfCredentialType) == + right.getFieldVL(sfCredentialType)) + { + throw std::runtime_error("duplicate credentials"); + } + return false; + } + return false; }); sle->setFieldArray(sfAcceptedCredentials, credentials); }; @@ -119,7 +129,14 @@ PermissionedDomainSet::doApply() // Modify existing permissioned domain. auto sleUpdate = view().peek( {ltPERMISSIONED_DOMAIN, ctx_.tx.getFieldH256(sfDomainID)}); - updateSle(sleUpdate); + try + { + updateSle(sleUpdate); + } + catch (...) + { + return temMALFORMED; + } view().update(sleUpdate); } else @@ -130,7 +147,14 @@ PermissionedDomainSet::doApply() auto slePd = std::make_shared(pdKeylet); slePd->setAccountID(sfOwner, account_); slePd->setFieldU32(sfSequence, ctx_.tx.getFieldU32(sfSequence)); - updateSle(slePd); + try + { + updateSle(slePd); + } + catch (...) + { + return temMALFORMED; + } view().insert(slePd); auto const page = view().dirInsert( keylet::ownerDir(account_), pdKeylet, describeOwnerDir(account_)); From 70dfad98b7c57d204c58ee73d538b6f343e67602 Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Tue, 1 Oct 2024 18:21:23 -0700 Subject: [PATCH 3/6] Test for 2 types of bad DomainIDs in PermissionedDomainSet. Also, test sorting of permissions. --- src/test/app/PermissionedDomains_test.cpp | 52 ++++++++---- src/test/jtx/PermissionedDomains.h | 7 +- src/test/jtx/impl/PermissionedDomains.cpp | 81 ++++++++++++++++--- .../app/tx/detail/PermissionedDomainSet.cpp | 1 - 4 files changed, 113 insertions(+), 28 deletions(-) diff --git a/src/test/app/PermissionedDomains_test.cpp b/src/test/app/PermissionedDomains_test.cpp index e96c88d55e9..f1016f3558b 100644 --- a/src/test/app/PermissionedDomains_test.cpp +++ b/src/test/app/PermissionedDomains_test.cpp @@ -175,6 +175,34 @@ class PermissionedDomains_test : public beast::unit_test::suite env(pd::setTx(account, credentialsDup, domain), fee(setFee), ter(temMALFORMED)); + + // Have equal issuers but different credentials and make sure they + // sort correctly. + { + pd::Credentials const credentialsSame{ + {alice2, pd::toBlob("credential3")}, + {alice3, pd::toBlob("credential2")}, + {alice2, pd::toBlob("credential9")}, + {alice5, pd::toBlob("credential4")}, + {alice2, pd::toBlob("credential6")}, + }; + BEAST_EXPECT( + credentialsSame != *pd::sortCredentials(credentialsSame)); + env(pd::setTx(account, credentialsSame, domain), fee(setFee)); + + uint256 d; + if (domain) + d = *domain; + else + d = pd::getNewDomain(env.meta()); + env.close(); + auto objects = pd::getObjects(account, env); + auto const fromObject = pd::credentialsFromJson(objects[d]); + auto const sortedCreds = *pd::sortCredentials(credentialsSame); + BEAST_EXPECT( + pd::credentialsFromJson(objects[d]) == + *pd::sortCredentials(credentialsSame)); + } } // Test PermissionedDomainSet @@ -238,24 +266,10 @@ class PermissionedDomains_test : public beast::unit_test::suite { BEAST_EXPECT( credentials10.size() == PermissionedDomainSet::PD_ARRAY_MAX); - BEAST_EXPECT(credentials10 != pd::sortCredentials(credentials10)); + BEAST_EXPECT(credentials10 != *pd::sortCredentials(credentials10)); env(pd::setTx(alice, credentials10), fee(setFee)); auto tx = env.tx()->getJson(JsonOptions::none); - auto meta = env.meta()->getJson(JsonOptions::none); - Json::Value a(Json::arrayValue); - a = meta["AffectedNodes"]; - - for (auto const& node : a) - { - if (!node.isMember("CreatedNode") || - node["CreatedNode"]["LedgerEntryType"] != - "PermissionedDomain") - { - continue; - } - std::ignore = domain2.parseHex( - node["CreatedNode"]["LedgerIndex"].asString()); - } + domain2 = pd::getNewDomain(env.meta()); auto objects = pd::getObjects(alice, env); auto object = objects[domain2]; BEAST_EXPECT( @@ -285,6 +299,12 @@ class PermissionedDomains_test : public beast::unit_test::suite env(pd::setTx(alice2, credentials1, domain2), ter(temINVALID_ACCOUNT_ID)); + // Update a uint256(0) domain + env(pd::setTx(alice, credentials1, uint256(0)), ter(temMALFORMED)); + + // Update non-existent domain + env(pd::setTx(alice, credentials1, uint256(75)), ter(tecNO_ENTRY)); + // Test bad data when creating a domain. testBadData(alice, env); // Test bad data when updating a domain. diff --git a/src/test/jtx/PermissionedDomains.h b/src/test/jtx/PermissionedDomains.h index ae7b673a946..95e4541ee66 100644 --- a/src/test/jtx/PermissionedDomains.h +++ b/src/test/jtx/PermissionedDomains.h @@ -21,6 +21,7 @@ #define RIPPLE_TEST_JTX_PERMISSIONEDDOMAINS_H_INCLUDED #include +#include namespace ripple { namespace test { @@ -63,13 +64,17 @@ Credentials credentialsFromJson(Json::Value const& object); // Sort credentials the same way as PermissionedDomainSet -Credentials +std::optional sortCredentials(Credentials const& input); // Get account_info Json::Value ownerInfo(Account const& account, Env& env); +// Get newly created domain from transaction metadata. +uint256 +getNewDomain(std::shared_ptr const& meta); + } // namespace pd } // namespace jtx } // namespace test diff --git a/src/test/jtx/impl/PermissionedDomains.cpp b/src/test/jtx/impl/PermissionedDomains.cpp index 7b27263e990..f773140b266 100644 --- a/src/test/jtx/impl/PermissionedDomains.cpp +++ b/src/test/jtx/impl/PermissionedDomains.cpp @@ -18,6 +18,7 @@ //============================================================================== #include +#include namespace ripple { namespace test { @@ -120,18 +121,56 @@ credentialsFromJson(Json::Value const& object) return ret; } -// Sort credentials the same way as PermissionedDomainSet -Credentials +// Sort credentials the same way as PermissionedDomainSet. None can +// be identical. +std::optional sortCredentials(Credentials const& input) { - Credentials ret = input; - std::sort( - ret.begin(), - ret.end(), - [](Credential const& left, Credential const& right) -> bool { - return left.first < right.first; - }); - return ret; + try + { + Credentials ret = input; + std::sort( + ret.begin(), + ret.end(), + [](Credential const& left, Credential const& right) -> bool { + if (left.first < right.first) + return true; + if (left.first == right.first) + { + if (left.second < right.second) + return true; + if (left.second == right.second) + throw std::runtime_error("duplicate"); + } + return false; + /* + if (left.getAccountID(sfIssuer) < right.getAccountID(sfIssuer)) + return true; + if (left.getAccountID(sfIssuer) == right.getAccountID(sfIssuer)) + { + if (left.getFieldVL(sfCredentialType) < + right.getFieldVL(sfCredentialType)) + { + return true; + } + if (left.getFieldVL(sfCredentialType) == + right.getFieldVL(sfCredentialType)) + { + throw std::runtime_error("duplicate credentials"); + } + return false; + } + return false; + return left.first < right.first; + */ + }); + return ret; + } + catch (...) + { + std::cerr << "wtf\n"; + return std::nullopt; + } } // Get account_info @@ -145,6 +184,28 @@ ownerInfo(Account const& account, Env& env) "json", "account_info", to_string(params))["result"]["account_data"]; } +uint256 +getNewDomain(std::shared_ptr const& meta) +{ + uint256 ret; + auto metaJson = meta->getJson(JsonOptions::none); + Json::Value a(Json::arrayValue); + a = metaJson["AffectedNodes"]; + + for (auto const& node : a) + { + if (!node.isMember("CreatedNode") || + node["CreatedNode"]["LedgerEntryType"] != "PermissionedDomain") + { + continue; + } + std::ignore = + ret.parseHex(node["CreatedNode"]["LedgerIndex"].asString()); + } + + return ret; +} + } // namespace pd } // namespace jtx } // namespace test diff --git a/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp index 42a91bb0c33..f3a5c2aaac7 100644 --- a/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp +++ b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp @@ -117,7 +117,6 @@ PermissionedDomainSet::doApply() { throw std::runtime_error("duplicate credentials"); } - return false; } return false; }); From e08bef81c8dd57d75e495d1c5bc56cd6f004f49c Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Mon, 7 Oct 2024 17:04:29 -0700 Subject: [PATCH 4/6] Silently remove duplicate credentials instead of failing the transaction. Conforms to Credentials spec. --- src/test/app/PermissionedDomains_test.cpp | 44 +++++++---- src/test/jtx/PermissionedDomains.h | 3 +- src/test/jtx/impl/PermissionedDomains.cpp | 73 +++++++------------ .../app/tx/detail/PermissionedDomainSet.cpp | 40 ++++------ 4 files changed, 72 insertions(+), 88 deletions(-) diff --git a/src/test/app/PermissionedDomains_test.cpp b/src/test/app/PermissionedDomains_test.cpp index f1016f3558b..6b1454f30d3 100644 --- a/src/test/app/PermissionedDomains_test.cpp +++ b/src/test/app/PermissionedDomains_test.cpp @@ -165,16 +165,32 @@ class PermissionedDomains_test : public beast::unit_test::suite .removeMember("Issuer"); env(txJsonMutable, fee(setFee), ter(temMALFORMED)); - // Make 2 identical credentials. - pd::Credentials const credentialsDup{ - {alice2, pd::toBlob("credential1")}, - {alice3, pd::toBlob("credential2")}, - {alice2, pd::toBlob("credential1")}, - {alice5, pd::toBlob("credential4")}, - }; - env(pd::setTx(account, credentialsDup, domain), - fee(setFee), - ter(temMALFORMED)); + // Make 2 identical credentials. The duplicate should be silently + // removed. + { + pd::Credentials const credentialsDup{ + {alice7, pd::toBlob("credential6")}, + {alice2, pd::toBlob("credential1")}, + {alice3, pd::toBlob("credential2")}, + {alice2, pd::toBlob("credential1")}, + {alice5, pd::toBlob("credential4")}, + }; + BEAST_EXPECT(pd::sortCredentials(credentialsDup).size() == 4); + env(pd::setTx(account, credentialsDup, domain), fee(setFee)); + + uint256 d; + if (domain) + d = *domain; + else + d = pd::getNewDomain(env.meta()); + env.close(); + auto objects = pd::getObjects(account, env); + auto const fromObject = pd::credentialsFromJson(objects[d]); + auto const sortedCreds = pd::sortCredentials(credentialsDup); + BEAST_EXPECT( + pd::credentialsFromJson(objects[d]) == + pd::sortCredentials(credentialsDup)); + } // Have equal issuers but different credentials and make sure they // sort correctly. @@ -187,7 +203,7 @@ class PermissionedDomains_test : public beast::unit_test::suite {alice2, pd::toBlob("credential6")}, }; BEAST_EXPECT( - credentialsSame != *pd::sortCredentials(credentialsSame)); + credentialsSame != pd::sortCredentials(credentialsSame)); env(pd::setTx(account, credentialsSame, domain), fee(setFee)); uint256 d; @@ -198,10 +214,10 @@ class PermissionedDomains_test : public beast::unit_test::suite env.close(); auto objects = pd::getObjects(account, env); auto const fromObject = pd::credentialsFromJson(objects[d]); - auto const sortedCreds = *pd::sortCredentials(credentialsSame); + auto const sortedCreds = pd::sortCredentials(credentialsSame); BEAST_EXPECT( pd::credentialsFromJson(objects[d]) == - *pd::sortCredentials(credentialsSame)); + pd::sortCredentials(credentialsSame)); } } @@ -266,7 +282,7 @@ class PermissionedDomains_test : public beast::unit_test::suite { BEAST_EXPECT( credentials10.size() == PermissionedDomainSet::PD_ARRAY_MAX); - BEAST_EXPECT(credentials10 != *pd::sortCredentials(credentials10)); + BEAST_EXPECT(credentials10 != pd::sortCredentials(credentials10)); env(pd::setTx(alice, credentials10), fee(setFee)); auto tx = env.tx()->getJson(JsonOptions::none); domain2 = pd::getNewDomain(env.meta()); diff --git a/src/test/jtx/PermissionedDomains.h b/src/test/jtx/PermissionedDomains.h index 95e4541ee66..20c5467b128 100644 --- a/src/test/jtx/PermissionedDomains.h +++ b/src/test/jtx/PermissionedDomains.h @@ -21,7 +21,6 @@ #define RIPPLE_TEST_JTX_PERMISSIONEDDOMAINS_H_INCLUDED #include -#include namespace ripple { namespace test { @@ -64,7 +63,7 @@ Credentials credentialsFromJson(Json::Value const& object); // Sort credentials the same way as PermissionedDomainSet -std::optional +Credentials sortCredentials(Credentials const& input); // Get account_info diff --git a/src/test/jtx/impl/PermissionedDomains.cpp b/src/test/jtx/impl/PermissionedDomains.cpp index f773140b266..5405ada76b1 100644 --- a/src/test/jtx/impl/PermissionedDomains.cpp +++ b/src/test/jtx/impl/PermissionedDomains.cpp @@ -121,56 +121,37 @@ credentialsFromJson(Json::Value const& object) return ret; } -// Sort credentials the same way as PermissionedDomainSet. None can -// be identical. -std::optional +// Sort credentials the same way as PermissionedDomainSet. Silently +// remove duplicates. +Credentials sortCredentials(Credentials const& input) { - try - { - Credentials ret = input; - std::sort( - ret.begin(), - ret.end(), - [](Credential const& left, Credential const& right) -> bool { - if (left.first < right.first) - return true; - if (left.first == right.first) - { - if (left.second < right.second) - return true; - if (left.second == right.second) - throw std::runtime_error("duplicate"); - } - return false; - /* - if (left.getAccountID(sfIssuer) < right.getAccountID(sfIssuer)) - return true; - if (left.getAccountID(sfIssuer) == right.getAccountID(sfIssuer)) - { - if (left.getFieldVL(sfCredentialType) < - right.getFieldVL(sfCredentialType)) - { - return true; - } - if (left.getFieldVL(sfCredentialType) == - right.getFieldVL(sfCredentialType)) - { - throw std::runtime_error("duplicate credentials"); - } - return false; - } - return false; - return left.first < right.first; - */ - }); - return ret; - } - catch (...) + Credentials ret = input; + + std::set cSet; + for (auto const& c : ret) + cSet.insert(c); + if (ret.size() > cSet.size()) { - std::cerr << "wtf\n"; - return std::nullopt; + ret = Credentials(); + for (auto const& c : cSet) + ret.push_back(c); } + + std::sort( + ret.begin(), + ret.end(), + [](Credential const& left, Credential const& right) -> bool { + if (left.first < right.first) + return true; + if (left.first == right.first) + { + if (left.second < right.second) + return true; + } + return false; + }); + return ret; } // Get account_info diff --git a/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp index f3a5c2aaac7..b3c4fa2f684 100644 --- a/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp +++ b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp @@ -21,7 +21,7 @@ #include #include #include - +#include #include namespace ripple { @@ -95,12 +95,19 @@ PermissionedDomainSet::doApply() // All checks have already been done. Just update credentials. Same logic // for either new domain or updating existing. + // Silently remove duplicates. auto updateSle = [this](std::shared_ptr const& sle) { auto credentials = ctx_.tx.getFieldArray(sfAcceptedCredentials); - // TODO when credentials are merged, it is possible that this will - // also need to sort on the CredentialType field in case there - // are multiple issuers in each set of credentials. The spec - // needs to be ironed out. + std::map hashed; + for (auto const& c : credentials) + hashed.insert({c.getHash(HashPrefix::transactionID), c}); + if (credentials.size() > hashed.size()) + { + credentials = STArray(); + for (auto const& e : hashed) + credentials.push_back(e.second); + } + credentials.sort( [](STObject const& left, STObject const& right) -> bool { if (left.getAccountID(sfIssuer) < right.getAccountID(sfIssuer)) @@ -112,11 +119,6 @@ PermissionedDomainSet::doApply() { return true; } - if (left.getFieldVL(sfCredentialType) == - right.getFieldVL(sfCredentialType)) - { - throw std::runtime_error("duplicate credentials"); - } } return false; }); @@ -128,14 +130,7 @@ PermissionedDomainSet::doApply() // Modify existing permissioned domain. auto sleUpdate = view().peek( {ltPERMISSIONED_DOMAIN, ctx_.tx.getFieldH256(sfDomainID)}); - try - { - updateSle(sleUpdate); - } - catch (...) - { - return temMALFORMED; - } + updateSle(sleUpdate); view().update(sleUpdate); } else @@ -146,14 +141,7 @@ PermissionedDomainSet::doApply() auto slePd = std::make_shared(pdKeylet); slePd->setAccountID(sfOwner, account_); slePd->setFieldU32(sfSequence, ctx_.tx.getFieldU32(sfSequence)); - try - { - updateSle(slePd); - } - catch (...) - { - return temMALFORMED; - } + updateSle(slePd); view().insert(slePd); auto const page = view().dirInsert( keylet::ownerDir(account_), pdKeylet, describeOwnerDir(account_)); From 7070c5695a05a8b0acbb9417e52ceac08d23802d Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Mon, 14 Oct 2024 13:24:23 -0700 Subject: [PATCH 5/6] Review fixes from Denis. --- include/xrpl/protocol/Indexes.h | 2 + src/libxrpl/protocol/Indexes.cpp | 6 + src/test/app/PermissionedDomains_test.cpp | 123 +++++++++++++----- src/test/jtx/PermissionedDomains.h | 2 +- src/test/jtx/impl/PermissionedDomains.cpp | 6 +- src/test/rpc/AccountObjects_test.cpp | 1 + .../app/tx/detail/PermissionedDomainSet.cpp | 53 ++++---- .../app/tx/detail/PermissionedDomainSet.h | 3 - src/xrpld/rpc/detail/RPCHelpers.cpp | 5 +- 9 files changed, 142 insertions(+), 59 deletions(-) diff --git a/include/xrpl/protocol/Indexes.h b/include/xrpl/protocol/Indexes.h index dcd7fd4b76b..d4e4965ee10 100644 --- a/include/xrpl/protocol/Indexes.h +++ b/include/xrpl/protocol/Indexes.h @@ -290,6 +290,8 @@ oracle(AccountID const& account, std::uint32_t const& documentID) noexcept; Keylet permissionedDomain(AccountID const& account, std::uint32_t seq) noexcept; +Keylet +permissionedDomain(uint256 const& domainID) noexcept; } // namespace keylet // Everything below is deprecated and should be removed in favor of keylets: diff --git a/src/libxrpl/protocol/Indexes.cpp b/src/libxrpl/protocol/Indexes.cpp index 0eddfdc6e2c..36b9a55a139 100644 --- a/src/libxrpl/protocol/Indexes.cpp +++ b/src/libxrpl/protocol/Indexes.cpp @@ -460,6 +460,12 @@ permissionedDomain(AccountID const& account, std::uint32_t seq) noexcept indexHash(LedgerNameSpace::PERMISSIONED_DOMAIN, account, seq)}; } +Keylet +permissionedDomain(uint256 const& domainID) noexcept +{ + return {ltPERMISSIONED_DOMAIN, domainID}; +} + } // namespace keylet } // namespace ripple diff --git a/src/test/app/PermissionedDomains_test.cpp b/src/test/app/PermissionedDomains_test.cpp index 6b1454f30d3..03dd6e98eb7 100644 --- a/src/test/app/PermissionedDomains_test.cpp +++ b/src/test/app/PermissionedDomains_test.cpp @@ -52,12 +52,13 @@ class PermissionedDomains_test : public beast::unit_test::suite Account const alice("alice"); Env env(*this, withFeature_); env.fund(XRP(1000), alice); - auto const setFee(drops(env.current()->fees().increment)); pd::Credentials credentials{{alice, pd::toBlob("first credential")}}; - env(pd::setTx(alice, credentials), fee(setFee)); + env(pd::setTx(alice, credentials)); BEAST_EXPECT(pd::ownerInfo(alice, env)["OwnerCount"].asUInt() == 1); auto objects = pd::getObjects(alice, env); BEAST_EXPECT(objects.size() == 1); + // Test that account_objects is correct without passing it the type + BEAST_EXPECT(objects == pd::getObjects(alice, env, false)); auto const domain = objects.begin()->first; env(pd::deleteTx(alice, domain)); } @@ -70,9 +71,8 @@ class PermissionedDomains_test : public beast::unit_test::suite Account const alice("alice"); Env env(*this, withoutFeature_); env.fund(XRP(1000), alice); - auto const setFee(drops(env.current()->fees().increment)); pd::Credentials credentials{{alice, pd::toBlob("first credential")}}; - env(pd::setTx(alice, credentials), fee(setFee), ter(temDISABLED)); + env(pd::setTx(alice, credentials), ter(temDISABLED)); env(pd::deleteTx(alice, uint256(75)), ter(temDISABLED)); } @@ -98,9 +98,7 @@ class PermissionedDomains_test : public beast::unit_test::suite auto const setFee(drops(env.current()->fees().increment)); // Test empty credentials. - env(pd::setTx(account, pd::Credentials(), domain), - fee(setFee), - ter(temMALFORMED)); + env(pd::setTx(account, pd::Credentials(), domain), ter(temMALFORMED)); // Test 11 credentials. pd::Credentials const credentials11{ @@ -117,9 +115,7 @@ class PermissionedDomains_test : public beast::unit_test::suite {alice12, pd::toBlob("credential11")}}; BEAST_EXPECT( credentials11.size() == PermissionedDomainSet::PD_ARRAY_MAX + 1); - env(pd::setTx(account, credentials11, domain), - fee(setFee), - ter(temMALFORMED)); + env(pd::setTx(account, credentials11, domain), ter(temMALFORMED)); // Test credentials including non-existent issuer. Account const nobody("nobody"); @@ -131,9 +127,7 @@ class PermissionedDomains_test : public beast::unit_test::suite {alice5, pd::toBlob("credential5")}, {alice6, pd::toBlob("credential6")}, {alice7, pd::toBlob("credential7")}}; - env(pd::setTx(account, credentialsNon, domain), - fee(setFee), - ter(temBAD_ISSUER)); + env(pd::setTx(account, credentialsNon, domain), ter(temBAD_ISSUER)); pd::Credentials const credentials4{ {alice2, pd::toBlob("credential1")}, @@ -147,23 +141,23 @@ class PermissionedDomains_test : public beast::unit_test::suite // Remove Issuer from a credential and apply. txJsonMutable["AcceptedCredentials"][2u]["AcceptedCredential"] .removeMember("Issuer"); - env(txJsonMutable, fee(setFee), ter(temMALFORMED)); + env(txJsonMutable, ter(temMALFORMED)); txJsonMutable["AcceptedCredentials"][2u] = credentialOrig; // Make an empty CredentialType. txJsonMutable["AcceptedCredentials"][2u]["AcceptedCredential"] ["CredentialType"] = ""; - env(txJsonMutable, fee(setFee), ter(temMALFORMED)); + env(txJsonMutable, ter(temMALFORMED)); // Remove Credentialtype from a credential and apply. txJsonMutable["AcceptedCredentials"][2u]["AcceptedCredential"] .removeMember("CredentialType"); - env(txJsonMutable, fee(setFee), ter(temMALFORMED)); + env(txJsonMutable, ter(temMALFORMED)); // Remove both txJsonMutable["AcceptedCredentials"][2u]["AcceptedCredential"] .removeMember("Issuer"); - env(txJsonMutable, fee(setFee), ter(temMALFORMED)); + env(txJsonMutable, ter(temMALFORMED)); // Make 2 identical credentials. The duplicate should be silently // removed. @@ -176,7 +170,7 @@ class PermissionedDomains_test : public beast::unit_test::suite {alice5, pd::toBlob("credential4")}, }; BEAST_EXPECT(pd::sortCredentials(credentialsDup).size() == 4); - env(pd::setTx(account, credentialsDup, domain), fee(setFee)); + env(pd::setTx(account, credentialsDup, domain)); uint256 d; if (domain) @@ -204,7 +198,7 @@ class PermissionedDomains_test : public beast::unit_test::suite }; BEAST_EXPECT( credentialsSame != pd::sortCredentials(credentialsSame)); - env(pd::setTx(account, credentialsSame, domain), fee(setFee)); + env(pd::setTx(account, credentialsSame, domain)); uint256 d; if (domain) @@ -245,13 +239,11 @@ class PermissionedDomains_test : public beast::unit_test::suite alice10, alice11, alice12); - auto const dropsFee = env.current()->fees().increment; - auto const setFee(drops(dropsFee)); // Create new from existing account with a single credential. pd::Credentials const credentials1{{alice2, pd::toBlob("credential1")}}; { - env(pd::setTx(alice, credentials1), fee(setFee)); + env(pd::setTx(alice, credentials1)); BEAST_EXPECT(pd::ownerInfo(alice, env)["OwnerCount"].asUInt() == 1); auto tx = env.tx()->getJson(JsonOptions::none); BEAST_EXPECT(tx[jss::TransactionType] == "PermissionedDomainSet"); @@ -283,7 +275,7 @@ class PermissionedDomains_test : public beast::unit_test::suite BEAST_EXPECT( credentials10.size() == PermissionedDomainSet::PD_ARRAY_MAX); BEAST_EXPECT(credentials10 != pd::sortCredentials(credentials10)); - env(pd::setTx(alice, credentials10), fee(setFee)); + env(pd::setTx(alice, credentials10)); auto tx = env.tx()->getJson(JsonOptions::none); domain2 = pd::getNewDomain(env.meta()); auto objects = pd::getObjects(alice, env); @@ -293,11 +285,6 @@ class PermissionedDomains_test : public beast::unit_test::suite pd::sortCredentials(credentials10)); } - // Make a new domain with insufficient fee. - env(pd::setTx(alice, credentials10), - fee(drops(dropsFee - 1)), - ter(telINSUF_FEE_P)); - // Update with 1 credential. env(pd::setTx(alice, credentials1, domain2)); BEAST_EXPECT( @@ -364,7 +351,7 @@ class PermissionedDomains_test : public beast::unit_test::suite env.fund(XRP(1000), alice); auto const setFee(drops(env.current()->fees().increment)); pd::Credentials credentials{{alice, pd::toBlob("first credential")}}; - env(pd::setTx(alice, credentials), fee(setFee)); + env(pd::setTx(alice, credentials)); env.close(); auto objects = pd::getObjects(alice, env); BEAST_EXPECT(objects.size() == 1); @@ -396,6 +383,83 @@ class PermissionedDomains_test : public beast::unit_test::suite BEAST_EXPECT(!pd::objectExists(objID, env)); } + void + testAccountReserve() + { + // Verify that the reserve behaves as expected for minting. + testcase("Account Reserve"); + + using namespace test::jtx; + + Env env(*this, withFeature_); + Account const alice("alice"); + + // Fund alice enough to exist, but not enough to meet + // the reserve. + auto const acctReserve = env.current()->fees().accountReserve(0); + auto const incReserve = env.current()->fees().increment; + env.fund(acctReserve, alice); + env.close(); + BEAST_EXPECT(env.balance(alice) == acctReserve); + BEAST_EXPECT(pd::ownerInfo(alice, env)["OwnerCount"].asUInt() == 0); + + // alice does not have enough XRP to cover the reserve. + pd::Credentials credentials{{alice, pd::toBlob("first credential")}}; + env(pd::setTx(alice, credentials), ter(tecINSUFFICIENT_RESERVE)); + BEAST_EXPECT(pd::ownerInfo(alice, env)["OwnerCount"].asUInt() == 0); + BEAST_EXPECT(pd::getObjects(alice, env).size() == 0); + env.close(); + + // Pay alice almost enough to make the reserve. + env(pay(env.master, alice, incReserve + drops(19))); + BEAST_EXPECT(env.balance(alice) == acctReserve + incReserve + drops(9)); + env.close(); + + // alice still does not have enough XRP for the reserve. + env(pd::setTx(alice, credentials), ter(tecINSUFFICIENT_RESERVE)); + env.close(); + BEAST_EXPECT(pd::ownerInfo(alice, env)["OwnerCount"].asUInt() == 0); + + // Pay alice enough to make the reserve. + env(pay(env.master, alice, drops(11))); + env.close(); + + // Now alice can create a PermissionedDomain. + env(pd::setTx(alice, credentials)); + env.close(); + BEAST_EXPECT(pd::ownerInfo(alice, env)["OwnerCount"].asUInt() == 1); + + /* + env(did::setValid(alice), ter(tecINSUFFICIENT_RESERVE)); + env.close(); + BEAST_EXPECT(ownerCount(env, alice) == 0); + + // Pay alice almost enough to make the reserve for a DID. + env(pay(env.master, alice, incReserve + drops(19))); + BEAST_EXPECT(env.balance(alice) == acctReserve + incReserve + + drops(9)); env.close(); + + // alice still does not have enough XRP for the reserve of a + DID. env(did::setValid(alice), ter(tecINSUFFICIENT_RESERVE)); + env.close(); + BEAST_EXPECT(ownerCount(env, alice) == 0); + + // Pay alice enough to make the reserve for a DID. + env(pay(env.master, alice, drops(11))); + env.close(); + + // Now alice can create a DID. + env(did::setValid(alice)); + env.close(); + BEAST_EXPECT(ownerCount(env, alice) == 1); + + // alice deletes her DID. + env(did::del(alice)); + BEAST_EXPECT(ownerCount(env, alice) == 0); + env.close(); + */ + } + public: void run() override @@ -404,6 +468,7 @@ class PermissionedDomains_test : public beast::unit_test::suite testDisabled(); testSet(); testDelete(); + testAccountReserve(); } }; diff --git a/src/test/jtx/PermissionedDomains.h b/src/test/jtx/PermissionedDomains.h index 20c5467b128..3ddf08096f7 100644 --- a/src/test/jtx/PermissionedDomains.h +++ b/src/test/jtx/PermissionedDomains.h @@ -45,7 +45,7 @@ deleteTx(AccountID const& account, uint256 const& domain); // Get PermissionedDomain objects from account_objects rpc call std::map -getObjects(Account const& account, Env& env); +getObjects(Account const& account, Env& env, bool withType = true); // Check if ledger object is there bool diff --git a/src/test/jtx/impl/PermissionedDomains.cpp b/src/test/jtx/impl/PermissionedDomains.cpp index 5405ada76b1..632efb7e485 100644 --- a/src/test/jtx/impl/PermissionedDomains.cpp +++ b/src/test/jtx/impl/PermissionedDomains.cpp @@ -64,13 +64,15 @@ deleteTx(AccountID const& account, uint256 const& domain) return jv; } -// Get PermissionedDomain objects from account_objects rpc call +// Get PermissionedDomain objects by type from account_objects rpc call std::map -getObjects(Account const& account, Env& env) +getObjects(Account const& account, Env& env, bool withType) { std::map ret; Json::Value params; params[jss::account] = account.human(); + if (withType) + params[jss::type] = jss::permissioned_domain; auto const& resp = env.rpc("json", "account_objects", to_string(params)); Json::Value a(Json::arrayValue); a = resp[jss::result][jss::account_objects]; diff --git a/src/test/rpc/AccountObjects_test.cpp b/src/test/rpc/AccountObjects_test.cpp index f58446e66c9..7acf8746c0e 100644 --- a/src/test/rpc/AccountObjects_test.cpp +++ b/src/test/rpc/AccountObjects_test.cpp @@ -627,6 +627,7 @@ class AccountObjects_test : public beast::unit_test::suite BEAST_EXPECT(acctObjsIsSize(acctObjs(gw, jss::ticket), 0)); BEAST_EXPECT(acctObjsIsSize(acctObjs(gw, jss::amm), 0)); BEAST_EXPECT(acctObjsIsSize(acctObjs(gw, jss::did), 0)); + BEAST_EXPECT(acctObjsIsSize(acctObjs(gw, jss::permissioned_domain), 0)); // we expect invalid field type reported for the following types BEAST_EXPECT(acctObjsTypeIsInvalid(acctObjs(gw, jss::amendments))); diff --git a/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp index b3c4fa2f684..c7c088d9121 100644 --- a/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp +++ b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp @@ -37,17 +37,22 @@ PermissionedDomainSet::preflight(PreflightContext const& ctx) auto const credentials = ctx.tx.getFieldArray(sfAcceptedCredentials); if (credentials.empty() || credentials.size() > PD_ARRAY_MAX) return temMALFORMED; + for (auto const& credential : credentials) + { + if (!credential.isFieldPresent(sfIssuer) || + !credential.isFieldPresent(sfCredentialType)) + { + return temMALFORMED; + } + if (credential.getFieldVL(sfCredentialType).empty()) + return temMALFORMED; + } - return preflight2(ctx); -} + auto const domain = ctx.tx.at(~sfDomainID); + if (domain && *domain == beast::zero) + return temMALFORMED; -XRPAmount -PermissionedDomainSet::calculateBaseFee(ReadView const& view, STTx const& tx) -{ - if (tx.isFieldPresent(sfDomainID)) - return Transactor::calculateBaseFee(view, tx); - // The fee required for a new PermissionedDomain is one owner reserve. - return view.fees().increment; + return preflight2(ctx); } TER @@ -56,17 +61,9 @@ PermissionedDomainSet::preclaim(PreclaimContext const& ctx) if (!ctx.view.read(keylet::account(ctx.tx.getAccountID(sfAccount)))) return tefINTERNAL; - assert(ctx.tx.isFieldPresent(sfAcceptedCredentials)); auto const credentials = ctx.tx.getFieldArray(sfAcceptedCredentials); for (auto const& credential : credentials) { - if (!credential.isFieldPresent(sfIssuer) || - !credential.isFieldPresent(sfCredentialType)) - { - return temMALFORMED; - } - if (credential.getFieldVL(sfCredentialType).empty()) - return temMALFORMED; if (!ctx.view.read(keylet::account(credential.getAccountID(sfIssuer)))) return temBAD_ISSUER; } @@ -74,9 +71,7 @@ PermissionedDomainSet::preclaim(PreclaimContext const& ctx) if (!ctx.tx.isFieldPresent(sfDomainID)) return tesSUCCESS; auto const domain = ctx.tx.getFieldH256(sfDomainID); - if (domain == beast::zero) - return temMALFORMED; - auto const sleDomain = ctx.view.read({ltPERMISSIONED_DOMAIN, domain}); + auto const sleDomain = ctx.view.read(keylet::permissionedDomain(domain)); if (!sleDomain) return tecNO_ENTRY; auto const owner = sleDomain->getAccountID(sfOwner); @@ -93,6 +88,18 @@ PermissionedDomainSet::doApply() { auto const ownerSle = view().peek(keylet::account(account_)); + // Check reserve availability for new object creation + { + auto const balance = STAmount((*ownerSle)[sfBalance]).xrp(); + auto const reserve = + ctx_.view().fees().accountReserve((*ownerSle)[sfOwnerCount] + 1); + + if (balance < reserve) + return tecINSUFFICIENT_RESERVE; + } + + // The purpose of this lambda is to modify the SLE for either creating a + // new or updating an existing object, to reduce code repetition. // All checks have already been done. Just update credentials. Same logic // for either new domain or updating existing. // Silently remove duplicates. @@ -129,7 +136,9 @@ PermissionedDomainSet::doApply() { // Modify existing permissioned domain. auto sleUpdate = view().peek( - {ltPERMISSIONED_DOMAIN, ctx_.tx.getFieldH256(sfDomainID)}); + keylet::permissionedDomain(ctx_.tx.getFieldH256(sfDomainID))); + // It should already be checked in preclaim(). + assert(sleUpdate); updateSle(sleUpdate); view().update(sleUpdate); } @@ -142,7 +151,6 @@ PermissionedDomainSet::doApply() slePd->setAccountID(sfOwner, account_); slePd->setFieldU32(sfSequence, ctx_.tx.getFieldU32(sfSequence)); updateSle(slePd); - view().insert(slePd); auto const page = view().dirInsert( keylet::ownerDir(account_), pdKeylet, describeOwnerDir(account_)); if (!page) @@ -150,6 +158,7 @@ PermissionedDomainSet::doApply() slePd->setFieldU64(sfOwnerNode, *page); // If we succeeded, the new entry counts against the creator's reserve. adjustOwnerCount(view(), ownerSle, 1, ctx_.journal); + view().insert(slePd); } return tesSUCCESS; diff --git a/src/xrpld/app/tx/detail/PermissionedDomainSet.h b/src/xrpld/app/tx/detail/PermissionedDomainSet.h index e1f105b1d15..fcc507d1867 100644 --- a/src/xrpld/app/tx/detail/PermissionedDomainSet.h +++ b/src/xrpld/app/tx/detail/PermissionedDomainSet.h @@ -38,9 +38,6 @@ class PermissionedDomainSet : public Transactor static NotTEC preflight(PreflightContext const& ctx); - static XRPAmount - calculateBaseFee(ReadView const& view, STTx const& tx); - static TER preclaim(PreclaimContext const& ctx); diff --git a/src/xrpld/rpc/detail/RPCHelpers.cpp b/src/xrpld/rpc/detail/RPCHelpers.cpp index fa66fecfbba..4a11020808d 100644 --- a/src/xrpld/rpc/detail/RPCHelpers.cpp +++ b/src/xrpld/rpc/detail/RPCHelpers.cpp @@ -915,7 +915,7 @@ chooseLedgerEntryType(Json::Value const& params) std::pair result{RPC::Status::OK, ltANY}; if (params.isMember(jss::type)) { - static constexpr std::array, 22> + static constexpr std::array, 23> types{ {{jss::account, ltACCOUNT_ROOT}, {jss::amendments, ltAMENDMENTS}, @@ -939,7 +939,8 @@ chooseLedgerEntryType(Json::Value const& params) {jss::ticket, ltTICKET}, {jss::xchain_owned_claim_id, ltXCHAIN_OWNED_CLAIM_ID}, {jss::xchain_owned_create_account_claim_id, - ltXCHAIN_OWNED_CREATE_ACCOUNT_CLAIM_ID}}}; + ltXCHAIN_OWNED_CREATE_ACCOUNT_CLAIM_ID}, + {jss::permissioned_domain, ltPERMISSIONED_DOMAIN}}}; auto const& p = params[jss::type]; if (!p.isString()) From bb5d6039c618d78994d9a2184025ba23c85cf8b6 Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Tue, 15 Oct 2024 04:19:46 -0700 Subject: [PATCH 6/6] Check for reserve only when creating a new object. --- .../app/tx/detail/PermissionedDomainSet.cpp | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp index c7c088d9121..7d296599cbc 100644 --- a/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp +++ b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp @@ -88,16 +88,6 @@ PermissionedDomainSet::doApply() { auto const ownerSle = view().peek(keylet::account(account_)); - // Check reserve availability for new object creation - { - auto const balance = STAmount((*ownerSle)[sfBalance]).xrp(); - auto const reserve = - ctx_.view().fees().accountReserve((*ownerSle)[sfOwnerCount] + 1); - - if (balance < reserve) - return tecINSUFFICIENT_RESERVE; - } - // The purpose of this lambda is to modify the SLE for either creating a // new or updating an existing object, to reduce code repetition. // All checks have already been done. Just update credentials. Same logic @@ -145,6 +135,13 @@ PermissionedDomainSet::doApply() else { // Create new permissioned domain. + // Check reserve availability for new object creation + auto const balance = STAmount((*ownerSle)[sfBalance]).xrp(); + auto const reserve = + ctx_.view().fees().accountReserve((*ownerSle)[sfOwnerCount] + 1); + if (balance < reserve) + return tecINSUFFICIENT_RESERVE; + Keylet const pdKeylet = keylet::permissionedDomain( account_, ctx_.tx.getFieldU32(sfSequence)); auto slePd = std::make_shared(pdKeylet);