Skip to content

Conversation

@mvadari
Copy link
Collaborator

@mvadari mvadari commented Sep 11, 2025

High Level Overview of Change

This PR adds WAMR as a dependency to rippled, in preparation for Smart Escrows.

Context of Change

Splitting up #5600 into more managably reviewable chunks

XLS-102

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 and everything compiles in properly without error.

Future Tasks

Additional PRs for the rest of the Smart Escrow functionality

@mvadari mvadari changed the title fear: add WAMR dependency feat: add WAMR dependency Sep 11, 2025
@codecov
Copy link

codecov bot commented Sep 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.2%. Comparing base (f28ba57) to head (334bcfa).
⚠️ Report is 50 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #5790     +/-   ##
=========================================
- Coverage     78.2%   78.2%   -0.0%     
=========================================
  Files          816     816             
  Lines        68948   68948             
  Branches      8352    8354      +2     
=========================================
- Hits         53950   53941      -9     
- Misses       14998   15007      +9     

see 5 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.

Copy link
Collaborator

@oleks-rip oleks-rip left a comment

Choose a reason for hiding this comment

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

LGTM

BUILD.md Outdated
git sparse-checkout add recipes/soci
git fetch origin master
git checkout master
conan export --version 2.4.1 external/wamr # TODO: needs to be added to the conan center index
Copy link
Contributor

Choose a reason for hiding this comment

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

We've just removed all conan packaging code from external, let's not add it back please.
Could you please create this PR in http://github.com/XRPLF/conan-center-index?

It's our fork of official CCI, so we will be reviewing code, not Conan team

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

XRPLF/conan-center-index#17, please add it to conan

@Tapanito Tapanito added the Triaged Issue/PR has been triaged for viability, liveliness, etc. label Oct 3, 2025
@mathbunnyru
Copy link
Contributor

This separate PR is no longer needed because we added the dependency to our CCI fork (and Artifactory).
So we can (and probably should) add the usage of the new package directly in the PR where it is actually needed (otherwise, this is untested code and a dependency which we don't yet use).

If you really need it, this PR should be refactored to use the new package we added.

Copy link
Contributor

@mathbunnyru mathbunnyru left a comment

Choose a reason for hiding this comment

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

Overall, LGTM, left some nitpicking comments

Copy link
Collaborator

@vvysokikh1 vvysokikh1 left a comment

Choose a reason for hiding this comment

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

lgtm, but I'll leave the approval to @mathbunnyru

@mathbunnyru
Copy link
Contributor

lgtm, but I'll leave the approval to @mathbunnyru

Already left my review, so waiting for these small things to be fixed

mvadari and others added 2 commits October 29, 2025 16:41
Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com>
xrpl.libpb
xxHash::xxhash
$<$<BOOL:${voidstar}>:antithesis-sdk-cpp>
)

if (WIN32)
target_link_libraries(xrpl.imports.main INTERFACE ntdll)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if this is related, so leaving this change to rippled team

@mvadari mvadari added QE test required RippleX QE Team must look at this PR. Blocked labels Oct 30, 2025
@mvadari
Copy link
Collaborator Author

mvadari commented Dec 2, 2025

Closed in favor of #5999

@mvadari mvadari closed this Dec 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Blocked QE test required RippleX QE Team must look at this PR. Triaged Issue/PR has been triaged for viability, liveliness, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants