feat: add WASM host functions #5791
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## ripple/wamr #5791 +/- ##
=============================================
+ Coverage 78.2% 78.7% +0.4%
=============================================
Files 816 825 +9
Lines 68948 70964 +2016
Branches 8354 8436 +82
=============================================
+ Hits 53941 55819 +1878
- Misses 15007 15145 +138
... and 5 files with indirect coverage changes 🚀 New features to boost your workflow:
|
…unctions (#5850) * remove `get_ledger_account_hash` and `get_ledger_tx_hash` * fix build+tests
|
|
||
| # Require the rpc-reviewers team to review changes to the rpc code. | ||
| include/xrpl/protocol/ @xrplf/rpc-reviewers | ||
| src/libxrpl/protocol/ @xrplf/rpc-reviewers | ||
| src/xrpld/rpc/ @xrplf/rpc-reviewers | ||
| src/xrpld/app/misc/ @xrplf/rpc-reviewers |
There was a problem hiding this comment.
Why is this being removed?
There was a problem hiding this comment.
This change won't be merged. It's just so the group isn't pinged for all of our PRs not going into develop.
| auto const r = 2 * z / denom; | ||
| return r; |
There was a problem hiding this comment.
nit: could be simply return 2 * z / denom;
| auto const r = lnX / ln10; | ||
| return r; |
| --cacheIdx; | ||
| if (cacheIdx < 0 || cacheIdx >= MAX_CACHE) | ||
| return Unexpected(HostFunctionError::SLOT_OUT_RANGE); |
There was a problem hiding this comment.
I think this is a possibly integer underflow (UB). If the user passes minimal int, this will flip
| if (!isLedgerObjCached) | ||
| { | ||
| isLedgerObjCached = true; | ||
| currentLedgerObj = ctx.view().read(leKey); | ||
| } | ||
| if (currentLedgerObj) | ||
| return currentLedgerObj; | ||
| return Unexpected(HostFunctionError::LEDGER_OBJ_NOT_FOUND); | ||
| } |
There was a problem hiding this comment.
this whole function does not make a lot of sense as API.
if we are talking only in context of escrow, then in escrowFinish::preflight we already verified that sle can be found. In this case we can just pass the SLE into the WasmHostFunctionImpl constructor and consider it cached already. There will be no need for the bool variable to track it, no error code etc.
Even if we expand on this to consider other types of SLE, it would still make more sense to just cache this SLE?
Even in case we want to do it this way, it still is enough to read the object, then return it or the error depending on if it's nullptr or not. The bool variable here is completely obsolete
|
Closed in favor of #6075 |
High Level Overview of Change
This PR adds all the WAMR integration code and the host functions specified in XLS-102. It also adds tests for all of this.
Note for reviewers: 10k lines of this PR are just WASM-compiled test fixtures, and another 4k lines are WASM test fixture source code. So while this PR is still quite large, it's not as large as it seems on the surface.
Context of Change
This PR depends on #5790
It is part of an effort to split up #5600 into smaller, more managable-to-review PRs.
XLS-102: XRPLF/XRPL-Standards#303
Type of Change
.gitignore, formatting, dropping support for older tooling)API Impact
N/A
Test Plan
Several tests are added to this PR.