From f6d63082c0037e0198be5bed17cd24648917b36f Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 5 Feb 2025 11:36:43 -0500 Subject: [PATCH 1/3] Improve git commit hash lookup (#5225) - Also get the branch name. - Use rev-parse instead of describe to get a clean hash. - Return the git hash and branch name in server_info for admin connections. - Include git hash and branch name on separate lines in --version. --- CMakeLists.txt | 10 +++++++++- include/xrpl/protocol/jss.h | 2 ++ src/test/rpc/ServerInfo_test.cpp | 31 ++++++++++++++++++++++++++----- src/xrpld/app/main/Main.cpp | 6 ++++++ src/xrpld/app/misc/NetworkOPs.cpp | 12 ++++++++++++ 5 files changed, 55 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 49ecd192b7..03dba51d0c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -19,13 +19,21 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON) # make GIT_COMMIT_HASH define available to all sources find_package(Git) if(Git_FOUND) - execute_process(COMMAND ${GIT_EXECUTABLE} --git-dir=${CMAKE_CURRENT_SOURCE_DIR}/.git describe --always --abbrev=40 + execute_process(COMMAND ${GIT_EXECUTABLE} --git-dir=${CMAKE_CURRENT_SOURCE_DIR}/.git rev-parse HEAD OUTPUT_STRIP_TRAILING_WHITESPACE OUTPUT_VARIABLE gch) if(gch) set(GIT_COMMIT_HASH "${gch}") message(STATUS gch: ${GIT_COMMIT_HASH}) add_definitions(-DGIT_COMMIT_HASH="${GIT_COMMIT_HASH}") endif() + + execute_process(COMMAND ${GIT_EXECUTABLE} --git-dir=${CMAKE_CURRENT_SOURCE_DIR}/.git rev-parse --abbrev-ref HEAD + OUTPUT_STRIP_TRAILING_WHITESPACE OUTPUT_VARIABLE gb) + if(gb) + set(GIT_BRANCH "${gb}") + message(STATUS gb: ${GIT_BRANCH}) + add_definitions(-DGIT_BRANCH="${GIT_BRANCH}") + endif() endif() #git if(thread_safety_analysis) diff --git a/include/xrpl/protocol/jss.h b/include/xrpl/protocol/jss.h index 4db8e0e32d..483b69a962 100644 --- a/include/xrpl/protocol/jss.h +++ b/include/xrpl/protocol/jss.h @@ -167,6 +167,7 @@ JSS(blobs_v2); // out: ValidatorList JSS(books); // in: Subscribe, Unsubscribe JSS(both); // in: Subscribe, Unsubscribe JSS(both_sides); // in: Subscribe, Unsubscribe +JSS(branch); // out: server_info JSS(broadcast); // out: SubmitTransaction JSS(bridge_account); // in: LedgerEntry JSS(build_path); // in: TransactionSign @@ -290,6 +291,7 @@ JSS(frozen_balances); // out: GatewayBalances JSS(full); // in: LedgerClearer, handlers/Ledger JSS(full_reply); // out: PathFind JSS(fullbelow_size); // out: GetCounts +JSS(git); // out: server_info JSS(good); // out: RPCVersion JSS(hash); // out: NetworkOPs, InboundLedger, // LedgerToJson, STTx; field diff --git a/src/test/rpc/ServerInfo_test.cpp b/src/test/rpc/ServerInfo_test.cpp index 2f0cdee77e..5e202f275a 100644 --- a/src/test/rpc/ServerInfo_test.cpp +++ b/src/test/rpc/ServerInfo_test.cpp @@ -86,21 +86,42 @@ admin = 127.0.0.1 { Env env(*this); - auto const result = env.rpc("server_info"); - BEAST_EXPECT(!result[jss::result].isMember(jss::error)); - BEAST_EXPECT(result[jss::result][jss::status] == "success"); - BEAST_EXPECT(result[jss::result].isMember(jss::info)); + auto const serverinfo = env.rpc("server_info"); + BEAST_EXPECT(serverinfo.isMember(jss::result)); + auto const& result = serverinfo[jss::result]; + BEAST_EXPECT(!result.isMember(jss::error)); + BEAST_EXPECT(result[jss::status] == "success"); + BEAST_EXPECT(result.isMember(jss::info)); + auto const& info = result[jss::info]; + BEAST_EXPECT(info.isMember(jss::build_version)); + // Git info is not guaranteed to be present + if (info.isMember(jss::git)) + { + auto const& git = info[jss::git]; + BEAST_EXPECT( + git.isMember(jss::hash) || git.isMember(jss::branch)); + BEAST_EXPECT( + !git.isMember(jss::hash) || + (git[jss::hash].isString() && + git[jss::hash].asString().size() == 40)); + BEAST_EXPECT( + !git.isMember(jss::branch) || + (git[jss::branch].isString() && + git[jss::branch].asString().size() != 0)); + } } { Env env(*this); // Call NetworkOPs directly and set the admin flag to false. - // Expect that the admin ports are not included in the result. auto const result = env.app().getOPs().getServerInfo(true, false, 0); + // Expect that the admin ports are not included in the result. auto const& ports = result[jss::ports]; BEAST_EXPECT(ports.isArray() && ports.size() == 0); + // Expect that git info is absent + BEAST_EXPECT(!result.isMember(jss::git)); } { diff --git a/src/xrpld/app/main/Main.cpp b/src/xrpld/app/main/Main.cpp index c945cfa85e..533cda75b5 100644 --- a/src/xrpld/app/main/Main.cpp +++ b/src/xrpld/app/main/Main.cpp @@ -521,6 +521,12 @@ run(int argc, char** argv) { std::cout << "rippled version " << BuildInfo::getVersionString() << std::endl; +#ifdef GIT_COMMIT_HASH + std::cout << "Git commit hash: " << GIT_COMMIT_HASH << std::endl; +#endif +#ifdef GIT_BRANCH + std::cout << "Git build branch: " << GIT_BRANCH << std::endl; +#endif return 0; } diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index cd653120c7..996a1fdf74 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -2493,6 +2493,18 @@ NetworkOPsImp::getServerInfo(bool human, bool admin, bool counters) x[jss::expiration] = "unknown"; } } + +#if defined(GIT_COMMIT_HASH) || defined(GIT_BRANCH) + { + auto& x = (info[jss::git] = Json::objectValue); +#ifdef GIT_COMMIT_HASH + x[jss::hash] = GIT_COMMIT_HASH; +#endif +#ifdef GIT_BRANCH + x[jss::branch] = GIT_BRANCH; +#endif + } +#endif } info[jss::io_latency_ms] = static_cast(app_.getIOLatency().count()); From fb3713bc2507d577aebe56fbc38872346811d76d Mon Sep 17 00:00:00 2001 From: Shawn Xie <35279399+shawnxie999@users.noreply.github.com> Date: Wed, 5 Feb 2025 10:05:24 -0800 Subject: [PATCH 2/3] Amendment `fixFrozenLPTokenTransfer` (#5227) Prohibits LPToken holders from sending LPToken to others if they have been frozen by one of the assets in AMM pool. --- .github/actions/dependencies/action.yml | 1 + include/xrpl/protocol/Feature.h | 2 +- include/xrpl/protocol/detail/features.macro | 1 + src/test/app/LPTokenTransfer_test.cpp | 486 ++++++++++++++++++++ src/xrpld/app/misc/detail/AMMUtils.cpp | 46 +- src/xrpld/app/paths/detail/DirectStep.cpp | 3 +- src/xrpld/app/paths/detail/StepChecks.h | 21 + src/xrpld/ledger/View.h | 7 + src/xrpld/ledger/detail/View.cpp | 40 +- 9 files changed, 597 insertions(+), 10 deletions(-) create mode 100644 src/test/app/LPTokenTransfer_test.cpp diff --git a/.github/actions/dependencies/action.yml b/.github/actions/dependencies/action.yml index 50e2999018..d3c67e8668 100644 --- a/.github/actions/dependencies/action.yml +++ b/.github/actions/dependencies/action.yml @@ -16,6 +16,7 @@ runs: conan export external/snappy snappy/1.1.10@ conan export external/rocksdb rocksdb/6.29.5@ conan export external/soci soci/4.0.3@ + conan export external/nudb nudb/2.0.8@ - name: add Ripple Conan remote shell: bash run: | diff --git a/include/xrpl/protocol/Feature.h b/include/xrpl/protocol/Feature.h index bff3e57597..c52f312cbf 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 = 86; +static constexpr std::size_t numFeatures = 87; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 322670c517..f82a05a7c1 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -29,6 +29,7 @@ // If you add an amendment here, then do not forget to increment `numFeatures` // in include/xrpl/protocol/Feature.h. +XRPL_FIX (FrozenLPTokenTransfer, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(DeepFreeze, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(PermissionedDomains, Supported::no, VoteBehavior::DefaultNo) XRPL_FEATURE(DynamicNFT, Supported::yes, VoteBehavior::DefaultNo) diff --git a/src/test/app/LPTokenTransfer_test.cpp b/src/test/app/LPTokenTransfer_test.cpp new file mode 100644 index 0000000000..96e621dccf --- /dev/null +++ b/src/test/app/LPTokenTransfer_test.cpp @@ -0,0 +1,486 @@ +//------------------------------------------------------------------------------ +/* + 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 + +namespace ripple { +namespace test { + +class LPTokenTransfer_test : public jtx::AMMTest +{ + void + testDirectStep(FeatureBitset features) + { + testcase("DirectStep"); + + using namespace jtx; + Env env{*this, features}; + fund(env, gw, {alice}, {USD(20'000), BTC(0.5)}, Fund::All); + env.close(); + + AMM ammAlice(env, alice, USD(20'000), BTC(0.5)); + BEAST_EXPECT( + ammAlice.expectBalances(USD(20'000), BTC(0.5), IOUAmount{100, 0})); + + fund(env, gw, {carol}, {USD(4'000), BTC(1)}, Fund::Acct); + ammAlice.deposit(carol, 10); + BEAST_EXPECT( + ammAlice.expectBalances(USD(22'000), BTC(0.55), IOUAmount{110, 0})); + + fund(env, gw, {bob}, {USD(4'000), BTC(1)}, Fund::Acct); + ammAlice.deposit(bob, 10); + BEAST_EXPECT( + ammAlice.expectBalances(USD(24'000), BTC(0.60), IOUAmount{120, 0})); + + auto const lpIssue = ammAlice.lptIssue(); + env.trust(STAmount{lpIssue, 500}, alice); + env.trust(STAmount{lpIssue, 500}, bob); + env.trust(STAmount{lpIssue, 500}, carol); + env.close(); + + // gateway freezes carol's USD + env(trust(gw, carol["USD"](0), tfSetFreeze)); + env.close(); + + // bob can still send lptoken to carol even tho carol's USD is + // frozen, regardless of whether fixFrozenLPTokenTransfer is enabled or + // not + // Note: Deep freeze is not considered for LPToken transfer + env(pay(bob, carol, STAmount{lpIssue, 5})); + env.close(); + + // cannot transfer to an amm account + env(pay(carol, lpIssue.getIssuer(), STAmount{lpIssue, 5}), + ter(tecNO_PERMISSION)); + env.close(); + + if (features[fixFrozenLPTokenTransfer]) + { + // carol is frozen on USD and therefore can't send lptoken to bob + env(pay(carol, bob, STAmount{lpIssue, 5}), ter(tecPATH_DRY)); + } + else + { + // carol can still send lptoken with frozen USD + env(pay(carol, bob, STAmount{lpIssue, 5})); + } + } + + void + testBookStep(FeatureBitset features) + { + testcase("BookStep"); + + using namespace jtx; + Env env{*this, features}; + + fund( + env, + gw, + {alice, bob, carol}, + {USD(10'000), EUR(10'000)}, + Fund::All); + AMM ammAlice(env, alice, USD(10'000), EUR(10'000)); + ammAlice.deposit(carol, 1'000); + ammAlice.deposit(bob, 1'000); + + auto const lpIssue = ammAlice.lptIssue(); + + // carols creates an offer to sell lptoken + env(offer(carol, XRP(10), STAmount{lpIssue, 10}), txflags(tfPassive)); + env.close(); + BEAST_EXPECT(expectOffers(env, carol, 1)); + + env.trust(STAmount{lpIssue, 1'000'000'000}, alice); + env.trust(STAmount{lpIssue, 1'000'000'000}, bob); + env.trust(STAmount{lpIssue, 1'000'000'000}, carol); + env.close(); + + // gateway freezes carol's USD + env(trust(gw, carol["USD"](0), tfSetFreeze)); + env.close(); + + // exercises alice's ability to consume carol's offer to sell lptoken + // when carol's USD is frozen pre/post fixFrozenLPTokenTransfer + // amendment + if (features[fixFrozenLPTokenTransfer]) + { + // with fixFrozenLPTokenTransfer, alice fails to consume carol's + // offer since carol's USD is frozen + env(pay(alice, bob, STAmount{lpIssue, 10}), + txflags(tfPartialPayment), + sendmax(XRP(10)), + ter(tecPATH_DRY)); + env.close(); + BEAST_EXPECT(expectOffers(env, carol, 1)); + + // gateway unfreezes carol's USD + env(trust(gw, carol["USD"](1'000'000'000), tfClearFreeze)); + env.close(); + + // alice successfully consumes carol's offer + env(pay(alice, bob, STAmount{lpIssue, 10}), + txflags(tfPartialPayment), + sendmax(XRP(10))); + env.close(); + BEAST_EXPECT(expectOffers(env, carol, 0)); + } + else + { + // without fixFrozenLPTokenTransfer, alice can consume carol's offer + // even when carol's USD is frozen + env(pay(alice, bob, STAmount{lpIssue, 10}), + txflags(tfPartialPayment), + sendmax(XRP(10))); + env.close(); + BEAST_EXPECT(expectOffers(env, carol, 0)); + } + + // make sure carol's USD is not frozen + env(trust(gw, carol["USD"](1'000'000'000), tfClearFreeze)); + env.close(); + + // ensure that carol's offer to buy lptoken can be consumed by alice + // even when carol's USD is frozen + { + // carol creates an offer to buy lptoken + env(offer(carol, STAmount{lpIssue, 10}, XRP(10)), + txflags(tfPassive)); + env.close(); + BEAST_EXPECT(expectOffers(env, carol, 1)); + + // gateway freezes carol's USD + env(trust(gw, carol["USD"](0), tfSetFreeze)); + env.close(); + + // alice successfully consumes carol's offer + env(pay(alice, bob, XRP(10)), + txflags(tfPartialPayment), + sendmax(STAmount{lpIssue, 10})); + env.close(); + BEAST_EXPECT(expectOffers(env, carol, 0)); + } + } + + void + testOfferCreation(FeatureBitset features) + { + testcase("Create offer"); + + using namespace jtx; + Env env{*this, features}; + + fund( + env, + gw, + {alice, bob, carol}, + {USD(10'000), EUR(10'000)}, + Fund::All); + AMM ammAlice(env, alice, USD(10'000), EUR(10'000)); + ammAlice.deposit(carol, 1'000); + ammAlice.deposit(bob, 1'000); + + auto const lpIssue = ammAlice.lptIssue(); + + // gateway freezes carol's USD + env(trust(gw, carol["USD"](0), tfSetFreeze)); + env.close(); + + // exercises carol's ability to create a new offer to sell lptoken with + // frozen USD, before and after fixFrozenLPTokenTransfer + if (features[fixFrozenLPTokenTransfer]) + { + // with fixFrozenLPTokenTransfer, carol can't create an offer to + // sell lptoken when one of the assets is frozen + + // carol can't create an offer to sell lptoken + env(offer(carol, XRP(10), STAmount{lpIssue, 10}), + txflags(tfPassive), + ter(tecUNFUNDED_OFFER)); + env.close(); + BEAST_EXPECT(expectOffers(env, carol, 0)); + + // gateway unfreezes carol's USD + env(trust(gw, carol["USD"](1'000'000'000), tfClearFreeze)); + env.close(); + + // carol can create an offer to sell lptoken after USD is unfrozen + env(offer(carol, XRP(10), STAmount{lpIssue, 10}), + txflags(tfPassive)); + env.close(); + BEAST_EXPECT(expectOffers(env, carol, 1)); + } + else + { + // without fixFrozenLPTokenTransfer, carol can create an offer + env(offer(carol, XRP(10), STAmount{lpIssue, 10}), + txflags(tfPassive)); + env.close(); + BEAST_EXPECT(expectOffers(env, carol, 1)); + } + + // gateway freezes carol's USD + env(trust(gw, carol["USD"](0), tfSetFreeze)); + env.close(); + + // carol can create offer to buy lptoken even if USD is frozen + env(offer(carol, STAmount{lpIssue, 10}, XRP(5)), txflags(tfPassive)); + env.close(); + BEAST_EXPECT(expectOffers(env, carol, 2)); + } + + void + testOfferCrossing(FeatureBitset features) + { + testcase("Offer crossing"); + + using namespace jtx; + Env env{*this, features}; + + // Offer crossing with two AMM LPTokens. + fund(env, gw, {alice, carol}, {USD(10'000)}, Fund::All); + AMM ammAlice1(env, alice, XRP(10'000), USD(10'000)); + ammAlice1.deposit(carol, 10'000'000); + + fund(env, gw, {alice, carol}, {EUR(10'000)}, Fund::IOUOnly); + AMM ammAlice2(env, alice, XRP(10'000), EUR(10'000)); + ammAlice2.deposit(carol, 10'000'000); + auto const token1 = ammAlice1.lptIssue(); + auto const token2 = ammAlice2.lptIssue(); + + // carol creates offer + env(offer(carol, STAmount{token2, 100}, STAmount{token1, 100})); + env.close(); + BEAST_EXPECT(expectOffers(env, carol, 1)); + + // gateway freezes carol's USD, carol's token1 should be frozen as well + env(trust(gw, carol["USD"](0), tfSetFreeze)); + env.close(); + + // alice creates an offer which exhibits different behavior on offer + // crossing depending on if fixFrozenLPTokenTransfer is enabled + env(offer(alice, STAmount{token1, 100}, STAmount{token2, 100})); + env.close(); + + // exercises carol's offer's ability to cross with alice's offer when + // carol's USD is frozen, before and after fixFrozenLPTokenTransfer + if (features[fixFrozenLPTokenTransfer]) + { + // with fixFrozenLPTokenTransfer enabled, alice's offer can no + // longer cross with carol's offer + BEAST_EXPECT( + expectLine(env, alice, STAmount{token1, 10'000'000}) && + expectLine(env, alice, STAmount{token2, 10'000'000})); + BEAST_EXPECT( + expectLine(env, carol, STAmount{token2, 10'000'000}) && + expectLine(env, carol, STAmount{token1, 10'000'000})); + BEAST_EXPECT( + expectOffers(env, alice, 1) && expectOffers(env, carol, 0)); + } + else + { + // alice's offer still crosses with carol's offer despite carol's + // token1 is frozen + BEAST_EXPECT( + expectLine(env, alice, STAmount{token1, 10'000'100}) && + expectLine(env, alice, STAmount{token2, 9'999'900})); + BEAST_EXPECT( + expectLine(env, carol, STAmount{token2, 10'000'100}) && + expectLine(env, carol, STAmount{token1, 9'999'900})); + BEAST_EXPECT( + expectOffers(env, alice, 0) && expectOffers(env, carol, 0)); + } + } + + void + testCheck(FeatureBitset features) + { + testcase("Check"); + + using namespace jtx; + Env env{*this, features}; + + fund( + env, + gw, + {alice, bob, carol}, + {USD(10'000), EUR(10'000)}, + Fund::All); + AMM ammAlice(env, alice, USD(10'000), EUR(10'000)); + ammAlice.deposit(carol, 1'000); + ammAlice.deposit(bob, 1'000); + + auto const lpIssue = ammAlice.lptIssue(); + + // gateway freezes carol's USD + env(trust(gw, carol["USD"](0), tfSetFreeze)); + env.close(); + + // carol can always create a check with lptoken that has frozen + // token + uint256 const carolChkId{keylet::check(carol, env.seq(carol)).key}; + env(check::create(carol, bob, STAmount{lpIssue, 10})); + env.close(); + + // with fixFrozenLPTokenTransfer enabled, bob fails to cash the check + if (features[fixFrozenLPTokenTransfer]) + env(check::cash(bob, carolChkId, STAmount{lpIssue, 10}), + ter(tecPATH_PARTIAL)); + else + env(check::cash(bob, carolChkId, STAmount{lpIssue, 10})); + + env.close(); + + // bob creates a check + uint256 const bobChkId{keylet::check(bob, env.seq(bob)).key}; + env(check::create(bob, carol, STAmount{lpIssue, 10})); + env.close(); + + // carol cashes the bob's check. Even though carol is frozen, she can + // still receive LPToken + env(check::cash(carol, bobChkId, STAmount{lpIssue, 10})); + env.close(); + } + + void + testNFTOffers(FeatureBitset features) + { + testcase("NFT Offers"); + using namespace test::jtx; + + Env env{*this, features}; + + // Setup AMM + fund( + env, + gw, + {alice, bob, carol}, + {USD(10'000), EUR(10'000)}, + Fund::All); + AMM ammAlice(env, alice, USD(10'000), EUR(10'000)); + ammAlice.deposit(carol, 1'000); + ammAlice.deposit(bob, 1'000); + + auto const lpIssue = ammAlice.lptIssue(); + + // bob mints a nft + uint256 const nftID{token::getNextID(env, bob, 0u, tfTransferable)}; + env(token::mint(bob, 0), txflags(tfTransferable)); + env.close(); + + // bob creates a sell offer for lptoken + uint256 const sellOfferIndex = keylet::nftoffer(bob, env.seq(bob)).key; + env(token::createOffer(bob, nftID, STAmount{lpIssue, 10}), + txflags(tfSellNFToken)); + env.close(); + + // gateway freezes carol's USD + env(trust(gw, carol["USD"](0), tfSetFreeze)); + env.close(); + + // exercises one's ability to transfer NFT using lptoken when one of the + // assets is frozen + if (features[fixFrozenLPTokenTransfer]) + { + // with fixFrozenLPTokenTransfer, freezing USD will prevent buy/sell + // offers with lptokens from being created/accepted + + // carol fails to accept bob's offer with lptoken because carol's + // USD is frozen + env(token::acceptSellOffer(carol, sellOfferIndex), + ter(tecINSUFFICIENT_FUNDS)); + env.close(); + + // gateway unfreezes carol's USD + env(trust(gw, carol["USD"](1'000'000), tfClearFreeze)); + env.close(); + + // carol can now accept the offer and own the nft + env(token::acceptSellOffer(carol, sellOfferIndex)); + env.close(); + + // gateway freezes bobs's USD + env(trust(gw, bob["USD"](0), tfSetFreeze)); + env.close(); + + // bob fails to create a buy offer with lptoken for carol's nft + // since bob's USD is frozen + env(token::createOffer(bob, nftID, STAmount{lpIssue, 10}), + token::owner(carol), + ter(tecUNFUNDED_OFFER)); + env.close(); + + // gateway unfreezes bob's USD + env(trust(gw, bob["USD"](1'000'000), tfClearFreeze)); + env.close(); + + // bob can now create a buy offer + env(token::createOffer(bob, nftID, STAmount{lpIssue, 10}), + token::owner(carol)); + env.close(); + } + else + { + // without fixFrozenLPTokenTransfer, freezing USD will still allow + // buy/sell offers to be created/accepted with lptoken + + // carol can still accept bob's offer despite carol's USD is frozen + env(token::acceptSellOffer(carol, sellOfferIndex)); + env.close(); + + // gateway freezes bob's USD + env(trust(gw, bob["USD"](0), tfSetFreeze)); + env.close(); + + // bob creates a buy offer with lptoken despite bob's USD is frozen + uint256 const buyOfferIndex = + keylet::nftoffer(bob, env.seq(bob)).key; + env(token::createOffer(bob, nftID, STAmount{lpIssue, 10}), + token::owner(carol)); + env.close(); + + // carol accepts bob's offer + env(token::acceptBuyOffer(carol, buyOfferIndex)); + env.close(); + } + } + +public: + void + run() override + { + FeatureBitset const all{jtx::supported_amendments()}; + + for (auto const features : {all, all - fixFrozenLPTokenTransfer}) + { + testDirectStep(features); + testBookStep(features); + testOfferCreation(features); + testOfferCrossing(features); + testCheck(features); + testNFTOffers(features); + } + } +}; + +BEAST_DEFINE_TESTSUITE(LPTokenTransfer, app, ripple); +} // namespace test +} // namespace ripple diff --git a/src/xrpld/app/misc/detail/AMMUtils.cpp b/src/xrpld/app/misc/detail/AMMUtils.cpp index f5f6ae6612..0b83afc6d3 100644 --- a/src/xrpld/app/misc/detail/AMMUtils.cpp +++ b/src/xrpld/app/misc/detail/AMMUtils.cpp @@ -116,13 +116,45 @@ ammLPHolds( AccountID const& lpAccount, beast::Journal const j) { - return accountHolds( - view, - lpAccount, - ammLPTCurrency(cur1, cur2), - ammAccount, - FreezeHandling::fhZERO_IF_FROZEN, - j); + // This function looks similar to `accountHolds`. However, it only checks if + // a LPToken holder has enough balance. On the other hand, `accountHolds` + // checks if the underlying assets of LPToken are frozen with the + // fixFrozenLPTokenTransfer amendment + + auto const currency = ammLPTCurrency(cur1, cur2); + STAmount amount; + + auto const sle = view.read(keylet::line(lpAccount, ammAccount, currency)); + if (!sle) + { + amount.clear(Issue{currency, ammAccount}); + JLOG(j.trace()) << "ammLPHolds: no SLE " + << " lpAccount=" << to_string(lpAccount) + << " amount=" << amount.getFullText(); + } + else if (isFrozen(view, lpAccount, currency, ammAccount)) + { + amount.clear(Issue{currency, ammAccount}); + JLOG(j.trace()) << "ammLPHolds: frozen currency " + << " lpAccount=" << to_string(lpAccount) + << " amount=" << amount.getFullText(); + } + else + { + amount = sle->getFieldAmount(sfBalance); + if (lpAccount > ammAccount) + { + // Put balance in account terms. + amount.negate(); + } + amount.setIssuer(ammAccount); + + JLOG(j.trace()) << "ammLPHolds:" + << " lpAccount=" << to_string(lpAccount) + << " amount=" << amount.getFullText(); + } + + return view.balanceHook(lpAccount, ammAccount, amount); } STAmount diff --git a/src/xrpld/app/paths/detail/DirectStep.cpp b/src/xrpld/app/paths/detail/DirectStep.cpp index ffd500009e..46aa129ac7 100644 --- a/src/xrpld/app/paths/detail/DirectStep.cpp +++ b/src/xrpld/app/paths/detail/DirectStep.cpp @@ -204,7 +204,8 @@ protected: logStringImpl(char const* name) const { std::ostringstream ostr; - ostr << name << ": " << "\nSrc: " << src_ << "\nDst: " << dst_; + ostr << name << ": " + << "\nSrc: " << src_ << "\nDst: " << dst_; return ostr.str(); } diff --git a/src/xrpld/app/paths/detail/StepChecks.h b/src/xrpld/app/paths/detail/StepChecks.h index 9cbc3ef0f9..d4fda2bfe6 100644 --- a/src/xrpld/app/paths/detail/StepChecks.h +++ b/src/xrpld/app/paths/detail/StepChecks.h @@ -21,6 +21,7 @@ #define RIPPLE_APP_PATHS_IMPL_STEP_CHECKS_H_INCLUDED #include +#include #include #include #include @@ -60,6 +61,26 @@ checkFreeze( } } + if (view.rules().enabled(fixFrozenLPTokenTransfer)) + { + if (auto const sleDst = view.read(keylet::account(dst)); + sleDst && sleDst->isFieldPresent(sfAMMID)) + { + auto const sleAmm = view.read(keylet::amm((*sleDst)[sfAMMID])); + if (!sleAmm) + return tecINTERNAL; // LCOV_EXCL_LINE + + if (isLPTokenFrozen( + view, + src, + (*sleAmm)[sfAsset].get(), + (*sleAmm)[sfAsset2].get())) + { + return terNO_LINE; + } + } + } + return tesSUCCESS; } diff --git a/src/xrpld/ledger/View.h b/src/xrpld/ledger/View.h index b964fc0ee7..aca3f9fa6d 100644 --- a/src/xrpld/ledger/View.h +++ b/src/xrpld/ledger/View.h @@ -160,6 +160,13 @@ isDeepFrozen( Currency const& currency, AccountID const& issuer); +[[nodiscard]] bool +isLPTokenFrozen( + ReadView const& view, + AccountID const& account, + Issue const& asset, + Issue const& asset2); + // Returns the amount an account can spend without going into debt. // // <-- saAmount: amount of currency held by account. May be negative. diff --git a/src/xrpld/ledger/detail/View.cpp b/src/xrpld/ledger/detail/View.cpp index 1422a50a3a..85abf7fc62 100644 --- a/src/xrpld/ledger/detail/View.cpp +++ b/src/xrpld/ledger/detail/View.cpp @@ -293,6 +293,17 @@ isDeepFrozen( return sle->isFlag(lsfHighDeepFreeze) || sle->isFlag(lsfLowDeepFreeze); } +bool +isLPTokenFrozen( + ReadView const& view, + AccountID const& account, + Issue const& asset, + Issue const& asset2) +{ + return isFrozen(view, account, asset.currency, asset.account) || + isFrozen(view, account, asset2.currency, asset2.account); +} + STAmount accountHolds( ReadView const& view, @@ -323,6 +334,32 @@ accountHolds( { return false; } + + // when fixFrozenLPTokenTransfer is enabled, if currency is lptoken, + // we need to check if the associated assets have been frozen + if (view.rules().enabled(fixFrozenLPTokenTransfer)) + { + auto const sleIssuer = view.read(keylet::account(issuer)); + if (!sleIssuer) + { + return false; // LCOV_EXCL_LINE + } + else if (sleIssuer->isFieldPresent(sfAMMID)) + { + auto const sleAmm = + view.read(keylet::amm((*sleIssuer)[sfAMMID])); + + if (!sleAmm || + isLPTokenFrozen( + view, + account, + (*sleAmm)[sfAsset].get(), + (*sleAmm)[sfAsset2].get())) + { + return false; + } + } + } } return true; @@ -492,7 +529,8 @@ xrpLiquid( STAmount const amount = (balance < reserve) ? STAmount{0} : balance - reserve; - JLOG(j.trace()) << "accountHolds:" << " account=" << to_string(id) + JLOG(j.trace()) << "accountHolds:" + << " account=" << to_string(id) << " amount=" << amount.getFullText() << " fullBalance=" << fullBalance.getFullText() << " balance=" << balance.getFullText() From 02387fd227d9b7307ec417138b6c6af5b9d156f0 Mon Sep 17 00:00:00 2001 From: Bart Date: Thu, 6 Feb 2025 13:11:49 -0800 Subject: [PATCH 3/3] Updates Conan dependencies (#5256) This PR updates several Conan dependencies: * boost * date * libarchive * libmysqlclient * libpq * lz4 * onetbb * openssl * sqlite3 * zlib * zstd --- conanfile.py | 14 +++++++------- external/rocksdb/conanfile.py | 6 +++--- external/soci/conanfile.py | 8 ++++---- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/conanfile.py b/conanfile.py index 14fc49a194..d4513068fb 100644 --- a/conanfile.py +++ b/conanfile.py @@ -24,14 +24,14 @@ class Xrpl(ConanFile): } requires = [ - 'date/3.0.1', + 'date/3.0.3', 'grpc/1.50.1', - 'libarchive/3.6.2', + 'libarchive/3.7.6', 'nudb/2.0.8', - 'openssl/1.1.1u', + 'openssl/1.1.1v', 'soci/4.0.3', 'xxhash/0.8.2', - 'zlib/1.2.13', + 'zlib/1.3.1', ] tool_requires = [ @@ -99,10 +99,10 @@ class Xrpl(ConanFile): self.options['boost'].visibility = 'global' def requirements(self): - self.requires('boost/1.82.0', force=True) - self.requires('lz4/1.9.3', force=True) + self.requires('boost/1.83.0', force=True) + self.requires('lz4/1.10.0', force=True) self.requires('protobuf/3.21.9', force=True) - self.requires('sqlite3/3.42.0', force=True) + self.requires('sqlite3/3.47.0', force=True) if self.options.jemalloc: self.requires('jemalloc/5.3.0') if self.options.rocksdb: diff --git a/external/rocksdb/conanfile.py b/external/rocksdb/conanfile.py index 09425b9f86..1c7853d814 100644 --- a/external/rocksdb/conanfile.py +++ b/external/rocksdb/conanfile.py @@ -89,13 +89,13 @@ class RocksDBConan(ConanFile): if self.options.with_snappy: self.requires("snappy/1.1.10") if self.options.with_lz4: - self.requires("lz4/1.9.4") + self.requires("lz4/1.10.0") if self.options.with_zlib: self.requires("zlib/[>=1.2.11 <2]") if self.options.with_zstd: - self.requires("zstd/1.5.5") + self.requires("zstd/1.5.6") if self.options.get_safe("with_tbb"): - self.requires("onetbb/2021.10.0") + self.requires("onetbb/2021.12.0") if self.options.with_jemalloc: self.requires("jemalloc/5.3.0") diff --git a/external/soci/conanfile.py b/external/soci/conanfile.py index 67c572d5ad..7e611493d7 100644 --- a/external/soci/conanfile.py +++ b/external/soci/conanfile.py @@ -62,15 +62,15 @@ class SociConan(ConanFile): def requirements(self): if self.options.with_sqlite3: - self.requires("sqlite3/3.41.1") + self.requires("sqlite3/3.47.0") if self.options.with_odbc and self.settings.os != "Windows": self.requires("odbc/2.3.11") if self.options.with_mysql: - self.requires("libmysqlclient/8.0.31") + self.requires("libmysqlclient/8.1.0") if self.options.with_postgresql: - self.requires("libpq/14.7") + self.requires("libpq/15.5") if self.options.with_boost: - self.requires("boost/1.81.0") + self.requires("boost/1.83.0") @property def _minimum_compilers_version(self):