Skip to content

Conversation

mvadari
Copy link
Collaborator

@mvadari mvadari commented Jul 23, 2025

High Level Overview of Change

This PR updates ripple/smart-escrow to contain the latest on the develop branch.

Context of Change

Tests are failing due to the conan2 upgrade. This PR handles that (alongside other upgrades).

We also need to keep the branch updated.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Performance (increase or change in throughput and/or latency)
  • Tests (you added tests for code that already exists, or your new feature included in this PR)
  • Documentation update
  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
  • Release

API Impact

N/A

Test Plan

CI passes

mvadari and others added 26 commits July 2, 2025 19:10
This PR fixes a crash in tests when the test `Env is run at trace/debug log level.

This issue only affects tests, and only if logging at trace/debug level, so really only relevant during rippled development, and does not affect production servers.
This PR refactors `CredentialHelpers` and removes some unnecessary dependencies as a step of modularization.

The ledger component is almost independent except that it references `MPTokenAuthorize` and `CredentialHelpers.h`, and the latter further references `Transactor.h`. This PR partially clears the path to modularizing the ledger component and decouples `CredentialHelpers` from xrpld.
Circular includes in header files can yield unpredictable results.
Removes clutter for old compilers, defaults to non-unity builds in cmake to match conanfile.py, and workaround for clang-20 compilation errors.
Having `boost::boost` in `self.requires` makes clio link with all boost libraries. There are additionally several Boost stacktrace backends that are both linked with, which violate ODR.
This change fixes the problem.
The current version was copied from `antithesis-sdk-cpp` but there is no logical reason to require this specific version of CMake. This change downgrades the version to make the project build with older CMake versions.
* Update the `account_info` API so that the `allowTrustLineLocking` flag is included in the response.
* The proposed `TokenEscrow` amendment added an `allowTrustLineLocking` flag in the `AccountRoot` object.
* In the API response, under `account_flags`, there is now an `allowTrustLineLocking` field with a boolean (`true` or `false`) value.
* For reference, the XLS-85 Token-Enabled Escrows implementation can be found in XRPLF#5185
Currently there is no easy way to track MPT related transactions for the issuer. This change allows MPT transactions to show up on issuer's AccountTx RPC (to align with how IOUs work).
This change silences a dummy warning, which is breaking builds with GCC 12 (but not newer versions of GCC) in release mode only.
)

Due to rounding, the LPTokenBalance of the last LP might not match the LP's trustline balance. This was fixed for `AMMWithdraw` in `fixAMMv1_1` by adjusting the LPTokenBalance to be the same as the trustline balance. Since `AMMClawback` is also performing a withdrawal, we need to adjust LPTokenBalance as well in `AMMClawback.`

This change includes:
1. Refactored `verifyAndAdjustLPTokenBalance` function in `AMMUtils`, which both`AMMWithdraw` and `AMMClawback` call to adjust LPTokenBalance.
2. Added the unit test `testLastHolderLPTokenBalance` to test the scenario.
3. Modify the existing unit tests for `fixAMMClawbackRounding`.
This is a minimally invasive update to use Conan 2 provided by our new build images.
The FlowCross amendment is now permanently enabled, so all code branches that have this amendment disabled are removed.
…kages (XRPLF#5567)

This PR updates several dependencies to their latest versions. Not all dependencies have been updated, as some need to be patched and some require additional code changes due to backward incompatibilities introduced by the version bump.
This issue was reported on the Javascript client library: XRPLF/xrpl.js#2611

The type filter (Note: as of the latest version of rippled, type parameter is deprecated) does not work as expected. This PR removes the type filter from the ledger command.
This change fixes the MacOS pipeline issue by limiting GitHub to choose the existing runners, ensuring the new experimental runners are excluded until they are ready.
This change reverts the usage of boost::shared_mutex back to std::shared_mutex. The change was originally introduced as a workaround for a bug in glibc 2.28 and older versions, which could cause threads using std::shared_mutex to stall. This issue primarily affected Ubuntu 18.04 and earlier distributions, which we no longer support.
Remove `include(default)` from `conan/profiles/libxrpl`. This means that we will now rely on compiler workarounds stored elsewhere e.g. in global.conf.
The current implementation of rngfill is prone to false warnings from GCC about array bounds violations. Looking at the code, the implementation naively manipulates both the bytes count and the buffer pointer directly to ensure the trailing memcpy doesn't overrun the buffer. As expressed, there is a data dependency on both fields between loop iterations.

Now, ideally, an optimizing compiler would realize that these dependencies were unnecessary and end up restructuring its intermediate representation into a functionally equivalent form with them absent. However, the point at which this occurs may be disjoint from when warning analyses are performed, potentially rendering them more difficult to
determine precisely.

In addition, it may also consume a portion of the budget the optimizer has allocated to attempting to improve a translation unit's performance. Given this is a function template which requires context-sensitive instantiation, this code would be more prone than most to being inlined, with a decrease in optimization budget corresponding to the effort the optimizer has already expended, having already optimized one or more calling functions. Thus, the scope for impacting the the ultimate quality of the code generated is elevated.

For this change, we rearrange things so that the location and contents of each memcpy can be computed independently, relying on a simple loop iteration counter as the only changing input between iterations.
For jobs running in containers, $GITHUB_WORKSPACE and ${{ github.workspace }} might not be the same directory. The actions/checkout step is supposed to checkout into `$GITHUB_WORKSPACE` and then add it to safe.directory (see instructions at https://github.com/actions/checkout), but that's apparently not happening for some container images. We can't be sure what is actually happening, so we preemptively add both directories to `safe.directory`. See also the GitHub issue opened in 2022 that still has not been resolved actions/runner#2058.
This change addresses the issue XRPLF#5336: Refactor HashRouter flags to be more type-safe.

* Switched numeric flags to enum type.
* Updated unit tests
…#5550)

If a feature was never voted on then it is safe to remove.
After the `FlowCross` amendment was retired (XRPLF#5562), there was still some unused code left. This change removes the remaining remnants.
)

This change adds support for `DomainID` to existing transactions `MPTokenIssuanceCreate` and `MPTokenIssuanceSet`.

In XRPLF#5224 `DomainID` was added as an access control mechanism for `SingleAssetVault`. The actual implementation of this feature lies in `MPToken` and `MPTokenIssuance`, hence it makes sense to enable the use of `DomainID` also in `MPTokenIssuanceCreate` and `MPTokenIssuanceSet`, following same rules as in Vault:

* `MPTokenIssuanceCreate` and `MPTokenIssuanceSet` can only set `DomainID` if flag `MPTRequireAuth` is set.
* `MPTokenIssuanceCreate` requires that `DomainID` be a non-zero, uint256 number.
* `MPTokenIssuanceSet` allows `DomainID` to be zero (or empty) in which case it will remove `DomainID` from the `MPTokenIssuance` object.

The change is amendment-gated by `SingleAssetVault`. This is a non-breaking change because `SingleAssetVault` amendment is `Supported::no`, i.e. at this moment considered a work in progress, which cannot be enabled on the network.
@mvadari mvadari requested a review from oleks-rip July 23, 2025 18:42
Copy link

codecov bot commented Jul 23, 2025

Codecov Report

Attention: Patch coverage is 90.04525% with 22 lines in your changes missing coverage. Please review.

Project coverage is 78.7%. Comparing base (ece3a8d) to head (98b8986).
Report is 9 commits behind head on ripple/smart-escrow.

Files with missing lines Patch % Lines
src/xrpld/overlay/detail/PeerImp.cpp 0.0% 11 Missing ⚠️
src/xrpld/app/misc/NetworkOPs.cpp 55.6% 4 Missing ⚠️
src/xrpld/app/ledger/detail/LedgerToJson.cpp 57.1% 3 Missing ⚠️
src/xrpld/app/misc/detail/AMMUtils.cpp 83.3% 2 Missing ⚠️
src/xrpld/app/tx/detail/Escrow.cpp 81.8% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                  Coverage Diff                  @@
##           ripple/smart-escrow   #5602     +/-   ##
=====================================================
- Coverage                 78.7%   78.7%   -0.0%     
=====================================================
  Files                      825     823      -2     
  Lines                    73098   73030     -68     
  Branches                  8548    8604     +56     
=====================================================
- Hits                     57553   57486     -67     
+ Misses                   15545   15544      -1     
Files with missing lines Coverage Δ
include/xrpl/basics/Buffer.h 100.0% <ø> (ø)
include/xrpl/basics/Expected.h 100.0% <ø> (ø)
include/xrpl/basics/StringUtilities.h 100.0% <ø> (ø)
include/xrpl/basics/algorithm.h 100.0% <ø> (ø)
include/xrpl/basics/hardened_hash.h 100.0% <ø> (ø)
include/xrpl/basics/tagged_integer.h 100.0% <ø> (ø)
include/xrpl/beast/clock/abstract_clock.h 100.0% <ø> (ø)
include/xrpl/beast/clock/manual_clock.h 100.0% <ø> (ø)
...east/container/detail/aged_associative_container.h 100.0% <ø> (ø)
...pl/beast/container/detail/aged_ordered_container.h 97.4% <ø> (ø)
... and 63 more

... and 100 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

conanfile.py Outdated
"soci::soci",
"sqlite3::sqlite",
"wamr::iwasm",
"wamr::wamr",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is something wrong it should build with iwasm

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

conanfile.py (example/0.1.0): Writing generators to /__w/rippled/rippled/tests/conan/.build/build/Release/generators
conanfile.py (example/0.1.0): Generator 'CMakeDeps' calling 'generate()'
ERROR: Error in generator 'CMakeDeps': error generating context for 'xrpl/head': Component 'wamr::iwasm' not found in 'wamr' package requirement

https://github.com/XRPLF/rippled/actions/runs/16478627682/job/46587132726?pr=5602

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wamr is command line tool which build with different parameters and shouldn't be enabled at all in this build. There is something wrong

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wamr::wamr appears to be the global target, not the command line tool.

@mvadari mvadari merged commit 61b2dcc into XRPLF:ripple/smart-escrow Jul 23, 2025
25 checks passed
@mvadari mvadari deleted the develop4.5 branch July 23, 2025 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.