Closed
Conversation
* Enable api_version 2, which is currently in beta. It is expected to be marked stable by the next stable release. * This does not change any defaults. * The only existing tests changed were one that set the same flag, which was now redundant, and a couple that tested versioning explicitly.
Curtail the occurrence of order books that are blocked by reduced offers with the implementation of the fixReducedOffersV1 amendment. This commit identifies three ways in which offers can be reduced: 1. A new offer can be partially crossed by existing offers, so the new offer is reduced when placed in the ledger. 2. An in-ledger offer can be partially crossed by a new offer in a transaction. So the in-ledger offer is reduced by the new offer. 3. An in-ledger offer may be under-funded. In this case the in-ledger offer is scaled down to match the available funds. Reduced offers can block order books if the effective quality of the reduced offer is worse than the quality of the original offer (from the perspective of the taker). It turns out that, for small values, the quality of the reduced offer can be significantly affected by the rounding mode used during scaling computations. This commit adjusts some rounding modes so that the quality of a reduced offer is always at least as good (from the taker's perspective) as the original offer. The amendment is titled fixReducedOffersV1 because additional ways of producing reduced offers may come to light. Therefore, there may be a future need for a V2 amendment.
Enhance the /crawl endpoint by publishing WebSocket/RPC ports in the server_info response. The function processing requests to the /crawl endpoint actually calls server_info internally, so this change enables a server to advertise its WebSocket/RPC port(s) to peers via the /crawl endpoint. `grpc` and `peer` ports are included as well. The new `ports` array contains objects, each containing a `port` for the listening port (number string), and an array `protocol` listing the supported protocol(s). This allows crawlers to build a richer topology without needing to port-scan nodes. For non-admin users (including peers), the info about *admin* ports is excluded. Also increase test coverage for RPC ServerInfo. Fix #2837.
Apply a minor cleanup in `TypedField`: * Remove a non-working and unused move constructor. * Constrain the remaining constructor to not be overly generic enough as to be used as a copy or move constructor.
Introduces: * AccountRoot flag: lsfAllowClawback * New Clawback transaction * More info on clawback spec: https://github.com/XRPLF/XRPL-Standards/tree/master/XLS-39d-clawback
Replace hand-rolled code with std::from_chars for better maintainability. The C++ std::from_chars function is intended to be as fast as possible, so it is unlikely to be slower than the code it replaces. This change is a net gain because it reduces the amount of hand-rolled code.
Remove deprecated fields from the ledger command: * accepted * hash (use ledger_hash instead) * seqNum (use ledger_index instead) * totalCoins (use total_coins instead) Update SHAMapStore unit tests to use `jss:ledger_hash` instead of the deprecated `hash` field. Fix #3214
Rename `ServerHandlerImp` to `ServerHandler`. There was no other ServerHandler definition despite the existence of a header suggesting that there was. This resolves a piece of historical confusion in the code, which was identified during a code review. The changes in the diff may look more extensive than they actually are. The contents of `impl/ServerHandlerImp.h` were merged into `ServerHandler.h`, making the latter file appear to have undergone significant modifications. However, this is a non-breaking refactor that only restructures code.
* Commits 0b812cd (#4427) and 11e914f (#4516) conflict. The first added references to `ServerHandlerImp` in files outside of that class's organizational unit (which is technically incorrect). The second removed `ServerHandlerImp`, but was not up to date with develop. This results in the build failing. * Fixes the build by changing references to `ServerHandlerImp` to the more correct `ServerHandler`.
Certain inputs for the AccountTx method should return an error. In other words, an invalid request from a user or client now results in an error message. Since this can change the response from the API, it is an API breaking change. This commit maintains backward compatibility by keeping the existing behavior for existing requests. When clients specify "api_version": 2, they will be able to get the updated error messages. Update unit tests to check the error based on the API version. * Fix #4288 * Fix #4545
- Use powers of two to clearly indicate the bitmask - Replace bitmask with explicit if-conditions to better indicate predicates Change enum values to be powers of two (fix #3417) #4239 Implement the simplified condition evaluation removes the complex bitwise and(&) operator Implement the second proposed solution in Nik Bougalis's comment - Software does not distinguish between different Conditions (Version: 1.5) #3417 (comment) I have tested this code change by performing RPC calls with the commands server_info, server_state, peers and validation_info. These commands worked as expected.
When requesting `account_info` with an invalid `signer_lists` value, the API should return an "invalidParams" error. `signer_lists` should have a value of type boolean. If it is not a boolean, then it is invalid input. The response now indicates that. * This is an API breaking change, so the change is only reflected for requests containing `"api_version": 2` * Fix #4539
* Update the `account_info` API so that the `allowClawback` flag is
included in the response.
* The proposed `Clawback` amendement added an `allowClawback` flag in
the `AccountRoot` object.
* In the API response, under `account_flags`, there is now an
`allowClawback` field with a boolean (`true` or `false`) value.
* For reference, the XLS-39 Clawback implementation can be found in
#4553
Fix #4588
- Previously, mulDiv had `std::pair<bool, uint64_t>` as the output type.
- This is an error-prone interface as it is easy to ignore when
overflow occurs.
- Using a return type of `std::optional` should decrease the likelihood
of ignoring overflow.
- It also allows for the use of optional::value_or() as a way to
explicitly recover from overflow.
- Include limits.h header file preprocessing directive in order to
satisfy gcc's numeric_limits incomplete_type requirement.
Fix #3495
---------
Co-authored-by: John Freeman <jfreeman08@gmail.com>
Improve error handling for ledger_entry by returning an "invalidParams" error when one or more request fields are specified incorrectly, or one or more required fields are missing. For example, if none of of the following fields is provided, then the API should return an invalidParams error: * index, account_root, directory, offer, ripple_state, check, escrow, payment_channel, deposit_preauth, ticket Prior to this commit, the API returned an "unknownOption" error instead. Since the error was actually due to invalid parameters, rather than unknown options, this error was misleading. Since this is an API breaking change, the "invalidParams" error is only returned for requests using api_version: 2 and above. To maintain backward compatibility, the "unknownOption" error is still returned for api_version: 1. Related: #4573 Fix #4303
Sections that were rewrapped were wrapped to 72 characters, the same as the recommendation for commit messages.
Add AMM functionality:
- InstanceCreate
- Deposit
- Withdraw
- Governance
- Auctioning
- payment engine integration
To support this functionality, add:
- New RPC method, `amm_info`, to fetch pool and LPT balances
- AMM Root Account
- trust line for each IOU AMM token
- trust line to track Liquidity Provider Tokens (LPT)
- `ltAMM` object
The `ltAMM` object tracks:
- fee votes
- auction slot bids
- AMM tokens pair
- total outstanding tokens balance
- `AMMID` to AMM `RootAccountID` mapping
Add new classes to facilitate AMM integration into the payment engine.
`BookStep` uses these classes to infer if AMM liquidity can be consumed.
The AMM formula implementation uses the new Number class added in #4192.
IOUAmount and STAmount use Number arithmetic.
Add AMM unit tests for all features.
AMM requires the following amendments:
- featureAMM
- fixUniversalNumber
- featureFlowCross
Notes:
- Current trading fee threshold is 1%
- AMM currency is generated by: 0x03 + 152 bits of sha256{cur1, cur2}
- Current max AMM Offers is 30
---------
Co-authored-by: Howard Hinnant <howard.hinnant@gmail.com>
Reason for this change is here XRPLF/XRPL-Standards#119 We would want to be explicit that this flag is exclusively for trustline. For new token types(eg. CFT), they will not utilize this flag for clawback, instead, they will turn clawback on/off on the token-level, which is more versatile.
* add CTIM to tx rpc --------- Co-authored-by: Rome Reginelli <mduo13@gmail.com> Co-authored-by: Elliot Lee <github.public@intelliot.com> Co-authored-by: Denis Angell <dangell@transia.co>
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
Signed-off-by: Manoj Doshi <mdoshi@ripple.com>
- 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 <mdoshi@ripple.com>
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 <mdoshi@ripple.com>
Fix the libxrpl library target for consumers using Conan. * Fix installation issues and update includes. * Update requirements in the Conan package info. * libxrpl requires openssl::crypto. (Conan is a software package manager for C++.)
- "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 XRPLF/XRPL-Standards#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 <mdoshi@ripple.com>
Signed-off-by: Manoj Doshi <mdoshi@ripple.com>
Introduce a new variadic template helper function, `forAllApiVersions`, that accepts callables to execute a set of functions over a range of versions - from RPC::apiMinimumSupportedVersion to RPC::apiBetaVersion. This avoids the duplication of code. Context: #4552
covert -> convert
Modify two error cases in AMMBid transactor to return `tecINTERNAL` to more clearly indicate that these errors should not be possible unless operating in unforeseen circumstances. It likely indicates a bug. The log level has been updated to `fatal()` since it indicates a (potentially network-wide) unexpected condition when either of these errors occurs. Details: The two specific transaction error cases changed are: - `tecAMM_BALANCE` - In this case, this error (total LP Tokens outstanding is lower than the amount to be burned for the bid) is a subset of the case where the user doesn't have enough LP Tokens to pay for the bid. When this case is reached, the bidder's LP Tokens balance has already been checked first. The user's LP Tokens should always be a subset of total LP Tokens issued, so this should be impossible. - `tecINSUFFICIENT_PAYMENT` - In this case, the amount to be refunded as a result of the bid is greater than the price paid for the auction slot. This should never occur unless something is wrong with the math for calculating the refund amount. Both error cases in question are "defense in depth" measures meant to protect against making things worse if the code has already reached a state that is supposed to be impossible, likely due to a bug elsewhere. Such "shouldn't ever occur" checks should use an error code that categorically indicates a larger problem. This is similar to how `tecINVARIANT_FAILED` is a warning sign that something went wrong and likely could've been worse, but since there isn't an Invariant Check applying here, `tecINTERNAL` is the appropriate error code. This is "debatably" a transaction processing change since it could hypothetically change how transactions are processed if there's a bug we don't know about.
- Update amm_info to fetch AMM by amm account id. - This is an additional way to retrieve an AMM object. - Alternatively, AMM can still be fetched by the asset pair as well. - Add owner directory entry for AMM object. Context: - Add back the AMM object directory entry, which was deleted by #4626. - This fixes `account_objects` for `amm` type.
Use the most recent versions in ConanCenter. * Due to a bug in Clang 16, you may get a compile error: "call to 'async_teardown' is ambiguous" * A compiler flag workaround is documented in `BUILD.md`. * At this time, building this with gcc 13 may require editing some files in `.conan/data` * A patch to support gcc13 may be added in a later PR. --------- Co-authored-by: Scott Schurr <scott@ripple.com>
tequdev
force-pushed
the
sync-1.12.0-conan
branch
from
41f355d to
52d3bab
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
18 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
High Level Overview of Change
syncing rippled (1.12.0) based on the Conan branch
The following files show differences in clang-format, but rippled ignores these differences and merges them into the develop branch.
changed files: src/secp256k1/src/bench_ecdh.c src/secp256k1/src/bench_recover.c src/secp256k1/src/bench_schnorr_verify.c src/secp256k1/src/bench_sign.c src/secp256k1/src/bench_verify.c src/secp256k1/src/gen_context.c src/secp256k1/src/java/org_bitcoin_NativeSecp256k1.c src/secp256k1/src/java/org_bitcoin_NativeSecp256k1.h src/secp256k1/src/java/org_bitcoin_Secp256k1Context.c src/secp256k1/src/java/org_bitcoin_Secp256k1Context.h src/secp256k1/src/modules/ecdh/main_impl.h src/secp256k1/src/tests_exhaustive.cContext of Change
Type of Change
.gitignore, formatting, dropping support for older tooling)API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)#4485 XRPLF/rippled@6b4437d (NOT MERGED - Build(Conan))Merged at below#4595 XRPLF/rippled@cb09e61 (NOT MERGED - Build/Update dependencies)