* Hook API Refactor2: Amendment Guards (#621)
* Hook API Refactor3: Consolidate the Hook API definitions from Enum.h and ApplyHook.h into a single file. (#622)
* Hook API Refactoring / Unit Testing (#581)
* Hook API Refactor2: Amendment Guards (#621)
* Hook API Refactor3: Consolidate the Hook API definitions from Enum.h and ApplyHook.h into a single file. (#622)
* Hook API Refactoring / Unit Testing (#581)
* fix: update clang-format to v18 and fix include ordering
- Update verify-generated-headers CI to use clang-format 18 (matching
clang-format.yml) instead of stale v10 which can't parse .clang-format
- Add .mise.toml for local clang-format 18 tooling
- Fix include ordering in cherry-picked files per clang-format 18
* chore: update levelization results for HookAPI changes
New loop: xrpl.hook <-> xrpld.app due to HookAPI.h including
Transaction.h from xrpld.app.
---------
Co-authored-by: tequ <git@tequ.dev>
Fixes the use of high and low in variable names, as these are determined by ripple::keylet::line processing.
Co-authored-by: RichardAH <richard.holland@starstone.co.nz>
std::endl triggers flush() which calls sync() on the shared log buffer.
Multiple threads racing in sync() cause str()/str("") operations to
corrupt buffer state, leading to crashes and double frees.
Added mutex to serialize access to suite.log, preventing concurrent
sync() calls on the same buffer.
SOCI's vendored conanfile was using boost::boost which links against
every single Boost library (40+ libraries) when only boost::headers
is needed for SOCI's template specializations (boost::optional and
boost::gregorian::date support).
This was causing excessive linking and potential symbol conflicts,
particularly on Linux CI where boost_stacktrace_from_exception was
causing multiple definition errors with libstdc++.
Changed SOCI's boost dependency from boost::boost to boost::headers
since SOCI only needs Boost headers for its template specializations,
not the compiled libraries. The project already provides all necessary
Boost libraries through the ripple_boost target.
This reduces the linked libraries from 40+ down to just the ~14 that
the project actually uses, fixing the Linux CI build failures and
reducing binary size.
Note: The SOCI Conan recipe for Conan 2.0 already implements this
fix correctly.
* Match unit tests on start of test name (#4634)
* For example, without this change, to run the TxQ tests, must specify
`--unittest=TxQ1,TxQ2` on the command line. With this change, can use
`--unittest=TxQ`, and both will be run.
* An exact match will prevent any further partial matching.
* This could have some side effects for different tests with a common
name beginning. For example, NFToken, NFTokenBurn, NFTokenDir. This
might be useful. If not, the shorter-named test(s) can be renamed. For
example, NFToken to NFTokens.
* Split the NFToken, NFTokenBurn, and Offer test classes. Potentially speeds
up parallel tests by a factor of 5.
* SetHook_test, SetHookTSH_test, XahauGenesis_test
---------
Co-authored-by: Ed Hennis <ed@ripple.com>
Combine multiple related debug log data points into a single
message. Allows quick correlation of events that
previously were either not logged or, if logged, strewn
across multiple lines, making correlation difficult.
The Heartbeat Timer and consensus ledger accept processing
each have this capability.
Also guarantees that log entries will be written if the
node is a validator, regardless of log severity level.
Otherwise, the level of these messages is at INFO severity.
* Add logging for amendment voting decision process
* When counting "received validations" to determine quorum, count the number of validators actually voting, not the total number of possible votes.
The current comment in the example cfg file incorrectly mentions both "may" and "must". This change fixes this comment to clarify that the default port of hosts is 2459 and that specifying it is therefore optional. It further sets the default port to 2459 instead of the legacy 51235.
This change enhances the filtering in the ledger, ledger_data, and account_objects methods by also supporting filtering by the canonical name of the LedgerEntryType using case-insensitive matching.
Rewrites the code so that the lock is not held during the callback. Instead it locks twice, once before, and once after. This is safe due to the structure of the code, but is checked after the second lock. This allows mutex_ to be changed back to a regular mutex.
In PeerImpl.cpp, if the function is a message handler (onMessage) or called directly from a message handler, then it should use fee_, since when the handler returns (OnMessageEnd) then the charge function is called. If the function is not a message handler, such as a job queue item, it should remain charge.
If the permissioned domains amendment XLS-80 is enabled before credentials XLS-70, then the permissioned domain users will not be able to match any credentials. The changes here prevent the creation of any permissioned domain objects if credentials are not enabled.
Make `simulate` RPC easier to use:
* Prevent the use of `seed`, `secret`, `seed_hex`, and `passphrase` fields (to avoid confusing with the signing methods).
* Add autofilling of the `NetworkID` field.
- Also get the branch name.
- Use rev-parse instead of describe to get a clean hash.
- Return the git hash and branch name in server_info for admin
connections.
- Include git hash and branch name on separate lines in --version.
- spec: XRPLF/XRPL-Standards#220
- amendment: "DeepFreeze"
- implemented deep freeze spec to allow token issuers to prevent currency holders from being able to acquire more of these tokens.
- in combination with normal freeze, deep freeze effectively prevents any balance trust line balance change of a currency holder (except direct issuer <-> holder payments).
- added 2 new invariant checks to verify that deep freeze cannot be enacted without normal freeze and transfer is not frozen.
- made some fixes to existing freeze handling.
Co-authored-by: Ed Hennis <ed@ripple.com>
Co-authored-by: Howard Hinnant <howard.hinnant@gmail.com>
* Resolves an issue introduced in #5111, which inadvertently removed the
-Wno-maybe-uninitialized compiler option from some xrpl.libxrpl
modules. This resulted in new "may be used uninitialized" build
warnings, first noticed in the "protocol" module. When compiling with
derr=TRUE, those warnings became errors, which made the build fail.
* Github CI actions will build with the assert and werr options turned
on. This will cause CI jobs to fail if a developer introduces a new
compiler warning, or causes an assert to fail in release builds.
* Includes the OS and compiler version in the linux dependencies jobs in
the "check environment" step.
* Translates the `unity` build option into `CMAKE_UNITY_BUILD` setting.
The LEDGER_ENTRY macro now takes an additional parameter, which makes it easier to avoid missing including the new field in jss.h and to the list of account_objects/ledger_data filters.
Replace Issue in STIssue with Asset. STIssue with MPTIssue is only used in MPT tests.
Will be used in Vault and in transactions with STIssue fields once MPT is integrated into DEX.
* Rename ASSERT to XRPL_ASSERT
* Upgrade to Anthithesis SDK 0.4.4, and use new 0.4.4 features
* automatic cast to bool, like assert
* Add instrumentation workflow to verify build with instrumentation enabled
Adds two CMake functions:
* add_module(library subdirectory): Declares an OBJECT "library" (a CMake abstraction for a collection of object files) with sources from the given subdirectory of the given library, representing a module. Isolates the module's headers by creating a subdirectory in the build directory, e.g. .build/tmp123, that contains just a symlink, e.g. .build/tmp123/basics, to the module's header directory, e.g. include/xrpl/basics, in the source directory, and putting .build/tmp123 (but not include/xrpl) on the include path of the module sources. This prevents the module sources from including headers not explicitly linked to the module in CMake with target_link_libraries.
* target_link_modules(library scope modules...): Links the library target to each of the module targets, and removes their sources from its source list (so they are not compiled and linked twice).
Uses these functions to separate and explicitly link modules in libxrpl:
Level 01: beast
Level 02: basics
Level 03: json, crypto
Level 04: protocol
Level 05: resource, server
* Copy Antithesis SDK version 0.4.0 to directory external/
* Add build option `voidstar` to enable instrumentation with Antithesis SDK
* Define instrumentation macros ASSERT and UNREACHABLE in terms of regular C assert
* Replace asserts with named ASSERT or UNREACHABLE
* Add UNREACHABLE to LogicError
* Document instrumentation macros in CONTRIBUTING.md
- Fix an erroneous high fee penalty that peers could incur for sending
older transactions.
- Update to the fees charged for imposing a load on the server.
- Prevent the relaying of internal pseudo-transactions.
- Before: Pseudo-transactions received from a peer will fail the signature
check, even if they were requested (using TMGetObjectByHash), because
they have no signature. This causes the peer to be charge for an
invalid signature.
- After: Pseudo-transactions, are put into the global cache
(TransactionMaster) only. If the transaction is not part of
a TMTransactions batch, the peer is charged an unwanted data fee.
These fees will not be a problem in the normal course of operations,
but should dissuade peers from behaving badly by sending a bunch of
junk.
- Improve logging: include the reason for fees charged to a peer.
Co-authored-by: Ed Hennis <ed@ripple.com>
`STNumber` lets objects and transactions contain multiple fields for
quantities of XRP, IOU, or MPT without duplicating information about the
"issue" (represented by `STIssue`). It is a straightforward serialization of
the `Number` type that uniformly represents those quantities.
---------
Co-authored-by: John Freeman <jfreeman08@gmail.com>
Co-authored-by: Howard Hinnant <howard.hinnant@gmail.com>
* 2.2.2 changed functions acquireAsync and NetworkOPsImp::recvValidation to add an item to a collection under lock, unlock, do some work, then lock again to do remove the item. It will deadlock if an exception is thrown while adding the item - before unlocking.
* Replace ScopedUnlock with scope_unlock.
Move the newest information to the top, i.e., use reverse chronological order within each of the two sections ("API Versions" and "XRP Ledger server versions")
The page_size will soon be made configurable with #5135, making this
re-ordering necessary.
When opening SQLite connection, there are specific pragmas set with
commonPragmas.
In particular, PRAGMA journal_mode creates journal file and locks the
page_size; as of this commit, this sets the page size to the default
value of 4096. Coincidentally, the hardcoded page_size was also 4096, so
no issue was noticed.
Update book_changes RPC to reduce latency, add "validated" field, and accept shortcut strings (current, closed, validated) for ledger_index.
`"validated": true` indicates that the transaction has been included in a validated ledger so the result of the transaction is immutable.
Fix#5033Fix#5034Fix#5035Fix#5036
---------
Co-authored-by: Bronek Kozicki <brok@incorrekt.com>
* Retry some failed RPC connections / commands in unit tests
* Remove orphaned `getAccounts` function
Co-authored-by: John Freeman <jfreeman08@gmail.com>
* Add fixNFTokenPageLinks amendment:
It was discovered that under rare circumstances the links between
NFTokenPages could be removed. If this happens, then the
account_objects and account_nfts RPC commands under-report the
NFTokens owned by an account.
The fixNFTokenPageLinks amendment does the following to address
the problem:
- It fixes the underlying problem so no further broken links
should be created.
- It adds Invariants so, if such damage were introduced in the
future, an invariant would stop it.
- It adds a new FixLedgerState transaction that repairs
directories that were damaged in this fashion.
- It adds unit tests for all of it.
* `account_objects` returns an invalid field error if `type` is not supported.
This includes objects an account can't own, or which are unsupported by `account_objects`
* Includes:
* Amendments
* Directory Node
* Fee Settings
* Ledger Hashes
* Negative UNL
The names of the files should reflect the name of the Dir class.
Co-authored-by: Zack Brunson <Zshooter@gmail.com>
Co-authored-by: Ed Hennis <ed@ripple.com>
* fix CTID in tx command returns invalidParams on lowercase hex
* test mixed case and change auto to explicit type
* add header cctype because std::tolower is called
* remove unused local variable
* change test case comment from 'lowercase' to 'mixed case'
---------
Co-authored-by: Zack Brunson <Zshooter@gmail.com>
* Add feature / amendment "InvariantsV1_1"
* Adds invariant AccountRootsDeletedClean:
* Checks that a deleted account doesn't leave any directly
accessible artifacts behind.
* Always tests, but only changes the transaction result if
featureInvariantsV1_1 is enabled.
* Unit tests.
* Resolves#4638
* [FOLD] Review feedback from @gregtatcam:
* Fix unused variable warning
* Improve Invariant test const correctness
* [FOLD] Review feedback from @mvadari:
* Centralize the account keylet function list, and some optimization
* [FOLD] Some structured binding doesn't work in clang
* [FOLD] Review feedback 2 from @mvadari:
* Clean up and clarify some comments.
* [FOLD] Change InvariantsV1_1 to unsupported
* Will allow multiple PRs to be merged over time using the same amendment.
* fixup! [FOLD] Change InvariantsV1_1 to unsupported
* [FOLD] Update and clarify some comments. No code changes.
* Move CMake directory
* Rearrange sources
* Rewrite includes
* Recompute loops
* Fix merge issue and formatting
---------
Co-authored-by: Pretty Printer <cpp@ripple.com>
* fix "account_nfts" with unassociated marker returning issue
* create unit test for fixing nft page invalid marker not returning error
add more test
change test name
create unit test
* fix "account_nfts" with unassociated marker returning issue
* fix "account_nfts" with unassociated marker returning issue
* fix "account_nfts" with unassociated marker returning issue
* fix "account_nfts" with unassociated marker returning issue
* fix "account_nfts" with unassociated marker returning issue
* fix "account_nfts" with unassociated marker returning issue
* fix "account_nfts" with unassociated marker returning issue
* fix "account_nfts" with unassociated marker returning issue
* [FOLD] accumulated review suggestions
* move BEAST check out of lambda function
---------
Authored-by: Scott Schurr <scott@ripple.com>
* fixInnerObjTemplate2 amendment:
Apply inner object templates to all remaining (non-AMM)
inner objects.
Adds a unit test for applying the template to sfMajorities.
Other remaining inner objects showed no problems having
templates applied.
* Move CMake directory
* Rearrange sources
* Rewrite includes
* Recompute loops
---------
Co-authored-by: Pretty Printer <cpp@ripple.com>
Fix interactions between NFTokenOffers and trust lines.
Since the NFTokenAcceptOffer does not check the trust line that
the issuer receives as a transfer fee in the NFTokenAcceptOffer,
if the issuer deletes the trust line after NFTokenCreateOffer,
the trust line is created for the issuer by the
NFTokenAcceptOffer. That's fixed.
Resolves#4925.
Fixes issue #4937.
The fixReducedOffersV1 amendment fixed certain forms of offer
modification that could lead to blocked order books. 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.
Issue #4937 identified an additional code path that modified
offers in a way that could lead to blocked order books. This
commit changes the rounding in that newly located code path so
the quality of the modified offer is never worse than the
quality of the offer as it was originally placed.
It is possible that additional ways of producing blocking
offers will come to light. Therefore there may be a future
need for a V3 amendment.
* Add trap_tx_hash command line option
This new option can be used only if replay is also enabled. It takes a transaction hash from the ledger loaded for replay, and will cause a specific line to be hit in Transactor.cpp, right before the selected transaction is applied.
When rippled initiates a connection to SQLite3, rippled sends a "PRAGMA"
statement defining the maximum number of pages allowed in the database.
Update the max_page_count so it is consistent with the default for newer
versions of SQLite3. Increasing max_page_count is critical for keeping
full history servers online.
Fix#5102
Price Oracle data-series logic uses `unordered_map` to update the Oracle object.
This results in different servers disagreeing on the order of that hash table.
Consequently, the generated ledgers will have different hashes.
The fix uses `map` instead to guarantee the order of the token pairs
in the data-series.
Due to the rounding, LPTokenBalance of the last
Liquidity Provider (LP), might not match this LP's
trustline balance. This fix sets LPTokenBalance on
last LP withdrawal to this LP's LPToken trustline
balance.
Single path AMM offer has to factor in the transfer in rate
when calculating the upper bound quality and the quality function
because single path AMM's offer quality is not constant.
This fix factors in the transfer fee in
BookStep::adjustQualityWithFees().
* Fix AMM offer rounding and low quality LOB offer blocking AMM:
A single-path AMM offer with account offer on DEX, is always generated
starting with the takerPays first, which is rounded up, and then
the takerGets, which is rounded down. This rounding ensures that the pool's
product invariant is maintained. However, when one of the offer's side
is XRP, this rounding can result in the AMM offer having a lower
quality, potentially causing offer generation to fail if the quality
is lower than the account's offer quality.
To address this issue, the proposed fix adjusts the offer generation process
to start with the XRP side first and always rounds it down. This results
in a smaller offer size, improving the offer's quality. Regardless if the offer
has XRP or not, the rounding is done so that the offer size is minimized.
This change still ensures the product invariant, as the other generated
side is the exact result of the swap-in or swap-out equations.
If a liquidity can be provided by both AMM and LOB offer on offer crossing
then AMM offer is generated so that it matches LOB offer quality. If LOB
offer quality is less than limit quality then generated AMM offer quality
is also less than limit quality and the offer doesn't cross. To address
this issue, if LOB quality is better than limit quality then use LOB
quality to generate AMM offer. Otherwise, don't use the quality to generate
AMM offer. In this case, limitOut() function in StrandFlow limits
the out amount to match strand's quality to limit quality and consume
maximum AMM liquidity.
Rounding in the payment engine is causing an assert to sometimes fire
with "dust" amounts. This is causing issues when running debug builds of
rippled. This issue will be addressed, but the assert is no longer
serving its purpose.
The AMM has an invariant for swaps where:
new_balance_1*new_balance_2 >= old_balance_1*old_balance_2
Due to rounding, this invariant could sometimes be violated (although by
very small amounts).
This patch introduces an amendment `fixAMMRounding` that changes the
rounding to always favor the AMM. Doing this should maintain the
invariant.
Co-authored-by: Bronek Kozicki
Co-authored-by: thejohnfreeman
It can be difficult to make transaction breaking changes to low level
code because the low level code does not have access to a ledger and the
current activated amendments in that ledger (the "rules"). This patch
adds global access to the current ledger rules as a `std::optional`. If
the optional is not seated, then there is no active transaction.
The `rotateWithLock` function holds a lock while it calls a callback
function that's passed in by the caller. This is a problematic design
that needs to be used very carefully. In this case, at least one caller
passed in a callback that eventually relocks the mutex on the same
thread, causing UB (a deadlock was observed). The caller was from
SHAMapStoreImpl, and it called `clearCaches`. This `clearCaches` can
potentially call `fetchNodeObject`, which tried to relock the mutex.
This patch resolves the issue by changing the mutex type to a
`recursive_mutex`. Ideally, the code should be rewritten so it doesn't
hold the mutex during the callback and the mutex should be changed back
to a regular mutex.
Co-authored-by: Ed Hennis <ed@ripple.com>
This amendment, `fixPreviousTxnID`, adds `PreviousTxnID` and
`PreviousTxnLgrSequence` as fields to all ledger objects that did
not already have them included (`DirectoryNode`, `Amendments`,
`FeeSettings`, `NegativeUNL`, and `AMM`). This makes it much easier
to go through the history of these ledger objects.
When calculating reward shares, the amount should always be rounded
down. If the `fixUniversalNumber` amendment is not active, this works
correctly. If it is not active, then the amount is incorrectly rounded
up. This patch introduces an amendment so it will be rounded down.
This fixes a case where a peer can desync under a certain timing
circumstance--if it reaches a certain point in consensus before it receives
proposals.
This was noticed under high transaction volumes. Namely, when we arrive at the
point of deciding whether consensus is reached after minimum establish phase
duration but before having received any proposals. This could be caused by
finishing the previous round slightly faster and/or having some delay in
receiving proposals. Existing behavior arrives at consensus immediately after
the minimum establish duration with no proposals. This causes us to desync
because we then close a non-validated ledger. The change in this PR causes us to
wait for a configured threshold before making the decision to arrive at
consensus with no proposals. This allows validators to catch up and for brief
delays in receiving proposals to be absorbed. There should be no drawback since,
with no proposals coming in, we needn't be in a huge rush to jump ahead.
We do not currently enforce that incoming peer connection does not have
remote_endpoint which is already used (either by incoming or outgoing
connection), hence already stored in slots_. If we happen to receive a
connection from such a duplicate remote_endpoint, it will eventually result in a
crash (when disconnecting) or weird behavior (when updating slot state), as a
result of an apparently matching remote_endpoint in slots_ being used by a
different connection.
This amendment fixes an edge case where an empty DID object can be
created. It adds an additional check to ensure that DIDs are
non-empty when created, and returns a `tecEMPTY_DID` error if the DID
would be empty.
* telENV_RPC_FAILED is a new code, reserved exclusively
for unit tests when RPC fails. This will
make those types of errors distinct and easier to test
for when expected and/or diagnose when not.
* Output RPC command result when result is not expected.
We are currently using old version 0.6.2 of `xxhash`, as a verbatim copy and paste of its header file `xxhash.h`. Switch to the more recent version 0.8.2. Since this version is in Conan Center (and properly protects its ABI by keeping the state object incomplete), add it as a Conan requirement. Switch to the SIMD instructions (in the new `XXH3` family) supported by the new version.
This algorithm is about an order of magnitude faster than the existing
algorithm (about 10x faster for encoding and about 15x faster for
decoding - including the double hash for the checksum). The algorithms
use gcc's int128 (fast MS version will have to wait, in the meantime MS
falls back to the slow code).
* It is now an invariant that all constructed Public Keys are valid,
non-empty and contain 33 bytes of data.
* Additionally, the memory footprint of the PublicKey class is reduced.
The size_ data member is declared as static.
* Distinguish and identify the PublisherList retrieved from the local
config file, versus the ones obtained from other validators.
* Fixes#2942
The compilation fails due to an issue in the initializer list
of an optional argument, which holds a vector of pairs.
The code compiles correctly on earlier gcc versions, but fails on gcc 13.
Implement native support for Price Oracles.
A Price Oracle is used to bring real-world data, such as market prices,
onto the blockchain, enabling dApps to access and utilize information
that resides outside the blockchain.
Add Price Oracle functionality:
- OracleSet: create or update the Oracle object
- OracleDelete: delete the Oracle object
To support this functionality add:
- New RPC method, `get_aggregate_price`, to calculate aggregate price for a token pair of the specified oracles
- `ltOracle` object
The `ltOracle` object maintains:
- Oracle Owner's account
- Oracle's metadata
- Up to ten token pairs with the scaled price
- The last update time the token pairs were updated
Add Oracle unit-tests
Add a new RPC / WS call for `server_definitions`, which returns an
SDK-compatible `definitions.json` (binary enum definitions) generated by
the server. This enables clients/libraries to dynamically work with new
fields and features, such as ones that may become available on side
chains. Clients query `server_definitions` on a node from the network
they want to work with, and immediately know how to speak that node's
binary "language", even if new features are added to it in the future
(as long as there are no new serialized types that the software doesn't
know how to serialize/deserialize).
Example:
```js
> {"command": "server_definitions"}
< {
"result": {
"FIELDS": [
[
"Generic",
{
"isSerialized": false,
"isSigningField": false,
"isVLEncoded": false,
"nth": 0,
"type": "Unknown"
}
],
[
"Invalid",
{
"isSerialized": false,
"isSigningField": false,
"isVLEncoded": false,
"nth": -1,
"type": "Unknown"
}
],
[
"ObjectEndMarker",
{
"isSerialized": false,
"isSigningField": true,
"isVLEncoded": false,
"nth": 1,
"type": "STObject"
}
],
...
```
Close#3657
---------
Co-authored-by: Richard Holland <richard.holland@starstone.co.nz>
A large synthetic offer was not handled correctly in the payment engine.
This patch fixes that issue and introduces a new invariant check while
processing synthetic offers.
* Commit 01c37fe introduced a change to the STTx unit test where a local
"defaultRules" object was created with a temporary inline "presets"
value provided to the ctor. Rules::Impl stores a const ref to the
presets provided to the ctor. This particular call provided an inline
temp variable, which goes out of scope as soon as the object is
created. On Windows, attempting to use the presets (e.g. via the
enabled() function) causes an access violation, which crashes the test
run.
* An audit of the code indicates that all other instances of Rules use
the Application's config.features list, which will have a sufficient
lifetime.
Add `STObject` constructor to explicitly set the inner object template.
This allows certain AMM transactions to apply in the same ledger:
There is no issue if the trading fee is greater than or equal to 0.01%.
If the trading fee is less than 0.01%, then:
- After AMM create, AMM transactions must wait for one ledger to close
(3-5 seconds).
- After one ledger is validated, all AMM transactions succeed, as
appropriate, except for AMMVote.
- The first AMMVote which votes for a 0 trading fee in a ledger will
succeed. Subsequent AMMVote transactions which vote for a 0 trading
fee will wait for the next ledger (3-5 seconds). This behavior repeats
for each ledger.
This has no effect on the ultimate correctness of AMM. This amendment
will allow the transactions described above to succeed as expected, even
if the trading fee is 0 and the transactions are applied within one
ledger (block).
Prior to this commit, `port_grpc` could not be added to the [server]
stanza. Instead of validating gRPC IP/Port/Protocol information in
ServerHandler, validate grpc port info in GRPCServer constructor. This
should not break backwards compatibility.
gRPC-related config info must be in a section (stanza) called
[port_gprc].
* Close#4015 - That was an alternate solution. It was decided that with
relaxed validation, it is not necessary to rename port_grpc.
* Fix#4557
These headers are required in the xrpl Conan package in order for
xbridge witness server (xbwd) to build. This change to libxrpl may help
any dependents of libxrpl. This addition does not change any C++ code.
Without this amendment, an NFTokenAcceptOffer transaction can succeed
even when the NFToken recipient does not have sufficient reserves for
the new NFTokenPage. This allowed accounts to accept NFT sell offers
without having a sufficient reserve. (However, there was no issue in
brokered mode or when a buy offer is involved.)
Instead, the transaction should fail with `tecINSUFFICIENT_RESERVE` as
appropriate. The `fixNFTokenReserve` amendment adds checks in the
NFTokenAcceptOffer transactor to check if the OwnerCount changed. If it
did, then it checks the new reserve requirement.
Fix#4679
Use consistent platform-agnostic library names on all platforms.
Fix an issue that prevents dependents like validator-keys-tool from
linking to libxrpl on Windows.
It is bad practice to change the binary base name depending on the
platform. CMake already manipulates the base name into a final name that
fits the conventions of the platform. Linkers accept base names on the
command line and then look for conventional names on disk.
Clients subscribed to `transactions` over WebSocket are being
disconnected because the traffic exceeds the default `send_queue_limit`
of 100.
This commit changes the default configuration, not the default in code.
Fix#4866
* Add logging for Application.cpp sweep()
* Improve lifetime management of ledger objects (`SLE`s)
* Only store SLE digest in CachedView; get SLEs from CachedSLEs
* Also force release of last ledger used for path finding if there are
no path finding requests to process
* Count more ST objects (derive from `CountedObject`)
* Track CachedView stats in CountedObjects
* Rename the CachedView counters
* Fix the scope of the digest lookup lock
Before this patch, if you asked "is it caching?" It was always caching.
Prevent WebSocket connections from trying to close twice.
The issue only occurs in debug builds (assertions are disabled in
release builds, including published packages), and when the WebSocket
connections are unprivileged. The assert (and WRN log) occurs when a
client drives up the resource balance enough to be forcibly disconnected
while there are still messages pending to be sent.
Thanks to @lathanbritz for discovering this issue in #4822.
Workaround for compilation errors with gcc-13 and other compilers
relying on `libstdc++` version 13. This is temporary until actual fix is
available for us to use: https://github.com/boostorg/beast/pull/2682
Some boost.beast files (which we do use) rely on an old gcc-12 behaviour
where `#include <cstdint>` was not needed even though types from this
header were used. This was broken by a change in libstdc++ version 13:
https://gcc.gnu.org/gcc-13/porting_to.html#header-dep-changes
The necessary fix was implemented in boost.beast, however it is not yet
available. Until it is available, we can use this workaround to enable
compilation of `rippled` with gcc-13, clang-16, etc.
* Promote API version 2 to supported
* Switch command line to API version 1
* Fix LedgerRequestRPC test
* Remove obsolete tx_account method
This method is not implemented, the only parts which are removed are related to command-line parsing
* Fix RPCCall test
* Reduce diff size, small test improvements
* Minor fixes
* Support for the mold linker
* [fold] handle case where both mold and gold are installed
* [fold] Use first non-default linker
* Fix TransactionEntry_test
* Fix AccountTx_test
---------
Co-authored-by: seelabs <scott.determan@yahoo.com>
* Remove include <ranges>
* Formatting fix
* Output for subscriptions
* Output from sign, submit etc.
* Output from ledger
* Output from account_tx
* Output from transaction_entry
* Output from tx
* Store close_time_iso in API v2 output
* Add small APIv2 unit test for subscribe
* Add unit test for transaction_entry
* Add unit test for tx
* Remove inLedger from API version 2
* Set ledger_hash and ledger_index
* Move isValidated from RPCHelpers to LedgerMaster
* Store closeTime in LedgerFill
* Time formatting fix
* additional tests for Subscribe unit tests
* Improved comments
* Rename mInLedger to mLedgerIndex
* Minor fixes
* Set ledger_hash on closed ledger, even if not validated
* Update API-CHANGELOG.md
* Add ledger_hash, ledger_index to transaction_entry
* Fix validated and close_time_iso in account_tx
* Fix typos
* Improve getJson for Transaction and STTx
* Minor improvements
* Replace class enum JsonOptions with struct
We may consider turning this into a general-purpose template and using it elsewhere
* simplify the extraction of transactionID from Transaction object
* Remove obsolete comments
* Unconditionally set validated in account_tx output
* Minor improvements
* Minor fixes
---------
Co-authored-by: Chenna Keshava <ckeshavabs@gmail.com>
The command line API still uses `apiMaximumSupportedVersion`.
The unit test RPCs use `apiMinimumSupportedVersion` if unspecified.
Context:
- #4568
- #4552
Introduce the `fixFillOrKill` amendment.
Fix an edge case occurring when an offer with `tfFillOrKill` set (but
without `tfSell` set) fails to cross an offer with a better rate. If
`tfFillOrKill` is set, then the owner must receive the full TakerPays.
Without this amendment, an offer fails if the entire `TakerGets` is not
spent. With this amendment, when `tfSell` is not set, the entire
`TakerGets` does not have to be spent.
For details about OfferCreate, see: https://xrpl.org/offercreate.htmlFix#4684
---------
Co-authored-by: Scott Schurr <scott@ripple.com>
Remove dependency on `<ranges>` header, since it is not implemented by
all compilers which we want to support.
This code change only affects unit tests.
Resolve https://github.com/XRPLF/rippled/issues/4787
Remove `tx_history` and `ledger_header` methods from API version 2.
Update `RPC::Handler` to allow for methods (or method implementations)
to be API version specific. This partially resolves#4727. We can now
store multiple handlers with the same name, as long as they belong to
different (non-overlapping) API versions. This necessarily impacts the
handler lookup algorithm and its complexity; however, there is no
performance loss on x86_64 architecture, and only minimal performance
loss on arm64 (around 10ns). This design change gives us extra
flexibility evolving the API in the future, including other parts of
In API version 2, `tx_history` and `ledger_header` are no longer
recognised; if they are called, `rippled` will return error
`unknownCmd`
Resolve#3638Resolve#3539
Using the "Amount" field in Payment transactions can cause incorrect
interpretation. There continue to be problems from the use of this
field. "Amount" is rarely the correct field to use; instead,
"delivered_amount" (or "DeliveredAmount") should be used.
Rename the "Amount" field to "DeliverMax", a less misleading name. With
api_version: 2, remove the "Amount" field from Payment transactions.
- Input: "DeliverMax" in `tx_json` is an alias for "Amount"
- sign
- submit (in sign-and-submit mode)
- submit_multisigned
- sign_for
- Output: Add "DeliverMax" where transactions are provided by the API
- ledger
- tx
- tx_history
- account_tx
- transaction_entry
- subscribe (transactions stream)
- Output: Remove "Amount" from API version 2
Fix#3484Fix#3902
Implement native support for W3C DIDs.
Add a new ledger object: `DID`.
Add two new transactions:
1. `DIDSet`: create or update the `DID` object.
2. `DIDDelete`: delete the `DID` object.
This meets the requirements specified in the DID v1.0 specification
currently recommended by the W3C Credentials Community Group.
The DID format for the XRP Ledger conforms to W3C DID standards.
The objects can be created and owned by any XRPL account holder.
The transactions can be integrated by any service, wallet, or application.
It might be possible for the server code to indirect through certain
`end()` iterators. While a debug build would catch this problem with
`assert()`s, a release build would crash. If there are problems in this
area in the future, it is best to get a definitive indication of the
nature of the error regardless of whether it's a debug or release build.
To accomplish this, these `assert`s are converted into `LogicError`s
that will produce a reasonable error message when they fire.
The assert is saying that the only reason `pathFinder` would be null is
if the request was aborted (connection dropped, etc.). That's what
`continueCallback()` checks. But that is very clearly not true if you
look at `getPathFinder`, which calls `findPaths`, which can return false
for many reasons.
Fix#4744
P2P link compression is a feature added in 1.6.0 by #3287.
https://xrpl.org/enable-link-compression.html
If the default changes in the future - for example, as currently
proposed by #4387 - the comment will be updated at that time.
Fix#4656
Context: The `DisallowIncoming` amendment provides an option to block
incoming trust lines from reaching your account. The
asfDisallowIncomingTrustline AccountSet Flag, when enabled, prevents any
incoming trust line from being created. However, it was too restrictive:
it would block an issuer from authorizing a trust line, even if the
trust line already exists. Consider:
1. Issuer sets asfRequireAuth on their account.
2. User sets asfDisallowIncomingTrustline on their account.
3. User submits tx to SetTrust to Issuer.
At this point, without `fixDisallowIncomingV1` active, the issuer would
not be able to authorize the trust line.
The `fixDisallowIncomingV1` amendment, once activated, allows an issuer
to authorize a trust line even after the user sets the
asfDisallowIncomingTrustline flag, as long as the trust line already
exists.
When a new transactor is added, there are several places in applySteps
that need to be modified. This patch refactors the code so only one
function needs to be modified.
Make transactions and pseudo-transactions share the same commonFields
again. This regularizes the code in a nice way.
While this technically allows pseudo-transactions to have a
TicketSequence field, pseudo-transactions are only ever constructed by
code paths that don't add such a field, so this is not a transaction
processing change. It may be possible to add a separate check to ensure
TicketSequence (and other fields that don't make sense on
pseudo-transactions) are never added to pseudo-transactions, but that
should not be necessary. (TicketSequence is not the only common field
that can not and does not appear in pseudo-transactions.) Note:
TicketSequence is already documented as a common field.
Related: #4637Fix#4714
Update minimum compiler requirement for building the codebase. The
feature "using enum" is required. This feature was introduced in C++20.
Updating the C++ compiler to version 11 or later fixes this error:
```
Building CXX object CMakeFiles/xrpl_core.dir/src/ripple/protocol/impl/STAmount.cpp.o
/build/ripple/binary/src/ripple/protocol/impl/STAmount.cpp: In lambda function:
/build/ripple/binary/src/ripple/protocol/impl/STAmount.cpp:1577:15: error: expected nested-name-specifier before 'enum'
1577 | using enum Number::rounding_mode;
| ^~~~
```
Fix#4693
Modify the `XChainBridge` amendment.
Before this patch, two door accounts on the same chain could could own
the same bridge spec (of course, one would have to be the issuer and one
would have to be the locker). While this is silly, it does not violate
any bridge invariants. However, on further review, if we allow this then
the `claim` transactions would need to change. Since it's hard to see a
use case for two doors to own the same bridge, this patch disallows
it. (The transaction will return tecDUPLICATE).
Amendment "flapping" (an amendment repeatedly gaining and losing
majority) usually occurs when an amendment is on the verge of gaining
majority, and a validator not in favor of the amendment goes offline or
loses sync. This fix makes two changes:
1. The number of validators in the UNL determines the threshold required
for an amendment to gain majority.
2. The AmendmentTable keeps a record of the most recent Amendment vote
received from each trusted validator (and, with `trustChanged`, stays
up-to-date when the set of trusted validators changes). If no
validation arrives from a given validator, then the AmendmentTable
assumes that the previously-received vote has not changed.
In other words, when missing an `STValidation` from a remote validator,
each server now uses the last vote seen. There is a 24 hour timeout for
recorded validator votes.
These changes do not require an amendment because they do not impact
transaction processing, but only the threshold at which each individual
validator decides to propose an EnableAmendment pseudo-transaction.
Fix#4350
Fix the Windows build by using `unsigned int` (instead of `uint`).
The error, introduced by #4618, looks something like:
rpc\impl\RPCHelpers.h(299,5): error C2061: syntax error: identifier
'uint' (compiling source file app\ledger\Ledger.cpp)
A few methods, including `book_offers`, take currency codes as
parameters. The XRPL doesn't care if the letters in those codes are
lowercase or uppercase, as long as they come from an alphabet defined
internally. rippled doesn't care either, when they are submitted in a
hex representation. When they are submitted in an ASCII string
representation, rippled, but not XRPL, is more restrictive, preventing
clients from interacting with some currencies already in the XRPL.
This change gets rippled out of the way and lets clients submit currency
codes in ASCII using the full alphabet.
Fixes#4112
Currently, the `BUILD.md` instructions suggest using `.build` as the
build directory, so this change helps to reduce confusion.
An alternative would be to instruct developers to add `/.build/` to
`.git/info/exclude` or to user-level `.gitignore` (although the latter
is very intrusive). However, it is being added here because it is a good
practice to have a sensible default that's consistent with the build
instructions.
gateway_balances
* When `account` does not exist in the ledger, return `actNotFound`
* (Previously, a normal response was returned)
* Fix#4290
* When required field(s) are missing, return `invalidParams`
* (Previously, `invalidHotWallet` was incorrectly returned)
* Fix#4548
channel_authorize
* When the specified `key_type` is invalid, return `badKeyType`
* (Previously, `invalidParams` was returned)
* Fix#4289
Since these are breaking changes, they apply only to API version 2.
Supersedes #4577
Copy the new code to `src/secp256k1` without changes:
`src/secp256k1` is identical to bitcoin-core/secp256k1@acf5c55 (v0.3.2).
We could consider changing to a Git submodule, though that would require
changes to the build instructions because we are not using submodules
anywhere else.
Remove the `verify` and `message` function declarations. The explicit
instantiation requests could not be completed because there were no
implementations for those two member functions. It is helpful that the
Microsoft (MSVC) compiler on Windows appears to be strict when it comes
to template instantiation.
This resolves the warning:
XChainAttestations.h(450): warning C4661: 'bool
ripple::XChainAttestationsBase<ripple::XChainClaimAttestation>::verify(void)
const': no suitable definition provided for explicit template
instantiation request
* For example, without this change, to run the TxQ tests, must specify
`--unittest=TxQ1,TxQ2` on the command line. With this change, can use
`--unittest=TxQ`, and both will be run.
* An exact match will prevent any further partial matching.
* This could have some side effects for different tests with a common
name beginning. For example, NFToken, NFTokenBurn, NFTokenDir. This
might be useful. If not, the shorter-named test(s) can be renamed. For
example, NFToken to NFTokens.
* Split the NFToken, NFTokenBurn, and Offer test classes. Potentially speeds
up parallel tests by a factor of 5.
A bridge connects two blockchains: a locking chain and an issuing
chain (also called a mainchain and a sidechain). Both are independent
ledgers, with their own validators and potentially their own custom
transactions. Importantly, there is a way to move assets from the
locking chain to the issuing chain and a way to return those assets from
the issuing chain back to the locking chain: the bridge. This key
operation is called a cross-chain transfer. A cross-chain transfer is
not a single transaction. It happens on two chains, requires multiple
transactions, and involves an additional server type called a "witness".
A bridge does not exchange assets between two ledgers. Instead, it locks
assets on one ledger (the "locking chain") and represents those assets
with wrapped assets on another chain (the "issuing chain"). A good model
to keep in mind is a box with an infinite supply of wrapped assets.
Putting an asset from the locking chain into the box will release a
wrapped asset onto the issuing chain. Putting a wrapped asset from the
issuing chain back into the box will release one of the existing locking
chain assets back onto the locking chain. There is no other way to get
assets into or out of the box. Note that there is no way for the box to
"run out of" wrapped assets - it has an infinite supply.
Co-authored-by: Gregory Popovitch <greg7mdp@gmail.com>
For the `account_tx` and `noripple_check` methods, perform input
validation for optional parameters such as "binary", "forward",
"strict", "transactions". Previously, when these parameters had invalid
values (e.g. not a bool), no error would be returned. Now, it returns an
`invalidParams` error.
* This updates the behavior to match Clio
(https://github.com/XRPLF/clio).
* Since this is potentially a breaking change, it only applies to
requests specifying api_version: 2.
* Fix#4543.
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.
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>
- 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.
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>
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++.)
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>
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 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
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
- 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.
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.
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.
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.
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.
* 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.
