From a93ab0eedc31d908a3a12a7c533ce6c5a57ad5df Mon Sep 17 00:00:00 2001 From: Elliot Lee Date: Wed, 19 Jul 2023 14:03:26 -0700 Subject: [PATCH 01/13] feat: support Concise Transaction Identifier (CTID) (XLS-37) (XRPLF#4418) The XLS-37 CTID (Concise Transaction ID) is a network-aware tx identifier which provides a way to efficiently locate a specific transaction without relying on transaction hashes. A CTID encodes the sequence number of the ledger that includes the tx, the transaction's index in that ledger, and the network ID. With the CTID, users can identify transactions and confirm their results. This applies even for transactions on sidechains, which may be difficult to find with only a transaction hash. Additionally, CTIDs require less storage space than transaction hashes, which can be beneficial for databases storing millions of transactions. The XLS-37 specification can be found at: XRPLF/XRPL-Standards#111 Add support for running a node on a different network. There is a new error code, `rpcWRONG_NETWORK`, returned when the requested CTID's network ID does not match the node's network ID. The error message looks like: Wrong network. You should submit this request to a node running on NetworkID: * Add RPC support for the CTID format * tx - you can specify "ctid", which is the CTID (16 hex digits, in a string, always starting with "C") * When retrieving a tx, the "ctid" may be returned * Add support for encoding, decoding, and validating CTIDs * Add tests --------- Co-authored-by: Rome Reginelli Co-authored-by: Denis Angell --- API-CHANGELOG.md | 77 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 API-CHANGELOG.md diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md new file mode 100644 index 00000000000..bbbe71d5545 --- /dev/null +++ b/API-CHANGELOG.md @@ -0,0 +1,77 @@ +# API Changelog + +This changelog is intended to list all updates to the API. + +For info about how API versioning works, view the [XLS-22d spec](https://github.com/XRPLF/XRPL-Standards/discussions/54). For details about the implementation of API versioning, view the [implementation PR](https://github.com/XRPLF/rippled/pull/3155). + +The API version controls the API behavior you see. This includes what properties you see in responses, what parameters you're permitted to send in requests, and so on. You specify the API version in each of your requests. When a breaking change is introduced to the `rippled` API, a new version is released. To avoid breaking your code, you should set (or increase) your version when you're ready to upgrade. + +For a log of breaking changes, see the **API Version [number]** headings. Breaking changes are associated with a particular API Version number. For non-breaking changes, scroll to the **XRP Ledger version [x.y.z]** headings. Non-breaking changes are associated with a particular XRP Ledger (`rippled`) release. + +## API Version 2 +This version will be supported by `rippled` version 1.12. + +#### V2 account_info response + +`signer_lists` is returned in the root of the response, instead of being nested under `account_data` (as it was in API version 1). ([#3770](https://github.com/XRPLF/rippled/pull/3770)) + +## API Version 1 +This version is supported by `rippled` version 1.11. + +### Idiosyncrasies + +#### V1 account_info response + +In [the response to the `account_info` command](https://xrpl.org/account_info.html#response-format), there is `account_data` - which is supposed to be an `AccountRoot` object - and `signer_lists` is in this object. However, the docs say that `signer_lists` should be at the root level of the reponse - and this makes sense, since signer lists are not part of the AccountRoot object. (First reported in [xrpl-dev-portal#938](https://github.com/XRPLF/xrpl-dev-portal/issues/938).) Thanks to [rippled#3770](https://github.com/XRPLF/rippled/pull/3770), this field will be moved in `api_version: 2`, to the root level of the response. + +#### server_info - network_id + +The `network_id` field was added in the `server_info` response in version 1.5.0 (2019), but it was not returned in [reporting mode](https://xrpl.org/rippled-server-modes.html#reporting-mode). + +## XRP Ledger version 1.12.0 + +Version 1.12.0 is in development. + +### Additions in 1.12 + +Additions are intended to be non-breaking (because they are purely additive). + +- `server_info`: Added `ports`, an array which advertises the RPC and WebSocket ports. This information is also included in the `/crawl` endpoint (which calls `server_info` internally). `grpc` and `peer` ports are also included. (https://github.com/XRPLF/rippled/pull/4427) + - `ports` contains objects, each containing a `port` for the listening port (a number string), and a `protocol` array listing the supported protocols on that port. + - This allows crawlers to build a more detailed topology without needing to port-scan nodes. + - (For peers and other non-admin clients, the info about admin ports is excluded.) +- Clawback: The following additions are gated by the Clawback amendment (`featureClawback`). (https://github.com/XRPLF/rippled/pull/4553) + - Adds an [AccountRoot flag](https://xrpl.org/accountroot.html#accountroot-flags) called `lsfAllowTrustLineClawback` (https://github.com/XRPLF/rippled/pull/4617) + - Adds the corresponding `asfAllowTrustLineClawback` [AccountSet Flag](https://xrpl.org/accountset.html#accountset-flags) as well. + - Clawback is disabled by default, so if an issuer desires the ability to claw back funds, they must use an `AccountSet` transaction to set the AllowTrustLineClawback flag. They must do this before creating any trust lines, offers, escrows, payment channels, or checks. + - Adds the [Clawback transaction type](https://github.com/XRPLF/XRPL-Standards/blob/master/XLS-39d-clawback/README.md#331-clawback-transaction), containing these fields: + - `Account`: The issuer of the asset being clawed back. Must also be the sender of the transaction. + - `Amount`: The amount being clawed back, with the `Amount.issuer` being the token holder's address. + +## XRP Ledger version 1.11.0 + +[Version 1.11.0](https://github.com/XRPLF/rippled/releases/tag/1.11.0) was released on Jun 20, 2023. + +### Breaking changes in 1.11 + +- Added the ability to mark amendments as obsolete. For the `feature` admin API, there is a new possible value for the `vetoed` field. ([#4291](https://github.com/XRPLF/rippled/pull/4291)) +- The API now won't accept seeds or public keys in place of account addresses. ([#4404](https://github.com/XRPLF/rippled/pull/4404)) +- For the `ledger_data` method, when all entries are filtered out, the API now returns an empty list (an empty array, `[]`). (Previously, it would return `null`.) While this is technically a breaking change, the new behavior is consistent with the documentation, so this is considered only a bug fix. ([#4398](https://github.com/XRPLF/rippled/pull/4398)) +- If and when the `fixNFTokenRemint` amendment activates, there will be a new AccountRoot field, `FirstNFTSequence`. This field is set to the current account sequence when the account issues their first NFT. If an account has not issued any NFTs, then the field is not set. ([#4406](https://github.com/XRPLF/rippled/pull/4406)) + - There is a new account deletion restriction: an account can only be deleted if `FirstNFTSequence` + `MintedNFTokens` + `256` is less than the current ledger sequence. + - This is potentially a breaking change if clients have logic for determining whether an account can be deleted. +- NetworkID + - For sidechains and networks with a network ID greater than 1024, there is a new [transaction common field](https://xrpl.org/transaction-common-fields.html), `NetworkID`. (https://github.com/XRPLF/rippled/pull/4370) + - This field helps to prevent replay attacks and is now required for chains whose network ID is 1025 or higher. + - The field must be omitted for Mainnet, so there is no change for Mainnet users. + - There are three new local error codes: + - `telNETWORK_ID_MAKES_TX_NON_CANONICAL`: a `NetworkID` is present but the chain's network ID is less than 1025. Remove the field from the transaction, and try again. + - `telREQUIRES_NETWORK_ID`: a `NetworkID` is required, but is not present. Add the field to the transaction, and try again. + - `telWRONG_NETWORK`: a `NetworkID` is specified, but it is for a different network. Submit the transaction to a different server which is connected to the correct network. + +### Additions and bug fixes in 1.11 + +- Added `nftoken_id`, `nftoken_ids` and `offer_id` meta fields into NFT `tx` and `account_tx` responses. (https://github.com/XRPLF/rippled/pull/4447) +- Added an `account_flags` object to the `account_info` method response. (https://github.com/XRPLF/rippled/pull/4459) +- Added `NFTokenPages` to the `account_objects` RPC. (https://github.com/XRPLF/rippled/pull/4352) +- Fixed: `marker` returned from the `account_lines` command would not work on subsequent commands. (https://github.com/XRPLF/rippled/pull/4361) From 4fed11b10e52cd82d8e8106a6900c157c08db619 Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Mon, 17 Jul 2023 08:05:11 -0400 Subject: [PATCH 02/13] fix(AMM): prevent orphaned objects, inconsistent ledger state: (#4626) When an AMM account is deleted, the owner directory entries must be deleted in order to ensure consistent ledger state. * When deleting AMM account: * Clean up AMM owner dir, linking AMM account and AMM object * Delete trust lines to AMM * Disallow `CheckCreate` to AMM accounts * AMM cannot cash a check * Constrain entries in AuthAccounts array to be accounts * AuthAccounts is an array of objects for the AMMBid transaction * SetTrust (TrustSet): Allow on AMM only for LP tokens * If the destination is an AMM account and the trust line doesn't exist, then: * If the asset is not the AMM LP token, then fail the tx with `tecNO_PERMISSION` * If the AMM is in empty state, then fail the tx with `tecAMM_EMPTY` * This disallows trustlines to AMM in empty state * Add AMMID to AMM root account * Remove lsfAMM flag and use sfAMMID instead * Remove owner dir entry for ltAMM * Add `AMMDelete` transaction type to handle amortized deletion * Limit number of trust lines to delete on final withdraw + AMMDelete * Put AMM in empty state when LPTokens is 0 upon final withdraw * Add `tfTwoAssetIfEmpty` deposit option in AMM empty state * Fail all AMM transactions in AMM empty state except special deposit * Add `tecINCOMPLETE` to indicate that not all AMM trust lines are deleted (i.e. partial deletion) * This is handled in Transactor similar to deleted offers * Fail AMMDelete with `tecINTERNAL` if AMM root account is nullptr * Don't validate for invalid asset pair in AMMDelete * AMMWithdraw deletes AMM trust lines and AMM account/object only if the number of trust lines is less than max * Current `maxDeletableAMMTrustLines` = 512 * Check no directory left after AMM trust lines are deleted * Enable partial trustline deletion in AMMWithdraw * Add `tecAMM_NOT_EMPTY` to fail any transaction that expects an AMM in empty state * Clawback considerations * Disallow clawback out of AMM account * Disallow AMM create if issuer can claw back This patch applies to the AMM implementation in #4294. Acknowledgements: Richard Holland and Nik Bougalis for responsibly disclosing this issue. Bug Bounties and Responsible Disclosures: We welcome reviews of the project code and urge researchers to responsibly disclose any issues they may find. To report a bug, please send a detailed report to: bugs@xrpl.org --- API-CHANGELOG.md | 22 + Builds/CMake/RippledCore.cmake | 1 + src/ripple/app/misc/AMMUtils.h | 20 + src/ripple/app/misc/impl/AMMUtils.cpp | 118 +++ src/ripple/app/paths/impl/BookStep.cpp | 3 +- src/ripple/app/tx/impl/AMMBid.cpp | 11 +- src/ripple/app/tx/impl/AMMCreate.cpp | 58 +- src/ripple/app/tx/impl/AMMDelete.cpp | 83 +++ src/ripple/app/tx/impl/AMMDelete.h | 54 ++ src/ripple/app/tx/impl/AMMDeposit.cpp | 91 ++- src/ripple/app/tx/impl/AMMDeposit.h | 17 + src/ripple/app/tx/impl/AMMVote.cpp | 10 +- src/ripple/app/tx/impl/AMMWithdraw.cpp | 57 +- src/ripple/app/tx/impl/AMMWithdraw.h | 8 - src/ripple/app/tx/impl/Clawback.cpp | 3 + src/ripple/app/tx/impl/CreateCheck.cpp | 4 + src/ripple/app/tx/impl/DeleteAccount.cpp | 77 +- src/ripple/app/tx/impl/Escrow.cpp | 2 +- src/ripple/app/tx/impl/InvariantCheck.cpp | 15 +- src/ripple/app/tx/impl/PayChan.cpp | 2 +- src/ripple/app/tx/impl/Payment.cpp | 2 +- src/ripple/app/tx/impl/SetTrust.cpp | 35 +- src/ripple/app/tx/impl/Transactor.cpp | 73 +- src/ripple/app/tx/impl/applySteps.cpp | 11 + src/ripple/ledger/View.h | 29 +- src/ripple/ledger/impl/View.cpp | 127 +++- src/ripple/protocol/LedgerFormats.h | 2 +- src/ripple/protocol/Protocol.h | 5 + src/ripple/protocol/TER.h | 6 +- src/ripple/protocol/TxFlags.h | 3 +- src/ripple/protocol/TxFormats.h | 3 + .../protocol/impl/InnerObjectFormats.cpp | 6 + src/ripple/protocol/impl/LedgerFormats.cpp | 3 +- src/ripple/protocol/impl/TER.cpp | 4 + src/ripple/protocol/impl/TxFormats.cpp | 10 + src/ripple/protocol/jss.h | 1 + src/test/app/AMMExtended_test.cpp | 1 - src/test/app/AMM_test.cpp | 695 +++++++++++++++++- src/test/app/CrossingLimits_test.cpp | 1 - src/test/app/Escrow_test.cpp | 1 - src/test/app/Flow_test.cpp | 1 - src/test/app/Freeze_test.cpp | 1 - src/test/app/Offer_test.cpp | 1 - src/test/app/Path_test.cpp | 1 - src/test/app/PayChan_test.cpp | 1 - src/test/app/PayStrand_test.cpp | 1 - src/test/app/TrustAndBalance_test.cpp | 1 - src/test/jtx.h | 1 + src/test/jtx/AMM.h | 20 + src/test/jtx/TestHelpers.h | 33 + src/test/jtx/check.h | 7 - src/test/jtx/impl/AMM.cpp | 22 +- src/test/jtx/impl/check.cpp | 16 - src/test/rpc/AccountOffers_test.cpp | 1 - src/test/rpc/LedgerData_test.cpp | 1 - src/test/rpc/NoRipple_test.cpp | 1 - 56 files changed, 1536 insertions(+), 247 deletions(-) create mode 100644 src/ripple/app/tx/impl/AMMDelete.cpp create mode 100644 src/ripple/app/tx/impl/AMMDelete.h diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md index bbbe71d5545..ae988beeb33 100644 --- a/API-CHANGELOG.md +++ b/API-CHANGELOG.md @@ -47,6 +47,28 @@ Additions are intended to be non-breaking (because they are purely additive). - Adds the [Clawback transaction type](https://github.com/XRPLF/XRPL-Standards/blob/master/XLS-39d-clawback/README.md#331-clawback-transaction), containing these fields: - `Account`: The issuer of the asset being clawed back. Must also be the sender of the transaction. - `Amount`: The amount being clawed back, with the `Amount.issuer` being the token holder's address. +- Adds [AMM](https://github.com/XRPLF/XRPL-Standards/discussions/78) ([#4294](https://github.com/XRPLF/rippled/pull/4294), [#4626](https://github.com/XRPLF/rippled/pull/4626)) feature: + - Adds `amm_info` API to retrieve AMM information for a given tokens pair. + - Adds `AMMCreate` transaction type to create `AMM` instance. + - Adds `AMMDeposit` transaction type to deposit funds into `AMM` instance. + - Adds `AMMWithdraw` transaction type to withdraw funds from `AMM` instance. + - Adds `AMMVote` transaction type to vote for the trading fee of `AMM` instance. + - Adds `AMMBid` transaction type to bid for the Auction Slot of `AMM` instance. + - Adds `AMMDelete` transaction type to delete `AMM` instance. + - Adds `sfAMMID` to `AccountRoot` to indicate that the account is `AMM`'s account. `AMMID` is used to fetch `ltAMM`. + - Adds `lsfAMMNode` `TrustLine` flag to indicate that one side of the `TrustLine` is `AMM` account. + - Adds `tfLPToken`, `tfSingleAsset`, `tfTwoAsset`, `tfOneAssetLPToken`, `tfLimitLPToken`, `tfTwoAssetIfEmpty`, + `tfWithdrawAll`, `tfOneAssetWithdrawAll` which allow a trader to specify different fields combination + for `AMMDeposit` and `AMMWithdraw` transactions. + - Adds new transaction result codes: + - tecUNFUNDED_AMM: insufficient balance to fund AMM. The account does not have funds for liquidity provision. + - tecAMM_BALANCE: AMM has invalid balance. Calculated balances greater than the current pool balances. + - tecAMM_FAILED: AMM transaction failed. Fails due to a processing failure. + - tecAMM_INVALID_TOKENS: AMM invalid LP tokens. Invalid input values, format, or calculated values. + - tecAMM_EMPTY: AMM is in empty state. Transaction expects AMM in non-empty state (LP tokens > 0). + - tecAMM_NOT_EMPTY: AMM is not in empty state. Transaction expects AMM in empty state (LP tokens == 0). + - tecAMM_ACCOUNT: AMM account. Clawback of AMM account. + - tecINCOMPLETE: Some work was completed, but more submissions required to finish. AMMDelete partially deletes the trustlines. ## XRP Ledger version 1.11.0 diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index b676c5ff5e9..6739afcbefa 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -476,6 +476,7 @@ target_sources (rippled PRIVATE src/ripple/app/rdb/impl/Wallet.cpp src/ripple/app/tx/impl/AMMBid.cpp src/ripple/app/tx/impl/AMMCreate.cpp + src/ripple/app/tx/impl/AMMDelete.cpp src/ripple/app/tx/impl/AMMDeposit.cpp src/ripple/app/tx/impl/AMMVote.cpp src/ripple/app/tx/impl/AMMWithdraw.cpp diff --git a/src/ripple/app/misc/AMMUtils.h b/src/ripple/app/misc/AMMUtils.h index 1718df496b8..c25503ceb9c 100644 --- a/src/ripple/app/misc/AMMUtils.h +++ b/src/ripple/app/misc/AMMUtils.h @@ -93,6 +93,26 @@ ammAccountHolds( AccountID const& ammAccountID, Issue const& issue); +/** Delete trustlines to AMM. If all trustlines are deleted then + * AMM object and account are deleted. Otherwise tecIMPCOMPLETE is returned. + */ +TER +deleteAMMAccount( + Sandbox& view, + Issue const& asset, + Issue const& asset2, + beast::Journal j); + +/** Initialize Auction and Voting slots and set the trading/discounted fee. + */ +void +initializeFeeAuctionVote( + ApplyView& view, + std::shared_ptr& ammSle, + AccountID const& account, + Issue const& lptIssue, + std::uint16_t tfee); + } // namespace ripple #endif // RIPPLE_APP_MISC_AMMUTILS_H_INLCUDED diff --git a/src/ripple/app/misc/impl/AMMUtils.cpp b/src/ripple/app/misc/impl/AMMUtils.cpp index 7156c77f21a..dcb403296c1 100644 --- a/src/ripple/app/misc/impl/AMMUtils.cpp +++ b/src/ripple/app/misc/impl/AMMUtils.cpp @@ -188,4 +188,122 @@ ammAccountHolds( return STAmount{issue}; } +static TER +deleteAMMTrustLines( + Sandbox& sb, + AccountID const& ammAccountID, + std::uint16_t maxTrustlinesToDelete, + beast::Journal j) +{ + return cleanupOnAccountDelete( + sb, + keylet::ownerDir(ammAccountID), + [&](LedgerEntryType nodeType, + uint256 const&, + std::shared_ptr& sleItem) -> TER { + // Should only have the trustlines + if (nodeType != LedgerEntryType::ltRIPPLE_STATE) + { + JLOG(j.error()) + << "deleteAMMTrustLines: deleting non-trustline " + << nodeType; + return tecINTERNAL; + } + + // Trustlines must have zero balance + if (sleItem->getFieldAmount(sfBalance) != beast::zero) + { + JLOG(j.error()) + << "deleteAMMTrustLines: deleting trustline with " + "non-zero balance."; + return tecINTERNAL; + } + + return deleteAMMTrustLine(sb, sleItem, ammAccountID, j); + }, + j, + maxTrustlinesToDelete); +} + +TER +deleteAMMAccount( + Sandbox& sb, + Issue const& asset, + Issue const& asset2, + beast::Journal j) +{ + auto ammSle = sb.peek(keylet::amm(asset, asset2)); + if (!ammSle) + { + JLOG(j.error()) << "deleteAMMAccount: AMM object does not exist " + << asset << " " << asset2; + return tecINTERNAL; + } + + auto const ammAccountID = (*ammSle)[sfAccount]; + auto sleAMMRoot = sb.peek(keylet::account(ammAccountID)); + if (!sleAMMRoot) + { + JLOG(j.error()) << "deleteAMMAccount: AMM account does not exist " + << to_string(ammAccountID); + return tecINTERNAL; + } + + if (auto const ter = + deleteAMMTrustLines(sb, ammAccountID, maxDeletableAMMTrustLines, j); + ter != tesSUCCESS) + return ter; + + auto const ownerDirKeylet = keylet::ownerDir(ammAccountID); + if (sb.exists(ownerDirKeylet) && !sb.emptyDirDelete(ownerDirKeylet)) + { + JLOG(j.error()) << "deleteAMMAccount: cannot delete root dir node of " + << toBase58(ammAccountID); + return tecINTERNAL; + } + + sb.erase(ammSle); + sb.erase(sleAMMRoot); + + return tesSUCCESS; +} + +void +initializeFeeAuctionVote( + ApplyView& view, + std::shared_ptr& ammSle, + AccountID const& account, + Issue const& lptIssue, + std::uint16_t tfee) +{ + // AMM creator gets the voting slot. + STArray voteSlots; + STObject voteEntry{sfVoteEntry}; + if (tfee != 0) + voteEntry.setFieldU16(sfTradingFee, tfee); + voteEntry.setFieldU32(sfVoteWeight, VOTE_WEIGHT_SCALE_FACTOR); + voteEntry.setAccountID(sfAccount, account); + voteSlots.push_back(voteEntry); + ammSle->setFieldArray(sfVoteSlots, voteSlots); + // AMM creator gets the auction slot for free. + auto& auctionSlot = ammSle->peekFieldObject(sfAuctionSlot); + auctionSlot.setAccountID(sfAccount, account); + // current + sec in 24h + auto const expiration = std::chrono::duration_cast( + view.info().parentCloseTime.time_since_epoch()) + .count() + + TOTAL_TIME_SLOT_SECS; + auctionSlot.setFieldU32(sfExpiration, expiration); + auctionSlot.setFieldAmount(sfPrice, STAmount{lptIssue, 0}); + // Set the fee + if (tfee != 0) + ammSle->setFieldU16(sfTradingFee, tfee); + else if (ammSle->isFieldPresent(sfTradingFee)) + ammSle->makeFieldAbsent(sfTradingFee); + if (auto const dfee = tfee / AUCTION_SLOT_DISCOUNTED_FEE_FRACTION) + auctionSlot.setFieldU16(sfDiscountedFee, dfee); + else if (auctionSlot.isFieldPresent(sfDiscountedFee)) + auctionSlot.makeFieldAbsent(sfDiscountedFee); +} + } // namespace ripple diff --git a/src/ripple/app/paths/impl/BookStep.cpp b/src/ripple/app/paths/impl/BookStep.cpp index e82acbde817..dd6abe577f5 100644 --- a/src/ripple/app/paths/impl/BookStep.cpp +++ b/src/ripple/app/paths/impl/BookStep.cpp @@ -99,7 +99,8 @@ class BookStep : public StepImp> , ownerPaysTransferFee_(ctx.ownerPaysTransferFee) , j_(ctx.j) { - if (auto const ammSle = ctx.view.read(keylet::amm(in, out))) + if (auto const ammSle = ctx.view.read(keylet::amm(in, out)); + ammSle && ammSle->getFieldAmount(sfLPTokenBalance) != beast::zero) ammLiquidity_.emplace( ctx.view, (*ammSle)[sfAccount], diff --git a/src/ripple/app/tx/impl/AMMBid.cpp b/src/ripple/app/tx/impl/AMMBid.cpp index 7654e842df1..d059f88c76c 100644 --- a/src/ripple/app/tx/impl/AMMBid.cpp +++ b/src/ripple/app/tx/impl/AMMBid.cpp @@ -94,6 +94,10 @@ AMMBid::preclaim(PreclaimContext const& ctx) return terNO_AMM; } + auto const lpTokensBalance = (*ammSle)[sfLPTokenBalance]; + if (lpTokensBalance == beast::zero) + return tecAMM_EMPTY; + if (ctx.tx.isFieldPresent(sfAuthAccounts)) { for (auto const& account : ctx.tx.getFieldArray(sfAuthAccounts)) @@ -114,7 +118,6 @@ AMMBid::preclaim(PreclaimContext const& ctx) JLOG(ctx.j.debug()) << "AMM Bid: account is not LP."; return tecAMM_INVALID_TOKENS; } - auto const lpTokensBalance = (*ammSle)[sfLPTokenBalance]; auto const bidMin = ctx.tx[~sfBidMin]; @@ -204,10 +207,10 @@ applyBid( Number const& burn) -> TER { auctionSlot.setAccountID(sfAccount, account_); auctionSlot.setFieldU32(sfExpiration, current + TOTAL_TIME_SLOT_SECS); - if (fee == 0) - auctionSlot.makeFieldAbsent(sfDiscountedFee); - else + if (fee != 0) auctionSlot.setFieldU16(sfDiscountedFee, fee); + else if (auctionSlot.isFieldPresent(sfDiscountedFee)) + auctionSlot.makeFieldAbsent(sfDiscountedFee); auctionSlot.setFieldAmount( sfPrice, toSTAmount(lpTokens.issue(), minPrice)); if (ctx_.tx.isFieldPresent(sfAuthAccounts)) diff --git a/src/ripple/app/tx/impl/AMMCreate.cpp b/src/ripple/app/tx/impl/AMMCreate.cpp index f3596a6f9f4..0c6874953a3 100644 --- a/src/ripple/app/tx/impl/AMMCreate.cpp +++ b/src/ripple/app/tx/impl/AMMCreate.cpp @@ -173,7 +173,7 @@ AMMCreate::preclaim(PreclaimContext const& ctx) auto isLPToken = [&](STAmount const& amount) -> bool { if (auto const sle = ctx.view.read(keylet::account(amount.issue().account))) - return (sle->getFlags() & lsfAMM); + return sle->isFieldPresent(sfAMMID); return false; }; @@ -184,7 +184,21 @@ AMMCreate::preclaim(PreclaimContext const& ctx) return tecAMM_INVALID_TOKENS; } - return tesSUCCESS; + // Disallow AMM if the issuer has clawback enabled + auto clawbackDisabled = [&](Issue const& issue) -> TER { + if (isXRP(issue)) + return tesSUCCESS; + if (auto const sle = ctx.view.read(keylet::account(issue.account)); + !sle) + return tecINTERNAL; + else if (sle->getFlags() & lsfAllowTrustLineClawback) + return tecNO_PERMISSION; + return tesSUCCESS; + }; + + if (auto const ter = clawbackDisabled(amount.issue()); ter != tesSUCCESS) + return ter; + return clawbackDisabled(amount2.issue()); } static std::pair @@ -247,7 +261,9 @@ applyCreate( // A user can only receive LPTokens through affirmative action - // either an AMMDeposit, TrustSet, crossing an offer, etc. sleAMMRoot->setFieldU32( - sfFlags, lsfAMM | lsfDisableMaster | lsfDefaultRipple | lsfDepositAuth); + sfFlags, lsfDisableMaster | lsfDefaultRipple | lsfDepositAuth); + // Link the root account and AMM object + sleAMMRoot->setFieldH256(sfAMMID, ammKeylet.key); sb.insert(sleAMMRoot); // Calculate initial LPT balance. @@ -255,46 +271,16 @@ applyCreate( // Create ltAMM auto ammSle = std::make_shared(ammKeylet); - if (ctx_.tx[sfTradingFee] != 0) - ammSle->setFieldU16(sfTradingFee, ctx_.tx[sfTradingFee]); ammSle->setAccountID(sfAccount, *ammAccount); ammSle->setFieldAmount(sfLPTokenBalance, lpTokens); auto const& [issue1, issue2] = std::minmax(amount.issue(), amount2.issue()); ammSle->setFieldIssue(sfAsset, STIssue{sfAsset, issue1}); ammSle->setFieldIssue(sfAsset2, STIssue{sfAsset2, issue2}); - // AMM creator gets the voting slot. - STArray voteSlots; - STObject voteEntry{sfVoteEntry}; - if (ctx_.tx[sfTradingFee] != 0) - voteEntry.setFieldU16(sfTradingFee, ctx_.tx[sfTradingFee]); - voteEntry.setFieldU32(sfVoteWeight, 100000); - voteEntry.setAccountID(sfAccount, account_); - voteSlots.push_back(voteEntry); - ammSle->setFieldArray(sfVoteSlots, voteSlots); - // AMM creator gets the auction slot for free. - auto& auctionSlot = ammSle->peekFieldObject(sfAuctionSlot); - auctionSlot.setAccountID(sfAccount, account_); - // current + sec in 24h - auto const expiration = - std::chrono::duration_cast( - ctx_.view().info().parentCloseTime.time_since_epoch()) - .count() + - TOTAL_TIME_SLOT_SECS; - auctionSlot.setFieldU32(sfExpiration, expiration); - auctionSlot.setFieldAmount(sfPrice, STAmount{lpTokens.issue(), 0}); + // AMM creator gets the auction slot and the voting slot. + initializeFeeAuctionVote( + ctx_.view(), ammSle, account_, lptIss, ctx_.tx[sfTradingFee]); sb.insert(ammSle); - // Add owner directory to link the root account and AMM object. - if (auto const page = sb.dirInsert( - keylet::ownerDir(*ammAccount), - ammSle->key(), - describeOwnerDir(*ammAccount)); - !page) - { - JLOG(j_.debug()) << "AMM Instance: failed to insert owner dir"; - return {tecDIR_FULL, false}; - } - // Send LPT to LP. auto res = accountSend(sb, *ammAccount, account_, lpTokens, ctx_.journal); if (res != tesSUCCESS) diff --git a/src/ripple/app/tx/impl/AMMDelete.cpp b/src/ripple/app/tx/impl/AMMDelete.cpp new file mode 100644 index 00000000000..25502be4f44 --- /dev/null +++ b/src/ripple/app/tx/impl/AMMDelete.cpp @@ -0,0 +1,83 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2022 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 + +namespace ripple { + +NotTEC +AMMDelete::preflight(PreflightContext const& ctx) +{ + if (!ammEnabled(ctx.rules)) + return temDISABLED; + + if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) + return ret; + + if (ctx.tx.getFlags() & tfUniversalMask) + { + JLOG(ctx.j.debug()) << "AMM Delete: invalid flags."; + return temINVALID_FLAG; + } + + return preflight2(ctx); +} + +TER +AMMDelete::preclaim(PreclaimContext const& ctx) +{ + auto const ammSle = + ctx.view.read(keylet::amm(ctx.tx[sfAsset], ctx.tx[sfAsset2])); + if (!ammSle) + { + JLOG(ctx.j.debug()) << "AMM Delete: Invalid asset pair."; + return terNO_AMM; + } + + auto const lpTokensBalance = (*ammSle)[sfLPTokenBalance]; + if (lpTokensBalance != beast::zero) + return tecAMM_NOT_EMPTY; + + return tesSUCCESS; +} + +TER +AMMDelete::doApply() +{ + // This is the ledger view that we work against. Transactions are applied + // as we go on processing transactions. + Sandbox sb(&ctx_.view()); + + auto const ter = + deleteAMMAccount(sb, ctx_.tx[sfAsset], ctx_.tx[sfAsset2], j_); + if (ter == tesSUCCESS || ter == tecINCOMPLETE) + sb.apply(ctx_.rawView()); + + return ter; +} + +} // namespace ripple diff --git a/src/ripple/app/tx/impl/AMMDelete.h b/src/ripple/app/tx/impl/AMMDelete.h new file mode 100644 index 00000000000..cf7f55cb715 --- /dev/null +++ b/src/ripple/app/tx/impl/AMMDelete.h @@ -0,0 +1,54 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2023 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_AMMDELETE_H_INCLUDED +#define RIPPLE_TX_AMMDELETE_H_INCLUDED + +#include + +namespace ripple { + +/** AMMDelete implements AMM delete transactor. This is a mechanism to + * delete AMM in an empty state when the number of LP tokens is 0. + * AMMDelete deletes the trustlines up to configured maximum. If all + * trustlines are deleted then AMM ltAMM and root account are deleted. + * Otherwise AMMDelete should be called again. + */ +class AMMDelete : public Transactor +{ +public: + static constexpr ConsequencesFactoryType ConsequencesFactory{Normal}; + + explicit AMMDelete(ApplyContext& ctx) : Transactor(ctx) + { + } + + static NotTEC + preflight(PreflightContext const& ctx); + + static TER + preclaim(PreclaimContext const& ctx); + + TER + doApply() override; +}; + +} // namespace ripple + +#endif // RIPPLE_TX_AMMDELETE_H_INCLUDED diff --git a/src/ripple/app/tx/impl/AMMDeposit.cpp b/src/ripple/app/tx/impl/AMMDeposit.cpp index 2fb9bd1eaab..5ea49f5445c 100644 --- a/src/ripple/app/tx/impl/AMMDeposit.cpp +++ b/src/ripple/app/tx/impl/AMMDeposit.cpp @@ -52,10 +52,12 @@ AMMDeposit::preflight(PreflightContext const& ctx) auto const amount2 = ctx.tx[~sfAmount2]; auto const ePrice = ctx.tx[~sfEPrice]; auto const lpTokens = ctx.tx[~sfLPTokenOut]; + auto const tradingFee = ctx.tx[~sfTradingFee]; // Valid options for the flags are: // tfLPTokens: LPTokenOut, [Amount, Amount2] // tfSingleAsset: Amount, [LPTokenOut] // tfTwoAsset: Amount, Amount2, [LPTokenOut] + // tfTwoAssetIfEmpty: Amount, Amount2, [sfTradingFee] // tfOnAssetLPToken: Amount and LPTokenOut // tfLimitLPToken: Amount and EPrice if (std::popcount(flags & tfDepositSubTx) != 1) @@ -66,29 +68,35 @@ AMMDeposit::preflight(PreflightContext const& ctx) if (flags & tfLPToken) { // if included then both amount and amount2 are deposit min - if (!lpTokens || ePrice || (amount && !amount2) || (!amount && amount2)) + if (!lpTokens || ePrice || (amount && !amount2) || + (!amount && amount2) || tradingFee) return temMALFORMED; } else if (flags & tfSingleAsset) { // if included then lpTokens is deposit min - if (!amount || amount2 || ePrice) + if (!amount || amount2 || ePrice || tradingFee) return temMALFORMED; } else if (flags & tfTwoAsset) { // if included then lpTokens is deposit min - if (!amount || !amount2 || ePrice) + if (!amount || !amount2 || ePrice || tradingFee) return temMALFORMED; } else if (flags & tfOneAssetLPToken) { - if (!amount || !lpTokens || amount2 || ePrice) + if (!amount || !lpTokens || amount2 || ePrice || tradingFee) return temMALFORMED; } else if (flags & tfLimitLPToken) { - if (!amount || !ePrice || lpTokens || amount2) + if (!amount || !ePrice || lpTokens || amount2 || tradingFee) + return temMALFORMED; + } + else if (flags & tfTwoAssetIfEmpty) + { + if (!amount || !amount2 || ePrice || lpTokens) return temMALFORMED; } @@ -148,6 +156,12 @@ AMMDeposit::preflight(PreflightContext const& ctx) } } + if (tradingFee > TRADING_FEE_THRESHOLD) + { + JLOG(ctx.j.debug()) << "AMM Deposit: invalid trading fee."; + return temBAD_FEE; + } + return preflight2(ctx); } @@ -174,12 +188,27 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) if (!expected) return expected.error(); auto const [amountBalance, amount2Balance, lptAMMBalance] = *expected; - if (amountBalance <= beast::zero || amount2Balance <= beast::zero || - lptAMMBalance <= beast::zero) + if (ctx.tx.getFlags() & tfTwoAssetIfEmpty) { - JLOG(ctx.j.debug()) - << "AMM Deposit: reserves or tokens balance is zero."; - return tecINTERNAL; + if (lptAMMBalance != beast::zero) + return tecAMM_NOT_EMPTY; + if (amountBalance != beast::zero || amount2Balance != beast::zero) + { + JLOG(ctx.j.debug()) << "AMM Deposit: tokens balance is not zero."; + return tecINTERNAL; + } + } + else + { + if (lptAMMBalance == beast::zero) + return tecAMM_EMPTY; + if (amountBalance <= beast::zero || amount2Balance <= beast::zero || + lptAMMBalance < beast::zero) + { + JLOG(ctx.j.debug()) + << "AMM Deposit: reserves or tokens balance is zero."; + return tecINTERNAL; + } } // Check account has sufficient funds. @@ -313,8 +342,6 @@ AMMDeposit::applyGuts(Sandbox& sb) return {tecINTERNAL, false}; auto const ammAccountID = (*ammSle)[sfAccount]; - auto const tfee = getTradingFee(ctx_.view(), *ammSle, account_); - auto const expected = ammHolds( sb, *ammSle, @@ -325,6 +352,9 @@ AMMDeposit::applyGuts(Sandbox& sb) if (!expected) return {expected.error(), false}; auto const [amountBalance, amount2Balance, lptAMMBalance] = *expected; + auto const tfee = (lptAMMBalance == beast::zero) + ? ctx_.tx[~sfTradingFee].value_or(0) + : getTradingFee(ctx_.view(), *ammSle, account_); auto const subTxType = ctx_.tx.getFlags() & tfDepositSubTx; @@ -382,6 +412,14 @@ AMMDeposit::applyGuts(Sandbox& sb) amount, amount2, tfee); + if (subTxType & tfTwoAssetIfEmpty) + return equalDepositInEmptyState( + sb, + ammAccountID, + *amount, + *amount2, + lptAMMBalance.issue(), + tfee); // should not happen. JLOG(j_.error()) << "AMM Deposit: invalid options."; return std::make_pair(tecINTERNAL, STAmount{}); @@ -391,6 +429,12 @@ AMMDeposit::applyGuts(Sandbox& sb) { assert(newLPTokenBalance > beast::zero); ammSle->setFieldAmount(sfLPTokenBalance, newLPTokenBalance); + // LP depositing into AMM empty state gets the auction slot + // and the voting + if (lptAMMBalance == beast::zero) + initializeFeeAuctionVote( + sb, ammSle, account_, lptAMMBalance.issue(), tfee); + sb.update(ammSle); } @@ -834,4 +878,27 @@ AMMDeposit::singleDepositEPrice( tfee); } +std::pair +AMMDeposit::equalDepositInEmptyState( + Sandbox& view, + AccountID const& ammAccount, + STAmount const& amount, + STAmount const& amount2, + Issue const& lptIssue, + std::uint16_t tfee) +{ + return deposit( + view, + ammAccount, + amount, + amount, + amount2, + STAmount{lptIssue, 0}, + ammLPTokens(amount, amount2, lptIssue), + std::nullopt, + std::nullopt, + std::nullopt, + tfee); +} + } // namespace ripple diff --git a/src/ripple/app/tx/impl/AMMDeposit.h b/src/ripple/app/tx/impl/AMMDeposit.h index 10da43594d3..385ce7c24e5 100644 --- a/src/ripple/app/tx/impl/AMMDeposit.h +++ b/src/ripple/app/tx/impl/AMMDeposit.h @@ -223,6 +223,23 @@ class AMMDeposit : public Transactor STAmount const& lptAMMBalance, STAmount const& ePrice, std::uint16_t tfee); + + /** Equal deposit in empty AMM state (LP tokens balance is 0) + * @param view + * @param ammAccount + * @param amount requested asset1 deposit amount + * @param amount2 requested asset2 deposit amount + * @param tfee + * @return + */ + std::pair + equalDepositInEmptyState( + Sandbox& view, + AccountID const& ammAccount, + STAmount const& amount, + STAmount const& amount2, + Issue const& lptIssue, + std::uint16_t tfee); }; } // namespace ripple diff --git a/src/ripple/app/tx/impl/AMMVote.cpp b/src/ripple/app/tx/impl/AMMVote.cpp index 09361356c3a..ff0598aaa40 100644 --- a/src/ripple/app/tx/impl/AMMVote.cpp +++ b/src/ripple/app/tx/impl/AMMVote.cpp @@ -69,6 +69,8 @@ AMMVote::preclaim(PreclaimContext const& ctx) JLOG(ctx.j.debug()) << "AMM Vote: Invalid asset pair."; return terNO_AMM; } + else if (ammSle->getFieldAmount(sfLPTokenBalance) == beast::zero) + return tecAMM_EMPTY; else if (auto const lpTokensNew = ammLPHolds(ctx.view, *ammSle, ctx.tx[sfAccount], ctx.j); lpTokensNew == beast::zero) @@ -208,17 +210,19 @@ applyVote( if (auto const discountedFee = fee / AUCTION_SLOT_DISCOUNTED_FEE_FRACTION) auctionSlot.setFieldU16(sfDiscountedFee, discountedFee); - else + else if (auctionSlot.isFieldPresent(sfDiscountedFee)) auctionSlot.makeFieldAbsent(sfDiscountedFee); } } else { - ammSle->makeFieldAbsent(sfTradingFee); + if (ammSle->isFieldPresent(sfTradingFee)) + ammSle->makeFieldAbsent(sfTradingFee); if (ammSle->isFieldPresent(sfAuctionSlot)) { auto& auctionSlot = ammSle->peekFieldObject(sfAuctionSlot); - auctionSlot.makeFieldAbsent(sfDiscountedFee); + if (auctionSlot.isFieldPresent(sfDiscountedFee)) + auctionSlot.makeFieldAbsent(sfDiscountedFee); } } sb.update(ammSle); diff --git a/src/ripple/app/tx/impl/AMMWithdraw.cpp b/src/ripple/app/tx/impl/AMMWithdraw.cpp index 8c583bc6170..58fc6b549ce 100644 --- a/src/ripple/app/tx/impl/AMMWithdraw.cpp +++ b/src/ripple/app/tx/impl/AMMWithdraw.cpp @@ -25,7 +25,6 @@ #include #include #include -#include #include #include @@ -195,8 +194,10 @@ AMMWithdraw::preclaim(PreclaimContext const& ctx) if (!expected) return expected.error(); auto const [amountBalance, amount2Balance, lptAMMBalance] = *expected; + if (lptAMMBalance == beast::zero) + return tecAMM_EMPTY; if (amountBalance <= beast::zero || amount2Balance <= beast::zero || - lptAMMBalance <= beast::zero) + lptAMMBalance < beast::zero) { JLOG(ctx.j.debug()) << "AMM Withdraw: reserves or tokens balance is zero."; @@ -301,6 +302,9 @@ AMMWithdraw::applyGuts(Sandbox& sb) if (!ammSle) return {tecINTERNAL, false}; auto const ammAccountID = (*ammSle)[sfAccount]; + auto const accountSle = sb.read(keylet::account(ammAccountID)); + if (!accountSle) + return {tecINTERNAL, false}; auto const lpTokens = ammLPHolds(ctx_.view(), *ammSle, ctx_.tx[sfAccount], ctx_.journal); auto const lpTokensWithdraw = @@ -372,19 +376,31 @@ AMMWithdraw::applyGuts(Sandbox& sb) return std::make_pair(tecINTERNAL, STAmount{}); }(); - // AMM is deleted if zero tokens balance - if (result == tesSUCCESS && newLPTokenBalance != beast::zero) + if (result != tesSUCCESS) + return {result, false}; + + bool updateBalance = true; + if (newLPTokenBalance == beast::zero) + { + if (auto const ter = + deleteAMMAccount(sb, ctx_.tx[sfAsset], ctx_.tx[sfAsset2], j_); + ter != tesSUCCESS && ter != tecINCOMPLETE) + return {ter, false}; + else + updateBalance = (ter == tecINCOMPLETE); + } + + if (updateBalance) { ammSle->setFieldAmount(sfLPTokenBalance, newLPTokenBalance); sb.update(ammSle); - - JLOG(ctx_.journal.trace()) - << "AMM Withdraw: tokens " << to_string(newLPTokenBalance.iou()) - << " " << to_string(lpTokens.iou()) << " " - << to_string(lptAMMBalance.iou()); } - return {result, result == tesSUCCESS}; + JLOG(ctx_.journal.trace()) + << "AMM Withdraw: tokens " << to_string(newLPTokenBalance.iou()) << " " + << to_string(lpTokens.iou()) << " " << to_string(lptAMMBalance.iou()); + + return {tesSUCCESS, true}; } TER @@ -401,24 +417,6 @@ AMMWithdraw::doApply() return result.first; } -TER -AMMWithdraw::deleteAccount(Sandbox& sb, AccountID const& ammAccountID) -{ - auto sleAMMRoot = sb.peek(keylet::account(ammAccountID)); - auto ammSle = sb.peek(keylet::amm(ctx_.tx[sfAsset], ctx_.tx[sfAsset2])); - - if (!sleAMMRoot || !ammSle) - return tecINTERNAL; - - // Note, the AMM trust lines are deleted since the balance - // goes to 0. It also means there are no linked - // ledger objects. - sb.erase(ammSle); - sb.erase(sleAMMRoot); - - return tesSUCCESS; -} - std::pair AMMWithdraw::withdraw( Sandbox& view, @@ -562,9 +560,6 @@ AMMWithdraw::withdraw( return {res, STAmount{}}; } - if (lpTokensWithdrawActual == lpTokensAMMBalance) - return {deleteAccount(view, ammAccount), STAmount{}}; - return {tesSUCCESS, lpTokensAMMBalance - lpTokensWithdrawActual}; } diff --git a/src/ripple/app/tx/impl/AMMWithdraw.h b/src/ripple/app/tx/impl/AMMWithdraw.h index 1e5b1a99e4f..40686266315 100644 --- a/src/ripple/app/tx/impl/AMMWithdraw.h +++ b/src/ripple/app/tx/impl/AMMWithdraw.h @@ -215,14 +215,6 @@ class AMMWithdraw : public Transactor STAmount const& amount, STAmount const& ePrice, std::uint16_t tfee); - - /** Delete AMM account. - * @param view - * @param ammAccountID - * @return - */ - TER - deleteAccount(Sandbox& view, AccountID const& ammAccountID); }; } // namespace ripple diff --git a/src/ripple/app/tx/impl/Clawback.cpp b/src/ripple/app/tx/impl/Clawback.cpp index 4fb4d4bc8f8..58546db5ca7 100644 --- a/src/ripple/app/tx/impl/Clawback.cpp +++ b/src/ripple/app/tx/impl/Clawback.cpp @@ -63,6 +63,9 @@ Clawback::preclaim(PreclaimContext const& ctx) if (!sleIssuer || !sleHolder) return terNO_ACCOUNT; + if (sleHolder->isFieldPresent(sfAMMID)) + return tecAMM_ACCOUNT; + std::uint32_t const issuerFlagsIn = sleIssuer->getFieldU32(sfFlags); // If AllowTrustLineClawback is not set or NoFreeze is set, return no diff --git a/src/ripple/app/tx/impl/CreateCheck.cpp b/src/ripple/app/tx/impl/CreateCheck.cpp index f5c2cbfbfd9..77ce4d017a1 100644 --- a/src/ripple/app/tx/impl/CreateCheck.cpp +++ b/src/ripple/app/tx/impl/CreateCheck.cpp @@ -97,6 +97,10 @@ CreateCheck::preclaim(PreclaimContext const& ctx) (flags & lsfDisallowIncomingCheck)) return tecNO_PERMISSION; + // AMM can not cash the check + if (sleDst->isFieldPresent(sfAMMID)) + return tecNO_PERMISSION; + if ((flags & lsfRequireDestTag) && !ctx.tx.isFieldPresent(sfDestinationTag)) { // The tag is basically account-specific information we don't diff --git a/src/ripple/app/tx/impl/DeleteAccount.cpp b/src/ripple/app/tx/impl/DeleteAccount.cpp index 62cc9e1fbbf..eeffc66d382 100644 --- a/src/ripple/app/tx/impl/DeleteAccount.cpp +++ b/src/ripple/app/tx/impl/DeleteAccount.cpp @@ -292,76 +292,29 @@ DeleteAccount::doApply() if (!src || !dst) return tefBAD_LEDGER; - // Delete all of the entries in the account directory. Keylet const ownerDirKeylet{keylet::ownerDir(account_)}; - std::shared_ptr sleDirNode{}; - unsigned int uDirEntry{0}; - uint256 dirEntry{beast::zero}; - - if (view().exists(ownerDirKeylet) && - dirFirst(view(), ownerDirKeylet.key, sleDirNode, uDirEntry, dirEntry)) - { - do - { - // Choose the right way to delete each directory node. - auto sleItem = view().peek(keylet::child(dirEntry)); - if (!sleItem) - { - // Directory node has an invalid index. Bail out. - JLOG(j_.fatal()) - << "DeleteAccount: Directory node in ledger " - << view().seq() << " has index to object that is missing: " - << to_string(dirEntry); - return tefBAD_LEDGER; - } - - LedgerEntryType const nodeType{safe_cast( - sleItem->getFieldU16(sfLedgerEntryType))}; - + auto const ter = cleanupOnAccountDelete( + view(), + ownerDirKeylet, + [&](LedgerEntryType nodeType, + uint256 const& dirEntry, + std::shared_ptr& sleItem) -> TER { if (auto deleter = nonObligationDeleter(nodeType)) { TER const result{ deleter(ctx_.app, view(), account_, dirEntry, sleItem, j_)}; - if (!isTesSuccess(result)) - return result; - } - else - { - assert(!"Undeletable entry should be found in preclaim."); - JLOG(j_.error()) - << "DeleteAccount undeletable item not found in preclaim."; - return tecHAS_OBLIGATIONS; - } - - // dirFirst() and dirNext() are like iterators with exposed - // internal state. We'll take advantage of that exposed state - // to solve a common C++ problem: iterator invalidation while - // deleting elements from a container. - // - // We have just deleted one directory entry, which means our - // "iterator state" is invalid. - // - // 1. During the process of getting an entry from the - // directory uDirEntry was incremented from 0 to 1. - // - // 2. We then deleted the entry at index 0, which means the - // entry that was at 1 has now moved to 0. - // - // 3. So we verify that uDirEntry is indeed 1. Then we jam it - // back to zero to "un-invalidate" the iterator. - assert(uDirEntry == 1); - if (uDirEntry != 1) - { - JLOG(j_.error()) - << "DeleteAccount iterator re-validation failed."; - return tefBAD_LEDGER; + return result; } - uDirEntry = 0; - } while (dirNext( - view(), ownerDirKeylet.key, sleDirNode, uDirEntry, dirEntry)); - } + assert(!"Undeletable entry should be found in preclaim."); + JLOG(j_.error()) << "DeleteAccount undeletable item not " + "found in preclaim."; + return tecHAS_OBLIGATIONS; + }, + ctx_.journal); + if (ter != tesSUCCESS) + return ter; // Transfer any XRP remaining after the fee is paid to the destination: (*dst)[sfBalance] = (*dst)[sfBalance] + mSourceBalance; diff --git a/src/ripple/app/tx/impl/Escrow.cpp b/src/ripple/app/tx/impl/Escrow.cpp index 981cc57e8fe..9d8aa6a2289 100644 --- a/src/ripple/app/tx/impl/Escrow.cpp +++ b/src/ripple/app/tx/impl/Escrow.cpp @@ -162,7 +162,7 @@ EscrowCreate::preclaim(PreclaimContext const& ctx) auto const sled = ctx.view.read(keylet::account(ctx.tx[sfDestination])); if (!sled) return tecNO_DST; - if (((*sled)[sfFlags] & lsfAMM)) + if (sled->isFieldPresent(sfAMMID)) return tecNO_PERMISSION; return tesSUCCESS; diff --git a/src/ripple/app/tx/impl/InvariantCheck.cpp b/src/ripple/app/tx/impl/InvariantCheck.cpp index 23fa7b17115..907611f1c9a 100644 --- a/src/ripple/app/tx/impl/InvariantCheck.cpp +++ b/src/ripple/app/tx/impl/InvariantCheck.cpp @@ -321,12 +321,12 @@ AccountRootsNotDeleted::finalize( ReadView const&, beast::Journal const& j) { - // AMM account root can be deleted as the result of AMM withdraw + // AMM account root can be deleted as the result of AMM withdraw/delete // transaction when the total AMM LP Tokens balance goes to 0. - // Not every AMM withdraw deletes the AMM account, accountsDeleted_ - // is set if it is deleted. + // A successful AccountDelete or AMMDelete MUST delete exactly + // one account root. if ((tx.getTxnType() == ttACCOUNT_DELETE || - (tx.getTxnType() == ttAMM_WITHDRAW && accountsDeleted_ == 1)) && + tx.getTxnType() == ttAMM_DELETE) && result == tesSUCCESS) { if (accountsDeleted_ == 1) @@ -341,6 +341,13 @@ AccountRootsNotDeleted::finalize( return false; } + // A successful AMMWithdraw MAY delete one account root + // when the total AMM LP Tokens balance goes to 0. Not every AMM withdraw + // deletes the AMM account, accountsDeleted_ is set if it is deleted. + if (tx.getTxnType() == ttAMM_WITHDRAW && result == tesSUCCESS && + accountsDeleted_ == 1) + return true; + if (accountsDeleted_ == 0) return true; diff --git a/src/ripple/app/tx/impl/PayChan.cpp b/src/ripple/app/tx/impl/PayChan.cpp index ac85603957f..3fe2a28a7cf 100644 --- a/src/ripple/app/tx/impl/PayChan.cpp +++ b/src/ripple/app/tx/impl/PayChan.cpp @@ -234,7 +234,7 @@ PayChanCreate::preclaim(PreclaimContext const& ctx) (flags & lsfDisallowXRP)) return tecNO_TARGET; - if (flags & lsfAMM) + if (sled->isFieldPresent(sfAMMID)) return tecNO_PERMISSION; } diff --git a/src/ripple/app/tx/impl/Payment.cpp b/src/ripple/app/tx/impl/Payment.cpp index a23e41bac2a..d3c26a37023 100644 --- a/src/ripple/app/tx/impl/Payment.cpp +++ b/src/ripple/app/tx/impl/Payment.cpp @@ -468,7 +468,7 @@ Payment::doApply() // AMMs can never receive an XRP payment. // Must use AMMDeposit transaction instead. - if (sleDst->getFlags() & lsfAMM) + if (sleDst->isFieldPresent(sfAMMID)) return tecNO_PERMISSION; // The source account does have enough money. Make sure the diff --git a/src/ripple/app/tx/impl/SetTrust.cpp b/src/ripple/app/tx/impl/SetTrust.cpp index acbbedabf10..7869cc7027d 100644 --- a/src/ripple/app/tx/impl/SetTrust.cpp +++ b/src/ripple/app/tx/impl/SetTrust.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -128,20 +129,46 @@ SetTrust::preclaim(PreclaimContext const& ctx) } } + // This might be nullptr + auto const sleDst = ctx.view.read(keylet::account(uDstAccountID)); + // If the destination has opted to disallow incoming trustlines // then honour that flag if (ctx.view.rules().enabled(featureDisallowIncoming)) { - auto const sleDst = ctx.view.read(keylet::account(uDstAccountID)); - if (!sleDst) return tecNO_DST; - auto const dstFlags = sleDst->getFlags(); - if (dstFlags & lsfDisallowIncomingTrustline) + if (sleDst->getFlags() & lsfDisallowIncomingTrustline) return tecNO_PERMISSION; } + // If destination is AMM and the trustline doesn't exist then only + // allow SetTrust if the asset is AMM LP token and AMM is not + // in empty state. + if (ammEnabled(ctx.view.rules())) + { + if (!sleDst) + return tecNO_DST; + + if (sleDst->isFieldPresent(sfAMMID) && + !ctx.view.read(keylet::line(id, uDstAccountID, currency))) + { + if (auto const ammSle = + ctx.view.read({ltAMM, sleDst->getFieldH256(sfAMMID)})) + { + if (auto const lpTokens = + ammSle->getFieldAmount(sfLPTokenBalance); + lpTokens == beast::zero) + return tecAMM_EMPTY; + else if (lpTokens.getCurrency() != saLimitAmount.getCurrency()) + return tecNO_PERMISSION; + } + else + return tecINTERNAL; + } + } + return tesSUCCESS; } diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index 66aa10227d4..449392531b8 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -757,6 +757,32 @@ removeExpiredNFTokenOffers( } } +static void +removeDeletedTrustLines( + ApplyView& view, + std::vector const& trustLines, + beast::Journal viewJ) +{ + if (trustLines.size() > maxDeletableAMMTrustLines) + { + JLOG(viewJ.error()) + << "removeDeletedTrustLines: deleted trustlines exceed max " + << trustLines.size(); + return; + } + + for (auto const& index : trustLines) + { + if (auto const sleState = view.peek({ltRIPPLE_STATE, index}); + deleteAMMTrustLine(view, sleState, std::nullopt, viewJ) != + tesSUCCESS) + { + JLOG(viewJ.error()) + << "removeDeletedTrustLines: failed to delete AMM trustline"; + } + } +} + /** Reset the context, discarding any changes made and adjust the fee */ std::pair Transactor::reset(XRPAmount fee) @@ -849,7 +875,8 @@ Transactor::operator()() } else if ( (result == tecOVERSIZE) || (result == tecKILLED) || - (result == tecEXPIRED) || (isTecClaimHardFail(result, view().flags()))) + (result == tecINCOMPLETE) || (result == tecEXPIRED) || + (isTecClaimHardFail(result, view().flags()))) { JLOG(j_.trace()) << "reapplying because of " << transToken(result); @@ -858,10 +885,21 @@ Transactor::operator()() // should be used, making it possible to do more useful work // when transactions fail with a `tec` code. std::vector removedOffers; + std::vector removedTrustLines; + std::vector expiredNFTokenOffers; - if ((result == tecOVERSIZE) || (result == tecKILLED)) + bool const doOffers = + ((result == tecOVERSIZE) || (result == tecKILLED)); + bool const doLines = (result == tecINCOMPLETE); + bool const doNFTokenOffers = (result == tecEXPIRED); + if (doOffers || doLines || doNFTokenOffers) { - ctx_.visit([&removedOffers]( + ctx_.visit([&doOffers, + &removedOffers, + &doLines, + &removedTrustLines, + &doNFTokenOffers, + &expiredNFTokenOffers]( uint256 const& index, bool isDelete, std::shared_ptr const& before, @@ -869,30 +907,23 @@ Transactor::operator()() if (isDelete) { assert(before && after); - if (before && after && (before->getType() == ltOFFER) && + if (doOffers && before && after && + (before->getType() == ltOFFER) && (before->getFieldAmount(sfTakerPays) == after->getFieldAmount(sfTakerPays))) { // Removal of offer found or made unfunded removedOffers.push_back(index); } - } - }); - } - std::vector expiredNFTokenOffers; + if (doLines && before && after && + (before->getType() == ltRIPPLE_STATE)) + { + // Removal of obsolete AMM trust line + removedTrustLines.push_back(index); + } - if (result == tecEXPIRED) - { - ctx_.visit([&expiredNFTokenOffers]( - uint256 const& index, - bool isDelete, - std::shared_ptr const& before, - std::shared_ptr const& after) { - if (isDelete) - { - assert(before && after); - if (before && after && + if (doNFTokenOffers && before && after && (before->getType() == ltNFTOKEN_OFFER)) expiredNFTokenOffers.push_back(index); } @@ -917,6 +948,10 @@ Transactor::operator()() removeExpiredNFTokenOffers( view(), expiredNFTokenOffers, ctx_.app.journal("View")); + if (result == tecINCOMPLETE) + removeDeletedTrustLines( + view(), removedTrustLines, ctx_.app.journal("View")); + applied = isTecClaim(result); } diff --git a/src/ripple/app/tx/impl/applySteps.cpp b/src/ripple/app/tx/impl/applySteps.cpp index f0d092d793d..fdb84c271a0 100644 --- a/src/ripple/app/tx/impl/applySteps.cpp +++ b/src/ripple/app/tx/impl/applySteps.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -165,6 +166,8 @@ invoke_preflight(PreflightContext const& ctx) return invoke_preflight_helper(ctx); case ttAMM_BID: return invoke_preflight_helper(ctx); + case ttAMM_DELETE: + return invoke_preflight_helper(ctx); default: assert(false); return {temUNKNOWN, TxConsequences{temUNKNOWN}}; @@ -278,6 +281,8 @@ invoke_preclaim(PreclaimContext const& ctx) return invoke_preclaim(ctx); case ttAMM_BID: return invoke_preclaim(ctx); + case ttAMM_DELETE: + return invoke_preclaim(ctx); default: assert(false); return temUNKNOWN; @@ -353,6 +358,8 @@ invoke_calculateBaseFee(ReadView const& view, STTx const& tx) return AMMVote::calculateBaseFee(view, tx); case ttAMM_BID: return AMMBid::calculateBaseFee(view, tx); + case ttAMM_DELETE: + return AMMDelete::calculateBaseFee(view, tx); default: assert(false); return XRPAmount{0}; @@ -529,6 +536,10 @@ invoke_apply(ApplyContext& ctx) AMMBid p(ctx); return p(); } + case ttAMM_DELETE: { + AMMDelete p(ctx); + return p(); + } default: assert(false); return {temUNKNOWN, false}; diff --git a/src/ripple/ledger/View.h b/src/ripple/ledger/View.h index 939624ae24e..2c8d2354e0c 100644 --- a/src/ripple/ledger/View.h +++ b/src/ripple/ledger/View.h @@ -42,7 +42,7 @@ namespace ripple { -enum class WaiveTransferFee { Yes, No }; +enum class WaiveTransferFee : bool { No = false, Yes }; //------------------------------------------------------------------------------ // @@ -458,6 +458,33 @@ transferXRP( [[nodiscard]] TER requireAuth(ReadView const& view, Issue const& issue, AccountID const& account); +/** Cleanup owner directory entries on account delete. + * Used for a regular and AMM accounts deletion. The caller + * has to provide the deleter function, which handles details of + * specific account-owned object deletion. + * @return tecINCOMPLETE indicates maxNodesToDelete + * are deleted and there remains more nodes to delete. + */ +[[nodiscard]] TER +cleanupOnAccountDelete( + ApplyView& view, + Keylet const& ownerDirKeylet, + std::function&)> + deleter, + beast::Journal j, + std::optional maxNodesToDelete = std::nullopt); + +/** Delete trustline to AMM. The passed `sle` must be obtained from a prior + * call to view.peek(). Fail if neither side of the trustline is AMM or + * if ammAccountID is seated and is not one of the trustline's side. + */ +[[nodiscard]] TER +deleteAMMTrustLine( + ApplyView& view, + std::shared_ptr sleState, + std::optional const& ammAccountID, + beast::Journal j); + } // namespace ripple #endif diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index 2de92bfc20f..8d059aaf4b7 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -375,7 +375,7 @@ xrpLiquid( view.ownerCountHook(id, sle->getFieldU32(sfOwnerCount)), ownerCountAdj); // AMMs have no reserve requirement - auto const reserve = (sle->getFlags() & lsfAMM) + auto const reserve = sle->isFieldPresent(sfAMMID) ? XRPAmount{0} : view.fees().accountReserve(ownerCount); @@ -1544,4 +1544,129 @@ requireAuth(ReadView const& view, Issue const& issue, AccountID const& account) return tesSUCCESS; } +TER +cleanupOnAccountDelete( + ApplyView& view, + Keylet const& ownerDirKeylet, + std::function&)> + deleter, + beast::Journal j, + std::optional maxNodesToDelete) +{ + // Delete all the entries in the account directory. + std::shared_ptr sleDirNode{}; + unsigned int uDirEntry{0}; + uint256 dirEntry{beast::zero}; + std::uint32_t deleted = 0; + + if (view.exists(ownerDirKeylet) && + dirFirst(view, ownerDirKeylet.key, sleDirNode, uDirEntry, dirEntry)) + { + do + { + if (maxNodesToDelete && ++deleted > *maxNodesToDelete) + return tecINCOMPLETE; + + // Choose the right way to delete each directory node. + auto sleItem = view.peek(keylet::child(dirEntry)); + if (!sleItem) + { + // Directory node has an invalid index. Bail out. + JLOG(j.fatal()) + << "DeleteAccount: Directory node in ledger " << view.seq() + << " has index to object that is missing: " + << to_string(dirEntry); + return tefBAD_LEDGER; + } + + LedgerEntryType const nodeType{safe_cast( + sleItem->getFieldU16(sfLedgerEntryType))}; + + // Deleter handles the details of specific account-owned object + // deletion + if (auto const ter = deleter(nodeType, dirEntry, sleItem); + ter != tesSUCCESS) + return ter; + + // dirFirst() and dirNext() are like iterators with exposed + // internal state. We'll take advantage of that exposed state + // to solve a common C++ problem: iterator invalidation while + // deleting elements from a container. + // + // We have just deleted one directory entry, which means our + // "iterator state" is invalid. + // + // 1. During the process of getting an entry from the + // directory uDirEntry was incremented from 0 to 1. + // + // 2. We then deleted the entry at index 0, which means the + // entry that was at 1 has now moved to 0. + // + // 3. So we verify that uDirEntry is indeed 1. Then we jam it + // back to zero to "un-invalidate" the iterator. + assert(uDirEntry == 1); + if (uDirEntry != 1) + { + JLOG(j.error()) + << "DeleteAccount iterator re-validation failed."; + return tefBAD_LEDGER; + } + uDirEntry = 0; + + } while ( + dirNext(view, ownerDirKeylet.key, sleDirNode, uDirEntry, dirEntry)); + } + + return tesSUCCESS; +} + +TER +deleteAMMTrustLine( + ApplyView& view, + std::shared_ptr sleState, + std::optional const& ammAccountID, + beast::Journal j) +{ + if (!sleState || sleState->getType() != ltRIPPLE_STATE) + return tecINTERNAL; + + auto const& [low, high] = std::minmax( + sleState->getFieldAmount(sfLowLimit).getIssuer(), + sleState->getFieldAmount(sfHighLimit).getIssuer()); + auto sleLow = view.peek(keylet::account(low)); + auto sleHigh = view.peek(keylet::account(high)); + if (!sleLow || !sleHigh) + return tecINTERNAL; + bool const ammLow = sleLow->isFieldPresent(sfAMMID); + bool const ammHigh = sleHigh->isFieldPresent(sfAMMID); + + // can't both be AMM + if (ammLow && ammHigh) + return tecINTERNAL; + + // at least one must be + if (!ammLow && !ammHigh) + return terNO_AMM; + + // one must be the target amm + if (ammAccountID && (low != *ammAccountID && high != *ammAccountID)) + return terNO_AMM; + + if (auto const ter = trustDelete(view, sleState, low, high, j); + ter != tesSUCCESS) + { + JLOG(j.error()) + << "deleteAMMTrustLine: failed to delete the trustline."; + return ter; + } + + auto const uFlags = !ammLow ? lsfLowReserve : lsfHighReserve; + if (!(sleState->getFlags() & uFlags)) + return tecINTERNAL; + + adjustOwnerCount(view, !ammLow ? sleLow : sleHigh, -1, j); + + return tesSUCCESS; +} + } // namespace ripple diff --git a/src/ripple/protocol/LedgerFormats.h b/src/ripple/protocol/LedgerFormats.h index a613c3a470d..b9205e7888a 100644 --- a/src/ripple/protocol/LedgerFormats.h +++ b/src/ripple/protocol/LedgerFormats.h @@ -249,7 +249,7 @@ enum LedgerSpecificFlags { 0x10000000, // True, reject new paychans lsfDisallowIncomingTrustline = 0x20000000, // True, reject new trustlines (only if no issued assets) - lsfAMM [[maybe_unused]] = 0x40000000, // True, AMM account + // 0x40000000 is available lsfAllowTrustLineClawback = 0x80000000, // True, enable clawback diff --git a/src/ripple/protocol/Protocol.h b/src/ripple/protocol/Protocol.h index 5df24271f68..6e4879cd746 100644 --- a/src/ripple/protocol/Protocol.h +++ b/src/ripple/protocol/Protocol.h @@ -95,6 +95,11 @@ using LedgerIndex = std::uint32_t; */ using TxID = uint256; +/** The maximum number of trustlines to delete as part of AMM account + * deletion cleanup. + */ +std::uint16_t constexpr maxDeletableAMMTrustLines = 512; + } // namespace ripple #endif diff --git a/src/ripple/protocol/TER.h b/src/ripple/protocol/TER.h index 955181de7e2..a2743bace8d 100644 --- a/src/ripple/protocol/TER.h +++ b/src/ripple/protocol/TER.h @@ -298,7 +298,11 @@ enum TECcodes : TERUnderlyingType { tecUNFUNDED_AMM = 162, tecAMM_BALANCE = 163, tecAMM_FAILED = 164, - tecAMM_INVALID_TOKENS = 165 + tecAMM_INVALID_TOKENS = 165, + tecAMM_EMPTY = 166, + tecAMM_NOT_EMPTY = 167, + tecAMM_ACCOUNT = 168, + tecINCOMPLETE = 169 }; //------------------------------------------------------------------------------ diff --git a/src/ripple/protocol/TxFlags.h b/src/ripple/protocol/TxFlags.h index 75d405cb827..39680e41d95 100644 --- a/src/ripple/protocol/TxFlags.h +++ b/src/ripple/protocol/TxFlags.h @@ -171,12 +171,13 @@ constexpr std::uint32_t tfSingleAsset = 0x00080000; constexpr std::uint32_t tfTwoAsset = 0x00100000; constexpr std::uint32_t tfOneAssetLPToken = 0x00200000; constexpr std::uint32_t tfLimitLPToken = 0x00400000; +constexpr std::uint32_t tfTwoAssetIfEmpty = 0x00800000; constexpr std::uint32_t tfWithdrawSubTx = tfLPToken | tfSingleAsset | tfTwoAsset | tfOneAssetLPToken | tfLimitLPToken | tfWithdrawAll | tfOneAssetWithdrawAll; constexpr std::uint32_t tfDepositSubTx = tfLPToken | tfSingleAsset | tfTwoAsset | tfOneAssetLPToken | - tfLimitLPToken; + tfLimitLPToken | tfTwoAssetIfEmpty; constexpr std::uint32_t tfWithdrawMask = ~(tfUniversal | tfWithdrawSubTx); constexpr std::uint32_t tfDepositMask = ~(tfUniversal | tfDepositSubTx); diff --git a/src/ripple/protocol/TxFormats.h b/src/ripple/protocol/TxFormats.h index a79f7dd79cb..2d7ba40c44c 100644 --- a/src/ripple/protocol/TxFormats.h +++ b/src/ripple/protocol/TxFormats.h @@ -157,6 +157,9 @@ enum TxType : std::uint16_t /** This transaction type bids for the auction slot */ ttAMM_BID = 39, + /** This transaction type deletes AMM in the empty state */ + ttAMM_DELETE = 40, + /** This system-generated transaction type is used to update the status of the various amendments. For details, see: https://xrpl.org/amendments.html diff --git a/src/ripple/protocol/impl/InnerObjectFormats.cpp b/src/ripple/protocol/impl/InnerObjectFormats.cpp index e6de3cc04e4..ba1a40a87ee 100644 --- a/src/ripple/protocol/impl/InnerObjectFormats.cpp +++ b/src/ripple/protocol/impl/InnerObjectFormats.cpp @@ -77,6 +77,12 @@ InnerObjectFormats::InnerObjectFormats() {sfPrice, soeREQUIRED}, {sfAuthAccounts, soeOPTIONAL}, }); + + add(sfAuthAccount.jsonName.c_str(), + sfAuthAccount.getCode(), + { + {sfAccount, soeREQUIRED}, + }); } InnerObjectFormats const& diff --git a/src/ripple/protocol/impl/LedgerFormats.cpp b/src/ripple/protocol/impl/LedgerFormats.cpp index 5228b625bb3..9192513457a 100644 --- a/src/ripple/protocol/impl/LedgerFormats.cpp +++ b/src/ripple/protocol/impl/LedgerFormats.cpp @@ -56,6 +56,7 @@ LedgerFormats::LedgerFormats() {sfMintedNFTokens, soeDEFAULT}, {sfBurnedNFTokens, soeDEFAULT}, {sfFirstNFTokenSequence, soeOPTIONAL}, + {sfAMMID, soeOPTIONAL}, }, commonFields); @@ -277,7 +278,7 @@ LedgerFormats::LedgerFormats() {sfAuctionSlot, soeOPTIONAL}, {sfLPTokenBalance, soeREQUIRED}, {sfAsset, soeREQUIRED}, - {sfAsset2, soeREQUIRED} + {sfAsset2, soeREQUIRED}, }, commonFields); diff --git a/src/ripple/protocol/impl/TER.cpp b/src/ripple/protocol/impl/TER.cpp index c16e9541fbf..29b87351204 100644 --- a/src/ripple/protocol/impl/TER.cpp +++ b/src/ripple/protocol/impl/TER.cpp @@ -45,6 +45,9 @@ transResults() MAKE_ERROR(tecAMM_BALANCE, "AMM has invalid balance."), MAKE_ERROR(tecAMM_INVALID_TOKENS, "AMM invalid LP tokens."), MAKE_ERROR(tecAMM_FAILED, "AMM transaction failed."), + MAKE_ERROR(tecAMM_EMPTY, "AMM is in empty state."), + MAKE_ERROR(tecAMM_NOT_EMPTY, "AMM is not in empty state."), + MAKE_ERROR(tecAMM_ACCOUNT, "This operation is not allowed on an AMM Account."), MAKE_ERROR(tecCLAIM, "Fee claimed. Sequence used. No action."), MAKE_ERROR(tecDIR_FULL, "Can not add entry to full directory."), MAKE_ERROR(tecFAILED_PROCESSING, "Failed to correctly process transaction."), @@ -92,6 +95,7 @@ transResults() MAKE_ERROR(tecINSUFFICIENT_FUNDS, "Not enough funds available to complete requested transaction."), MAKE_ERROR(tecOBJECT_NOT_FOUND, "A requested object could not be located."), MAKE_ERROR(tecINSUFFICIENT_PAYMENT, "The payment is not sufficient."), + MAKE_ERROR(tecINCOMPLETE, "Some work was completed, but more submissions required to finish."), MAKE_ERROR(tefALREADY, "The exact transaction was already in this ledger."), MAKE_ERROR(tefBAD_ADD_AUTH, "Not authorized to add account."), diff --git a/src/ripple/protocol/impl/TxFormats.cpp b/src/ripple/protocol/impl/TxFormats.cpp index 062174815c9..549e6619c1a 100644 --- a/src/ripple/protocol/impl/TxFormats.cpp +++ b/src/ripple/protocol/impl/TxFormats.cpp @@ -101,6 +101,7 @@ TxFormats::TxFormats() {sfEPrice, soeOPTIONAL}, {sfLPTokenOut, soeOPTIONAL}, {sfTicketSequence, soeOPTIONAL}, + {sfTradingFee, soeOPTIONAL}, }, commonFields); @@ -139,6 +140,15 @@ TxFormats::TxFormats() }, commonFields); + add(jss::AMMDelete, + ttAMM_DELETE, + { + {sfAsset, soeREQUIRED}, + {sfAsset2, soeREQUIRED}, + {sfTicketSequence, soeOPTIONAL}, + }, + commonFields); + add(jss::OfferCancel, ttOFFER_CANCEL, { diff --git a/src/ripple/protocol/jss.h b/src/ripple/protocol/jss.h index c7faa4ff98b..a0f81858637 100644 --- a/src/ripple/protocol/jss.h +++ b/src/ripple/protocol/jss.h @@ -52,6 +52,7 @@ JSS(AMMBid); // transaction type JSS(AMMID); // field JSS(AMMCreate); // transaction type JSS(AMMDeposit); // transaction type +JSS(AMMDelete); // transaction type JSS(AMMVote); // transaction type JSS(AMMWithdraw); // transaction type JSS(Amendments); // ledger type. diff --git a/src/test/app/AMMExtended_test.cpp b/src/test/app/AMMExtended_test.cpp index 6b888ce8a2c..ad2c1f67805 100644 --- a/src/test/app/AMMExtended_test.cpp +++ b/src/test/app/AMMExtended_test.cpp @@ -32,7 +32,6 @@ #include #include #include -#include #include #include diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 98804e77b74..b0e5106f9eb 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -27,7 +27,6 @@ #include #include #include -#include #include #include @@ -109,6 +108,15 @@ struct AMM_test : public jtx::AMMTest env.close(); AMM ammAlice(env, alice, XRP(10'000), USD(10'000)); } + + // Trading fee + testAMM( + [&](AMM& amm, Env&) { + BEAST_EXPECT(amm.expectTradingFee(1'000)); + BEAST_EXPECT(amm.expectAuctionSlot(100, 0, IOUAmount{0})); + }, + std::nullopt, + 1'000); } void @@ -392,6 +400,21 @@ struct AMM_test : public jtx::AMMTest AMM ammAlice1( env, alice, USD(10'000), USD1(10'000), ter(terNO_RIPPLE)); } + + // Issuer has clawback enabled + { + Env env(*this); + env.fund(XRP(1'000), gw); + env(fset(gw, asfAllowTrustLineClawback)); + fund(env, gw, {alice}, XRP(1'000), {USD(1'000)}, Fund::Acct); + env.close(); + AMM amm(env, gw, XRP(100), USD(100), ter(tecNO_PERMISSION)); + AMM amm1(env, alice, USD(100), XRP(100), ter(tecNO_PERMISSION)); + env(fclear(gw, asfAllowTrustLineClawback)); + env.close(); + // Can't be cleared + AMM amm2(env, gw, XRP(100), USD(100), ter(tecNO_PERMISSION)); + } } void @@ -416,96 +439,167 @@ struct AMM_test : public jtx::AMMTest std::optional, std::optional, std::optional, - std::optional>> + std::optional, + std::optional>> invalidOptions = { - // flags, tokens, asset1In, asset2in, EPrice - {tfLPToken, 1'000, std::nullopt, USD(100), std::nullopt}, - {tfLPToken, 1'000, XRP(100), std::nullopt, std::nullopt}, + // flags, tokens, asset1In, asset2in, EPrice, tfee + {tfLPToken, + 1'000, + std::nullopt, + USD(100), + std::nullopt, + std::nullopt}, {tfLPToken, 1'000, + XRP(100), std::nullopt, std::nullopt, - STAmount{USD, 1, -1}}, + std::nullopt}, + {tfLPToken, + 1'000, + std::nullopt, + std::nullopt, + STAmount{USD, 1, -1}, + std::nullopt}, {tfLPToken, std::nullopt, USD(100), std::nullopt, - STAmount{USD, 1, -1}}, + STAmount{USD, 1, -1}, + std::nullopt}, {tfLPToken, 1'000, XRP(100), std::nullopt, - STAmount{USD, 1, -1}}, + STAmount{USD, 1, -1}, + std::nullopt}, + {tfLPToken, + 1'000, + std::nullopt, + std::nullopt, + std::nullopt, + 1'000}, {tfSingleAsset, 1'000, std::nullopt, std::nullopt, + std::nullopt, std::nullopt}, {tfSingleAsset, std::nullopt, std::nullopt, USD(100), + std::nullopt, std::nullopt}, {tfSingleAsset, std::nullopt, std::nullopt, std::nullopt, - STAmount{USD, 1, -1}}, + STAmount{USD, 1, -1}, + std::nullopt}, + {tfSingleAsset, + std::nullopt, + USD(100), + std::nullopt, + std::nullopt, + 1'000}, {tfTwoAsset, 1'000, std::nullopt, std::nullopt, + std::nullopt, std::nullopt}, {tfTwoAsset, std::nullopt, XRP(100), USD(100), - STAmount{USD, 1, -1}}, + STAmount{USD, 1, -1}, + std::nullopt}, {tfTwoAsset, std::nullopt, XRP(100), std::nullopt, + std::nullopt, std::nullopt}, + {tfTwoAsset, + std::nullopt, + XRP(100), + USD(100), + std::nullopt, + 1'000}, {tfTwoAsset, std::nullopt, std::nullopt, USD(100), - STAmount{USD, 1, -1}}, + STAmount{USD, 1, -1}, + std::nullopt}, {tfOneAssetLPToken, 1'000, std::nullopt, std::nullopt, + std::nullopt, std::nullopt}, {tfOneAssetLPToken, std::nullopt, XRP(100), USD(100), + std::nullopt, std::nullopt}, {tfOneAssetLPToken, std::nullopt, XRP(100), std::nullopt, - STAmount{USD, 1, -1}}, + STAmount{USD, 1, -1}, + std::nullopt}, + {tfOneAssetLPToken, + 1'000, + XRP(100), + std::nullopt, + std::nullopt, + 1'000}, {tfLimitLPToken, 1'000, std::nullopt, std::nullopt, + std::nullopt, std::nullopt}, {tfLimitLPToken, 1'000, USD(100), std::nullopt, + std::nullopt, std::nullopt}, {tfLimitLPToken, std::nullopt, USD(100), XRP(100), + std::nullopt, std::nullopt}, {tfLimitLPToken, std::nullopt, + XRP(100), + std::nullopt, + STAmount{USD, 1, -1}, + 1'000}, + {tfTwoAssetIfEmpty, + std::nullopt, + std::nullopt, + std::nullopt, + std::nullopt, + 1'000}, + {tfTwoAssetIfEmpty, + 1'000, std::nullopt, std::nullopt, - STAmount{USD, 1, -1}}}; + std::nullopt, + std::nullopt}, + {tfTwoAssetIfEmpty, + std::nullopt, + XRP(100), + USD(100), + STAmount{USD, 1, -1}, + std::nullopt}, + }; for (auto const& it : invalidOptions) { ammAlice.deposit( @@ -517,6 +611,7 @@ struct AMM_test : public jtx::AMMTest std::get<0>(it), std::nullopt, std::nullopt, + std::get<5>(it), ter(temMALFORMED)); } @@ -543,6 +638,7 @@ struct AMM_test : public jtx::AMMTest std::nullopt, {{iss1, iss2}}, std::nullopt, + std::nullopt, ter(terNO_AMM)); } @@ -617,6 +713,7 @@ struct AMM_test : public jtx::AMMTest std::nullopt, std::nullopt, seq(1), + std::nullopt, ter(terNO_ACCOUNT)); // Invalid AMM @@ -629,6 +726,7 @@ struct AMM_test : public jtx::AMMTest std::nullopt, {{USD, GBP}}, std::nullopt, + std::nullopt, ter(terNO_AMM)); // Single deposit: 100000 tokens worth of USD @@ -642,6 +740,7 @@ struct AMM_test : public jtx::AMMTest std::nullopt, std::nullopt, std::nullopt, + std::nullopt, ter(tecAMM_FAILED)); // Single deposit: 100000 tokens worth of XRP @@ -655,6 +754,7 @@ struct AMM_test : public jtx::AMMTest std::nullopt, std::nullopt, std::nullopt, + std::nullopt, ter(tecAMM_FAILED)); // Deposit amount is invalid @@ -689,6 +789,15 @@ struct AMM_test : public jtx::AMMTest std::nullopt, std::nullopt, ter(tecAMM_INVALID_TOKENS)); + + // Deposit non-empty AMM + ammAlice.deposit( + carol, + XRP(100), + USD(100), + std::nullopt, + tfTwoAssetIfEmpty, + ter(tecAMM_NOT_EMPTY)); }); // Invalid AMM @@ -790,6 +899,7 @@ struct AMM_test : public jtx::AMMTest std::nullopt, std::nullopt, std::nullopt, + std::nullopt, ter(tecUNFUNDED_AMM)); }); @@ -809,6 +919,7 @@ struct AMM_test : public jtx::AMMTest std::nullopt, std::nullopt, std::nullopt, + std::nullopt, ter(tecUNFUNDED_AMM)); }); @@ -884,6 +995,7 @@ struct AMM_test : public jtx::AMMTest tfTwoAsset, std::nullopt, std::nullopt, + std::nullopt, ter(temBAD_AMM_TOKENS)); // min amounts can't be <= zero ammAlice.deposit( @@ -895,6 +1007,7 @@ struct AMM_test : public jtx::AMMTest tfTwoAsset, std::nullopt, std::nullopt, + std::nullopt, ter(temBAD_AMOUNT)); ammAlice.deposit( carol, @@ -905,6 +1018,7 @@ struct AMM_test : public jtx::AMMTest tfTwoAsset, std::nullopt, std::nullopt, + std::nullopt, ter(temBAD_AMOUNT)); // min amount bad currency ammAlice.deposit( @@ -916,6 +1030,7 @@ struct AMM_test : public jtx::AMMTest tfTwoAsset, std::nullopt, std::nullopt, + std::nullopt, ter(temBAD_CURRENCY)); // min amount bad token pair ammAlice.deposit( @@ -927,6 +1042,7 @@ struct AMM_test : public jtx::AMMTest tfTwoAsset, std::nullopt, std::nullopt, + std::nullopt, ter(temBAD_AMM_TOKENS)); ammAlice.deposit( carol, @@ -937,6 +1053,7 @@ struct AMM_test : public jtx::AMMTest tfTwoAsset, std::nullopt, std::nullopt, + std::nullopt, ter(temBAD_AMM_TOKENS)); }); @@ -952,6 +1069,7 @@ struct AMM_test : public jtx::AMMTest tfLPToken, std::nullopt, std::nullopt, + std::nullopt, ter(tecAMM_FAILED)); ammAlice.deposit( carol, @@ -962,6 +1080,7 @@ struct AMM_test : public jtx::AMMTest tfLPToken, std::nullopt, std::nullopt, + std::nullopt, ter(tecAMM_FAILED)); // Equal deposit by asset ammAlice.deposit( @@ -973,6 +1092,7 @@ struct AMM_test : public jtx::AMMTest tfTwoAsset, std::nullopt, std::nullopt, + std::nullopt, ter(tecAMM_FAILED)); // Single deposit by asset ammAlice.deposit( @@ -984,6 +1104,7 @@ struct AMM_test : public jtx::AMMTest tfSingleAsset, std::nullopt, std::nullopt, + std::nullopt, ter(tecAMM_FAILED)); }); } @@ -1250,6 +1371,16 @@ struct AMM_test : public jtx::AMMTest std::nullopt, std::nullopt, ter(temINVALID_FLAG)); + ammAlice.withdraw( + alice, + 1'000'000, + std::nullopt, + std::nullopt, + std::nullopt, + tfTwoAssetIfEmpty, + std::nullopt, + std::nullopt, + ter(temINVALID_FLAG)); // Invalid options std::vector cfg) { + cfg->FEES.reference_fee = XRPAmount(1); + return cfg; + }), + all); + fund(env, gw, {alice}, XRP(20'000), {USD(10'000)}); + AMM amm(env, gw, XRP(10'000), USD(10'000)); + for (auto i = 0; i < maxDeletableAMMTrustLines + 10; ++i) + { + Account const a{std::to_string(i)}; + env.fund(XRP(1'000), a); + env(trust(a, STAmount{amm.lptIssue(), 10'000})); + env.close(); + } + // The trustlines are partially deleted, + // AMM is set to an empty state. + amm.withdrawAll(gw); + BEAST_EXPECT(amm.ammExists()); + + // Bid,Vote,Deposit,Withdraw,SetTrust failing with + // tecAMM_EMPTY. Deposit succeeds with tfTwoAssetIfEmpty option. + amm.bid( + alice, + 1000, + std::nullopt, + {}, + std::nullopt, + std::nullopt, + std::nullopt, + ter(tecAMM_EMPTY)); + amm.vote( + std::nullopt, + 100, + std::nullopt, + std::nullopt, + std::nullopt, + ter(tecAMM_EMPTY)); + amm.withdraw( + alice, 100, std::nullopt, std::nullopt, ter(tecAMM_EMPTY)); + amm.deposit( + alice, + USD(100), + std::nullopt, + std::nullopt, + std::nullopt, + ter(tecAMM_EMPTY)); + env(trust(alice, STAmount{amm.lptIssue(), 10'000}), + ter(tecAMM_EMPTY)); + + // Can deposit with tfTwoAssetIfEmpty option + amm.deposit( + alice, + std::nullopt, + XRP(10'000), + USD(10'000), + std::nullopt, + tfTwoAssetIfEmpty, + std::nullopt, + std::nullopt, + 1'000); + BEAST_EXPECT( + amm.expectBalances(XRP(10'000), USD(10'000), amm.tokens())); + BEAST_EXPECT(amm.expectTradingFee(1'000)); + BEAST_EXPECT(amm.expectAuctionSlot(100, 0, IOUAmount{0})); + + // Withdrawing all tokens deletes AMM since the number + // of remaining trustlines is less than max + amm.withdrawAll(alice); + BEAST_EXPECT(!amm.ammExists()); + BEAST_EXPECT(!env.le(keylet::ownerDir(amm.ammAccount()))); + } + + { + Env env( + *this, + envconfig([](std::unique_ptr cfg) { + cfg->FEES.reference_fee = XRPAmount(1); + return cfg; + }), + all); + fund(env, gw, {alice}, XRP(20'000), {USD(10'000)}); + AMM amm(env, gw, XRP(10'000), USD(10'000)); + for (auto i = 0; i < maxDeletableAMMTrustLines * 2 + 10; ++i) + { + Account const a{std::to_string(i)}; + env.fund(XRP(1'000), a); + env(trust(a, STAmount{amm.lptIssue(), 10'000})); + env.close(); + } + // The trustlines are partially deleted. + amm.withdrawAll(gw); + BEAST_EXPECT(amm.ammExists()); + + // AMMDelete has to be called twice to delete AMM. + amm.ammDelete(alice, ter(tecINCOMPLETE)); + BEAST_EXPECT(amm.ammExists()); + // Deletes remaining trustlines and deletes AMM. + amm.ammDelete(alice); + BEAST_EXPECT(!amm.ammExists()); + BEAST_EXPECT(!env.le(keylet::ownerDir(amm.ammAccount()))); + } + } + + void + testClawback() + { + testcase("Clawback"); + using namespace jtx; + Env env(*this); + env.fund(XRP(2'000), gw); + env.fund(XRP(2'000), alice); + AMM amm(env, gw, XRP(1'000), USD(1'000)); + env(fset(gw, asfAllowTrustLineClawback), ter(tecOWNERS)); + } + + void + testAMMID() + { + testcase("AMMID"); + using namespace jtx; + testAMM([&](AMM& amm, Env& env) { + amm.setClose(false); + auto const info = env.rpc( + "json", + "account_info", + std::string( + "{\"account\": \"" + to_string(amm.ammAccount()) + "\"}")); + try + { + BEAST_EXPECT( + info[jss::result][jss::account_data][jss::AMMID] + .asString() == to_string(amm.ammID())); + } + catch (...) + { + fail(); + } + amm.deposit(carol, 1'000); + auto affected = env.meta()->getJson( + JsonOptions::none)[sfAffectedNodes.fieldName]; + try + { + bool found = false; + for (auto const& node : affected) + { + if (node.isMember(sfModifiedNode.fieldName) && + node[sfModifiedNode.fieldName] + [sfLedgerEntryType.fieldName] + .asString() == "AccountRoot" && + node[sfModifiedNode.fieldName][sfFinalFields.fieldName] + [jss::Account] + .asString() == to_string(amm.ammAccount())) + { + found = node[sfModifiedNode.fieldName] + [sfFinalFields.fieldName][jss::AMMID] + .asString() == to_string(amm.ammID()); + break; + } + } + BEAST_EXPECT(found); + } + catch (...) + { + fail(); + } + }); + } + + void + testSelection() + { + testcase("Offer/Strand Selection"); + using namespace jtx; + Account const ed("ed"); + Account const gw1("gw1"); + auto const ETH = gw1["ETH"]; + auto const CAN = gw1["CAN"]; + + // These tests are expected to fail if the OwnerPaysFee feature + // is ever supported. Updates will need to be made to AMM handling + // in the payment engine, and these tests will need to be updated. + + auto prep = [&](Env& env, auto gwRate, auto gw1Rate) { + fund(env, gw, {alice, carol, bob, ed}, XRP(2'000), {USD(2'000)}); + env.fund(XRP(2'000), gw1); + fund( + env, + gw1, + {alice, carol, bob, ed}, + {ETH(2'000), CAN(2'000)}, + Fund::IOUOnly); + env(rate(gw, gwRate)); + env(rate(gw1, gw1Rate)); + env.close(); + }; + + for (auto const& rates : + {std::make_pair(1.5, 1.9), std::make_pair(1.9, 1.5)}) + { + // Offer Selection + + // Cross-currency payment: AMM has the same spot price quality + // as CLOB's offer and can't generate a better quality offer. + // The transfer fee in this case doesn't change the CLOB quality + // because trIn is ignored on adjustment and trOut on payment is + // also ignored because ownerPaysTransferFee is false in this case. + // Run test for 0) offer, 1) AMM, 2) offer and AMM + // to verify that the quality is better in the first case, + // and CLOB is selected in the second case. + { + std::array q; + for (auto i = 0; i < 3; ++i) + { + Env env(*this); + prep(env, rates.first, rates.second); + std::optional amm; + if (i == 0 || i == 2) + { + env(offer(ed, ETH(400), USD(400)), txflags(tfPassive)); + env.close(); + } + if (i > 0) + amm.emplace(env, ed, USD(1'000), ETH(1'000)); + env(pay(carol, bob, USD(100)), + path(~USD), + sendmax(ETH(500))); + env.close(); + // CLOB and AMM, AMM is not selected + if (i == 2) + { + BEAST_EXPECT(amm->expectBalances( + USD(1'000), ETH(1'000), amm->tokens())); + } + BEAST_EXPECT(expectLine(env, bob, USD(2'100))); + q[i] = Quality(Amounts{ + ETH(2'000) - env.balance(carol, ETH), + env.balance(bob, USD) - USD(2'000)}); + } + // CLOB is better quality than AMM + BEAST_EXPECT(q[0] > q[1]); + // AMM is not selected with CLOB + BEAST_EXPECT(q[0] == q[2]); + } + // Offer crossing: AMM has the same spot price quality + // as CLOB's offer and can't generate a better quality offer. + // The transfer fee in this case doesn't change the CLOB quality + // because the quality adjustment is ignored for the offer crossing. + for (auto i = 0; i < 3; ++i) + { + Env env(*this); + prep(env, rates.first, rates.second); + std::optional amm; + if (i == 0 || i == 2) + { + env(offer(ed, ETH(400), USD(400)), txflags(tfPassive)); + env.close(); + } + if (i > 0) + amm.emplace(env, ed, USD(1'000), ETH(1'000)); + env(offer(alice, USD(400), ETH(400))); + env.close(); + // AMM is not selected + if (i > 0) + { + BEAST_EXPECT(amm->expectBalances( + USD(1'000), ETH(1'000), amm->tokens())); + } + if (i == 0 || i == 2) + { + // Fully crosses + BEAST_EXPECT(expectOffers(env, alice, 0)); + } + // Fails to cross because AMM is not selected + else + { + BEAST_EXPECT(expectOffers( + env, alice, 1, {Amounts{USD(400), ETH(400)}})); + } + BEAST_EXPECT(expectOffers(env, ed, 0)); + } + + // Show that the CLOB quality reduction + // results in AMM offer selection. + + // Same as the payment but reduced offer quality + { + std::array q; + for (auto i = 0; i < 3; ++i) + { + Env env(*this); + prep(env, rates.first, rates.second); + std::optional amm; + if (i == 0 || i == 2) + { + env(offer(ed, ETH(400), USD(300)), txflags(tfPassive)); + env.close(); + } + if (i > 0) + amm.emplace(env, ed, USD(1'000), ETH(1'000)); + env(pay(carol, bob, USD(100)), + path(~USD), + sendmax(ETH(500))); + env.close(); + // AMM and CLOB are selected + if (i > 0) + { + BEAST_EXPECT(!amm->expectBalances( + USD(1'000), ETH(1'000), amm->tokens())); + } + if (i == 2) + { + if (rates.first == 1.5) + { + BEAST_EXPECT(expectOffers( + env, + ed, + 1, + {{Amounts{ + STAmount{ + ETH, UINT64_C(378'6327949540823), -13}, + STAmount{ + USD, + UINT64_C(283'9745962155617), + -13}}}})); + } + else + { + BEAST_EXPECT(expectOffers( + env, + ed, + 1, + {{Amounts{ + STAmount{ + ETH, UINT64_C(325'299461620749), -12}, + STAmount{ + USD, + UINT64_C(243'9745962155617), + -13}}}})); + } + } + BEAST_EXPECT(expectLine(env, bob, USD(2'100))); + q[i] = Quality(Amounts{ + ETH(2'000) - env.balance(carol, ETH), + env.balance(bob, USD) - USD(2'000)}); + } + // AMM is better quality + BEAST_EXPECT(q[1] > q[0]); + // AMM and CLOB produce better quality + BEAST_EXPECT(q[2] > q[1]); + } + + // Same as the offer-crossing but reduced offer quality + for (auto i = 0; i < 3; ++i) + { + Env env(*this); + prep(env, rates.first, rates.second); + std::optional amm; + if (i == 0 || i == 2) + { + env(offer(ed, ETH(400), USD(250)), txflags(tfPassive)); + env.close(); + } + if (i > 0) + amm.emplace(env, ed, USD(1'000), ETH(1'000)); + env(offer(alice, USD(250), ETH(400))); + env.close(); + // AMM is selected in both cases + if (i > 0) + { + BEAST_EXPECT(!amm->expectBalances( + USD(1'000), ETH(1'000), amm->tokens())); + } + // Partially crosses, AMM is selected, CLOB fails limitQuality + if (i == 2) + { + if (rates.first == 1.5) + { + BEAST_EXPECT(expectOffers( + env, ed, 1, {{Amounts{ETH(400), USD(250)}}})); + BEAST_EXPECT(expectOffers( + env, + alice, + 1, + {{Amounts{ + STAmount{USD, UINT64_C(40'5694150420947), -13}, + STAmount{ETH, UINT64_C(64'91106406735152), -14}, + }}})); + } + else + { + // Ed offer is partially crossed. + BEAST_EXPECT(expectOffers( + env, + ed, + 1, + {{Amounts{ + STAmount{ETH, UINT64_C(335'0889359326485), -13}, + STAmount{USD, UINT64_C(209'4305849579053), -13}, + }}})); + BEAST_EXPECT(expectOffers(env, alice, 0)); + } + } + } + + // Strand selection + + // Two book steps strand quality is 1. + // AMM strand's best quality is equal to AMM's spot price + // quality, which is 1. Both strands (steps) are adjusted + // for the transfer fee in qualityUpperBound. In case + // of two strands, AMM offers have better quality and are consumed + // first, remaining liquidity is generated by CLOB offers. + // Liquidity from two strands is better in this case than in case + // of one strand with two book steps. Liquidity from one strand + // with AMM has better quality than either one strand with two book + // steps or two strands. It may appear unintuitive, but one strand + // with AMM is optimized and generates one AMM offer, while in case + // of two strands, multiple AMM offers are generated, which results + // in slightly worse overall quality. + { + std::array q; + for (auto i = 0; i < 3; ++i) + { + Env env(*this); + prep(env, rates.first, rates.second); + std::optional amm; + + if (i == 0 || i == 2) + { + env(offer(ed, ETH(400), CAN(400)), txflags(tfPassive)); + env(offer(ed, CAN(400), USD(400))), txflags(tfPassive); + env.close(); + } + + if (i > 0) + amm.emplace(env, ed, ETH(1'000), USD(1'000)); + + env(pay(carol, bob, USD(100)), + path(~USD), + path(~CAN, ~USD), + sendmax(ETH(600))); + env.close(); + + BEAST_EXPECT(expectLine(env, bob, USD(2'100))); + + if (i == 2) + { + if (rates.first == 1.5) + { + // Liquidity is consumed from AMM strand only + BEAST_EXPECT(amm->expectBalances( + STAmount{ETH, UINT64_C(1'176'66038955758), -11}, + USD(850), + amm->tokens())); + } + else + { + BEAST_EXPECT(amm->expectBalances( + STAmount{ + ETH, UINT64_C(1'179'540094339627), -12}, + STAmount{USD, UINT64_C(847'7880529867501), -13}, + amm->tokens())); + BEAST_EXPECT(expectOffers( + env, + ed, + 2, + {{Amounts{ + STAmount{ + ETH, + UINT64_C(343'3179205198749), + -13}, + STAmount{ + CAN, + UINT64_C(343'3179205198749), + -13}, + }, + Amounts{ + STAmount{ + CAN, + UINT64_C(362'2119470132499), + -13}, + STAmount{ + USD, + UINT64_C(362'2119470132499), + -13}, + }}})); + } + } + q[i] = Quality(Amounts{ + ETH(2'000) - env.balance(carol, ETH), + env.balance(bob, USD) - USD(2'000)}); + } + BEAST_EXPECT(q[1] > q[0]); + BEAST_EXPECT(q[2] > q[0] && q[2] < q[1]); + } + } + } + void testCore() { @@ -4154,6 +4811,10 @@ struct AMM_test : public jtx::AMMTest testAMMAndCLOB(); testTradingFee(); testAdjustedTokens(); + testAutoDelete(); + testClawback(); + testAMMID(); + testSelection(); } void @@ -4166,4 +4827,4 @@ struct AMM_test : public jtx::AMMTest BEAST_DEFINE_TESTSUITE_PRIO(AMM, app, ripple, 1); } // namespace test -} // namespace ripple \ No newline at end of file +} // namespace ripple diff --git a/src/test/app/CrossingLimits_test.cpp b/src/test/app/CrossingLimits_test.cpp index d2ae9799ce8..09cca1e82af 100644 --- a/src/test/app/CrossingLimits_test.cpp +++ b/src/test/app/CrossingLimits_test.cpp @@ -18,7 +18,6 @@ #include #include #include -#include namespace ripple { namespace test { diff --git a/src/test/app/Escrow_test.cpp b/src/test/app/Escrow_test.cpp index 255325ca8ac..083764fd371 100644 --- a/src/test/app/Escrow_test.cpp +++ b/src/test/app/Escrow_test.cpp @@ -24,7 +24,6 @@ #include #include #include -#include #include #include diff --git a/src/test/app/Flow_test.cpp b/src/test/app/Flow_test.cpp index b9d2b715aa5..131cad6f042 100644 --- a/src/test/app/Flow_test.cpp +++ b/src/test/app/Flow_test.cpp @@ -28,7 +28,6 @@ #include #include #include -#include namespace ripple { namespace test { diff --git a/src/test/app/Freeze_test.cpp b/src/test/app/Freeze_test.cpp index fc51a6869a6..cb4653c086d 100644 --- a/src/test/app/Freeze_test.cpp +++ b/src/test/app/Freeze_test.cpp @@ -22,7 +22,6 @@ #include #include #include -#include namespace ripple { diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp index e54f9706b36..1c8e544af30 100644 --- a/src/test/app/Offer_test.cpp +++ b/src/test/app/Offer_test.cpp @@ -22,7 +22,6 @@ #include #include #include -#include #include namespace ripple { diff --git a/src/test/app/Path_test.cpp b/src/test/app/Path_test.cpp index ea2dc51132a..0f9916cbcff 100644 --- a/src/test/app/Path_test.cpp +++ b/src/test/app/Path_test.cpp @@ -35,7 +35,6 @@ #include #include #include -#include #include #include diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index dd82a622cc9..f8162dd107d 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -25,7 +25,6 @@ #include #include #include -#include #include diff --git a/src/test/app/PayStrand_test.cpp b/src/test/app/PayStrand_test.cpp index 4eb6c9fecb6..a70ab099745 100644 --- a/src/test/app/PayStrand_test.cpp +++ b/src/test/app/PayStrand_test.cpp @@ -29,7 +29,6 @@ #include "ripple/app/paths/AMMContext.h" #include #include -#include namespace ripple { namespace test { diff --git a/src/test/app/TrustAndBalance_test.cpp b/src/test/app/TrustAndBalance_test.cpp index f83eeac27ec..5b0c1d6480e 100644 --- a/src/test/app/TrustAndBalance_test.cpp +++ b/src/test/app/TrustAndBalance_test.cpp @@ -22,7 +22,6 @@ #include #include #include -#include #include namespace ripple { diff --git a/src/test/jtx.h b/src/test/jtx.h index bcf51398d5c..dd987472cc8 100644 --- a/src/test/jtx.h +++ b/src/test/jtx.h @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include diff --git a/src/test/jtx/AMM.h b/src/test/jtx/AMM.h index dd7e91a3ef8..3cf06bfe401 100644 --- a/src/test/jtx/AMM.h +++ b/src/test/jtx/AMM.h @@ -65,8 +65,10 @@ class AMM Account const creatorAccount_; STAmount const asset1_; STAmount const asset2_; + uint256 const ammID_; IOUAmount const initialLPTokens_; bool log_; + bool doClose_; // Predict next purchase price IOUAmount lastPurchasePrice_; std::optional bidMin_; @@ -180,6 +182,7 @@ class AMM std::optional const& flags, std::optional> const& assets, std::optional const& seq, + std::optional const& tfee = std::nullopt, std::optional const& ter = std::nullopt); IOUAmount @@ -286,6 +289,23 @@ class AMM return ammRpcInfo(lp); } + void + ammDelete( + AccountID const& deleter, + std::optional const& ter = std::nullopt); + + void + setClose(bool close) + { + doClose_ = close; + } + + uint256 + ammID() const + { + return ammID_; + } + private: void setTokens( diff --git a/src/test/jtx/TestHelpers.h b/src/test/jtx/TestHelpers.h index a29a894452b..ff681ffa50b 100644 --- a/src/test/jtx/TestHelpers.h +++ b/src/test/jtx/TestHelpers.h @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -405,6 +406,38 @@ STPathElement allpe(AccountID const& a, Issue const& iss); /***************************************************************/ +/* Check */ +/***************************************************************/ +namespace check { + +/** Create a check. */ +// clang-format off +template + requires std::is_same_v +Json::Value +create(A const& account, A const& dest, STAmount const& sendMax) +{ + Json::Value jv; + jv[sfAccount.jsonName] = to_string(account); + jv[sfSendMax.jsonName] = sendMax.getJson(JsonOptions::none); + jv[sfDestination.jsonName] = to_string(dest); + jv[sfTransactionType.jsonName] = jss::CheckCreate; + jv[sfFlags.jsonName] = tfUniversal; + return jv; +} +// clang-format on + +inline Json::Value +create( + jtx::Account const& account, + jtx::Account const& dest, + STAmount const& sendMax) +{ + return create(account.id(), dest.id(), sendMax); +} + +} // namespace check + } // namespace jtx } // namespace test } // namespace ripple diff --git a/src/test/jtx/check.h b/src/test/jtx/check.h index 325e5897258..6bad6b9db5e 100644 --- a/src/test/jtx/check.h +++ b/src/test/jtx/check.h @@ -31,13 +31,6 @@ namespace jtx { /** Check operations. */ namespace check { -/** Create a check. */ -Json::Value -create( - jtx::Account const& account, - jtx::Account const& dest, - STAmount const& sendMax); - /** Cash a check requiring that a specific amount be delivered. */ Json::Value cash(jtx::Account const& dest, uint256 const& checkId, STAmount const& amount); diff --git a/src/test/jtx/impl/AMM.cpp b/src/test/jtx/impl/AMM.cpp index 37efdc112a0..c09d496f439 100644 --- a/src/test/jtx/impl/AMM.cpp +++ b/src/test/jtx/impl/AMM.cpp @@ -62,8 +62,10 @@ AMM::AMM( , creatorAccount_(account) , asset1_(asset1) , asset2_(asset2) + , ammID_(keylet::amm(asset1_.issue(), asset2_.issue()).key) , initialLPTokens_(initialTokens(asset1, asset2)) , log_(log) + , doClose_(true) , lastPurchasePrice_(0) , bidMin_() , bidMax_() @@ -382,6 +384,7 @@ AMM::deposit( flags, std::nullopt, std::nullopt, + std::nullopt, ter); } @@ -404,6 +407,7 @@ AMM::deposit( flags, std::nullopt, std::nullopt, + std::nullopt, ter); } @@ -417,6 +421,7 @@ AMM::deposit( std::optional const& flags, std::optional> const& assets, std::optional const& seq, + std::optional const& tfee, std::optional const& ter) { Json::Value jv; @@ -428,6 +433,8 @@ AMM::deposit( asset2In->setJson(jv[jss::Amount2]); if (maxEP) maxEP->setJson(jv[jss::EPrice]); + if (tfee) + jv[jss::TradingFee] = *tfee; std::uint32_t jvflags = 0; if (flags) jvflags = *flags; @@ -671,7 +678,8 @@ AMM::submit( env_(jv, *ter); else env_(jv); - env_.close(); + if (doClose_) + env_.close(); } bool @@ -697,6 +705,18 @@ AMM::expectAuctionSlot(auto&& cb) const return false; } +void +AMM::ammDelete(AccountID const& deleter, std::optional const& ter) +{ + Json::Value jv; + jv[jss::Account] = to_string(deleter); + setTokens(jv); + jv[jss::TransactionType] = jss::AMMDelete; + if (fee_ != 0) + jv[jss::Fee] = std::to_string(fee_); + submit(jv, std::nullopt, ter); +} + namespace amm { Json::Value trust(AccountID const& account, STAmount const& amount, std::uint32_t flags) diff --git a/src/test/jtx/impl/check.cpp b/src/test/jtx/impl/check.cpp index 8abcaea1b34..d862130bc70 100644 --- a/src/test/jtx/impl/check.cpp +++ b/src/test/jtx/impl/check.cpp @@ -27,22 +27,6 @@ namespace jtx { namespace check { -// Create a check. -Json::Value -create( - jtx::Account const& account, - jtx::Account const& dest, - STAmount const& sendMax) -{ - Json::Value jv; - jv[sfAccount.jsonName] = account.human(); - jv[sfSendMax.jsonName] = sendMax.getJson(JsonOptions::none); - jv[sfDestination.jsonName] = dest.human(); - jv[sfTransactionType.jsonName] = jss::CheckCreate; - jv[sfFlags.jsonName] = tfUniversal; - return jv; -} - // Cash a check requiring that a specific amount be delivered. Json::Value cash(jtx::Account const& dest, uint256 const& checkId, STAmount const& amount) diff --git a/src/test/rpc/AccountOffers_test.cpp b/src/test/rpc/AccountOffers_test.cpp index 4979ac07216..d94442ea8e5 100644 --- a/src/test/rpc/AccountOffers_test.cpp +++ b/src/test/rpc/AccountOffers_test.cpp @@ -19,7 +19,6 @@ #include #include -#include namespace ripple { namespace test { diff --git a/src/test/rpc/LedgerData_test.cpp b/src/test/rpc/LedgerData_test.cpp index ebc1b9fb553..f0811ba34c4 100644 --- a/src/test/rpc/LedgerData_test.cpp +++ b/src/test/rpc/LedgerData_test.cpp @@ -20,7 +20,6 @@ #include #include #include -#include namespace ripple { diff --git a/src/test/rpc/NoRipple_test.cpp b/src/test/rpc/NoRipple_test.cpp index 268cd1f14a3..472706f0b73 100644 --- a/src/test/rpc/NoRipple_test.cpp +++ b/src/test/rpc/NoRipple_test.cpp @@ -20,7 +20,6 @@ #include #include #include -#include namespace ripple { From 230ebca6d750e3c5c95a6476298cdb2aaa791dbd Mon Sep 17 00:00:00 2001 From: "Alphonse N. Mousse" <39067955+a-noni-mousse@users.noreply.github.com> Date: Sun, 2 Jul 2023 15:52:04 +0000 Subject: [PATCH 03/13] refactor: use C++20 function std::popcount (#4389) - Replace custom popcnt16 implementation with std::popcount from C++20 - Maintain compatibility with older compilers and MacOS by providing a conditional compilation fallback to __builtin_popcount and a lookup table method - Move and inline related functions within SHAMapInnerNode for performance and readability Signed-off-by: Manoj Doshi --- src/ripple/shamap/SHAMapInnerNode.h | 13 +++++++++++ src/ripple/shamap/impl/SHAMapInnerNode.cpp | 16 ++----------- src/ripple/shamap/impl/TaggedPointer.h | 27 ++++++++++++++++++++++ src/ripple/shamap/impl/TaggedPointer.ipp | 26 ++------------------- 4 files changed, 44 insertions(+), 38 deletions(-) diff --git a/src/ripple/shamap/SHAMapInnerNode.h b/src/ripple/shamap/SHAMapInnerNode.h index c85cdcbbc85..44ac05799f2 100644 --- a/src/ripple/shamap/SHAMapInnerNode.h +++ b/src/ripple/shamap/SHAMapInnerNode.h @@ -28,6 +28,7 @@ #include #include +#include #include #include #include @@ -194,12 +195,24 @@ class SHAMapInnerNode final : public SHAMapTreeNode, makeCompressedInner(Slice data); }; +inline bool +SHAMapInnerNode::isEmpty() const +{ + return isBranch_ == 0; +} + inline bool SHAMapInnerNode::isEmptyBranch(int m) const { return (isBranch_ & (1 << m)) == 0; } +inline int +SHAMapInnerNode::getBranchCount() const +{ + return popcnt16(isBranch_); +} + inline bool SHAMapInnerNode::isFullBelow(std::uint32_t generation) const { diff --git a/src/ripple/shamap/impl/SHAMapInnerNode.cpp b/src/ripple/shamap/impl/SHAMapInnerNode.cpp index 1cac616b00c..c9884955914 100644 --- a/src/ripple/shamap/impl/SHAMapInnerNode.cpp +++ b/src/ripple/shamap/impl/SHAMapInnerNode.cpp @@ -256,18 +256,6 @@ SHAMapInnerNode::serializeWithPrefix(Serializer& s) const [&](SHAMapHash const& hh) { s.addBitString(hh.as_uint256()); }); } -bool -SHAMapInnerNode::isEmpty() const -{ - return isBranch_ == 0; -} - -int -SHAMapInnerNode::getBranchCount() const -{ - return popcnt16(isBranch_); -} - std::string SHAMapInnerNode::getString(const SHAMapNodeID& id) const { @@ -292,9 +280,9 @@ SHAMapInnerNode::setChild(int m, std::shared_ptr child) auto const dstIsBranch = [&] { if (child) - return isBranch_ | (1 << m); + return isBranch_ | (1u << m); else - return isBranch_ & ~(1 << m); + return isBranch_ & ~(1u << m); }(); auto const dstToAllocate = popcnt16(dstIsBranch); diff --git a/src/ripple/shamap/impl/TaggedPointer.h b/src/ripple/shamap/impl/TaggedPointer.h index 2371d8ddb23..afc0ef582ae 100644 --- a/src/ripple/shamap/impl/TaggedPointer.h +++ b/src/ripple/shamap/impl/TaggedPointer.h @@ -22,6 +22,8 @@ #include +#include +#include #include #include @@ -217,6 +219,31 @@ class TaggedPointer getChildIndex(std::uint16_t isBranch, int i) const; }; +[[nodiscard]] inline int +popcnt16(std::uint16_t a) +{ +#if __cpp_lib_bitops + return std::popcount(a); +#elif defined(__clang__) || defined(__GNUC__) + return __builtin_popcount(a); +#else + // fallback to table lookup + static auto constexpr const tbl = []() { + std::array ret{}; + for (int i = 0; i != 256; ++i) + { + for (int j = 0; j != 8; ++j) + { + if (i & (1 << j)) + ret[i]++; + } + } + return ret; + }(); + return tbl[a & 0xff] + tbl[a >> 8]; +#endif +} + } // namespace ripple #endif diff --git a/src/ripple/shamap/impl/TaggedPointer.ipp b/src/ripple/shamap/impl/TaggedPointer.ipp index 30bf68426b6..7cdff6b4944 100644 --- a/src/ripple/shamap/impl/TaggedPointer.ipp +++ b/src/ripple/shamap/impl/TaggedPointer.ipp @@ -22,6 +22,7 @@ #include #include +#include #include @@ -159,29 +160,6 @@ deallocateArrays(std::uint8_t boundaryIndex, void* p) freeArrayFuns[boundaryIndex](p); } -[[nodiscard]] inline int -popcnt16(std::uint16_t a) -{ -#if defined(__clang__) || defined(__GNUC__) - return __builtin_popcount(a); -#else - // fallback to table lookup - static auto constexpr const tbl = []() { - std::array ret{}; - for (int i = 0; i != 256; ++i) - { - for (int j = 0; j != 8; ++j) - { - if (i & (1 << j)) - ret[i]++; - } - } - return ret; - }(); - return tbl[a & 0xff] + tbl[a >> 8]; -#endif -} - // Used in `iterChildren` and elsewhere as the hash value for sparse arrays when // the hash isn't actually stored in the array. static SHAMapHash const zeroSHAMapHash; @@ -285,7 +263,7 @@ TaggedPointer::getChildIndex(std::uint16_t isBranch, int i) const // mask sets all the bits >=i to zero and all the bits Date: Sun, 7 May 2023 20:33:29 +0000 Subject: [PATCH 04/13] refactor: improve checking of path lengths (#4519) Improve the checking of the path lengths during Payments. Previously, the code that did the check of the payment path lengths was sometimes executed, but without any effect. This changes it to only check when it matters, and to not make unnecessary copies of the path vectors. Signed-off-by: Manoj Doshi --- src/ripple/app/tx/impl/Payment.cpp | 31 +++++++++--------------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/src/ripple/app/tx/impl/Payment.cpp b/src/ripple/app/tx/impl/Payment.cpp index d3c26a37023..3903aa75045 100644 --- a/src/ripple/app/tx/impl/Payment.cpp +++ b/src/ripple/app/tx/impl/Payment.cpp @@ -270,27 +270,17 @@ Payment::preclaim(PreclaimContext const& ctx) return tecDST_TAG_NEEDED; } - if (paths || sendMax || !saDstAmount.native()) + // Payment with at least one intermediate step and uses transitive balances. + if ((paths || sendMax || !saDstAmount.native()) && ctx.view.open()) { - // Ripple payment with at least one intermediate step and uses - // transitive balances. - - // Copy paths into an editable class. - STPathSet const spsPaths = ctx.tx.getFieldPathSet(sfPaths); + STPathSet const& paths = ctx.tx.getFieldPathSet(sfPaths); - auto pathTooBig = spsPaths.size() > MaxPathSize; - - if (!pathTooBig) - for (auto const& path : spsPaths) - if (path.size() > MaxPathLength) - { - pathTooBig = true; - break; - } - - if (ctx.view.open() && pathTooBig) + if (paths.size() > MaxPathSize || + std::any_of(paths.begin(), paths.end(), [](STPath const& path) { + return path.size() > MaxPathLength; + })) { - return telBAD_PATH_COUNT; // Too many paths for proposed ledger. + return telBAD_PATH_COUNT; } } @@ -384,9 +374,6 @@ Payment::doApply() } } - // Copy paths into an editable class. - STPathSet spsPaths = ctx_.tx.getFieldPathSet(sfPaths); - path::RippleCalc::Input rcInput; rcInput.partialPaymentAllowed = partialPaymentAllowed; rcInput.defaultPathsAllowed = defaultPathsAllowed; @@ -404,7 +391,7 @@ Payment::doApply() saDstAmount, uDstAccountID, account_, - spsPaths, + ctx_.tx.getFieldPathSet(sfPaths), ctx_.app.logs(), &rcInput); // VFALCO NOTE We might not need to apply, depending From ede5fbb0f5b1204c1d02011226fbf1dbafaa9bcb Mon Sep 17 00:00:00 2001 From: John Freeman Date: Tue, 18 Jul 2023 12:09:51 -0500 Subject: [PATCH 05/13] Fix the package recipe for consumers of libxrpl (#4631) - "Rename" the type `LedgerInfo` to `LedgerHeader` (but leave an alias for `LedgerInfo` to not yet disturb existing uses). Put it in its own public header, named after itself, so that it is more easily found. - Move the type `Fees` and NFT serialization functions into public (installed) headers. - Compile the XRPL and gRPC protocol buffers directly into `libxrpl` and install their headers. Fix the Conan recipe to correctly export these types. Addresses change (2) in https://github.com/XRPLF/XRPL-Standards/discussions/121. For context: This work supports Clio's dependence on libxrpl. Clio is just an example consumer. These changes should benefit all current and future consumers. --------- Co-authored-by: cyan317 <120398799+cindyyan317@users.noreply.github.com> Signed-off-by: Manoj Doshi --- Builds/CMake/RippledCore.cmake | 24 +++- Builds/CMake/RippledMultiConfig.cmake | 2 +- Builds/CMake/deps/Protobuf.cmake | 23 ++-- Builds/CMake/deps/gRPC.cmake | 38 ++++-- CMakeLists.txt | 6 +- conanfile.py | 5 +- src/ripple/app/ledger/InboundLedger.h | 8 -- src/ripple/app/ledger/LedgerToJson.h | 17 +-- src/ripple/app/ledger/impl/InboundLedger.cpp | 35 +---- .../ledger/impl/LedgerReplayMsgHandler.cpp | 1 + src/ripple/app/reporting/ETLSource.h | 2 +- src/ripple/app/reporting/P2pProxy.h | 3 +- src/ripple/app/tx/impl/details/NFTokenUtils.h | 90 +------------ src/ripple/ledger/ReadView.h | 87 +----------- src/ripple/ledger/impl/View.cpp | 17 --- .../nodestore/impl/DatabaseShardImp.cpp | 1 + src/ripple/nodestore/impl/Shard.cpp | 1 + src/ripple/overlay/Slot.h | 2 +- src/ripple/overlay/impl/ProtocolMessage.h | 1 - src/ripple/protocol/Fees.h | 57 ++++++++ src/ripple/protocol/LedgerHeader.h | 103 ++++++++++++++ .../NFTSyntheticSerializer.h | 23 +--- src/ripple/{rpc => protocol}/NFTokenID.h | 22 ++- src/ripple/{rpc => protocol}/NFTokenOfferID.h | 21 +-- src/ripple/protocol/Serializer.h | 1 + src/ripple/protocol/impl/LedgerHeader.cpp | 71 ++++++++++ .../impl/NFTSyntheticSerializer.cpp | 21 +-- .../{rpc => protocol}/impl/NFTokenID.cpp | 16 +-- .../{rpc => protocol}/impl/NFTokenOfferID.cpp | 16 +-- src/ripple/protocol/messages.h | 2 +- src/ripple/protocol/nft.h | 127 ++++++++++++++++++ src/ripple/protocol/serialize.h | 48 +++++++ src/ripple/rpc/GRPCHandlers.h | 2 +- src/ripple/rpc/handlers/AccountTx.cpp | 4 +- src/ripple/rpc/handlers/Tx.cpp | 4 +- src/ripple/rpc/impl/RPCHelpers.h | 2 +- src/test/overlay/compression_test.cpp | 2 +- src/test/overlay/reduce_relay_test.cpp | 4 +- 38 files changed, 528 insertions(+), 381 deletions(-) create mode 100644 src/ripple/protocol/Fees.h create mode 100644 src/ripple/protocol/LedgerHeader.h rename src/ripple/{rpc => protocol}/NFTSyntheticSerializer.h (79%) rename src/ripple/{rpc => protocol}/NFTokenID.h (86%) rename src/ripple/{rpc => protocol}/NFTokenOfferID.h (86%) create mode 100644 src/ripple/protocol/impl/LedgerHeader.cpp rename src/ripple/{rpc => protocol}/impl/NFTSyntheticSerializer.cpp (70%) rename src/ripple/{rpc => protocol}/impl/NFTokenID.cpp (94%) rename src/ripple/{rpc => protocol}/impl/NFTokenOfferID.cpp (83%) create mode 100644 src/ripple/protocol/nft.h create mode 100644 src/ripple/protocol/serialize.h diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index 6739afcbefa..b2f52ff8bf0 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -85,6 +85,7 @@ target_sources (xrpl_core PRIVATE src/ripple/protocol/impl/STIssue.cpp src/ripple/protocol/impl/Keylet.cpp src/ripple/protocol/impl/LedgerFormats.cpp + src/ripple/protocol/impl/LedgerHeader.cpp src/ripple/protocol/impl/PublicKey.cpp src/ripple/protocol/impl/Quality.cpp src/ripple/protocol/impl/QualityFunction.cpp @@ -116,6 +117,9 @@ target_sources (xrpl_core PRIVATE src/ripple/protocol/impl/UintTypes.cpp src/ripple/protocol/impl/digest.cpp src/ripple/protocol/impl/tokens.cpp + src/ripple/protocol/impl/NFTSyntheticSerializer.cpp + src/ripple/protocol/impl/NFTokenID.cpp + src/ripple/protocol/impl/NFTokenOfferID.cpp #[===============================[ main sources: subdir: crypto @@ -232,6 +236,7 @@ install ( src/ripple/protocol/BuildInfo.h src/ripple/protocol/ErrorCodes.h src/ripple/protocol/Feature.h + src/ripple/protocol/Fees.h src/ripple/protocol/HashPrefix.h src/ripple/protocol/Indexes.h src/ripple/protocol/InnerObjectFormats.h @@ -240,6 +245,10 @@ install ( src/ripple/protocol/Keylet.h src/ripple/protocol/KnownFormats.h src/ripple/protocol/LedgerFormats.h + src/ripple/protocol/LedgerHeader.h + src/ripple/protocol/NFTSyntheticSerializer.h + src/ripple/protocol/NFTokenID.h + src/ripple/protocol/NFTokenOfferID.h src/ripple/protocol/Protocol.h src/ripple/protocol/PublicKey.h src/ripple/protocol/Quality.h @@ -277,6 +286,9 @@ install ( src/ripple/protocol/UintTypes.h src/ripple/protocol/digest.h src/ripple/protocol/jss.h + src/ripple/protocol/serialize.h + src/ripple/protocol/nft.h + src/ripple/protocol/nftPageMask.h src/ripple/protocol/tokens.h DESTINATION include/ripple/protocol) install ( @@ -296,6 +308,7 @@ install ( DESTINATION include/ripple/beast/clock) install ( FILES + src/ripple/beast/core/CurrentThreadName.h src/ripple/beast/core/LexicalCast.h src/ripple/beast/core/List.h src/ripple/beast/core/SemanticVersion.h @@ -309,6 +322,14 @@ install ( install ( FILES src/ripple/beast/hash/impl/xxhash.h DESTINATION include/ripple/beast/hash/impl) +install ( + FILES + src/ripple/beast/net/IPAddress.h + src/ripple/beast/net/IPAddressConversion.h + src/ripple/beast/net/IPAddressV4.h + src/ripple/beast/net/IPAddressV6.h + src/ripple/beast/net/IPEndpoint.h + DESTINATION include/ripple/beast/net) install ( FILES src/ripple/beast/rfc2616.h @@ -714,9 +735,6 @@ target_sources (rippled PRIVATE src/ripple/rpc/impl/ShardVerificationScheduler.cpp src/ripple/rpc/impl/Status.cpp src/ripple/rpc/impl/TransactionSign.cpp - src/ripple/rpc/impl/NFTokenID.cpp - src/ripple/rpc/impl/NFTokenOfferID.cpp - src/ripple/rpc/impl/NFTSyntheticSerializer.cpp #[===============================[ main sources: subdir: perflog diff --git a/Builds/CMake/RippledMultiConfig.cmake b/Builds/CMake/RippledMultiConfig.cmake index ae9b182a3fc..6df48a3d964 100644 --- a/Builds/CMake/RippledMultiConfig.cmake +++ b/Builds/CMake/RippledMultiConfig.cmake @@ -14,7 +14,7 @@ if (is_multiconfig) file(GLOB md_files RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} CONFIGURE_DEPENDS *.md) LIST(APPEND all_sources ${md_files}) - foreach (_target secp256k1::secp256k1 ed25519::ed25519 pbufs xrpl_core rippled) + foreach (_target secp256k1::secp256k1 ed25519::ed25519 xrpl_core rippled) get_target_property (_type ${_target} TYPE) if(_type STREQUAL "INTERFACE_LIBRARY") continue() diff --git a/Builds/CMake/deps/Protobuf.cmake b/Builds/CMake/deps/Protobuf.cmake index 0706ae32243..e5d866cb069 100644 --- a/Builds/CMake/deps/Protobuf.cmake +++ b/Builds/CMake/deps/Protobuf.cmake @@ -1,22 +1,27 @@ find_package(Protobuf 3.8) -file(MAKE_DIRECTORY ${CMAKE_BINARY_DIR}/proto_gen) +set(output_dir ${CMAKE_BINARY_DIR}/proto_gen) +file(MAKE_DIRECTORY ${output_dir}) set(ccbd ${CMAKE_CURRENT_BINARY_DIR}) -set(CMAKE_CURRENT_BINARY_DIR ${CMAKE_BINARY_DIR}/proto_gen) +set(CMAKE_CURRENT_BINARY_DIR ${output_dir}) protobuf_generate_cpp(PROTO_SRCS PROTO_HDRS src/ripple/proto/ripple.proto) set(CMAKE_CURRENT_BINARY_DIR ${ccbd}) -add_library(pbufs STATIC ${PROTO_SRCS} ${PROTO_HDRS}) -target_include_directories(pbufs SYSTEM PUBLIC - ${CMAKE_BINARY_DIR}/proto_gen - ${CMAKE_BINARY_DIR}/proto_gen/src/ripple/proto +target_include_directories(xrpl_core SYSTEM PUBLIC + # The generated implementation imports the header relative to the output + # directory. + $ + $ ) -target_link_libraries(pbufs protobuf::libprotobuf) -target_compile_options(pbufs +target_sources(xrpl_core PRIVATE ${output_dir}/src/ripple/proto/ripple.pb.cc) +install( + FILES ${output_dir}/src/ripple/proto/ripple.pb.h + DESTINATION include/ripple/proto) +target_link_libraries(xrpl_core PUBLIC protobuf::libprotobuf) +target_compile_options(xrpl_core PUBLIC $<$: --system-header-prefix="google/protobuf" -Wno-deprecated-dynamic-exception-spec > ) -add_library(Ripple::pbufs ALIAS pbufs) diff --git a/Builds/CMake/deps/gRPC.cmake b/Builds/CMake/deps/gRPC.cmake index 44185b3a248..de3a88b7bf9 100644 --- a/Builds/CMake/deps/gRPC.cmake +++ b/Builds/CMake/deps/gRPC.cmake @@ -5,21 +5,30 @@ find_package(gRPC 1.23) grpc defs and bundle into a static lib #]=================================] -set(GRPC_GEN_DIR "${CMAKE_BINARY_DIR}/proto_gen_grpc") +set(output_dir "${CMAKE_BINARY_DIR}/proto_gen_grpc") +set(GRPC_GEN_DIR "${output_dir}/ripple/proto") file(MAKE_DIRECTORY ${GRPC_GEN_DIR}) set(GRPC_PROTO_SRCS) set(GRPC_PROTO_HDRS) set(GRPC_PROTO_ROOT "${CMAKE_CURRENT_SOURCE_DIR}/src/ripple/proto/org") -file(GLOB_RECURSE GRPC_DEFINITION_FILES LIST_DIRECTORIES false "${GRPC_PROTO_ROOT}/*.proto") +file(GLOB_RECURSE GRPC_DEFINITION_FILES "${GRPC_PROTO_ROOT}/*.proto") foreach(file ${GRPC_DEFINITION_FILES}) + # /home/user/rippled/src/ripple/proto/org/.../v1/get_ledger.proto get_filename_component(_abs_file ${file} ABSOLUTE) + # /home/user/rippled/src/ripple/proto/org/.../v1 get_filename_component(_abs_dir ${_abs_file} DIRECTORY) + # get_ledger get_filename_component(_basename ${file} NAME_WE) + # /home/user/rippled/src/ripple/proto get_filename_component(_proto_inc ${GRPC_PROTO_ROOT} DIRECTORY) # updir one level + # org/.../v1/get_ledger.proto file(RELATIVE_PATH _rel_root_file ${_proto_inc} ${_abs_file}) + # org/.../v1 get_filename_component(_rel_root_dir ${_rel_root_file} DIRECTORY) + # src/ripple/proto/org/.../v1 file(RELATIVE_PATH _rel_dir ${CMAKE_CURRENT_SOURCE_DIR} ${_abs_dir}) + # .cmake/proto_gen_grpc/ripple/proto/org/.../v1/get_ledger.grpc.pb.cc set(src_1 "${GRPC_GEN_DIR}/${_rel_root_dir}/${_basename}.grpc.pb.cc") set(src_2 "${GRPC_GEN_DIR}/${_rel_root_dir}/${_basename}.pb.cc") set(hdr_1 "${GRPC_GEN_DIR}/${_rel_root_dir}/${_basename}.grpc.pb.h") @@ -36,20 +45,32 @@ foreach(file ${GRPC_DEFINITION_FILES}) WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} COMMENT "Running gRPC C++ protocol buffer compiler on ${file}" VERBATIM) - set_source_files_properties(${src_1} ${src_2} ${hdr_1} ${hdr_2} PROPERTIES GENERATED TRUE) + set_source_files_properties(${src_1} ${src_2} ${hdr_1} ${hdr_2} PROPERTIES + GENERATED TRUE + SKIP_UNITY_BUILD_INCLUSION ON + ) list(APPEND GRPC_PROTO_SRCS ${src_1} ${src_2}) list(APPEND GRPC_PROTO_HDRS ${hdr_1} ${hdr_2}) endforeach() -add_library(grpc_pbufs STATIC ${GRPC_PROTO_SRCS} ${GRPC_PROTO_HDRS}) -#target_include_directories(grpc_pbufs PRIVATE src) -target_include_directories(grpc_pbufs SYSTEM PUBLIC ${GRPC_GEN_DIR}) -target_link_libraries(grpc_pbufs +target_include_directories(xrpl_core SYSTEM PUBLIC + $ + $ + # The generated sources include headers relative to this path. Fix it later. + $ +) +target_sources(xrpl_core PRIVATE ${GRPC_PROTO_SRCS}) +install( + DIRECTORY ${output_dir}/ripple + DESTINATION include/ + FILES_MATCHING PATTERN "*.h" +) +target_link_libraries(xrpl_core PUBLIC "gRPC::grpc++" # libgrpc is missing references. absl::random_random ) -target_compile_options(grpc_pbufs +target_compile_options(xrpl_core PRIVATE $<$:-wd4065> $<$>:-Wno-deprecated-declarations> @@ -59,4 +80,3 @@ target_compile_options(grpc_pbufs --system-header-prefix="google/protobuf" -Wno-deprecated-dynamic-exception-spec >) -add_library(Ripple::grpc_pbufs ALIAS grpc_pbufs) diff --git a/CMakeLists.txt b/CMakeLists.txt index 256f8dc7fc5..5723c831ad4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -93,8 +93,6 @@ endif() find_package(nudb REQUIRED) find_package(date REQUIRED) -include(deps/Protobuf) -include(deps/gRPC) target_link_libraries(ripple_libs INTERFACE ed25519::ed25519 @@ -102,8 +100,6 @@ target_link_libraries(ripple_libs INTERFACE lz4::lz4 OpenSSL::Crypto OpenSSL::SSL - Ripple::grpc_pbufs - Ripple::pbufs secp256k1::secp256k1 soci::soci SQLite::SQLite3 @@ -131,6 +127,8 @@ endif() ### include(RippledCore) +include(deps/Protobuf) +include(deps/gRPC) include(RippledInstall) include(RippledCov) include(RippledMultiConfig) diff --git a/conanfile.py b/conanfile.py index 25d30d23c37..c789cacf568 100644 --- a/conanfile.py +++ b/conanfile.py @@ -151,9 +151,12 @@ def package_info(self): 'libed25519.a', 'libsecp256k1.a', ] - libxrpl.includedirs = ['include'] + # TODO: Fix the protobufs to include each other relative to + # `include/`, not `include/ripple/proto/`. + libxrpl.includedirs = ['include', 'include/ripple/proto'] libxrpl.requires = [ 'boost::boost', 'openssl::crypto', 'date::date', + 'grpc::grpc++', ] diff --git a/src/ripple/app/ledger/InboundLedger.h b/src/ripple/app/ledger/InboundLedger.h index 287dbaf7f16..357a6fcad1e 100644 --- a/src/ripple/app/ledger/InboundLedger.h +++ b/src/ripple/app/ledger/InboundLedger.h @@ -197,14 +197,6 @@ class InboundLedger final : public TimeoutCounter, std::unique_ptr mPeerSet; }; -/** Deserialize a ledger header from a byte array. */ -LedgerInfo -deserializeHeader(Slice data, bool hasHash = false); - -/** Deserialize a ledger header (prefixed with 4 bytes) from a byte array. */ -LedgerInfo -deserializePrefixedHeader(Slice data, bool hasHash = false); - } // namespace ripple #endif diff --git a/src/ripple/app/ledger/LedgerToJson.h b/src/ripple/app/ledger/LedgerToJson.h index 7f3777bc805..f658583885f 100644 --- a/src/ripple/app/ledger/LedgerToJson.h +++ b/src/ripple/app/ledger/LedgerToJson.h @@ -26,6 +26,7 @@ #include #include #include +#include #include namespace ripple { @@ -70,22 +71,6 @@ addJson(Json::Value&, LedgerFill const&); Json::Value getJson(LedgerFill const&); -/** Serialize an object to a blob. */ -template -Blob -serializeBlob(Object const& o) -{ - Serializer s; - o.add(s); - return s.peekData(); -} - -/** Serialize an object to a hex string. */ -inline std::string -serializeHex(STObject const& o) -{ - return strHex(serializeBlob(o)); -} } // namespace ripple #endif diff --git a/src/ripple/app/ledger/impl/InboundLedger.cpp b/src/ripple/app/ledger/impl/InboundLedger.cpp index af3ba8a7a9b..53475988cbf 100644 --- a/src/ripple/app/ledger/impl/InboundLedger.cpp +++ b/src/ripple/app/ledger/impl/InboundLedger.cpp @@ -271,36 +271,6 @@ InboundLedger::neededStateHashes(int max, SHAMapSyncFilter* filter) const mLedger->info().accountHash, mLedger->stateMap(), max, filter); } -LedgerInfo -deserializeHeader(Slice data, bool hasHash) -{ - SerialIter sit(data.data(), data.size()); - - LedgerInfo info; - - info.seq = sit.get32(); - info.drops = sit.get64(); - info.parentHash = sit.get256(); - info.txHash = sit.get256(); - info.accountHash = sit.get256(); - info.parentCloseTime = - NetClock::time_point{NetClock::duration{sit.get32()}}; - info.closeTime = NetClock::time_point{NetClock::duration{sit.get32()}}; - info.closeTimeResolution = NetClock::duration{sit.get8()}; - info.closeFlags = sit.get8(); - - if (hasHash) - info.hash = sit.get256(); - - return info; -} - -LedgerInfo -deserializePrefixedHeader(Slice data, bool hasHash) -{ - return deserializeHeader(data + 4, hasHash); -} - // See how much of the ledger data is stored locally // Data found in a fetch pack will be stored void @@ -568,10 +538,9 @@ InboundLedger::trigger(std::shared_ptr const& peer, TriggerReason reason) if (auto stream = journal_.trace()) { + stream << "Trigger acquiring ledger " << hash_; if (peer) - stream << "Trigger acquiring ledger " << hash_ << " from " << peer; - else - stream << "Trigger acquiring ledger " << hash_; + stream << " from " << peer; if (complete_ || failed_) stream << "complete=" << complete_ << " failed=" << failed_; diff --git a/src/ripple/app/ledger/impl/LedgerReplayMsgHandler.cpp b/src/ripple/app/ledger/impl/LedgerReplayMsgHandler.cpp index 57c2fd08344..c5301be7ea2 100644 --- a/src/ripple/app/ledger/impl/LedgerReplayMsgHandler.cpp +++ b/src/ripple/app/ledger/impl/LedgerReplayMsgHandler.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include diff --git a/src/ripple/app/reporting/ETLSource.h b/src/ripple/app/reporting/ETLSource.h index 66cb61bcc7a..b9f27a1bf1f 100644 --- a/src/ripple/app/reporting/ETLSource.h +++ b/src/ripple/app/reporting/ETLSource.h @@ -21,6 +21,7 @@ #define RIPPLE_APP_REPORTING_ETLSOURCE_H_INCLUDED #include #include +#include #include #include @@ -29,7 +30,6 @@ #include #include -#include "org/xrpl/rpc/v1/xrp_ledger.grpc.pb.h" #include namespace ripple { diff --git a/src/ripple/app/reporting/P2pProxy.h b/src/ripple/app/reporting/P2pProxy.h index 92cc508a1a0..55ebc726726 100644 --- a/src/ripple/app/reporting/P2pProxy.h +++ b/src/ripple/app/reporting/P2pProxy.h @@ -21,12 +21,11 @@ #define RIPPLE_APP_REPORTING_P2PPROXY_H_INCLUDED #include +#include #include #include #include - -#include "org/xrpl/rpc/v1/xrp_ledger.grpc.pb.h" #include namespace ripple { diff --git a/src/ripple/app/tx/impl/details/NFTokenUtils.h b/src/ripple/app/tx/impl/details/NFTokenUtils.h index db7cf00be10..5242bf38ff3 100644 --- a/src/ripple/app/tx/impl/details/NFTokenUtils.h +++ b/src/ripple/app/tx/impl/details/NFTokenUtils.h @@ -25,34 +25,12 @@ #include #include #include +#include namespace ripple { namespace nft { -// Separate taxons from regular integers. -struct TaxonTag -{ -}; -using Taxon = tagged_integer; - -inline Taxon -toTaxon(std::uint32_t i) -{ - return static_cast(i); -} - -inline std::uint32_t -toUInt32(Taxon t) -{ - return static_cast(t); -} - -constexpr std::uint16_t const flagBurnable = 0x0001; -constexpr std::uint16_t const flagOnlyXRP = 0x0002; -constexpr std::uint16_t const flagCreateTrustLines = 0x0004; -constexpr std::uint16_t const flagTransferable = 0x0008; - /** Delete up to a specified number of offers from the specified token offer * directory. */ std::size_t @@ -116,72 +94,6 @@ removeToken( bool deleteTokenOffer(ApplyView& view, std::shared_ptr const& offer); -inline std::uint16_t -getFlags(uint256 const& id) -{ - std::uint16_t flags; - memcpy(&flags, id.begin(), 2); - return boost::endian::big_to_native(flags); -} - -inline std::uint16_t -getTransferFee(uint256 const& id) -{ - std::uint16_t fee; - memcpy(&fee, id.begin() + 2, 2); - return boost::endian::big_to_native(fee); -} - -inline std::uint32_t -getSerial(uint256 const& id) -{ - std::uint32_t seq; - memcpy(&seq, id.begin() + 28, 4); - return boost::endian::big_to_native(seq); -} - -inline Taxon -cipheredTaxon(std::uint32_t tokenSeq, Taxon taxon) -{ - // An issuer may issue several NFTs with the same taxon; to ensure that NFTs - // are spread across multiple pages we lightly mix the taxon up by using the - // sequence (which is not under the issuer's direct control) as the seed for - // a simple linear congruential generator. - // - // From the Hull-Dobell theorem we know that f(x)=(m*x+c) mod n will yield a - // permutation of [0, n) when n is a power of 2 if m is congruent to 1 mod 4 - // and c is odd. - // - // Here we use m = 384160001 and c = 2459. The modulo is implicit because we - // use 2^32 for n and the arithmetic gives it to us for "free". - // - // Note that the scramble value we calculate is not cryptographically secure - // but that's fine since all we're looking for is some dispersion. - // - // **IMPORTANT** Changing these numbers would be a breaking change requiring - // an amendment along with a way to distinguish token IDs that - // were generated with the old code. - return taxon ^ toTaxon(((384160001 * tokenSeq) + 2459)); -} - -inline Taxon -getTaxon(uint256 const& id) -{ - std::uint32_t taxon; - memcpy(&taxon, id.begin() + 24, 4); - taxon = boost::endian::big_to_native(taxon); - - // The taxon cipher is just an XOR, so it is reversible by applying the - // XOR a second time. - return cipheredTaxon(getSerial(id), toTaxon(taxon)); -} - -inline AccountID -getIssuer(uint256 const& id) -{ - return AccountID::fromVoid(id.data() + 4); -} - bool compareTokens(uint256 const& a, uint256 const& b); diff --git a/src/ripple/ledger/ReadView.h b/src/ripple/ledger/ReadView.h index e019d602f07..2ea3ab492ed 100644 --- a/src/ripple/ledger/ReadView.h +++ b/src/ripple/ledger/ReadView.h @@ -27,7 +27,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -41,79 +43,6 @@ namespace ripple { -/** Reflects the fee settings for a particular ledger. - - The fees are always the same for any transactions applied - to a ledger. Changes to fees occur in between ledgers. -*/ -struct Fees -{ - XRPAmount base{0}; // Reference tx cost (drops) - XRPAmount reserve{0}; // Reserve base (drops) - XRPAmount increment{0}; // Reserve increment (drops) - - explicit Fees() = default; - Fees(Fees const&) = default; - Fees& - operator=(Fees const&) = default; - - /** Returns the account reserve given the owner count, in drops. - - The reserve is calculated as the reserve base plus - the reserve increment times the number of increments. - */ - XRPAmount - accountReserve(std::size_t ownerCount) const - { - return reserve + ownerCount * increment; - } -}; - -//------------------------------------------------------------------------------ - -/** Information about the notional ledger backing the view. */ -struct LedgerInfo -{ - explicit LedgerInfo() = default; - - // - // For all ledgers - // - - LedgerIndex seq = 0; - NetClock::time_point parentCloseTime = {}; - - // - // For closed ledgers - // - - // Closed means "tx set already determined" - uint256 hash = beast::zero; - uint256 txHash = beast::zero; - uint256 accountHash = beast::zero; - uint256 parentHash = beast::zero; - - XRPAmount drops = beast::zero; - - // If validated is false, it means "not yet validated." - // Once validated is true, it will never be set false at a later time. - // VFALCO TODO Make this not mutable - bool mutable validated = false; - bool accepted = false; - - // flags indicating how this ledger close took place - int closeFlags = 0; - - // the resolution for this ledger close time (2-120 seconds) - NetClock::duration closeTimeResolution = {}; - - // For closed ledgers, the time the ledger - // closed. For open ledgers, the time the ledger - // will close if there's no transactions. - // - NetClock::time_point closeTime = {}; -}; - //------------------------------------------------------------------------------ /** A view into a ledger. @@ -344,18 +273,6 @@ class DigestAwareReadView : public ReadView //------------------------------------------------------------------------------ -// ledger close flags -static std::uint32_t const sLCF_NoConsensusTime = 0x01; - -inline bool -getCloseAgree(LedgerInfo const& info) -{ - return (info.closeFlags & sLCF_NoConsensusTime) == 0; -} - -void -addRaw(LedgerInfo const&, Serializer&, bool includeHash = false); - Rules makeRulesGivenLedger(DigestAwareReadView const& ledger, Rules const& current); diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index 8d059aaf4b7..75fd35782b6 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -158,23 +158,6 @@ cdirNext( // //------------------------------------------------------------------------------ -void -addRaw(LedgerInfo const& info, Serializer& s, bool includeHash) -{ - s.add32(info.seq); - s.add64(info.drops.drops()); - s.addBitString(info.parentHash); - s.addBitString(info.txHash); - s.addBitString(info.accountHash); - s.add32(info.parentCloseTime.time_since_epoch().count()); - s.add32(info.closeTime.time_since_epoch().count()); - s.add8(info.closeTimeResolution.count()); - s.add8(info.closeFlags); - - if (includeHash) - s.addBitString(info.hash); -} - bool hasExpired(ReadView const& view, std::optional const& exp) { diff --git a/src/ripple/nodestore/impl/DatabaseShardImp.cpp b/src/ripple/nodestore/impl/DatabaseShardImp.cpp index 1efcbe2ac26..33000b5d24c 100644 --- a/src/ripple/nodestore/impl/DatabaseShardImp.cpp +++ b/src/ripple/nodestore/impl/DatabaseShardImp.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include diff --git a/src/ripple/nodestore/impl/Shard.cpp b/src/ripple/nodestore/impl/Shard.cpp index 8d0eab81153..10adf298361 100644 --- a/src/ripple/nodestore/impl/Shard.cpp +++ b/src/ripple/nodestore/impl/Shard.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include namespace ripple { diff --git a/src/ripple/overlay/Slot.h b/src/ripple/overlay/Slot.h index 1197eff56ef..e58619e66f8 100644 --- a/src/ripple/overlay/Slot.h +++ b/src/ripple/overlay/Slot.h @@ -28,7 +28,7 @@ #include #include #include -#include +#include #include #include diff --git a/src/ripple/overlay/impl/ProtocolMessage.h b/src/ripple/overlay/impl/ProtocolMessage.h index 09861353a10..d6fb14bc78c 100644 --- a/src/ripple/overlay/impl/ProtocolMessage.h +++ b/src/ripple/overlay/impl/ProtocolMessage.h @@ -32,7 +32,6 @@ #include #include #include -#include #include #include diff --git a/src/ripple/protocol/Fees.h b/src/ripple/protocol/Fees.h new file mode 100644 index 00000000000..d155b869d1d --- /dev/null +++ b/src/ripple/protocol/Fees.h @@ -0,0 +1,57 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2023 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_PROTOCOL_FEES_H_INCLUDED +#define RIPPLE_PROTOCOL_FEES_H_INCLUDED + +#include + +namespace ripple { + +/** Reflects the fee settings for a particular ledger. + + The fees are always the same for any transactions applied + to a ledger. Changes to fees occur in between ledgers. +*/ +struct Fees +{ + XRPAmount base{0}; // Reference tx cost (drops) + XRPAmount reserve{0}; // Reserve base (drops) + XRPAmount increment{0}; // Reserve increment (drops) + + explicit Fees() = default; + Fees(Fees const&) = default; + Fees& + operator=(Fees const&) = default; + + /** Returns the account reserve given the owner count, in drops. + + The reserve is calculated as the reserve base plus + the reserve increment times the number of increments. + */ + XRPAmount + accountReserve(std::size_t ownerCount) const + { + return reserve + ownerCount * increment; + } +}; + +} // namespace ripple + +#endif diff --git a/src/ripple/protocol/LedgerHeader.h b/src/ripple/protocol/LedgerHeader.h new file mode 100644 index 00000000000..2adcfcfd209 --- /dev/null +++ b/src/ripple/protocol/LedgerHeader.h @@ -0,0 +1,103 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2023 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_PROTOCOL_LEDGERHEADER_H_INCLUDED +#define RIPPLE_PROTOCOL_LEDGERHEADER_H_INCLUDED + +#include +#include +#include +#include +#include +#include + +namespace ripple { + +/** Information about the notional ledger backing the view. */ +struct LedgerHeader +{ + explicit LedgerHeader() = default; + + // + // For all ledgers + // + + LedgerIndex seq = 0; + NetClock::time_point parentCloseTime = {}; + + // + // For closed ledgers + // + + // Closed means "tx set already determined" + uint256 hash = beast::zero; + uint256 txHash = beast::zero; + uint256 accountHash = beast::zero; + uint256 parentHash = beast::zero; + + XRPAmount drops = beast::zero; + + // If validated is false, it means "not yet validated." + // Once validated is true, it will never be set false at a later time. + // VFALCO TODO Make this not mutable + bool mutable validated = false; + bool accepted = false; + + // flags indicating how this ledger close took place + int closeFlags = 0; + + // the resolution for this ledger close time (2-120 seconds) + NetClock::duration closeTimeResolution = {}; + + // For closed ledgers, the time the ledger + // closed. For open ledgers, the time the ledger + // will close if there's no transactions. + // + NetClock::time_point closeTime = {}; +}; + +// We call them "headers" in conversation +// but "info" in code. Unintuitive. +// This alias lets us give the "correct" name to the class +// without yet disturbing existing uses. +using LedgerInfo = LedgerHeader; + +// ledger close flags +static std::uint32_t const sLCF_NoConsensusTime = 0x01; + +inline bool +getCloseAgree(LedgerHeader const& info) +{ + return (info.closeFlags & sLCF_NoConsensusTime) == 0; +} + +void +addRaw(LedgerHeader const&, Serializer&, bool includeHash = false); + +/** Deserialize a ledger header from a byte array. */ +LedgerHeader +deserializeHeader(Slice data, bool hasHash = false); + +/** Deserialize a ledger header (prefixed with 4 bytes) from a byte array. */ +LedgerHeader +deserializePrefixedHeader(Slice data, bool hasHash = false); + +} // namespace ripple + +#endif diff --git a/src/ripple/rpc/NFTSyntheticSerializer.h b/src/ripple/protocol/NFTSyntheticSerializer.h similarity index 79% rename from src/ripple/rpc/NFTSyntheticSerializer.h rename to src/ripple/protocol/NFTSyntheticSerializer.h index 090e8937869..f9a0cd50a46 100644 --- a/src/ripple/rpc/NFTSyntheticSerializer.h +++ b/src/ripple/protocol/NFTSyntheticSerializer.h @@ -17,28 +17,17 @@ */ //============================================================================== -#ifndef RIPPLE_RPC_NFTSYNTHETICSERIALIZER_H_INCLUDED -#define RIPPLE_RPC_NFTSYNTHETICSERIALIZER_H_INCLUDED +#ifndef RIPPLE_PROTOCOL_NFTSYNTHETICSERIALIZER_H_INCLUDED +#define RIPPLE_PROTOCOL_NFTSYNTHETICSERIALIZER_H_INCLUDED -#include -#include +#include +#include +#include -#include #include -namespace Json { -class Value; -} - namespace ripple { -class TxMeta; -class STTx; - -namespace RPC { - -struct JsonContext; - /** Adds common synthetic fields to transaction-related JSON responses @@ -47,12 +36,10 @@ struct JsonContext; void insertNFTSyntheticInJson( Json::Value&, - RPC::JsonContext const&, std::shared_ptr const&, TxMeta const&); /** @} */ -} // namespace RPC } // namespace ripple #endif diff --git a/src/ripple/rpc/NFTokenID.h b/src/ripple/protocol/NFTokenID.h similarity index 86% rename from src/ripple/rpc/NFTokenID.h rename to src/ripple/protocol/NFTokenID.h index cb218966fda..f29713aba7b 100644 --- a/src/ripple/rpc/NFTokenID.h +++ b/src/ripple/protocol/NFTokenID.h @@ -17,25 +17,20 @@ */ //============================================================================== -#ifndef RIPPLE_RPC_NFTOKENID_H_INCLUDED -#define RIPPLE_RPC_NFTOKENID_H_INCLUDED +#ifndef RIPPLE_PROTOCOL_NFTOKENID_H_INCLUDED +#define RIPPLE_PROTOCOL_NFTOKENID_H_INCLUDED -#include +#include +#include +#include +#include -#include #include - -namespace Json { -class Value; -} +#include +#include namespace ripple { -class TxMeta; -class STTx; - -namespace RPC { - /** Add a `nftoken_ids` field to the `meta` output parameter. The field is only added to successful NFTokenMint, NFTokenAcceptOffer, @@ -62,7 +57,6 @@ insertNFTokenID( TxMeta const& transactionMeta); /** @} */ -} // namespace RPC } // namespace ripple #endif diff --git a/src/ripple/rpc/NFTokenOfferID.h b/src/ripple/protocol/NFTokenOfferID.h similarity index 86% rename from src/ripple/rpc/NFTokenOfferID.h rename to src/ripple/protocol/NFTokenOfferID.h index 6c1bef3d127..777324f4243 100644 --- a/src/ripple/rpc/NFTokenOfferID.h +++ b/src/ripple/protocol/NFTokenOfferID.h @@ -17,25 +17,19 @@ */ //============================================================================== -#ifndef RIPPLE_RPC_NFTOKENOFFERID_H_INCLUDED -#define RIPPLE_RPC_NFTOKENOFFERID_H_INCLUDED +#ifndef RIPPLE_PROTOCOL_NFTOKENOFFERID_H_INCLUDED +#define RIPPLE_PROTOCOL_NFTOKENOFFERID_H_INCLUDED -#include +#include +#include +#include +#include -#include #include - -namespace Json { -class Value; -} +#include namespace ripple { -class TxMeta; -class STTx; - -namespace RPC { - /** Add an `offer_id` field to the `meta` output parameter. The field is only added to successful NFTokenCreateOffer transactions. @@ -58,7 +52,6 @@ insertNFTokenOfferID( TxMeta const& transactionMeta); /** @} */ -} // namespace RPC } // namespace ripple #endif diff --git a/src/ripple/protocol/Serializer.h b/src/ripple/protocol/Serializer.h index 7c3ccf9580b..e3ef00eaf65 100644 --- a/src/ripple/protocol/Serializer.h +++ b/src/ripple/protocol/Serializer.h @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include diff --git a/src/ripple/protocol/impl/LedgerHeader.cpp b/src/ripple/protocol/impl/LedgerHeader.cpp new file mode 100644 index 00000000000..6d13db224fd --- /dev/null +++ b/src/ripple/protocol/impl/LedgerHeader.cpp @@ -0,0 +1,71 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2023 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 { + +void +addRaw(LedgerHeader const& info, Serializer& s, bool includeHash) +{ + s.add32(info.seq); + s.add64(info.drops.drops()); + s.addBitString(info.parentHash); + s.addBitString(info.txHash); + s.addBitString(info.accountHash); + s.add32(info.parentCloseTime.time_since_epoch().count()); + s.add32(info.closeTime.time_since_epoch().count()); + s.add8(info.closeTimeResolution.count()); + s.add8(info.closeFlags); + + if (includeHash) + s.addBitString(info.hash); +} + +LedgerHeader +deserializeHeader(Slice data, bool hasHash) +{ + SerialIter sit(data.data(), data.size()); + + LedgerHeader header; + + header.seq = sit.get32(); + header.drops = sit.get64(); + header.parentHash = sit.get256(); + header.txHash = sit.get256(); + header.accountHash = sit.get256(); + header.parentCloseTime = + NetClock::time_point{NetClock::duration{sit.get32()}}; + header.closeTime = NetClock::time_point{NetClock::duration{sit.get32()}}; + header.closeTimeResolution = NetClock::duration{sit.get8()}; + header.closeFlags = sit.get8(); + + if (hasHash) + header.hash = sit.get256(); + + return header; +} + +LedgerHeader +deserializePrefixedHeader(Slice data, bool hasHash) +{ + return deserializeHeader(data + 4, hasHash); +} + +} // namespace ripple diff --git a/src/ripple/rpc/impl/NFTSyntheticSerializer.cpp b/src/ripple/protocol/impl/NFTSyntheticSerializer.cpp similarity index 70% rename from src/ripple/rpc/impl/NFTSyntheticSerializer.cpp rename to src/ripple/protocol/impl/NFTSyntheticSerializer.cpp index f4692cfd4f8..e34397ada6a 100644 --- a/src/ripple/rpc/impl/NFTSyntheticSerializer.cpp +++ b/src/ripple/protocol/impl/NFTSyntheticSerializer.cpp @@ -17,28 +17,16 @@ */ //============================================================================== -#include - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include +#include +#include +#include +#include namespace ripple { -namespace RPC { void insertNFTSyntheticInJson( Json::Value& response, - RPC::JsonContext const& context, std::shared_ptr const& transaction, TxMeta const& transactionMeta) { @@ -46,5 +34,4 @@ insertNFTSyntheticInJson( insertNFTokenOfferID(response[jss::meta], transaction, transactionMeta); } -} // namespace RPC } // namespace ripple diff --git a/src/ripple/rpc/impl/NFTokenID.cpp b/src/ripple/protocol/impl/NFTokenID.cpp similarity index 94% rename from src/ripple/rpc/impl/NFTokenID.cpp rename to src/ripple/protocol/impl/NFTokenID.cpp index d0be439ec6a..ea6ab984c19 100644 --- a/src/ripple/rpc/impl/NFTokenID.cpp +++ b/src/ripple/protocol/impl/NFTokenID.cpp @@ -17,21 +17,10 @@ */ //============================================================================== -#include - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include +#include +#include namespace ripple { -namespace RPC { bool canHaveNFTokenID( @@ -198,5 +187,4 @@ insertNFTokenID( } } -} // namespace RPC } // namespace ripple diff --git a/src/ripple/rpc/impl/NFTokenOfferID.cpp b/src/ripple/protocol/impl/NFTokenOfferID.cpp similarity index 83% rename from src/ripple/rpc/impl/NFTokenOfferID.cpp rename to src/ripple/protocol/impl/NFTokenOfferID.cpp index 05d110aac56..c9c3118cf2a 100644 --- a/src/ripple/rpc/impl/NFTokenOfferID.cpp +++ b/src/ripple/protocol/impl/NFTokenOfferID.cpp @@ -17,21 +17,10 @@ */ //============================================================================== -#include - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include +#include +#include namespace ripple { -namespace RPC { bool canHaveNFTokenOfferID( @@ -81,5 +70,4 @@ insertNFTokenOfferID( response[jss::offer_id] = to_string(result.value()); } -} // namespace RPC } // namespace ripple diff --git a/src/ripple/protocol/messages.h b/src/ripple/protocol/messages.h index 81f6dc6c758..e9c9cf60b21 100644 --- a/src/ripple/protocol/messages.h +++ b/src/ripple/protocol/messages.h @@ -31,6 +31,6 @@ #undef TYPE_BOOL #endif -#include "ripple.pb.h" +#include #endif diff --git a/src/ripple/protocol/nft.h b/src/ripple/protocol/nft.h new file mode 100644 index 00000000000..2df8e0b89ce --- /dev/null +++ b/src/ripple/protocol/nft.h @@ -0,0 +1,127 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2023 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_PROTOCOL_NFT_H_INCLUDED +#define RIPPLE_PROTOCOL_NFT_H_INCLUDED + +#include +#include +#include + +#include + +#include +#include + +namespace ripple { +namespace nft { + +// Separate taxons from regular integers. +struct TaxonTag +{ +}; +using Taxon = tagged_integer; + +inline Taxon +toTaxon(std::uint32_t i) +{ + return static_cast(i); +} + +inline std::uint32_t +toUInt32(Taxon t) +{ + return static_cast(t); +} + +constexpr std::uint16_t const flagBurnable = 0x0001; +constexpr std::uint16_t const flagOnlyXRP = 0x0002; +constexpr std::uint16_t const flagCreateTrustLines = 0x0004; +constexpr std::uint16_t const flagTransferable = 0x0008; + +inline std::uint16_t +getFlags(uint256 const& id) +{ + std::uint16_t flags; + memcpy(&flags, id.begin(), 2); + return boost::endian::big_to_native(flags); +} + +inline std::uint16_t +getTransferFee(uint256 const& id) +{ + std::uint16_t fee; + memcpy(&fee, id.begin() + 2, 2); + return boost::endian::big_to_native(fee); +} + +inline std::uint32_t +getSerial(uint256 const& id) +{ + std::uint32_t seq; + memcpy(&seq, id.begin() + 28, 4); + return boost::endian::big_to_native(seq); +} + +inline Taxon +cipheredTaxon(std::uint32_t tokenSeq, Taxon taxon) +{ + // An issuer may issue several NFTs with the same taxon; to ensure that NFTs + // are spread across multiple pages we lightly mix the taxon up by using the + // sequence (which is not under the issuer's direct control) as the seed for + // a simple linear congruential generator. + // + // From the Hull-Dobell theorem we know that f(x)=(m*x+c) mod n will yield a + // permutation of [0, n) when n is a power of 2 if m is congruent to 1 mod 4 + // and c is odd. + // + // Here we use m = 384160001 and c = 2459. The modulo is implicit because we + // use 2^32 for n and the arithmetic gives it to us for "free". + // + // Note that the scramble value we calculate is not cryptographically secure + // but that's fine since all we're looking for is some dispersion. + // + // **IMPORTANT** Changing these numbers would be a breaking change requiring + // an amendment along with a way to distinguish token IDs that + // were generated with the old code. + return taxon ^ toTaxon(((384160001 * tokenSeq) + 2459)); +} + +inline Taxon +getTaxon(uint256 const& id) +{ + std::uint32_t taxon; + memcpy(&taxon, id.begin() + 24, 4); + taxon = boost::endian::big_to_native(taxon); + + // The taxon cipher is just an XOR, so it is reversible by applying the + // XOR a second time. + return cipheredTaxon(getSerial(id), toTaxon(taxon)); +} + +inline AccountID +getIssuer(uint256 const& id) +{ + return AccountID::fromVoid(id.data() + 4); +} + +} // namespace nft +} // namespace ripple + +#endif diff --git a/src/ripple/protocol/serialize.h b/src/ripple/protocol/serialize.h new file mode 100644 index 00000000000..93c59acfd0a --- /dev/null +++ b/src/ripple/protocol/serialize.h @@ -0,0 +1,48 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2023 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_PROTOCOL_SERIALIZE_H_INCLUDED +#define RIPPLE_PROTOCOL_SERIALIZE_H_INCLUDED + +#include +#include +#include + +namespace ripple { + +/** Serialize an object to a blob. */ +template +Blob +serializeBlob(Object const& o) +{ + Serializer s; + o.add(s); + return s.peekData(); +} + +/** Serialize an object to a hex string. */ +inline std::string +serializeHex(STObject const& o) +{ + return strHex(serializeBlob(o)); +} + +} // namespace ripple + +#endif diff --git a/src/ripple/rpc/GRPCHandlers.h b/src/ripple/rpc/GRPCHandlers.h index 493de7a5c47..9fb8d0909fe 100644 --- a/src/ripple/rpc/GRPCHandlers.h +++ b/src/ripple/rpc/GRPCHandlers.h @@ -20,9 +20,9 @@ #ifndef RIPPLE_RPC_GRPCHANDLER_H_INCLUDED #define RIPPLE_RPC_GRPCHANDLER_H_INCLUDED +#include #include #include -#include namespace ripple { diff --git a/src/ripple/rpc/handlers/AccountTx.cpp b/src/ripple/rpc/handlers/AccountTx.cpp index bfbc76362a3..84d087939e7 100644 --- a/src/ripple/rpc/handlers/AccountTx.cpp +++ b/src/ripple/rpc/handlers/AccountTx.cpp @@ -29,12 +29,12 @@ #include #include #include +#include #include #include #include #include #include -#include #include #include @@ -334,7 +334,7 @@ populateJsonResponse( insertDeliveredAmount( jvObj[jss::meta], context, txn, *txnMeta); insertNFTSyntheticInJson( - jvObj, context, txn->getSTransaction(), *txnMeta); + jvObj, txn->getSTransaction(), *txnMeta); } } } diff --git a/src/ripple/rpc/handlers/Tx.cpp b/src/ripple/rpc/handlers/Tx.cpp index e79997ec8f1..a14f82f0b02 100644 --- a/src/ripple/rpc/handlers/Tx.cpp +++ b/src/ripple/rpc/handlers/Tx.cpp @@ -24,11 +24,11 @@ #include #include #include +#include #include #include #include #include -#include #include namespace ripple { @@ -297,7 +297,7 @@ populateJsonResponse( insertDeliveredAmount( response[jss::meta], context, result.txn, *meta); insertNFTSyntheticInJson( - response, context, result.txn->getSTransaction(), *meta); + response, result.txn->getSTransaction(), *meta); } } response[jss::validated] = result.validated; diff --git a/src/ripple/rpc/impl/RPCHelpers.h b/src/ripple/rpc/impl/RPCHelpers.h index 6184b357515..0293fb15a21 100644 --- a/src/ripple/rpc/impl/RPCHelpers.h +++ b/src/ripple/rpc/impl/RPCHelpers.h @@ -21,6 +21,7 @@ #define RIPPLE_RPC_RPCHELPERS_H_INCLUDED #include +#include #include #include @@ -30,7 +31,6 @@ #include #include #include -#include #include namespace Json { diff --git a/src/test/overlay/compression_test.cpp b/src/test/overlay/compression_test.cpp index e24cb09f965..3b61b2b3a09 100644 --- a/src/test/overlay/compression_test.cpp +++ b/src/test/overlay/compression_test.cpp @@ -34,12 +34,12 @@ #include #include #include +#include #include #include #include #include #include -#include #include #include #include diff --git a/src/test/overlay/reduce_relay_test.cpp b/src/test/overlay/reduce_relay_test.cpp index 1839220dc4f..c722476dbaa 100644 --- a/src/test/overlay/reduce_relay_test.cpp +++ b/src/test/overlay/reduce_relay_test.cpp @@ -23,7 +23,7 @@ #include #include #include -#include +#include #include #include @@ -1597,4 +1597,4 @@ BEAST_DEFINE_TESTSUITE_MANUAL(reduce_relay_simulate, ripple_data, ripple); } // namespace test -} // namespace ripple \ No newline at end of file +} // namespace ripple From d96e59713f70bb1106fe72d42d2f325d5d347399 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 18 Jul 2023 16:06:41 -0700 Subject: [PATCH 06/13] add view updates for account SLEs (#4629) Signed-off-by: Manoj Doshi --- src/ripple/app/tx/impl/SetAccount.cpp | 2 ++ src/ripple/app/tx/impl/SetRegularKey.cpp | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/ripple/app/tx/impl/SetAccount.cpp b/src/ripple/app/tx/impl/SetAccount.cpp index b080fcfe86a..d43b3b339e5 100644 --- a/src/ripple/app/tx/impl/SetAccount.cpp +++ b/src/ripple/app/tx/impl/SetAccount.cpp @@ -604,6 +604,8 @@ SetAccount::doApply() if (uFlagsIn != uFlagsOut) sle->setFieldU32(sfFlags, uFlagsOut); + ctx_.view().update(sle); + return tesSUCCESS; } diff --git a/src/ripple/app/tx/impl/SetRegularKey.cpp b/src/ripple/app/tx/impl/SetRegularKey.cpp index 34a8b7d238c..8b789d75aed 100644 --- a/src/ripple/app/tx/impl/SetRegularKey.cpp +++ b/src/ripple/app/tx/impl/SetRegularKey.cpp @@ -96,6 +96,8 @@ SetRegularKey::doApply() sle->makeFieldAbsent(sfRegularKey); } + ctx_.view().update(sle); + return tesSUCCESS; } From 966507a180f6a4b854c787d42003e656dc0a2374 Mon Sep 17 00:00:00 2001 From: Ikko Eltociear Ashimine Date: Wed, 9 Aug 2023 19:53:44 +0900 Subject: [PATCH 07/13] refactor: fix typo in FeeUnits.h (#4644) covert -> convert Signed-off-by: Manoj Doshi --- src/ripple/basics/FeeUnits.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/basics/FeeUnits.h b/src/ripple/basics/FeeUnits.h index c0f1afbe6c6..35e0ff24d05 100644 --- a/src/ripple/basics/FeeUnits.h +++ b/src/ripple/basics/FeeUnits.h @@ -128,7 +128,7 @@ class TaggedFee : private boost::totally_ordered>, } /** Instances with the same unit, and a type that is - "safe" to covert to this one can be converted + "safe" to convert to this one can be converted implicitly */ template < class Other, From 7cf929e13b6800fbdeb96c1f361df977c0099c1c Mon Sep 17 00:00:00 2001 From: Michael Legleux Date: Sat, 12 Aug 2023 18:20:54 -0400 Subject: [PATCH 08/13] Update Ubuntu build image (#4650) Signed-off-by: Manoj Doshi --- Builds/CMake/RippledRelease.cmake | 2 +- Builds/containers/gitlab-ci/pkgbuild.yml | 2 +- .../dpkg/debian/rippled-reporting.install | 4 +- Builds/containers/packaging/dpkg/debian/rules | 97 ++++++++----------- Builds/containers/packaging/rpm/rippled.spec | 75 +++++++------- Builds/containers/ubuntu-builder/Dockerfile | 7 ++ .../containers/ubuntu-builder/ubuntu_setup.sh | 49 ++-------- .../ubuntu-builder/ubuntu_setup2.sh | 49 ++++++++++ .../ubuntu-builder/ubuntu_setup3.sh | 7 ++ 9 files changed, 158 insertions(+), 134 deletions(-) create mode 100755 Builds/containers/ubuntu-builder/ubuntu_setup2.sh create mode 100755 Builds/containers/ubuntu-builder/ubuntu_setup3.sh diff --git a/Builds/CMake/RippledRelease.cmake b/Builds/CMake/RippledRelease.cmake index 1c60ea80d7b..a0ad3696572 100644 --- a/Builds/CMake/RippledRelease.cmake +++ b/Builds/CMake/RippledRelease.cmake @@ -49,7 +49,7 @@ if (is_root_project) docker run -v ${CMAKE_CURRENT_SOURCE_DIR}:/opt/rippled_bld/pkg/rippled -v ${CMAKE_CURRENT_BINARY_DIR}/packages:/opt/rippled_bld/pkg/out - -t rippleci/rippled-rpm-builder:${container_label} + -t rippled-rpm-builder:${container_label} /bin/bash -c "cp -fpu rippled/Builds/containers/packaging/rpm/build_rpm.sh . && ./build_rpm.sh" VERBATIM USES_TERMINAL diff --git a/Builds/containers/gitlab-ci/pkgbuild.yml b/Builds/containers/gitlab-ci/pkgbuild.yml index f35186acb8e..e2326852867 100644 --- a/Builds/containers/gitlab-ci/pkgbuild.yml +++ b/Builds/containers/gitlab-ci/pkgbuild.yml @@ -14,7 +14,7 @@ variables: RPM_CONTAINER_TAG: "2023-02-13" RPM_CONTAINER_NAME: "rippled-rpm-builder" RPM_CONTAINER_FULLNAME: "${RPM_CONTAINER_NAME}:${RPM_CONTAINER_TAG}" - DPKG_CONTAINER_TAG: "2023-03-20" + DPKG_CONTAINER_TAG: "2023-07-31" DPKG_CONTAINER_NAME: "rippled-dpkg-builder" DPKG_CONTAINER_FULLNAME: "${DPKG_CONTAINER_NAME}:${DPKG_CONTAINER_TAG}" ARTIFACTORY_HOST: "artifactory.ops.ripple.com" diff --git a/Builds/containers/packaging/dpkg/debian/rippled-reporting.install b/Builds/containers/packaging/dpkg/debian/rippled-reporting.install index 255c7b0b5c4..0cee940f59e 100644 --- a/Builds/containers/packaging/dpkg/debian/rippled-reporting.install +++ b/Builds/containers/packaging/dpkg/debian/rippled-reporting.install @@ -1,8 +1,8 @@ -bld/rippled-reporting/rippled-reporting opt/rippled-reporting/bin +build.rippled-reporting/rippled-reporting opt/rippled-reporting/bin cfg/rippled-reporting.cfg opt/rippled-reporting/etc debian/tmp/opt/rippled-reporting/etc/validators.txt opt/rippled-reporting/etc opt/rippled-reporting/bin/update-rippled-reporting.sh opt/rippled-reporting/bin/getRippledReportingInfo opt/rippled-reporting/etc/update-rippled-reporting-cron -etc/logrotate.d/rippled-reporting \ No newline at end of file +etc/logrotate.d/rippled-reporting diff --git a/Builds/containers/packaging/dpkg/debian/rules b/Builds/containers/packaging/dpkg/debian/rules index c7234c847dd..08b180e104c 100755 --- a/Builds/containers/packaging/dpkg/debian/rules +++ b/Builds/containers/packaging/dpkg/debian/rules @@ -15,69 +15,56 @@ override_dh_systemd_start: dh_systemd_start --no-restart-on-upgrade override_dh_auto_configure: - apt install --yes gcc-11 g++-11 - update-alternatives --install \ - /usr/bin/gcc gcc /usr/bin/gcc-11 100 \ - --slave /usr/bin/g++ g++ /usr/bin/g++-11 \ - --slave /usr/bin/gcc-ar gcc-ar /usr/bin/gcc-ar-11 \ - --slave /usr/bin/gcc-nm gcc-nm /usr/bin/gcc-nm-11 \ - --slave /usr/bin/gcc-ranlib gcc-ranlib /usr/bin/gcc-ranlib-11 \ - --slave /usr/bin/gcov gcov /usr/bin/gcov-11 \ - --slave /usr/bin/gcov-tool gcov-tool /usr/bin/gcov-dump-11 \ - --slave /usr/bin/gcov-dump gcov-dump /usr/bin/gcov-tool-11 - update-alternatives --set gcc /usr/bin/gcc-11 - env - rm -rf bld - conan profile update settings.compiler.cppstd=20 gcc - conan profile update settings.compiler.version=11 gcc - conan export external/snappy snappy/1.1.9@ + /root/.pyenv/shims/conan export external/snappy snappy/1.1.10@ + /root/.pyenv/shims/conan export external/soci soci/4.0.3@ - conan install . \ - --profile gcc \ - --install-folder bld/rippled \ - --build missing \ - --build boost \ - --build sqlite3 \ - --settings build_type=Release + mkdir build.rippled - cmake -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \ - -G Ninja \ - -DCMAKE_BUILD_TYPE=Release \ - -DCMAKE_INSTALL_PREFIX=/opt/ripple \ - -Dstatic=ON \ - -Dunity=OFF \ - -DCMAKE_VERBOSE_MAKEFILE=ON \ - -Dvalidator_keys=ON \ - -B bld/rippled + cd build.rippled && \ + /root/.pyenv/shims/conan install .. \ + --profile gcc \ + --output-folder . \ + --build missing \ + --settings build_type=Release - conan install . \ - --profile gcc \ - --install-folder bld/rippled-reporting \ - --build missing \ - --build boost \ - --build sqlite3 \ - --build libuv \ - --settings build_type=Release \ - --settings compiler.cppstd=17 \ - --options reporting=True + cd build.rippled && \ + cmake .. \ + -DCMAKE_BUILD_TYPE=Release \ + -Dvalidator_keys=ON \ + -DCMAKE_VERBOSE_MAKEFILE=ON \ + -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake - cmake -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \ - -G Ninja \ - -DCMAKE_BUILD_TYPE=Release \ - -DCMAKE_INSTALL_PREFIX=/opt/rippled-reporting \ - -Dstatic=ON \ - -Dunity=OFF \ - -DCMAKE_VERBOSE_MAKEFILE=ON \ - -Dreporting=ON \ - -B bld/rippled-reporting + mkdir build.rippled-reporting + + cd build.rippled-reporting && \ + /root/.pyenv/shims/conan install .. \ + --profile gcc \ + --output-folder . \ + --settings compiler.cppstd=17 \ + --settings build_type=Release \ + --build missing \ + --build boost \ + --build sqlite3 \ + --build libuv \ + --options reporting=True + + cd build.rippled-reporting && \ + cmake .. \ + -DCMAKE_BUILD_TYPE=Release \ + -Dvalidator_keys=ON \ + -Dstatic=ON \ + -Dunity=OFF \ + -Dreporting=ON \ + -DCMAKE_VERBOSE_MAKEFILE=ON \ + -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake override_dh_auto_build: - cmake --build bld/rippled --target rippled --target validator-keys -j${nproc} - cmake --build bld/rippled-reporting --target rippled -j${nproc} + cmake --build build.rippled --target rippled --target validator-keys --parallel 8 + cmake --build build.rippled-reporting --target rippled --parallel 8 override_dh_auto_install: - cmake --install bld/rippled --prefix debian/tmp/opt/ripple - install -D bld/rippled/validator-keys/validator-keys debian/tmp/opt/ripple/bin/validator-keys + cmake --install build.rippled --prefix debian/tmp/opt/ripple + install -D build.rippled/validator-keys/validator-keys debian/tmp/opt/ripple/bin/validator-keys install -D Builds/containers/shared/update-rippled.sh debian/tmp/opt/ripple/bin/update-rippled.sh install -D bin/getRippledInfo debian/tmp/opt/ripple/bin/getRippledInfo install -D Builds/containers/shared/update-rippled-cron debian/tmp/opt/ripple/etc/update-rippled-cron diff --git a/Builds/containers/packaging/rpm/rippled.spec b/Builds/containers/packaging/rpm/rippled.spec index fe451d645d4..0c2f454fa42 100644 --- a/Builds/containers/packaging/rpm/rippled.spec +++ b/Builds/containers/packaging/rpm/rippled.spec @@ -36,41 +36,44 @@ History server for XRP Ledger %setup -c -n rippled %build -rm -rf ~/.conan/profiles/default - -cp /opt/libcstd/libstdc++.so.6.0.22 /usr/lib64 -cp /opt/libcstd/libstdc++.so.6.0.22 /lib64 -ln -sf /usr/lib64/libstdc++.so.6.0.22 /usr/lib64/libstdc++.so.6 -ln -sf /lib64/libstdc++.so.6.0.22 /usr/lib64/libstdc++.so.6 +source /opt/rh/devtoolset-11/enable source /opt/rh/rh-python38/enable + pip install "conan<2" + conan profile new default --detect -conan profile update settings.compiler.libcxx=libstdc++11 default conan profile update settings.compiler.cppstd=20 default +conan profile update settings.compiler.libcxx=libstdc++11 default cd rippled -mkdir -p bld.rippled -conan export external/snappy snappy/1.1.9@ +conan export external/snappy snappy/1.1.10@ +conan export external/soci soci/4.0.3@ +mkdir -p bld.rippled pushd bld.rippled + +cp /opt/libcstd/libstdc++.so.6.0.22 /usr/lib64 +cp /opt/libcstd/libstdc++.so.6.0.22 /lib64 +ln -sf /usr/lib64/libstdc++.so.6.0.22 /usr/lib64/libstdc++.so.6 +ln -sf /lib64/libstdc++.so.6.0.22 /usr/lib64/libstdc++.so.6 + conan install .. \ - --settings build_type=Release \ - --output-folder . \ - --build missing - -cmake -G Ninja \ - -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \ - -DCMAKE_INSTALL_PREFIX=%{_prefix} \ - -DCMAKE_BUILD_TYPE=Release \ - -Dunity=OFF \ - -Dstatic=ON \ - -Dvalidator_keys=ON \ - -DCMAKE_VERBOSE_MAKEFILE=ON \ - .. + --profile default \ + --output-folder . \ + --build missing \ + --settings build_type=Release + +cmake .. \ + -DCMAKE_BUILD_TYPE=Release \ + -Dvalidator_keys=ON \ + -DCMAKE_INSTALL_PREFIX=%{_prefix} \ + -DCMAKE_VERBOSE_MAKEFILE=ON \ + -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake cmake --build . --parallel $(nproc) --target rippled --target validator-keys + popd mkdir -p bld.rippled-reporting @@ -83,16 +86,16 @@ conan install .. \ --settings compiler.cppstd=17 \ --options reporting=True -cmake -G Ninja \ - -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \ - -DCMAKE_INSTALL_PREFIX=%{_prefix} \ - -DCMAKE_BUILD_TYPE=Release \ - -Dunity=OFF \ - -Dstatic=ON \ - -Dvalidator_keys=ON \ - -Dreporting=ON \ - -DCMAKE_VERBOSE_MAKEFILE=ON \ - .. +cmake .. \ + -G Ninja \ + -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \ + -DCMAKE_INSTALL_PREFIX=%{_prefix} \ + -DCMAKE_BUILD_TYPE=Release \ + -Dunity=OFF \ + -Dstatic=ON \ + -Dvalidator_keys=ON \ + -Dreporting=ON \ + -DCMAKE_VERBOSE_MAKEFILE=ON \ cmake --build . --parallel $(nproc) --target rippled @@ -103,7 +106,7 @@ test -e /etc/pki/tls || { mkdir -p /etc/pki; ln -s /usr/lib/ssl /etc/pki/tls; } rm -rf $RPM_BUILD_ROOT DESTDIR=$RPM_BUILD_ROOT cmake --build rippled/bld.rippled --target install #-- -v mkdir -p $RPM_BUILD_ROOT -rm -rf ${RPM_BUILD_ROOT}/%{_prefix}/lib64/ +rm -rf ${RPM_BUILD_ROOT}%{_prefix}/lib64/ install -d ${RPM_BUILD_ROOT}/etc/opt/ripple install -d ${RPM_BUILD_ROOT}/usr/local/bin @@ -130,9 +133,9 @@ install -D rippled/bld.rippled-reporting/rippled-reporting ${RPM_BUILD_ROOT}%{_b install -D ./rippled/cfg/rippled-reporting.cfg ${RPM_BUILD_ROOT}%{_prefix}/etc/rippled-reporting.cfg install -D ./rippled/cfg/validators-example.txt ${RPM_BUILD_ROOT}%{_prefix}/etc/validators.txt install -D ./rippled/Builds/containers/packaging/rpm/50-rippled-reporting.preset ${RPM_BUILD_ROOT}/usr/lib/systemd/system-preset/50-rippled-reporting.preset -ln -s %{_prefix}/bin/rippled-reporting ${RPM_BUILD_ROOT}/usr/local/bin/rippled-reporting -ln -s %{_prefix}/etc/rippled-reporting.cfg ${RPM_BUILD_ROOT}/etc/opt/rippled-reporting/rippled-reporting.cfg -ln -s %{_prefix}/etc/validators.txt ${RPM_BUILD_ROOT}/etc/opt/rippled-reporting/validators.txt +ln -sf %{_prefix}/bin/rippled-reporting ${RPM_BUILD_ROOT}/usr/local/bin/rippled-reporting +ln -sf %{_prefix}/etc/rippled-reporting.cfg ${RPM_BUILD_ROOT}/etc/opt/rippled-reporting/rippled-reporting.cfg +ln -sf %{_prefix}/etc/validators.txt ${RPM_BUILD_ROOT}/etc/opt/rippled-reporting/validators.txt install -d $RPM_BUILD_ROOT/var/log/rippled-reporting install -d $RPM_BUILD_ROOT/var/lib/rippled-reporting install -D ./rippled/Builds/containers/shared/rippled-reporting.service ${RPM_BUILD_ROOT}/usr/lib/systemd/system/rippled-reporting.service diff --git a/Builds/containers/ubuntu-builder/Dockerfile b/Builds/containers/ubuntu-builder/Dockerfile index 23723967fc2..e2604d3a1ca 100644 --- a/Builds/containers/ubuntu-builder/Dockerfile +++ b/Builds/containers/ubuntu-builder/Dockerfile @@ -6,8 +6,15 @@ LABEL git-commit=$GIT_COMMIT WORKDIR /root COPY ubuntu-builder/ubuntu_setup.sh . +COPY ubuntu-builder/ubuntu_setup2.sh . + RUN ./ubuntu_setup.sh && rm ubuntu_setup.sh +RUN ./ubuntu_setup2.sh && rm ubuntu_setup2.sh + +COPY ubuntu-builder/ubuntu_setup3.sh . +RUN ./ubuntu_setup3.sh && rm ubuntu_setup3.sh + RUN mkdir -m 777 -p /opt/rippled_bld/pkg/ WORKDIR /opt/rippled_bld/pkg diff --git a/Builds/containers/ubuntu-builder/ubuntu_setup.sh b/Builds/containers/ubuntu-builder/ubuntu_setup.sh index cd8db75153d..778bcf53c0b 100755 --- a/Builds/containers/ubuntu-builder/ubuntu_setup.sh +++ b/Builds/containers/ubuntu-builder/ubuntu_setup.sh @@ -6,37 +6,33 @@ set -o xtrace # Parameters -gcc_version=${GCC_VERSION:-10} -cmake_version=${CMAKE_VERSION:-3.25.1} -conan_version=${CONAN_VERSION:-1.59} +gcc_version=${GCC_VERSION:-11} + +export DEBIAN_FRONTEND=noninteractive apt update # Iteratively build the list of packages to install so that we can interleave # the lines with comments explaining their inclusion. dependencies='' -# - to identify the Ubuntu version -dependencies+=' lsb-release' # - for add-apt-repository dependencies+=' software-properties-common' # - to download CMake dependencies+=' curl' # - to build CMake dependencies+=' libssl-dev' -# - Python headers for Boost.Python -dependencies+=' python3-dev' -# - to install Conan -dependencies+=' python3-pip' +# - for Python +dependencies+=' libbz2-dev liblzma-dev libsqlite3-dev' # - to download rippled dependencies+=' git' # - CMake generators (but not CMake itself) dependencies+=' make ninja-build' -apt install --yes ${dependencies} +apt-get install --yes ${dependencies} add-apt-repository --yes ppa:ubuntu-toolchain-r/test -apt install --yes gcc-${gcc_version} g++-${gcc_version} \ - debhelper debmake debsums gnupg dh-buildinfo dh-make dh-systemd cmake \ - ninja-build zlib1g-dev make cmake ninja-build autoconf automake \ - pkg-config apt-transport-https +apt-get install --yes gcc-${gcc_version} g++-${gcc_version} +apt-get install --yes build-essential libssl-dev zlib1g-dev \ +libbz2-dev libreadline-dev libsqlite3-dev curl \ +libncursesw5-dev xz-utils tk-dev libxml2-dev libxmlsec1-dev libffi-dev liblzma-dev # Give us nice unversioned aliases for gcc and company. update-alternatives --install \ @@ -49,28 +45,3 @@ update-alternatives --install \ --slave /usr/bin/gcov-tool gcov-tool /usr/bin/gcov-dump-${gcc_version} \ --slave /usr/bin/gcov-dump gcov-dump /usr/bin/gcov-tool-${gcc_version} update-alternatives --auto gcc - -# Download and unpack CMake. -cmake_slug="cmake-${cmake_version}" -curl --location --remote-name \ - "https://github.com/Kitware/CMake/releases/download/v${cmake_version}/${cmake_slug}.tar.gz" -tar xzf ${cmake_slug}.tar.gz -rm ${cmake_slug}.tar.gz - -# Build and install CMake. -cd ${cmake_slug} -./bootstrap --parallel=$(nproc) -make --jobs $(nproc) -make install -cd .. -rm --recursive --force ${cmake_slug} - -# Install Conan. -pip3 install conan==${conan_version} - -conan profile new --detect gcc -conan profile update settings.compiler=gcc gcc -conan profile update settings.compiler.version=${gcc_version} gcc -conan profile update settings.compiler.libcxx=libstdc++11 gcc -conan profile update env.CC=/usr/bin/gcc gcc -conan profile update env.CXX=/usr/bin/g++ gcc diff --git a/Builds/containers/ubuntu-builder/ubuntu_setup2.sh b/Builds/containers/ubuntu-builder/ubuntu_setup2.sh new file mode 100755 index 00000000000..07ea577c68a --- /dev/null +++ b/Builds/containers/ubuntu-builder/ubuntu_setup2.sh @@ -0,0 +1,49 @@ +#!/usr/bin/env bash + +set -o errexit +set -o nounset +set -o xtrace + +# Parameters + +gcc_version=${GCC_VERSION:-11} +cmake_version=${CMAKE_VERSION:-3.25.1} +cmake_sha256=1c511d09516af493694ed9baf13c55947a36389674d657a2d5e0ccedc6b291d8 +conan_version=${CONAN_VERSION:-1.60} + +curl https://pyenv.run | bash +export PYENV_ROOT="$HOME/.pyenv" +command -v pyenv >/dev/null || export PATH="$PYENV_ROOT/bin:$PATH" +eval "$(pyenv init -)" + +pyenv install 3.11.2 +pyenv global 3.11.2 + +# Download and unpack CMake. +cmake_slug="cmake-${cmake_version}" +cmake_archive="${cmake_slug}.tar.gz" +curl --location --remote-name \ + "https://github.com/Kitware/CMake/releases/download/v${cmake_version}/${cmake_archive}" +echo "${cmake_sha256} ${cmake_archive}" | sha256sum --check +tar -xzf ${cmake_archive} +rm ${cmake_archive} + +# Build and install CMake. +cd ${cmake_slug} +./bootstrap --parallel=$(nproc) +make --jobs $(nproc) +make install +cd .. +rm --recursive --force ${cmake_slug} + +# Install Conan. +pip install --upgrade pip +pip install conan==${conan_version} + +conan profile new --detect gcc +conan profile update settings.compiler=gcc gcc +conan profile update settings.compiler.version=${gcc_version} gcc +conan profile update settings.compiler.libcxx=libstdc++11 gcc +conan profile update settings.compiler.cppstd=20 gcc +conan profile update env.CC=/usr/bin/gcc gcc +conan profile update env.CXX=/usr/bin/g++ gcc diff --git a/Builds/containers/ubuntu-builder/ubuntu_setup3.sh b/Builds/containers/ubuntu-builder/ubuntu_setup3.sh new file mode 100755 index 00000000000..91c7ca2406a --- /dev/null +++ b/Builds/containers/ubuntu-builder/ubuntu_setup3.sh @@ -0,0 +1,7 @@ +#!/usr/bin/env bash + +set -o errexit +set -o nounset +set -o xtrace + +apt-get install --yes build-essential fakeroot devscripts cmake debhelper dh-systemd From 9523184faa89b7710e44dec967a27871686296e5 Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Thu, 20 Apr 2023 11:12:31 -0700 Subject: [PATCH 09/13] Apply transaction batches in periodic intervals. (#4504) Add new transaction submission API field, "sync", which determines behavior of the server while submitting transactions: - sync (default): Process transactions in a batch immediately, and return only once the transaction has been processed. - async: Put transaction into the batch for the next processing interval and return immediately. - wait: Put transaction into the batch for the next processing interval and return only after it is processed. Signed-off-by: Manoj Doshi --- Builds/CMake/RippledCore.cmake | 1 + cfg/rippled-example.cfg | 2 +- cfg/rippled-reporting.cfg | 2 +- src/ripple/app/ledger/impl/LedgerMaster.cpp | 33 +-- src/ripple/app/main/Application.cpp | 1 + src/ripple/app/misc/NetworkOPs.cpp | 219 +++++++++--------- src/ripple/app/misc/NetworkOPs.h | 43 +++- src/ripple/app/tx/impl/apply.cpp | 10 +- src/ripple/basics/SubmitSync.h | 41 ++++ src/ripple/core/Config.h | 11 +- src/ripple/overlay/impl/PeerImp.cpp | 10 +- src/ripple/protocol/TER.h | 1 + src/ripple/protocol/impl/TER.cpp | 1 + src/ripple/protocol/jss.h | 1 + src/ripple/rpc/handlers/Submit.cpp | 10 +- src/ripple/rpc/handlers/SubmitMultiSigned.cpp | 9 +- src/ripple/rpc/impl/RPCHelpers.cpp | 21 ++ src/ripple/rpc/impl/RPCHelpers.h | 10 + src/ripple/rpc/impl/TransactionSign.cpp | 11 +- src/ripple/rpc/impl/TransactionSign.h | 14 +- src/test/app/Transaction_ordering_test.cpp | 4 + src/test/jtx/Env_test.cpp | 91 ++++++++ src/test/rpc/JSONRPC_test.cpp | 9 +- src/test/rpc/RobustTransaction_test.cpp | 6 +- 24 files changed, 398 insertions(+), 163 deletions(-) create mode 100644 src/ripple/basics/SubmitSync.h diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index b2f52ff8bf0..e08fe35e46b 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -199,6 +199,7 @@ install ( src/ripple/basics/StringUtilities.h src/ripple/basics/TaggedCache.h src/ripple/basics/tagged_integer.h + src/ripple/basics/SubmitSync.h src/ripple/basics/ThreadSafetyAnalysis.h src/ripple/basics/ToString.h src/ripple/basics/UnorderedContainers.h diff --git a/cfg/rippled-example.cfg b/cfg/rippled-example.cfg index 0a669313066..ef450ce3c10 100644 --- a/cfg/rippled-example.cfg +++ b/cfg/rippled-example.cfg @@ -478,7 +478,7 @@ # # Configure the maximum number of transactions to have in the job queue # -# Must be a number between 100 and 1000, defaults to 250 +# Must be a number between 1000 and 100000, defaults to 10000 # # # [overlay] diff --git a/cfg/rippled-reporting.cfg b/cfg/rippled-reporting.cfg index dbafdd497fa..f09c17ae637 100644 --- a/cfg/rippled-reporting.cfg +++ b/cfg/rippled-reporting.cfg @@ -467,7 +467,7 @@ # # Configure the maximum number of transactions to have in the job queue # -# Must be a number between 100 and 1000, defaults to 250 +# Must be a number between 1000 and 100000, defaults to 10000 # # # [overlay] diff --git a/src/ripple/app/ledger/impl/LedgerMaster.cpp b/src/ripple/app/ledger/impl/LedgerMaster.cpp index 7ae7476948b..e051006cf05 100644 --- a/src/ripple/app/ledger/impl/LedgerMaster.cpp +++ b/src/ripple/app/ledger/impl/LedgerMaster.cpp @@ -549,22 +549,25 @@ void LedgerMaster::applyHeldTransactions() { std::lock_guard sl(m_mutex); + // It can be expensive to modify the open ledger even with no transactions + // to process. Regardless, make sure to reset held transactions with + // the parent. + if (mHeldTransactions.size()) + { + app_.openLedger().modify([&](OpenView& view, beast::Journal j) { + bool any = false; + for (auto const& it : mHeldTransactions) + { + ApplyFlags flags = tapNONE; + auto const result = + app_.getTxQ().apply(app_, view, it.second, flags, j); + if (result.second) + any = true; + } + return any; + }); + } - app_.openLedger().modify([&](OpenView& view, beast::Journal j) { - bool any = false; - for (auto const& it : mHeldTransactions) - { - ApplyFlags flags = tapNONE; - auto const result = - app_.getTxQ().apply(app_, view, it.second, flags, j); - if (result.second) - any = true; - } - return any; - }); - - // VFALCO TODO recreate the CanonicalTxSet object instead of resetting - // it. // VFALCO NOTE The hash for an open ledger is undefined so we use // something that is a reasonable substitute. mHeldTransactions.reset(app_.openLedger().current()->info().parentHash); diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index 42bf6d66c9d..83cf762cfcb 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -1527,6 +1527,7 @@ ApplicationImp::start(bool withTimers) { setSweepTimer(); setEntropyTimer(); + m_networkOPs->setBatchApplyTimer(); } m_io_latency_sampler.start(); diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 4d90e0622f8..e59dd1128ff 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -42,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -233,6 +234,7 @@ class NetworkOPsImp final : public NetworkOPs , heartbeatTimer_(io_svc) , clusterTimer_(io_svc) , accountHistoryTxTimer_(io_svc) + , batchApplyTimer_(io_svc) , mConsensus( app, make_FeeVote( @@ -282,43 +284,12 @@ class NetworkOPsImp final : public NetworkOPs processTransaction( std::shared_ptr& transaction, bool bUnlimited, + RPC::SubmitSync sync, bool bLocal, FailHard failType) override; - /** - * For transactions submitted directly by a client, apply batch of - * transactions and wait for this transaction to complete. - * - * @param transaction Transaction object. - * @param bUnliimited Whether a privileged client connection submitted it. - * @param failType fail_hard setting from transaction submission. - */ - void - doTransactionSync( - std::shared_ptr transaction, - bool bUnlimited, - FailHard failType); - - /** - * For transactions not submitted by a locally connected client, fire and - * forget. Add to batch and trigger it to be processed if there's no batch - * currently being applied. - * - * @param transaction Transaction object - * @param bUnlimited Whether a privileged client connection submitted it. - * @param failType fail_hard setting from transaction submission. - */ - void - doTransactionAsync( - std::shared_ptr transaction, - bool bUnlimited, - FailHard failtype); - - /** - * Apply transactions in batches. Continue until none are queued. - */ - void - transactionBatch(); + bool + transactionBatch(bool drain) override; /** * Attempt to apply transactions and post-process based on the results. @@ -592,6 +563,15 @@ class NetworkOPsImp final : public NetworkOPs << "NetworkOPs: accountHistoryTxTimer cancel error: " << ec.message(); } + + ec.clear(); + batchApplyTimer_.cancel(ec); + if (ec) + { + JLOG(m_journal.error()) + << "NetworkOPs: batchApplyTimer cancel error: " + << ec.message(); + } } // Make sure that any waitHandlers pending in our timers are done. using namespace std::chrono_literals; @@ -710,6 +690,9 @@ class NetworkOPsImp final : public NetworkOPs void setAccountHistoryJobTimer(SubAccountHistoryInfoWeak subInfo); + void + setBatchApplyTimer() override; + Application& app_; beast::Journal m_journal; @@ -728,6 +711,8 @@ class NetworkOPsImp final : public NetworkOPs boost::asio::steady_timer heartbeatTimer_; boost::asio::steady_timer clusterTimer_; boost::asio::steady_timer accountHistoryTxTimer_; + //! This timer is for applying transaction batches. + boost::asio::steady_timer batchApplyTimer_; RCLConsensus mConsensus; @@ -1002,6 +987,42 @@ NetworkOPsImp::setAccountHistoryJobTimer(SubAccountHistoryInfoWeak subInfo) [this, subInfo]() { setAccountHistoryJobTimer(subInfo); }); } +void +NetworkOPsImp::setBatchApplyTimer() +{ + using namespace std::chrono_literals; + // 100ms lag between batch intervals provides significant throughput gains + // with little increased latency. Tuning this figure further will + // require further testing. In general, increasing this figure will + // also increase theoretical throughput, but with diminishing returns. + auto constexpr batchInterval = 100ms; + + setTimer( + batchApplyTimer_, + batchInterval, + [this]() { + { + std::lock_guard lock(mMutex); + // Only do the job if there's work to do and it's not currently + // being done. + if (mTransactions.size() && + mDispatchState == DispatchState::none) + { + if (m_job_queue.addJob( + jtBATCH, "transactionBatch", [this]() { + transactionBatch(false); + })) + { + mDispatchState = DispatchState::scheduled; + } + return; + } + } + setBatchApplyTimer(); + }, + [this]() { setBatchApplyTimer(); }); +} + void NetworkOPsImp::processHeartbeatTimer() { @@ -1178,7 +1199,8 @@ NetworkOPsImp::submitTransaction(std::shared_ptr const& iTrans) m_job_queue.addJob(jtTRANSACTION, "submitTxn", [this, tx]() { auto t = tx; - processTransaction(t, false, false, FailHard::no); + processTransaction( + t, false, RPC::SubmitSync::async, false, FailHard::no); }); } @@ -1186,6 +1208,7 @@ void NetworkOPsImp::processTransaction( std::shared_ptr& transaction, bool bUnlimited, + RPC::SubmitSync sync, bool bLocal, FailHard failType) { @@ -1215,7 +1238,7 @@ NetworkOPsImp::processTransaction( // Not concerned with local checks at this point. if (validity == Validity::SigBad) { - JLOG(m_journal.info()) << "Transaction has bad signature: " << reason; + JLOG(m_journal.trace()) << "Transaction has bad signature: " << reason; transaction->setStatus(INVALID); transaction->setResult(temBAD_SIGNATURE); app_.getHashRouter().setFlags(transaction->getID(), SF_BAD); @@ -1225,100 +1248,72 @@ NetworkOPsImp::processTransaction( // canonicalize can change our pointer app_.getMasterTransaction().canonicalize(&transaction); - if (bLocal) - doTransactionSync(transaction, bUnlimited, failType); - else - doTransactionAsync(transaction, bUnlimited, failType); -} - -void -NetworkOPsImp::doTransactionAsync( - std::shared_ptr transaction, - bool bUnlimited, - FailHard failType) -{ - std::lock_guard lock(mMutex); - - if (transaction->getApplying()) - return; - - mTransactions.push_back( - TransactionStatus(transaction, bUnlimited, false, failType)); - transaction->setApplying(); - - if (mDispatchState == DispatchState::none) - { - if (m_job_queue.addJob( - jtBATCH, "transactionBatch", [this]() { transactionBatch(); })) - { - mDispatchState = DispatchState::scheduled; - } - } -} - -void -NetworkOPsImp::doTransactionSync( - std::shared_ptr transaction, - bool bUnlimited, - FailHard failType) -{ - std::unique_lock lock(mMutex); - + std::unique_lock lock(mMutex); if (!transaction->getApplying()) { - mTransactions.push_back( - TransactionStatus(transaction, bUnlimited, true, failType)); transaction->setApplying(); + mTransactions.push_back( + TransactionStatus(transaction, bUnlimited, bLocal, failType)); } - - do + switch (sync) { - if (mDispatchState == DispatchState::running) - { - // A batch processing job is already running, so wait. - mCond.wait(lock); - } - else - { - apply(lock); - - if (mTransactions.size()) + using enum RPC::SubmitSync; + case sync: + do { - // More transactions need to be applied, but by another job. - if (m_job_queue.addJob(jtBATCH, "transactionBatch", [this]() { - transactionBatch(); - })) - { - mDispatchState = DispatchState::scheduled; - } - } - } - } while (transaction->getApplying()); + // If a batch is being processed, then wait. Otherwise, + // process a batch. + if (mDispatchState == DispatchState::running) + mCond.wait(lock); + else + apply(lock); + } while (transaction->getApplying()); + break; + + case async: + // It's conceivable for the submitted transaction to be + // processed and its result to be modified before being returned + // to the client. Make a copy of the transaction and set its + // status to guarantee that the client gets the terSUBMITTED + // result in all cases. + transaction = std::make_shared(*transaction); + transaction->setResult(terSUBMITTED); + break; + + case wait: + mCond.wait( + lock, [&transaction] { return !transaction->getApplying(); }); + break; + + default: + assert(false); + } } -void -NetworkOPsImp::transactionBatch() +bool +NetworkOPsImp::transactionBatch(bool const drain) { - std::unique_lock lock(mMutex); - - if (mDispatchState == DispatchState::running) - return; - - while (mTransactions.size()) { - apply(lock); + std::unique_lock lock(mMutex); + if (mDispatchState == DispatchState::running || mTransactions.empty()) + return false; + + do + apply(lock); + while (drain && mTransactions.size()); } + setBatchApplyTimer(); + return true; } void NetworkOPsImp::apply(std::unique_lock& batchLock) { + assert(!mTransactions.empty()); + assert(mDispatchState != DispatchState::running); std::vector submit_held; std::vector transactions; mTransactions.swap(transactions); - assert(!transactions.empty()); - - assert(mDispatchState != DispatchState::running); mDispatchState = DispatchState::running; batchLock.unlock(); @@ -1702,7 +1697,9 @@ NetworkOPsImp::checkLastClosedLedger( switchLedgers = false; } else + { networkClosed = closedLedger; + } if (!switchLedgers) return false; diff --git a/src/ripple/app/misc/NetworkOPs.h b/src/ripple/app/misc/NetworkOPs.h index d53127ed3b6..59285311172 100644 --- a/src/ripple/app/misc/NetworkOPs.h +++ b/src/ripple/app/misc/NetworkOPs.h @@ -71,6 +71,10 @@ enum class OperatingMode { FULL = 4 //!< we have the ledger and can even validate }; +namespace RPC { +enum class SubmitSync; +} + /** Provides server functionality for clients. Clients include backend applications, local commands, and connected @@ -123,22 +127,47 @@ class NetworkOPs : public InfoSub::Source virtual void submitTransaction(std::shared_ptr const&) = 0; - /** - * Process transactions as they arrive from the network or which are - * submitted by clients. Process local transactions synchronously + /** Process a transaction. + * + * The transaction has been submitted either from the peer network or + * from a client. For client submissions, there are 3 distinct behaviors: + * 1) sync (default): process transactions in a batch immediately, + * and return only once the transaction has been processed. + * 2) async: Put transaction into the batch for the next processing + * interval and return immediately. + * 3) wait: Put transaction into the batch for the next processing + * interval and return only after it is processed. * - * @param transaction Transaction object + * @param transaction Transaction object. * @param bUnlimited Whether a privileged client connection submitted it. - * @param bLocal Client submission. - * @param failType fail_hard setting from transaction submission. + * @param sync Client submission synchronous behavior type requested. + * @param bLocal Whether submitted by client (local) or peer. + * @param failType Whether to fail hard or not. */ virtual void processTransaction( std::shared_ptr& transaction, bool bUnlimited, + RPC::SubmitSync sync, bool bLocal, FailHard failType) = 0; + /** Apply transactions in batches. + * + * Only a single batch unless drain is set. This is to optimize performance + * because there is significant overhead in applying each batch, whereas + * processing an individual transaction is fast. + * + * Setting the drain parameter is relevant for some transaction + * processing unit tests that expect all submitted transactions to + * be processed synchronously. + * + * @param drain Whether to process batches until none remain. + * @return Whether any transactions were processed. + */ + virtual bool + transactionBatch(bool drain) = 0; + //-------------------------------------------------------------------------- // // Owner functions @@ -187,6 +216,8 @@ class NetworkOPs : public InfoSub::Source setStandAlone() = 0; virtual void setStateTimer() = 0; + virtual void + setBatchApplyTimer() = 0; virtual void setNeedNetworkLedger() = 0; diff --git a/src/ripple/app/tx/impl/apply.cpp b/src/ripple/app/tx/impl/apply.cpp index c0704c5c3ae..4881f2a49b7 100644 --- a/src/ripple/app/tx/impl/apply.cpp +++ b/src/ripple/app/tx/impl/apply.cpp @@ -134,7 +134,7 @@ applyTransaction( if (retryAssured) flags = flags | tapRETRY; - JLOG(j.debug()) << "TXN " << txn.getTransactionID() + JLOG(j.trace()) << "TXN " << txn.getTransactionID() << (retryAssured ? "/retry" : "/final"); try @@ -142,7 +142,7 @@ applyTransaction( auto const result = apply(app, view, txn, flags, j); if (result.second) { - JLOG(j.debug()) + JLOG(j.trace()) << "Transaction applied: " << transHuman(result.first); return ApplyResult::Success; } @@ -151,17 +151,17 @@ applyTransaction( isTelLocal(result.first)) { // failure - JLOG(j.debug()) + JLOG(j.trace()) << "Transaction failure: " << transHuman(result.first); return ApplyResult::Fail; } - JLOG(j.debug()) << "Transaction retry: " << transHuman(result.first); + JLOG(j.trace()) << "Transaction retry: " << transHuman(result.first); return ApplyResult::Retry; } catch (std::exception const& ex) { - JLOG(j.warn()) << "Throws: " << ex.what(); + JLOG(j.trace()) << "Throws: " << ex.what(); return ApplyResult::Fail; } } diff --git a/src/ripple/basics/SubmitSync.h b/src/ripple/basics/SubmitSync.h new file mode 100644 index 00000000000..12311c676e8 --- /dev/null +++ b/src/ripple/basics/SubmitSync.h @@ -0,0 +1,41 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2023 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_BASICS_SUBMITSYNC_H_INCLUDED +#define RIPPLE_BASICS_SUBMITSYNC_H_INCLUDED + +namespace ripple { +namespace RPC { + +/** + * Possible values for defining synchronous behavior of the transaction + * submission API. + * 1) sync (default): Process transactions in a batch immediately, + * and return only once the transaction has been processed. + * 2) async: Put transaction into the batch for the next processing + * interval and return immediately. + * 3) wait: Put transaction into the batch for the next processing + * interval and return only after it is processed. + */ +enum class SubmitSync { sync, async, wait }; + +} // namespace RPC +} // namespace ripple + +#endif \ No newline at end of file diff --git a/src/ripple/core/Config.h b/src/ripple/core/Config.h index e86157762b3..6236f89fc52 100644 --- a/src/ripple/core/Config.h +++ b/src/ripple/core/Config.h @@ -210,7 +210,7 @@ class Config : public BasicConfig // Node storage configuration std::uint32_t LEDGER_HISTORY = 256; - std::uint32_t FETCH_DEPTH = 1000000000; + std::uint32_t FETCH_DEPTH = 1'000'000'000; // Tunable that adjusts various parameters, typically associated // with hardware parameters (RAM size and CPU cores). The default @@ -227,10 +227,11 @@ class Config : public BasicConfig // Enable the experimental Ledger Replay functionality bool LEDGER_REPLAY = false; - // Work queue limits - int MAX_TRANSACTIONS = 250; - static constexpr int MAX_JOB_QUEUE_TX = 1000; - static constexpr int MIN_JOB_QUEUE_TX = 100; + // Work queue limits. 10000 transactions is 2 full seconds of slowdown at + // 5000/s. + int MAX_TRANSACTIONS = 10'000; + static constexpr int MAX_JOB_QUEUE_TX = 100'000; + static constexpr int MIN_JOB_QUEUE_TX = 1'000; // Amendment majority time std::chrono::seconds AMENDMENT_MAJORITY_TIME = defaultAmendmentMajorityTime; diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 0d58a10abac..3afec605cfa 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -39,13 +40,14 @@ #include #include #include +#include #include -#include #include #include #include +#include #include #include #include @@ -3109,7 +3111,11 @@ PeerImp::checkTransaction( bool const trusted(flags & SF_TRUSTED); app_.getOPs().processTransaction( - tx, trusted, false, NetworkOPs::FailHard::no); + tx, + trusted, + RPC::SubmitSync::async, + false, + NetworkOPs::FailHard::no); } catch (std::exception const& ex) { diff --git a/src/ripple/protocol/TER.h b/src/ripple/protocol/TER.h index a2743bace8d..edae58d83c9 100644 --- a/src/ripple/protocol/TER.h +++ b/src/ripple/protocol/TER.h @@ -208,6 +208,7 @@ enum TERcodes : TERUnderlyingType { terQUEUED, // Transaction is being held in TxQ until fee drops terPRE_TICKET, // Ticket is not yet in ledger but might be on its way terNO_AMM, // AMM doesn't exist for the asset pair + terSUBMITTED // Has been submitted async. }; //------------------------------------------------------------------------------ diff --git a/src/ripple/protocol/impl/TER.cpp b/src/ripple/protocol/impl/TER.cpp index 29b87351204..9da1bc70757 100644 --- a/src/ripple/protocol/impl/TER.cpp +++ b/src/ripple/protocol/impl/TER.cpp @@ -189,6 +189,7 @@ transResults() MAKE_ERROR(terQUEUED, "Held until escalated fee drops."), MAKE_ERROR(terPRE_TICKET, "Ticket is not yet in ledger."), MAKE_ERROR(terNO_AMM, "AMM doesn't exist for the asset pair."), + MAKE_ERROR(terSUBMITTED, "Transaction has been submitted."), MAKE_ERROR(tesSUCCESS, "The transaction was applied. Only final in a validated ledger."), }; diff --git a/src/ripple/protocol/jss.h b/src/ripple/protocol/jss.h index a0f81858637..a74b505009b 100644 --- a/src/ripple/protocol/jss.h +++ b/src/ripple/protocol/jss.h @@ -598,6 +598,7 @@ JSS(sub_index); // in: LedgerEntry JSS(subcommand); // in: PathFind JSS(success); // rpc JSS(supported); // out: AmendmentTableImpl +JSS(sync_mode); // in: Submit JSS(system_time_offset); // out: NetworkOPs JSS(tag); // out: Peers JSS(taker); // in: Subscribe, BookOffers diff --git a/src/ripple/rpc/handlers/Submit.cpp b/src/ripple/rpc/handlers/Submit.cpp index 2b5c8bba925..8a702c5bd3e 100644 --- a/src/ripple/rpc/handlers/Submit.cpp +++ b/src/ripple/rpc/handlers/Submit.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -48,6 +49,10 @@ doSubmit(RPC::JsonContext& context) { context.loadType = Resource::feeMediumBurdenRPC; + auto const sync = RPC::getSubmitSyncMode(context.params); + if (!sync) + return sync.error(); + if (!context.params.isMember(jss::tx_blob)) { auto const failType = getFailHard(context); @@ -62,7 +67,8 @@ doSubmit(RPC::JsonContext& context) context.role, context.ledgerMaster.getValidatedLedgerAge(), context.app, - RPC::getProcessTxnFn(context.netOps)); + RPC::getProcessTxnFn(context.netOps), + *sync); ret[jss::deprecated] = "Signing support in the 'submit' command has been " @@ -131,7 +137,7 @@ doSubmit(RPC::JsonContext& context) auto const failType = getFailHard(context); context.netOps.processTransaction( - tpTrans, isUnlimited(context.role), true, failType); + tpTrans, isUnlimited(context.role), *sync, true, failType); } catch (std::exception& e) { diff --git a/src/ripple/rpc/handlers/SubmitMultiSigned.cpp b/src/ripple/rpc/handlers/SubmitMultiSigned.cpp index dbae6e95f7a..9b455a1961f 100644 --- a/src/ripple/rpc/handlers/SubmitMultiSigned.cpp +++ b/src/ripple/rpc/handlers/SubmitMultiSigned.cpp @@ -18,10 +18,12 @@ //============================================================================== #include +#include #include #include #include #include +#include #include namespace ripple { @@ -37,13 +39,18 @@ doSubmitMultiSigned(RPC::JsonContext& context) auto const failHard = context.params[jss::fail_hard].asBool(); auto const failType = NetworkOPs::doFailHard(failHard); + auto const sync = RPC::getSubmitSyncMode(context.params); + if (!sync) + return sync.error(); + return RPC::transactionSubmitMultiSigned( context.params, failType, context.role, context.ledgerMaster.getValidatedLedgerAge(), context.app, - RPC::getProcessTxnFn(context.netOps)); + RPC::getProcessTxnFn(context.netOps), + *sync); } } // namespace ripple diff --git a/src/ripple/rpc/impl/RPCHelpers.cpp b/src/ripple/rpc/impl/RPCHelpers.cpp index bc38df62fc9..d9d24c81f79 100644 --- a/src/ripple/rpc/impl/RPCHelpers.cpp +++ b/src/ripple/rpc/impl/RPCHelpers.cpp @@ -1165,5 +1165,26 @@ getLedgerByContext(RPC::JsonContext& context) return RPC::make_error( rpcNOT_READY, "findCreate failed to return an inbound ledger"); } + +ripple::Expected +getSubmitSyncMode(Json::Value const& params) +{ + using enum RPC::SubmitSync; + if (params.isMember(jss::sync_mode)) + { + std::string const syncMode = params[jss::sync_mode].asString(); + if (syncMode == "async") + return async; + else if (syncMode == "wait") + return wait; + else if (syncMode != "sync") + return Unexpected(RPC::make_error( + rpcINVALID_PARAMS, + "sync_mode parameter must be one of \"sync\", \"async\", or " + "\"wait\".")); + } + return sync; +} + } // namespace RPC } // namespace ripple diff --git a/src/ripple/rpc/impl/RPCHelpers.h b/src/ripple/rpc/impl/RPCHelpers.h index 0293fb15a21..5fa7ae804a2 100644 --- a/src/ripple/rpc/impl/RPCHelpers.h +++ b/src/ripple/rpc/impl/RPCHelpers.h @@ -26,6 +26,8 @@ #include #include +#include +#include #include #include #include @@ -293,6 +295,14 @@ getAPIVersionNumber(const Json::Value& value, bool betaEnabled); std::variant, Json::Value> getLedgerByContext(RPC::JsonContext& context); +/** Helper to parse submit_mode parameter to RPC submit. + * + * @param params RPC parameters + * @return Either the mode or an error object. + */ +ripple::Expected +getSubmitSyncMode(Json::Value const& params); + } // namespace RPC } // namespace ripple diff --git a/src/ripple/rpc/impl/TransactionSign.cpp b/src/ripple/rpc/impl/TransactionSign.cpp index 7610682fd1a..5cd0c9edee3 100644 --- a/src/ripple/rpc/impl/TransactionSign.cpp +++ b/src/ripple/rpc/impl/TransactionSign.cpp @@ -802,7 +802,8 @@ transactionSubmit( Role role, std::chrono::seconds validatedLedgerAge, Application& app, - ProcessTransactionFn const& processTransaction) + ProcessTransactionFn const& processTransaction, + RPC::SubmitSync sync) { using namespace detail; @@ -828,8 +829,7 @@ transactionSubmit( // Finally, submit the transaction. try { - // FIXME: For performance, should use asynch interface - processTransaction(txn.second, isUnlimited(role), true, failType); + processTransaction(txn.second, isUnlimited(role), sync, failType); } catch (std::exception&) { @@ -1038,7 +1038,8 @@ transactionSubmitMultiSigned( Role role, std::chrono::seconds validatedLedgerAge, Application& app, - ProcessTransactionFn const& processTransaction) + ProcessTransactionFn const& processTransaction, + RPC::SubmitSync sync) { auto const& ledger = app.openLedger().current(); auto j = app.journal("RPCHandler"); @@ -1211,7 +1212,7 @@ transactionSubmitMultiSigned( try { // FIXME: For performance, should use asynch interface - processTransaction(txn.second, isUnlimited(role), true, failType); + processTransaction(txn.second, isUnlimited(role), sync, failType); } catch (std::exception&) { diff --git a/src/ripple/rpc/impl/TransactionSign.h b/src/ripple/rpc/impl/TransactionSign.h index d9c76e189f0..a396e65af52 100644 --- a/src/ripple/rpc/impl/TransactionSign.h +++ b/src/ripple/rpc/impl/TransactionSign.h @@ -21,6 +21,7 @@ #define RIPPLE_RPC_TRANSACTIONSIGN_H_INCLUDED #include +#include #include #include @@ -75,7 +76,7 @@ checkFee( using ProcessTransactionFn = std::function& transaction, bool bUnlimited, - bool bLocal, + RPC::SubmitSync sync, NetworkOPs::FailHard failType)>; inline ProcessTransactionFn @@ -84,9 +85,10 @@ getProcessTxnFn(NetworkOPs& netOPs) return [&netOPs]( std::shared_ptr& transaction, bool bUnlimited, - bool bLocal, + RPC::SubmitSync sync, NetworkOPs::FailHard failType) { - netOPs.processTransaction(transaction, bUnlimited, bLocal, failType); + netOPs.processTransaction( + transaction, bUnlimited, sync, true, failType); }; } @@ -107,7 +109,8 @@ transactionSubmit( Role role, std::chrono::seconds validatedLedgerAge, Application& app, - ProcessTransactionFn const& processTransaction); + ProcessTransactionFn const& processTransaction, + RPC::SubmitSync sync); /** Returns a Json::objectValue. */ Json::Value @@ -126,7 +129,8 @@ transactionSubmitMultiSigned( Role role, std::chrono::seconds validatedLedgerAge, Application& app, - ProcessTransactionFn const& processTransaction); + ProcessTransactionFn const& processTransaction, + RPC::SubmitSync sync); } // namespace RPC } // namespace ripple diff --git a/src/test/app/Transaction_ordering_test.cpp b/src/test/app/Transaction_ordering_test.cpp index 0353df90663..01f870d0668 100644 --- a/src/test/app/Transaction_ordering_test.cpp +++ b/src/test/app/Transaction_ordering_test.cpp @@ -15,6 +15,7 @@ */ //============================================================================== +#include #include #include #include @@ -91,6 +92,7 @@ struct Transaction_ordering_test : public beast::unit_test::suite env(tx2, ter(terPRE_SEQ)); BEAST_EXPECT(env.seq(alice) == aliceSequence); env(tx1); + BEAST_EXPECT(env.app().getOPs().transactionBatch(false)); env.app().getJobQueue().rendezvous(); BEAST_EXPECT(env.seq(alice) == aliceSequence + 2); @@ -143,6 +145,8 @@ struct Transaction_ordering_test : public beast::unit_test::suite } env(tx[0]); + // Apply until no more deferred/held transactions. + BEAST_EXPECT(env.app().getOPs().transactionBatch(true)); env.app().getJobQueue().rendezvous(); BEAST_EXPECT(env.seq(alice) == aliceSequence + 5); diff --git a/src/test/jtx/Env_test.cpp b/src/test/jtx/Env_test.cpp index 6f09f49ed5d..6e26a40e25a 100644 --- a/src/test/jtx/Env_test.cpp +++ b/src/test/jtx/Env_test.cpp @@ -29,6 +29,7 @@ #include #include +#include #include namespace ripple { @@ -900,6 +901,95 @@ class Env_test : public beast::unit_test::suite pass(); } + void + testSyncSubmit() + { + using namespace jtx; + Env env(*this); + + auto const alice = Account{"alice"}; + auto const n = XRP(10000); + env.fund(n, alice); + BEAST_EXPECT(env.balance(alice) == n); + + // submit only + auto applyBlobTxn = [&env](char const* syncMode, auto&&... txnArgs) { + auto jt = env.jt(txnArgs...); + Serializer s; + jt.stx->add(s); + + Json::Value args{Json::objectValue}; + + args[jss::tx_blob] = strHex(s.slice()); + args[jss::fail_hard] = true; + args[jss::sync_mode] = syncMode; + + return env.rpc("json", "submit", args.toStyledString()); + }; + + auto jr = applyBlobTxn("sync", noop(alice)); + BEAST_EXPECT(jr[jss::result][jss::engine_result] == "tesSUCCESS"); + + jr = applyBlobTxn("async", noop(alice)); + BEAST_EXPECT(jr[jss::result][jss::engine_result] == "terSUBMITTED"); + // Make sure it gets processed before submitting and waiting. + env.app().getOPs().transactionBatch(true); + + auto applier = [&env]() { + while (!env.app().getOPs().transactionBatch(false)) + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + }; + auto t = std::thread(applier); + + jr = applyBlobTxn("wait", noop(alice)); + BEAST_EXPECT(jr[jss::result][jss::engine_result] == "tesSUCCESS"); + t.join(); + + jr = applyBlobTxn("scott", noop(alice)); + BEAST_EXPECT(jr[jss::result][jss::error] == "invalidParams"); + + // sign and submit + auto applyJsonTxn = [&env]( + char const* syncMode, + std::string const secret, + Json::Value const& val) { + Json::Value args{Json::objectValue}; + args[jss::secret] = secret; + args[jss::tx_json] = val; + args[jss::fail_hard] = true; + args[jss::sync_mode] = syncMode; + + return env.rpc("json", "submit", args.toStyledString()); + }; + + Json::Value payment; + auto secret = toBase58(generateSeed("alice")); + payment = noop("alice"); + payment[sfSequence.fieldName] = env.seq("alice"); + payment[sfSetFlag.fieldName] = 0; + jr = applyJsonTxn("sync", secret, payment); + BEAST_EXPECT(jr[jss::result][jss::engine_result] == "tesSUCCESS"); + + payment[sfSequence.fieldName] = env.seq("alice"); + jr = applyJsonTxn("async", secret, payment); + BEAST_EXPECT(jr[jss::result][jss::engine_result] == "terSUBMITTED"); + + env.app().getOPs().transactionBatch(true); + payment[sfSequence.fieldName] = env.seq("alice"); + + auto aSeq = env.seq("alice"); + t = std::thread(applier); + jr = applyJsonTxn("wait", secret, payment); + BEAST_EXPECT(jr[jss::result][jss::engine_result] == "tesSUCCESS"); + t.join(); + // Ensure the last transaction was processed. + BEAST_EXPECT(env.seq("alice") == aSeq + 1); + + payment[sfSequence.fieldName] = env.seq("alice"); + jr = applyJsonTxn("scott", secret, payment); + BEAST_EXPECT(jr[jss::result][jss::error] == "invalidParams"); + } + void run() override { @@ -925,6 +1015,7 @@ class Env_test : public beast::unit_test::suite testSignAndSubmit(); testFeatures(); testExceptionalShutdown(); + testSyncSubmit(); } }; diff --git a/src/test/rpc/JSONRPC_test.cpp b/src/test/rpc/JSONRPC_test.cpp index a0970bbd746..b6e54967c40 100644 --- a/src/test/rpc/JSONRPC_test.cpp +++ b/src/test/rpc/JSONRPC_test.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -2384,7 +2385,7 @@ class JSONRPC_test : public beast::unit_test::suite fakeProcessTransaction( std::shared_ptr&, bool, - bool, + SubmitSync, NetworkOPs::FailHard) { ; @@ -2432,7 +2433,8 @@ class JSONRPC_test : public beast::unit_test::suite Role role, std::chrono::seconds validatedLedgerAge, Application & app, - ProcessTransactionFn const& processTransaction); + ProcessTransactionFn const& processTransaction, + RPC::SubmitSync sync); using TestStuff = std::tuple; @@ -2485,7 +2487,8 @@ class JSONRPC_test : public beast::unit_test::suite testRole, 1s, env.app(), - processTxn); + processTxn, + RPC::SubmitSync::sync); } std::string errStr; diff --git a/src/test/rpc/RobustTransaction_test.cpp b/src/test/rpc/RobustTransaction_test.cpp index 37b16c58d7f..01ac71e272a 100644 --- a/src/test/rpc/RobustTransaction_test.cpp +++ b/src/test/rpc/RobustTransaction_test.cpp @@ -17,6 +17,7 @@ */ //============================================================================== +#include #include #include #include @@ -88,7 +89,8 @@ class RobustTransaction_test : public beast::unit_test::suite } BEAST_EXPECT(jv[jss::result][jss::engine_result] == "tefPAST_SEQ"); - // Submit future sequence transaction + // Submit future sequence transaction -- this transaction should be + // held until the sequence gap is closed. payment[jss::tx_json][sfSequence.fieldName] = env.seq("alice") + 1; jv = wsc->invoke("submit", payment); if (wsc->version() == 2) @@ -114,6 +116,8 @@ class RobustTransaction_test : public beast::unit_test::suite } BEAST_EXPECT(jv[jss::result][jss::engine_result] == "tesSUCCESS"); + // Apply held transactions. + env.app().getOPs().transactionBatch(true); // Wait for the jobqueue to process everything env.app().getJobQueue().rendezvous(); From a082a2e64308af5884ca0ab21391082f94d6d117 Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Thu, 20 Apr 2023 12:01:27 -0700 Subject: [PATCH 10/13] Asynchronously write batches to NuDB. (#4503) Signed-off-by: Manoj Doshi --- src/ripple/nodestore/Types.h | 3 ++- src/ripple/nodestore/backend/NuDBFactory.cpp | 22 ++++++++++++-------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/ripple/nodestore/Types.h b/src/ripple/nodestore/Types.h index 6d8583ed9d1..08bcac2d25c 100644 --- a/src/ripple/nodestore/Types.h +++ b/src/ripple/nodestore/Types.h @@ -36,8 +36,9 @@ enum { // This sets a limit on the maximum number of writes // in a batch. Actual usage can be twice this since // we have a new batch growing as we write the old. + // The main goal is to avoid delays while persisting the ledger. // - batchWriteLimitSize = 65536 + batchWriteLimitSize = 262144 }; /** Return codes from Backend operations. */ diff --git a/src/ripple/nodestore/backend/NuDBFactory.cpp b/src/ripple/nodestore/backend/NuDBFactory.cpp index 30b848e82af..74afe2ed7f1 100644 --- a/src/ripple/nodestore/backend/NuDBFactory.cpp +++ b/src/ripple/nodestore/backend/NuDBFactory.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -35,7 +36,7 @@ namespace ripple { namespace NodeStore { -class NuDBBackend : public Backend +class NuDBBackend : public Backend, public BatchWriter::Callback { public: static constexpr std::uint64_t currentType = 1; @@ -46,6 +47,7 @@ class NuDBBackend : public Backend beast::Journal const j_; size_t const keyBytes_; + BatchWriter batch_; std::size_t const burstSize_; std::string const name_; nudb::store db_; @@ -60,6 +62,7 @@ class NuDBBackend : public Backend beast::Journal journal) : j_(journal) , keyBytes_(keyBytes) + , batch_(*this, scheduler) , burstSize_(burstSize) , name_(get(keyValues, "path")) , deletePath_(false) @@ -79,6 +82,7 @@ class NuDBBackend : public Backend beast::Journal journal) : j_(journal) , keyBytes_(keyBytes) + , batch_(*this, scheduler) , burstSize_(burstSize) , name_(get(keyValues, "path")) , db_(context) @@ -262,13 +266,7 @@ class NuDBBackend : public Backend void store(std::shared_ptr const& no) override { - BatchWriteReport report; - report.writeCount = 1; - auto const start = std::chrono::steady_clock::now(); - do_insert(no); - report.elapsed = std::chrono::duration_cast( - std::chrono::steady_clock::now() - start); - scheduler_.onBatchWrite(report); + batch_.store(no); } void @@ -329,7 +327,7 @@ class NuDBBackend : public Backend int getWriteLoad() override { - return 0; + return batch_.getWriteLoad(); } void @@ -357,6 +355,12 @@ class NuDBBackend : public Backend Throw(ec); } + void + writeBatch(Batch const& batch) override + { + storeBatch(batch); + } + int fdRequired() const override { From df655487c597837b8a8eb6a6a8378c25e63725d1 Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Thu, 20 Apr 2023 13:46:40 -0700 Subject: [PATCH 11/13] Several changes to improve Consensus stability: (#4505) * Verify accepted ledger becomes validated, and retry with a new consensus transaction set if not. * Always store proposals. * Track proposals by ledger sequence. This helps slow peers catch up with the rest of the network. * Acquire transaction sets for proposals with future ledger sequences. This also helps slow peers catch up. * Optimize timer delay for establish phase to wait based on how long validators have been sending proposals. This also helps slow peers to catch up. * Fix impasse achieving close time consensus. * Don't wait between open and establish phases. --- 1 | 32 ++ Builds/levelization/results/ordering.txt | 3 + src/ripple/app/consensus/RCLConsensus.cpp | 118 +++-- src/ripple/app/consensus/RCLConsensus.h | 178 ++++++- src/ripple/app/consensus/RCLCxPeerPos.h | 3 +- src/ripple/app/ledger/LedgerMaster.h | 30 +- src/ripple/app/ledger/impl/LedgerMaster.cpp | 2 + src/ripple/app/misc/NetworkOPs.cpp | 17 +- src/ripple/app/misc/impl/ValidatorList.cpp | 5 +- .../detail/aged_unordered_container.h | 5 + src/ripple/consensus/Consensus.cpp | 16 +- src/ripple/consensus/Consensus.h | 478 +++++++++++++++--- src/ripple/consensus/ConsensusParms.h | 10 +- src/ripple/consensus/ConsensusProposal.h | 41 +- src/ripple/consensus/ConsensusTypes.h | 6 +- src/ripple/consensus/DisputedTx.h | 20 +- src/ripple/overlay/impl/PeerImp.cpp | 9 +- src/ripple/proto/ripple.proto | 2 + src/test/consensus/Consensus_test.cpp | 31 +- src/test/csf/Peer.h | 124 ++++- src/test/csf/Proposal.h | 2 +- 21 files changed, 945 insertions(+), 187 deletions(-) create mode 100644 1 diff --git a/1 b/1 new file mode 100644 index 00000000000..8484bf265ce --- /dev/null +++ b/1 @@ -0,0 +1,32 @@ +r aded4a7a92 docs: add API Changelog (#4612) + +# Rebase 01df19c08b..aded4a7a92 onto 01df19c08b (1 command) +# +# Commands: +# p, pick = use commit +# r, reword = use commit, but edit the commit message +# e, edit = use commit, but stop for amending +# s, squash = use commit, but meld into previous commit +# f, fixup [-C | -c] = like "squash" but keep only the previous +# commit's log message, unless -C is used, in which case +# keep only this commit's message; -c is same as -C but +# opens the editor +# x, exec = run command (the rest of the line) using shell +# b, break = stop here (continue rebase later with 'git rebase --continue') +# d, drop = remove commit +# l, label