Skip to content

Disable reusing wasm module#6364

Open
oleks-rip wants to merge 5 commits intoXRPLF:ripple/wasmi-host-functionsfrom
oleks-rip:one_inst
Open

Disable reusing wasm module#6364
oleks-rip wants to merge 5 commits intoXRPLF:ripple/wasmi-host-functionsfrom
oleks-rip:one_inst

Conversation

@oleks-rip
Copy link
Collaborator

High Level Overview of Change

Reusing module functionality left only for performance testing and will not be available in release.

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

@oleks-rip oleks-rip requested a review from mvadari February 12, 2026 18:15
@codecov
Copy link

codecov bot commented Feb 12, 2026

Codecov Report

❌ Patch coverage is 95.90164% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.5%. Comparing base (1b4a564) to head (1768855).

Files with missing lines Patch % Lines
src/xrpld/app/wasm/HostFuncImpl.h 76.9% 3 Missing ⚠️
src/xrpld/app/wasm/detail/WasmVM.cpp 90.0% 1 Missing ⚠️
src/xrpld/app/wasm/detail/WasmiVM.cpp 95.0% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                      Coverage Diff                      @@
##           ripple/wasmi-host-functions   #6364     +/-   ##
=============================================================
- Coverage                         80.5%   80.5%   -0.0%     
=============================================================
  Files                              861     861             
  Lines                            67751   67702     -49     
  Branches                          7285    7286      +1     
=============================================================
- Hits                             54544   54490     -54     
- Misses                           13207   13212      +5     
Files with missing lines Coverage Δ
src/xrpld/app/wasm/HostFunc.h 100.0% <ø> (ø)
src/xrpld/app/wasm/ParamsHelper.h 100.0% <ø> (ø)
src/xrpld/app/wasm/WasmVM.h 100.0% <ø> (ø)
src/xrpld/app/wasm/WasmiVM.h 100.0% <ø> (ø)
src/xrpld/app/wasm/detail/HostFuncImpl.cpp 100.0% <100.0%> (ø)
src/xrpld/app/wasm/detail/HostFuncImplFloat.cpp 98.2% <100.0%> (ø)
src/xrpld/app/wasm/detail/HostFuncImplGetter.cpp 100.0% <100.0%> (ø)
src/xrpld/app/wasm/detail/HostFuncImplKeylet.cpp 100.0% <100.0%> (ø)
...xrpld/app/wasm/detail/HostFuncImplLedgerHeader.cpp 100.0% <100.0%> (ø)
src/xrpld/app/wasm/detail/HostFuncImplNFT.cpp 100.0% <100.0%> (ø)
... and 4 more

... and 2 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.


virtual Expected<std::uint32_t, HostFunctionError>
getLedgerSqn()
getLedgerSqn() const
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are these functions all changing to const?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because they can

Copy link
Collaborator

Choose a reason for hiding this comment

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

For next time, it would make the PR easier to read if the const changes were in their own PR (maybe a follow-up).

@oleks-rip oleks-rip force-pushed the one_inst branch 2 times, most recently from f5dbc0e to e2f88c9 Compare February 13, 2026 17:26

extern std::string const allHostFunctionsWasmHex;

extern std::string const deepRecursionHex;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are so many tests removed here? They're not all perf tests

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

recursion moved to loop tests, other tests using vector in parameters, which is disabled now, float tests grouped with float

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do the loop tests check recursion? Don't they just check loops?

Copy link
Collaborator Author

@oleks-rip oleks-rip Feb 18, 2026

Choose a reason for hiding this comment

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

recursion is another kind of a loop, so just group them for better perception

Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you sure? There doesn't seem to be another recursion stack overflow test. Same with badAlloc

Copy link
Collaborator Author

@oleks-rip oleks-rip Feb 18, 2026

Choose a reason for hiding this comment

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

Recursion can stop on 2 conditions: 1) gas finished - just like infinity loop; 2) stack finished. So ye, they are similar.

Bad alloc is impossible now, so it was removed. And bad alloc test has nothing in common with recursion or loop.

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.

3 participants

Comments