Minor refactor to `TxFormats.cpp`:
- Rename `commonFields` to `pseudoCommonFields` (since it is the common fields
that all pseudo-transactions need)
- Add a new static variable, `commonFields`, which represents all the common
fields that non-pseudo transactions need (essentially everything that
`pseudoCommonFields` contains, plus `sfTicketSequence`)
This makes it harder to accidentally leave out `sfTicketSequence` in a new
transaction.
- Verify "check", used to retrieve a Check object, is a string.
- Verify "nft_page", used to retrieve an NFT Page, is a string.
- Verify "index", used to retrieve any type of ledger object by its
unique ID, is a string.
- Verify "directory", used to retrieve a DirectoryNode, is a string or
an object.
This change only impacts api_version 2 since it is a breaking change.
https://xrpl.org/ledger_entry.htmlFix#4550
* In namespace ripple, introduces get_name function that takes a
std:🧵:native_handle_type and returns a std::string.
* In namespace ripple, introduces get_name function that takes a
std::thread or std::jthread and returns a std::string.
* In namespace ripple::this_thread, introduces get_name function
that takes no parameters and returns the name of the current
thread as a std::string.
* In namespace ripple::this_thread, introduces set_name function
that takes a std::string_view and sets the name of the current
thread.
* Intended to replace the beast utilities setCurrentThreadName
and getCurrentThreadName.
- 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.
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.
* 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.
Add new transaction submission API field, "sync", which
determines behavior of the server while submitting transactions:
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.
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
- "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 <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>
- 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>
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>
Introduce an API Changelog, which logs the changes that have been made
to the API.
Without this changelog, it is difficult to keep track of the changes and
additions that are made to the API. While all changes are surfaced in
PRs and Release Notes, these are mixed in with other non-API-affecting
changes. PRs that affect the API have the `API Change` label applied,
but it is hard to identify which PRs have been included in each release.
Furthermore, some API changes will take effect based on `api_version`
(starting with rippled version 1.12, which will introduce `api_version:
2`), while others are based on the `rippled` version.
The API Changelog clarifies the details of the changes in a way that is
easily understood by API consumers, and breaks down the changes to be
clear which ones are gated by api_version (versus `rippled` version).
From now on, all PR authors are responsible for updating the API
Changelog according to the additions/changes that their PR makes to the
APIs.
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.
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>
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>
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: #4573Fix#4303
- 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>
* 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
#4553Fix#4588
Enhance security during the build process:
* The '-fstack-protector' flag enables stack protection for preventing
buffer overflow vulnerabilities. If an attempt is made to overflow the
buffer, the program will terminate, thus protecting the integrity of
the stack.
* The '-Wl,-z,relro,-z,now' linker flag enables Read-only Relocations
(RELRO), a feature that helps harden the binary against certain types
of exploits, particularly those that involve overwriting the Global
Offset Table (GOT).
* This flag is only set for Linux builds, due to compatibility issues
with apple-clang.
* The `relro` option makes certain sections of memory read-only after
initialization to prevent them from being overwritten, while `now`
ensures that all dynamic symbols are resolved immediately on program
start, reducing the window of opportunity for attacks.
* Add a new YAML file (.pre-commit-config.yaml) to set up pre-commit
hook for clang-format
* The pre-commit hook is opt-in and needs to be installed in order to
run automatically
* Update CONTRIBUTING.md with instructions on how to set up and use the
clang-format linter
Automating the process of running clang-format before committing code
helps to save time by removing the need to fix formatting issues later.
This is a tooling improvement and doesn't change C++ code.
* Update the version of the checkout action (for GitHub Actions) in
`clang-format.yml` and `levelization.yml`.
* The previous version, v2, was raising deprecation warnings due to
its reliance on Node.js 12.
* The latest checkout action version, v3, uses Node.js 16.
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
The debug packages were named with the extension ".ddeb", but due to a
bug in Artifactory, they need to have the ".deb" extension. Debug symbol
packages with ".ddeb" extensions are not indexed, and thus are not
visible in apt clients.
* Fix the issue by renaming the debug packages in the build script.
* Use GCC-11 and update GCC Conan profile.
* This software requires GCC 11 and C++20. However, reporting mode is
built with C++17.
This is a quick band-aid to fix the build. Later, it will be better to
remove this package-building code.
For context, a Debian (deb) package contains bundled software and
resources necessary for installing and managing software on a
Debian-based system, including Ubuntu and derivatives.
- 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.
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
* 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`.
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.
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++.)
