update wasm structure #12
Conversation
nguyen-zung
requested review from
seunghwalee-cramiumlabs,
zkrypt-crossbar and
sookyung-eom
WalkthroughThe changes update the package configuration to reference compiled output files, adjust TypeScript compiler settings for output and type declarations, and remove trailing blank lines in a method within the main wrapper class without altering functionality. Changes
Estimated code review effort2 (~15 minutes) Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
wasmjs/dist/index.d.tsis excluded by!**/dist/**wasmjs/dist/index.jsis excluded by!**/dist/**wasmjs/dist/test/tw.test.jsis excluded by!**/dist/**
📒 Files selected for processing (3)
wasmjs/package.json(1 hunks)wasmjs/src/index.ts(1 hunks)wasmjs/tsconfig.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: test-wasm
- GitHub Check: build-and-test
- GitHub Check: test
🔇 Additional comments (5)
wasmjs/package.json (2)
3-3: Version bump looks appropriate for structural changes.The version increment from 1.0.0 to 1.0.4 reflects the structural changes to the build output and new methods being added.
5-6: Good shift to compiled distribution files.The change from referencing source files directly (
src/index.ts) to compiled output (dist/index.jsanddist/index.d.ts) is the correct approach for npm package distribution. This ensures consumers get optimized JavaScript and proper TypeScript support.wasmjs/tsconfig.json (3)
6-7: Build output configuration aligns with package.json.The
outDir: "dist"anddeclaration: truesettings properly support the package.json changes that referencedist/index.jsanddist/index.d.ts.
12-12: Include scope appropriately narrowed for build.Focusing the compilation on
src/**/*is correct for the build process, removing test files from the compilation scope.
5-5: Confirm moduleResolution compatibility with importsI scanned wasmjs/src/index.ts and found only these external imports:
"craminiumlab-wallet-core""long"Switching from
"bundler"to"node"defers resolution to TypeScript’s Node-style algorithm. Please verify that both packages expose entrypoints compatible with Node resolution (viamain/moduleor anexportsfield). If either relies on bundler-specific mappings, you may need to switch to"node16"/"nodenext"or add explicitpathsin your tsconfig.
wasmjs/src/index.ts
Outdated
| buildEVMUnsignedTx(jsonStr: string): Uint8Array { | ||
| const req = JSON.parse(jsonStr); | ||
|
|
||
| const opPayment = TW.Stellar.Proto.OperationPayment.create({ | ||
| destination: req.destination, | ||
| amount: Long.fromString(String(req.amount)), | ||
| }) | ||
| const memoText = this.TW.Stellar.Proto.MemoText.create({ | ||
| text: req.memoText | ||
| }) | ||
| const input = { | ||
| fee : req.fee, | ||
| account: req.account, | ||
| passphrase: 'Public Global Stellar Network ; September 2015', | ||
| sequence: Long.fromString(String(req.sequence)), | ||
| opPayment: opPayment, | ||
| memoText: memoText | ||
| }; | ||
| return this.TW.Stellar.Proto.SigningInput.encode(input).finish(); | ||
| } |
There was a problem hiding this comment.
Critical: EVM method incorrectly implements Stellar logic.
The buildEVMUnsignedTx method is misleadingly named as it actually builds Stellar transactions using TW.Stellar.Proto types. This will cause confusion and runtime errors when used for EVM transactions.
Either:
- Rename the method to
buildStellarUnsignedTx2if this is intentional duplication, or - Implement proper EVM transaction building using Ethereum protobuf types:
- buildEVMUnsignedTx(jsonStr: string): Uint8Array {
- const req = JSON.parse(jsonStr);
-
- const opPayment = TW.Stellar.Proto.OperationPayment.create({
- destination: req.destination,
- amount: Long.fromString(String(req.amount)),
- })
- const memoText = this.TW.Stellar.Proto.MemoText.create({
- text: req.memoText
- })
- const input = {
- fee : req.fee,
- account: req.account,
- passphrase: 'Public Global Stellar Network ; September 2015',
- sequence: Long.fromString(String(req.sequence)),
- opPayment: opPayment,
- memoText: memoText
- };
- return this.TW.Stellar.Proto.SigningInput.encode(input).finish();
+ buildEVMUnsignedTx(jsonStr: string): Uint8Array {
+ const req = JSON.parse(jsonStr);
+ const input = {
+ chainId: this.HexCoding.decode(req.chainId),
+ nonce: this.HexCoding.decode(req.nonce),
+ gasPrice: this.HexCoding.decode(req.gasPrice),
+ gasLimit: this.HexCoding.decode(req.gasLimit),
+ toAddress: req.toAddress,
+ transaction: {
+ transfer: this.TW.Ethereum.Proto.Transaction.Transfer.create({
+ amount: this.HexCoding.decode(req.amount)
+ })
+ }
+ };
+ return this.TW.Ethereum.Proto.SigningInput.encode(input).finish();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| buildEVMUnsignedTx(jsonStr: string): Uint8Array { | |
| const req = JSON.parse(jsonStr); | |
| const opPayment = TW.Stellar.Proto.OperationPayment.create({ | |
| destination: req.destination, | |
| amount: Long.fromString(String(req.amount)), | |
| }) | |
| const memoText = this.TW.Stellar.Proto.MemoText.create({ | |
| text: req.memoText | |
| }) | |
| const input = { | |
| fee : req.fee, | |
| account: req.account, | |
| passphrase: 'Public Global Stellar Network ; September 2015', | |
| sequence: Long.fromString(String(req.sequence)), | |
| opPayment: opPayment, | |
| memoText: memoText | |
| }; | |
| return this.TW.Stellar.Proto.SigningInput.encode(input).finish(); | |
| } | |
| buildEVMUnsignedTx(jsonStr: string): Uint8Array { | |
| const req = JSON.parse(jsonStr); | |
| const input = { | |
| chainId: this.HexCoding.decode(req.chainId), | |
| nonce: this.HexCoding.decode(req.nonce), | |
| gasPrice: this.HexCoding.decode(req.gasPrice), | |
| gasLimit: this.HexCoding.decode(req.gasLimit), | |
| toAddress: req.toAddress, | |
| transaction: { | |
| transfer: this.TW.Ethereum.Proto.Transaction.Transfer.create({ | |
| amount: this.HexCoding.decode(req.amount) | |
| }) | |
| } | |
| }; | |
| return this.TW.Ethereum.Proto.SigningInput.encode(input).finish(); | |
| } |
🤖 Prompt for AI Agents
In wasmjs/src/index.ts lines 239 to 258, the method named buildEVMUnsignedTx is
incorrectly building Stellar transactions instead of EVM transactions, causing
confusion and potential runtime errors. To fix this, either rename the method to
buildStellarUnsignedTx2 to reflect its actual purpose or rewrite the method to
construct unsigned EVM transactions using the appropriate Ethereum protobuf
types instead of Stellar types.
wasmjs/src/index.ts
Outdated
| buildEVMUnsignedMessage(txInput: Uint8Array): Uint8Array { | ||
| const preimage = this.preImageHashes(this.CoinType.stellar, txInput); | ||
| const out = this.TW.TxCompiler.Proto.PreSigningOutput.decode(preimage) | ||
| return out.dataHash | ||
| } |
There was a problem hiding this comment.
Critical: EVM method uses wrong coin type.
The buildEVMUnsignedMessage method uses CoinType.stellar instead of an appropriate EVM coin type, which will produce incorrect results for EVM transactions.
- buildEVMUnsignedMessage(txInput: Uint8Array): Uint8Array {
- const preimage = this.preImageHashes(this.CoinType.stellar, txInput);
- const out = this.TW.TxCompiler.Proto.PreSigningOutput.decode(preimage)
- return out.dataHash
- }
+ buildEVMUnsignedMessage(txInput: Uint8Array): Uint8Array {
+ const preimage = this.preImageHashes(this.CoinType.ethereum, txInput);
+ const out = this.TW.TxCompiler.Proto.PreSigningOutput.decode(preimage)
+ return out.data
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| buildEVMUnsignedMessage(txInput: Uint8Array): Uint8Array { | |
| const preimage = this.preImageHashes(this.CoinType.stellar, txInput); | |
| const out = this.TW.TxCompiler.Proto.PreSigningOutput.decode(preimage) | |
| return out.dataHash | |
| } | |
| buildEVMUnsignedMessage(txInput: Uint8Array): Uint8Array { | |
| const preimage = this.preImageHashes(this.CoinType.ethereum, txInput); | |
| const out = this.TW.TxCompiler.Proto.PreSigningOutput.decode(preimage) | |
| return out.data | |
| } |
🤖 Prompt for AI Agents
In wasmjs/src/index.ts around lines 260 to 264, the buildEVMUnsignedMessage
method incorrectly uses CoinType.stellar when calling preImageHashes, which is
wrong for EVM transactions. Replace CoinType.stellar with the correct EVM coin
type constant to ensure the method processes EVM transactions properly and
produces correct results.
| marshalInput(input: protobuf.Writer): Uint8Array { | ||
| return input.finish(); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify protobuf import for marshalInput method.
The marshalInput method references protobuf.Writer but there's no protobuf import visible in this file. This will cause a TypeScript compilation error.
🏁 Script executed:
#!/bin/bash
# Check if protobuf is imported in the file
rg "import.*protobuf" wasmjs/src/index.tsLength of output: 43
🏁 Script executed:
#!/bin/bash
# Display the top of the file to inspect imports
sed -n '1,200p' wasmjs/src/index.ts
# Search for any protobuf references in the file
rg "protobuf" -n wasmjs/src/index.tsLength of output: 6196
Import missing protobuf in wasmjs/src/index.ts
The marshalInput method refers to protobuf.Writer but there’s no protobuf import in this file, which will trigger a TypeScript compilation error.
• File: wasmjs/src/index.ts
• Around line 266
Add at the top of the file:
import Long from "long";
+import * as protobuf from "protobufjs/minimal";If you’d rather import just the Writer type, you can instead do:
-import * as protobuf from "protobufjs/minimal";
+import { Writer } from "protobufjs/minimal";and update the signature to:
marshalInput(input: Writer): Uint8Array { … }🤖 Prompt for AI Agents
In wasmjs/src/index.ts around lines 266 to 268, the marshalInput method uses
protobuf.Writer but protobuf is not imported, causing a TypeScript error. Fix
this by adding an import statement for protobuf or just the Writer type at the
top of the file. Then update the marshalInput method signature accordingly to
use the imported Writer type if you choose the latter approach.
- https://www.npmjs.com/package/craminiumlab-wallet-core
- https://www.npmjs.com/package/craminiumlab-wasmjs
Thank you!
Summary by CodeRabbit