-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(quickjs): add test_sto_emplace #448
Conversation
sublimator
commented
Feb 25, 2025
•
edited
Loading
edited
- Adds JSHooks amendment to cfg/rippled-standalone.cfg and cfg/genesis.json
- Adds test_sto_emplace
- Fixes WASM impl of sto_emplace to pass nullptr
- Update build_test_jshooks.sh to include dependency install info and generated comments
var sto = [ | ||
17, 0, 97, 34, 0, 0, 0, 0, 36, 4, 31, 148, 217, 37, 4, 94, 132, 183, 45, 0, 0, | ||
0, 0, 85, 19, 64, 179, 37, 134, 49, 150, 181, 111, 65, 245, 137, 235, 125, 47, | ||
217, 76, 13, 125, 184, 14, 75, 44, 103, 167, 120, 42, 214, 194, 176, 119, 80, | ||
98, 64, 0, 0, 0, 0, 164, 121, 148, 129, 20, 55, 223, 68, 7, 231, 170, 7, 241, | ||
213, 201, 145, 242, 211, 111, 158, 184, 199, 52, 175, 108, | ||
] | ||
var ins = [ | ||
86, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, | ||
17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, | ||
] | ||
var ans = [ | ||
17, 0, 97, 34, 0, 0, 0, 0, 36, 4, 31, 148, 217, 37, 4, 94, 132, 183, 45, 0, 0, | ||
0, 0, 85, 19, 64, 179, 37, 134, 49, 150, 181, 111, 65, 245, 137, 235, 125, 47, | ||
217, 76, 13, 125, 184, 14, 75, 44, 103, 167, 120, 42, 214, 194, 176, 119, 80, | ||
86, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, | ||
17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 98, 64, 0, 0, 0, 0, | ||
164, 121, 148, 129, 20, 55, 223, 68, 7, 231, 170, 7, 241, 213, 201, 145, 242, | ||
211, 111, 158, 184, 199, 52, 175, 108, | ||
] | ||
var ans2 = [ | ||
17, 0, 97, 34, 0, 0, 0, 0, 36, 4, 31, 148, 217, 37, 4, 94, 132, 183, 45, 0, 0, | ||
0, 0, 84, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, | ||
17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 85, 19, 64, 179, | ||
37, 134, 49, 150, 181, 111, 65, 245, 137, 235, 125, 47, 217, 76, 13, 125, 184, | ||
14, 75, 44, 103, 167, 120, 42, 214, 194, 176, 119, 80, 98, 64, 0, 0, 0, 0, | ||
164, 121, 148, 129, 20, 55, 223, 68, 7, 231, 170, 7, 241, 213, 201, 145, 242, | ||
211, 111, 158, 184, 199, 52, 175, 108, | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good if those were written in hexadecimal to match the other test cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See: #448 (comment)
ASSERT(buf.length == sto.length + ins.length) | ||
for (let i = 0; i < ans2.length; ++i) ASSERT(ans2[i] == buf[i]) | ||
} | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the CHook test, there is a comment for test front insertion
, but it would be easier to compare if there were similar comments here too.
The same for other parts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't actually go over this test that deeply tbh, as it was one that was already done:
I do note that the source typescript had lovely 0x hexadecimal literals, but the hooks-cli compile-js contracts-js/toolbox/base.ts build
does not seem to maintain them in the output dist/base.js
.
I can simply copy them ofc, but it may be worth seeing if we can get the compile-js
command to do "the right thing" here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its fine for now. We will do another full pass through of all tests again.
Can you also make a PR for the test in hooks-toolkit
@@ -62,7 +62,8 @@ | |||
"3C43D9A973AA4443EF3FC38E42DD306160FBFFDAB901CD8BAA15D09F2597EB87", | |||
"0285B7E5E08E1A8E4C15636F0591D87F73CB6A7B6452A932AD72BBC8E5D1CBE3", | |||
"6E739F4F8B07BED29FC9FF440DA3C301CD14A180DF45819F658FEC2F7DE31427", | |||
"36799EA497B1369B170805C078AEFE6188345F9B3E324C21E9CA3FF574E3C3D6" | |||
"36799EA497B1369B170805C078AEFE6188345F9B3E324C21E9CA3FF574E3C3D6", | |||
"DD4F86291F142A20761B32B4D0CE4291F86CA33F0B46F0D04171482FBA52E536" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this shouldn't be changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, maybe not
We are targeting the jshooks branch though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did note that the xrpld-network-gen project uses a --ledgerfile with no amendments at all, and must rely on some other way of enabling them:
https://github.com/Transia-RnD/xrpld-network-gen/blob/main/xrpld_netgen/genesis.xahau.json
In other words, maybe we should remove /all/ the Amendments here, but shrug. I'm not actually sure what this file is used for. I suppose also starting test nets?!