-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/wasm #10
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/wasm #10
Conversation
WalkthroughThis update introduces Bitcoin Cash support to the Go wrapper, including new constants and a sample signing function. It also adds a new JavaScript/TypeScript module ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WalletCoreWrapper
participant WASM_Core
participant Blockchain_Protobuf
User->>WalletCoreWrapper: init()
WalletCoreWrapper->>WASM_Core: initWasm()
WASM_Core-->>WalletCoreWrapper: WASM instance
User->>WalletCoreWrapper: buildAptosUnsignedTx(json)
WalletCoreWrapper->>Blockchain_Protobuf: parse JSON, marshal to protobuf
Blockchain_Protobuf-->>WalletCoreWrapper: UnsignedTx bytes
User->>WalletCoreWrapper: preImageHashes(coinType, txInput)
WalletCoreWrapper->>WASM_Core: preImageHashes(coinType, txInput)
WASM_Core-->>WalletCoreWrapper: Preimage hash
User->>WalletCoreWrapper: compileWithSignatures(coinType, txInput, sigs, pubKeys)
WalletCoreWrapper->>WASM_Core: compileWithSignatures(coinType, txInput, sigs, pubKeys)
WASM_Core-->>WalletCoreWrapper: SignedTx bytes
sequenceDiagram
participant GoMain
participant sample.bitcoincash
participant core
participant protobuf
GoMain->>sample.bitcoincash: TestBitcoinCash()
sample.bitcoincash->>protobuf: Marshal SigningInput
sample.bitcoincash->>core: PreImageHashes(CoinTypeBitcoinCash, input)
core-->>sample.bitcoincash: PreSigningOutput
sample.bitcoincash->>core: CompileWithSignatures(CoinTypeBitcoinCash, input, sig, pubkey)
core-->>sample.bitcoincash: SigningOutput
sample.bitcoincash->>core: PublicKeyVerifyAsDER(pubkey, sig, hash)
core-->>sample.bitcoincash: Verification result
Possibly related PRs
Suggested reviewers
Poem
✨ 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 (
|
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.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
wrapper/go-wrapper/main.go (1)
76-80
: Avoid float math when building Wei amounts
0.01 * math.Pow10(ew.CoinType.Decimals())
mixesfloat64
withint
→ a rounding step viaint64
, which can silently lose precision for 18-decimals chains. Build the amount with integer maths instead:-Amount: big.NewInt(int64( - 0.01 * math.Pow10(ew.CoinType.Decimals()), -)).Bytes(), +amountWei := new(big.Int).Exp(big.NewInt(10), big.NewInt(int64(ew.CoinType.Decimals())), nil) // 1e18 +amountWei.Div(amountWei, big.NewInt(100)) // 1% +Amount: amountWei.Bytes(),
🧹 Nitpick comments (15)
wrapper/go-wrapper/main.go (1)
60-60
: Guard samples that maypanic
sample.TestBitcoinCash()
panics on any failure (it re-uses the test helper verbatim). Invoking it unconditionally frommain()
will crash the CLI on network glitches or signature mismatch. Wrap it, or move it behind a-demo
flag so production consumers aren’t affected.wasm/tests/Blockchain/Cosmos.test.ts (1)
1-3
: Incomplete test file - no test cases implemented.This file only contains imports with no actual test implementation. Consider adding test cases for Cosmos blockchain functionality to provide proper test coverage.
Would you like me to generate a sample test case structure for Cosmos blockchain testing, similar to the existing Aptos tests?
wasm/tests/Blockchain/Aptos.test.ts (2)
32-36
: Remove console.log statements before merging.These debugging console.log statements should be removed before merging to production to avoid cluttering test output.
Apply this diff to remove the debugging statements:
- console.log('Aptos input:', input.length) - console.log(TransactionCompiler) - const hash = TransactionCompiler.preImageHashes(CoinType.aptos, input) - console.log('hash:', hash) - console.log('type:', CoinType.aptos.value) + const hash = TransactionCompiler.preImageHashes(CoinType.aptos, input)
42-42
: Remove console.log statement.This debugging statement should be removed before merging.
Apply this diff:
- console.log(globalThis.walletCore)
wasmjs/package.json (2)
4-4
: Add package description.The description field is empty. Consider adding a meaningful description for the package.
12-12
: Add author information.The author field is empty. Consider adding author information for the package.
wasmjs/test/tw.test.ts (5)
31-31
: Remove console.log statement.This debugging statement should be removed before merging to production.
Apply this diff:
- console.log('sigHash raw:', (sigHash))
80-80
: Remove console.log statement.This debugging statement should be removed before merging to production.
Apply this diff:
- console.log('sigHash raw:', (sigHash))
65-67
: Consider using expect().toEqual() instead of expect().to.equal().For consistency with Vitest testing framework, consider using
toEqual()
instead ofto.equal()
which is Chai syntax.Apply this diff:
- expect(wallet.encodeHex(decoded.encoded)).to.equal( + expect(wallet.encodeHex(decoded.encoded)).toEqual(
94-94
: Consider using expect().toEqual() instead of expect().to.equal().For consistency with Vitest testing framework, consider using
toEqual()
instead ofto.equal()
which is Chai syntax.Apply this diff:
- expect(txencoded).to.equal("0x83a400828258208316e5007d61fb90652cabb41141972a38b5bc60954d602cf843476aa3f67f6300825820e29392c59c903fefb905730587d22cae8bda30bd8d9aeec3eca082ae77675946000182825839015fcbab3d70db82b3b9da5686d1ff51c83b97e032e52bd45a6ce6fe7908ec32633484b152fa756444e5fc62128210bc1fd7b8253ec5490b281a002dc6c082583901a9426fe0cee6d01d1fe32af650e1e7b5d52c35d8a53218f3d0861531621c2b1ebdf4f11f96da67fdcb0e1d97a7e778566166be55f193c30f1a000f9ec1021a0002b0bf031a0b532b80a20081825820d163c8c4f0be7c22cd3a1152abb013c855ea614b92201497a568c5d93ceeb41e58406a23ab9267867fbf021c1cb2232bc83d2cdd663d651d22d59b6cddbca5cb106d4db99da50672f69a2309ca8a329a3f9576438afe4538b013de4591a6dfcd4d090281845820d163c8c4f0be7c22cd3a1152abb013c855ea614b92201497a568c5d93ceeb41e58406a23ab9267867fbf021c1cb2232bc83d2cdd663d651d22d59b6cddbca5cb106d4db99da50672f69a2309ca8a329a3f9576438afe4538b013de4591a6dfcd4d095820a7f484aa383806735c46fd769c679ee41f8952952036a6e2338ada940b8a91f441a0f6") + expect(txencoded).toEqual("0x83a400828258208316e5007d61fb90652cabb41141972a38b5bc60954d602cf843476aa3f67f6300825820e29392c59c903fefb905730587d22cae8bda30bd8d9aeec3eca082ae77675946000182825839015fcbab3d70db82b3b9da5686d1ff51c83b97e032e52bd45a6ce6fe7908ec32633484b152fa756444e5fc62128210bc1fd7b8253ec5490b281a002dc6c082583901a9426fe0cee6d01d1fe32af650e1e7b5d52c35d8a53218f3d0861531621c2b1ebdf4f11f96da67fdcb0e1d97a7e778566166be55f193c30f1a000f9ec1021a0002b0bf031a0b532b80a20081825820d163c8c4f0be7c22cd3a1152abb013c855ea614b92201497a568c5d93ceeb41e58406a23ab9267867fbf021c1cb2232bc83d2cdd663d651d22d59b6cddbca5cb106d4db99da50672f69a2309ca8a329a3f9576438afe4538b013de4591a6dfcd4d090281845820d163c8c4f0be7c22cd3a1152abb013c855ea614b92201497a568c5d93ceeb41e58406a23ab9267867fbf021c1cb2232bc83d2cdd663d651d22d59b6cddbca5cb106d4db99da50672f69a2309ca8a329a3f9576438afe4538b013de4591a6dfcd4d095820a7f484aa383806735c46fd769c679ee41f8952952036a6e2338ada940b8a91f441a0f6")
73-73
: Consider extracting long JSON string to a variable or external file.The inline JSON string is quite long and makes the test harder to read. Consider extracting it to a variable or external test data file.
Apply this diff:
+ const cardanoTestData = { + "address": "", + "toAddress": "addr1q90uh2eawrdc9vaemftgd50l28yrh9lqxtjjh4z6dnn0u7ggasexxdyyk9f05atygnjlccsjsggtc87hhqjna32fpv5qeq96ls", + "changeAddress": "addr1qx55ymlqemndq8gluv40v58pu76a2tp4mzjnyx8n6zrp2vtzrs43a0057y0edkn8lh9su8vh5lnhs4npv6l9tuvncv8swc7t08", + "amount": 3000000, + "ttl": 190000000, + "utxos": [ + { + "tx_hash": "gxblAH1h+5BlLKu0EUGXKji1vGCVTWAs+ENHaqP2f2M=", + "output_index": 0, + "address": "Ae2tdPwUPEZ6vkqxSjJxaQYmDxHf5DTnxtZ67pFLJGTb9LTnCGkDP6ca3f8", + "amount": 2500000 + }, + { + "tx_hash": "4pOSxZyQP++5BXMFh9IsrovaML2Nmu7D7KCCrndnWUY=", + "output_index": 0, + "address": "Ae2tdPwUPEZ6vkqxSjJxaQYmDxHf5DTnxtZ67pFLJGTb9LTnCGkDP6ca3f8", + "amount": 1700000 + } + ] + }; + const jsonString = JSON.stringify(cardanoTestData); - const jsonString = '{\"address\":\"\",\"toAddress\":\"addr1q90uh2eawrdc9vaemftgd50l28yrh9lqxtjjh4z6dnn0u7ggasexxdyyk9f05atygnjlccsjsggtc87hhqjna32fpv5qeq96ls\",\"changeAddress\":\"addr1qx55ymlqemndq8gluv40v58pu76a2tp4mzjnyx8n6zrp2vtzrs43a0057y0edkn8lh9su8vh5lnhs4npv6l9tuvncv8swc7t08\",\"amount\":3000000,\"ttl\":190000000,\"utxos\":[{\"tx_hash\":\"gxblAH1h+5BlLKu0EUGXKji1vGCVTWAs+ENHaqP2f2M=\",\"output_index\":0,\"address\":\"Ae2tdPwUPEZ6vkqxSjJxaQYmDxHf5DTnxtZ67pFLJGTb9LTnCGkDP6ca3f8\",\"amount\":2500000},{\"tx_hash\":\"4pOSxZyQP++5BXMFh9IsrovaML2Nmu7D7KCCrndnWUY=\",\"output_index\":0,\"address\":\"Ae2tdPwUPEZ6vkqxSjJxaQYmDxHf5DTnxtZ67pFLJGTb9LTnCGkDP6ca3f8\",\"amount\":1700000}]}';wrapper/go-wrapper/sample/bitcoincash.go (2)
13-76
: Consider improving error handling and test structureThe function uses
panic
for all error cases, which isn't ideal for a sample/test function. Consider:
- Using proper test framework (e.g.,
testing.T
) witht.Fatal()
ort.Error()
- Returning errors instead of panicking
- Adding descriptive test assertions
-func TestBitcoinCash() { +func TestBitcoinCash(t *testing.T) error { // https://blockchair.com/bitcoin-cash/transaction/96ee20002b34e468f9d3c5ee54f6a8ddaa61c118889c4f35395c2cd93ba5bbb4 Address := "bitcoincash:qzhlrcrcne07x94h99thved2pgzdtv8ccujjy73xya" input := &bitcoin.SigningInput{ HashType: uint32(core.BitcoinSigHashTypeAll), Amount: 600, ByteFee: 1, ToAddress: "1Bp9U1ogV3A14FMvKbRJms7ctyso4Z4Tcx", ChangeAddress: "1FQc5LdgGHMHEN9nwkjmz6tWkxhPpxBvBU", } utxoHash, err := hex.DecodeString("e28c2b955293159898e34c6840d99bf4d390e2ee1c6f606939f18ee1e2000d05") if err != nil { - panic(err) + return fmt.Errorf("failed to decode UTXO hash: %w", err) }
62-63
: Document the hardcoded test valuesThe hardcoded signature and public key should have comments explaining their origin (e.g., from the referenced transaction).
+ // Signature and public key from the referenced transaction + // https://blockchair.com/bitcoin-cash/transaction/96ee20002b34e468f9d3c5ee54f6a8ddaa61c118889c4f35395c2cd93ba5bbb4 signature, _ := hex.DecodeString("304402204c8506e96808b672fce2a0c2ee958f9fa1e9b39fbb62741efb9857e77daa94f102202f560e3c05d215e6b397b5dd49df289bbdbbb7d80faea31bc37f20f6aa92b88d") pubkey, _ := hex.DecodeString("038eab72ec78e639d02758e7860cdec018b49498c307791f785aa3019622f4ea5b")wasmjs/src/index.ts (1)
17-17
: Remove underscore prefix from used parameterThe
_tw
parameter is stored inthis.TW
, so it's not unused. The underscore prefix is misleading.- constructor(core: WalletCore, _tw = TW) { + constructor(core: WalletCore, tw = TW) { const { CoinType, HexCoding, AnySigner, TransactionCompiler, DataVector, PrivateKey } = core; this.CoinType = CoinType; this.HexCoding = HexCoding; this.AnySigner = AnySigner; this.TransactionCompiler = TransactionCompiler; this.DataVector = DataVector; this.PrivateKey = PrivateKey; - this.TW = _tw; + this.TW = tw; }wasm/tsconfig.json (1)
23-23
: Preserve trailing newline for cross-tool consistencyRemoving the final newline can trigger lint warnings (e.g.,
no-eol-last
) and cause noisy diffs in the future. Most editors and CI linters expect files to end with\n
.} +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
wasm/package-lock.json
is excluded by!**/package-lock.json
wasmjs/dist/src/index.js
is excluded by!**/dist/**
wasmjs/dist/test/tw.test.js
is excluded by!**/dist/**
wasmjs/package-lock.json
is excluded by!**/package-lock.json
wasmjs/public/wallet-core.wasm
is excluded by!**/*.wasm
📒 Files selected for processing (13)
wasm/index.ts
(1 hunks)wasm/package.json
(1 hunks)wasm/tests/Blockchain/Aptos.test.ts
(2 hunks)wasm/tests/Blockchain/Cosmos.test.ts
(1 hunks)wasm/tsconfig.json
(1 hunks)wasmjs/package.json
(1 hunks)wasmjs/public/wallet-core.js
(1 hunks)wasmjs/src/index.ts
(1 hunks)wasmjs/test/tw.test.ts
(1 hunks)wasmjs/tsconfig.json
(1 hunks)wrapper/go-wrapper/core/coin.go
(1 hunks)wrapper/go-wrapper/main.go
(1 hunks)wrapper/go-wrapper/sample/bitcoincash.go
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
wrapper/go-wrapper/main.go (1)
wrapper/go-wrapper/sample/bitcoincash.go (1)
TestBitcoinCash
(13-76)
wrapper/go-wrapper/sample/bitcoincash.go (1)
wrapper/go-wrapper/core/coin.go (2)
CoinType
(11-11)CoinTypeBitcoinCash
(26-26)
🪛 Biome (1.9.4)
wasmjs/public/wallet-core.js
[error] 30-30: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 53-53: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 63-63: Comparing to itself is potentially pointless.
(lint/suspicious/noSelfCompare)
[error] 67-67: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 68-68: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 71-71: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 72-72: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 76-76: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 76-76: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 77-77: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 77-77: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 82-82: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 83-83: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 83-83: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 88-88: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 89-89: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 89-89: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 93-93: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 102-102: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 124-124: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 125-125: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 126-126: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 128-128: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 144-144: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 146-146: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 147-147: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.
(lint/suspicious/noConfusingLabels)
[error] 149-149: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 149-149: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the catch clause.
(lint/complexity/noUselessCatch)
[error] 149-149: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.
(lint/suspicious/noConfusingLabels)
[error] 151-151: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.
(lint/suspicious/noConfusingLabels)
[error] 154-154: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 154-154: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 161-161: stdin is assigned to itself.
This is where is assigned.
(lint/correctness/noSelfAssign)
[error] 161-161: stdout is assigned to itself.
This is where is assigned.
(lint/correctness/noSelfAssign)
[error] 161-161: stderr is assigned to itself.
This is where is assigned.
(lint/correctness/noSelfAssign)
[error] 15-15: A global variable should not be reassigned.
Assigning to a global variable can override essential functionality.
(lint/suspicious/noGlobalAssign)
[error] 114-114: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 114-114: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 114-114: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 138-138: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 77-77: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 81-81: Do not reassign a function declaration.
Reassigned here.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 147-147: Do not use the a variable name as a label
The variable is declared here
Creating a label with the same name as an in-scope variable leads to confusion.
(lint/suspicious/noLabelVar)
[error] 149-149: Do not use the a variable name as a label
The variable is declared here
Creating a label with the same name as an in-scope variable leads to confusion.
(lint/suspicious/noLabelVar)
[error] 151-151: Do not use the a variable name as a label
The variable is declared here
Creating a label with the same name as an in-scope variable leads to confusion.
(lint/suspicious/noLabelVar)
[error] 156-156: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 156-156: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 156-156: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
⏰ 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: test-wasm
- GitHub Check: build-and-test
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: test
🔇 Additional comments (6)
wasm/index.ts (1)
12-13
: Whitespace-only change looks fine
No functional impact.wrapper/go-wrapper/core/coin.go (1)
14-27
: Remember to propagate the new CoinType everywhere
CoinTypeBitcoinCash
is added, but make sure:
- All switch/if statements mapping
CoinType
→ coin-specific behaviour (fee calculation, HRP, script generator, etc.) have aBitcoinCash
branch.- Tests cover
GetName()
,Decimals()
, address validation and signing for BCH.Consider running the integration test suite with the help of the sample already included.
wasm/package.json (1)
2-4
: Package rename may break downstream imports
Changing"@trustwallet/wallet-core"
→"craminiumlab-wallet-core"
is a breaking change. Confirm that:
- All internal
require()
/import
statements in this repo & the newwasmjs
package point to the new name.- External consumers are notified (release notes or
npm deprecate
message on the old package).- Consider updating
"repository"
to the new GitHub org to avoid confusion.wasmjs/tsconfig.json (1)
1-12
: TypeScript configuration looks good.The configuration is well-structured with appropriate modern settings for a WASM project. The ES2020 target and ESNext modules are suitable choices, and enabling strict mode is a good practice.
wasmjs/package.json (1)
24-28
: All dependencies are at their latest stable versions
The entries in wasmjs/package.json are current—no updates are available for:
- craminiumlab-wallet-core ^1.0.2 (latest 1.0.2)
- long ^5.3.2 (latest 5.3.2)
- protobufjs ^7.5.3 (latest 7.5.3)
Please run
npm audit
(or your preferred scanner) to verify there are no known security vulnerabilities.wasmjs/public/wallet-core.js (1)
1-179
: Auto-generated file - minimal review neededThis appears to be an auto-generated WASM binding file. While static analysis shows various issues, most are expected in generated code and don't require manual fixes. Consider adding a header comment to indicate this file is auto-generated and should not be manually edited.
"name": "craminiumlab-wasmjs", | ||
"version": "1.0.0", | ||
"description": "", | ||
"main": "src/index.ts", |
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.
🛠️ Refactor suggestion
Main field should point to compiled JavaScript.
The main field points to a TypeScript file (src/index.ts
) instead of the compiled JavaScript output. This should typically point to the compiled version in the dist directory.
Apply this diff:
- "main": "src/index.ts",
+ "main": "dist/index.js",
📝 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.
"main": "src/index.ts", | |
"main": "dist/index.js", |
🤖 Prompt for AI Agents
In wasmjs/package.json at line 5, the "main" field incorrectly points to the
TypeScript source file "src/index.ts". Update this field to point to the
compiled JavaScript output, typically located in the "dist" directory, such as
"dist/index.js", to ensure proper module resolution.
msgForSign := core.PreImageHashes(core.CoinTypeBitcoinCash, txInputData) | ||
if msgForSign == nil || len(msgForSign) == 0 { | ||
panic(err) | ||
} |
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.
Fix incorrect error variable usage
The code panics with err
but should create a proper error message since err
wasn't set by PreImageHashes
.
msgForSign := core.PreImageHashes(core.CoinTypeBitcoinCash, txInputData)
if msgForSign == nil || len(msgForSign) == 0 {
- panic(err)
+ panic(errors.New("PreImageHashes returned empty result"))
}
🤖 Prompt for AI Agents
In wrapper/go-wrapper/sample/bitcoincash.go around lines 50 to 53, the code
panics using an undefined variable `err` when `msgForSign` is nil or empty.
Replace the panic argument with a newly created error message that clearly
indicates the failure to generate pre-image hashes, since `err` is not set by
`PreImageHashes`.
marshalInput(input: protobuf.Writer): Uint8Array { | ||
return input.finish(); | ||
} | ||
|
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.
💡 Verification agent
🧩 Analysis chain
Verify protobuf.Writer type is available
The protobuf.Writer
type is used but may not be imported. Ensure the type is available or use the appropriate type from the imported modules.
🏁 Script executed:
#!/bin/bash
# Check if protobuf types are imported or available
rg -A 5 "import.*protobuf" wasmjs/src/
rg -A 5 "Writer.*finish" wasmjs/
Length of output: 75
🏁 Script executed:
#!/bin/bash
set -e
echo "=== Imports in wasmjs/src/index.ts ==="
rg '^import' -n wasmjs/src/index.ts || true
echo
echo "=== 'Writer' references in wasmjs/src/index.ts ==="
rg -n 'Writer' wasmjs/src/index.ts || true
Length of output: 545
Add missing import for protobuf.Writer
The method marshalInput(input: protobuf.Writer): Uint8Array { … }
references protobuf.Writer
, but there’s no import for protobuf
in wasmjs/src/index.ts
. Please import the correct module (for example, protobufjs/minimal
) so that protobuf.Writer
is recognized.
• File: wasmjs/src/index.ts
• Around line 147
Suggested diff:
import Long from "long";
+import * as protobuf from "protobufjs/minimal"; // or the appropriate protobufjs entrypoint
marshalInput(input: protobuf.Writer): Uint8Array {
return 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.
marshalInput(input: protobuf.Writer): Uint8Array { | |
return input.finish(); | |
} | |
import Long from "long"; | |
import * as protobuf from "protobufjs/minimal"; // or the appropriate protobufjs entrypoint | |
marshalInput(input: protobuf.Writer): Uint8Array { | |
return input.finish(); | |
} |
🤖 Prompt for AI Agents
In wasmjs/src/index.ts around lines 147 to 150, the method marshalInput uses
protobuf.Writer but the protobuf module is not imported. Add an import statement
for protobuf from the appropriate package (e.g., protobufjs/minimal) at the top
of the file to ensure protobuf.Writer is recognized and avoid reference errors.
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.
Actionable comments posted: 5
♻️ Duplicate comments (1)
wasmjs/src/index.ts (1)
239-241
: Add missing import for protobuf.Writer.The method references
protobuf.Writer
but the protobuf module is not imported.import Long from "long"; +import * as protobuf from "protobufjs/minimal"; marshalInput(input: protobuf.Writer): Uint8Array { return input.finish(); }
🧹 Nitpick comments (1)
wasmjs/src/index.ts (1)
1-4
: Consider consolidating imports from the same package.Multiple imports from the same package can be consolidated for better readability and maintainability.
-import * as Core from "craminiumlab-wallet-core"; -import { TW } from "craminiumlab-wallet-core"; -import { WalletCore, initWasm } from "craminiumlab-wallet-core"; +import * as Core from "craminiumlab-wallet-core"; +import { TW, WalletCore, initWasm } from "craminiumlab-wallet-core";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
wasmjs/dist/src/index.js
is excluded by!**/dist/**
wasmjs/dist/test/tw.test.js
is excluded by!**/dist/**
📒 Files selected for processing (2)
wasmjs/src/index.ts
(1 hunks)wasmjs/test/tw.test.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- wasmjs/test/tw.test.ts
⏰ 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 (2)
wasmjs/src/index.ts (2)
8-39
: Well-structured wrapper class with proper encapsulation.The class design effectively encapsulates WalletCore functionality and provides a clean interface for WASM initialization. The static async init pattern is appropriate for WebAssembly modules.
60-83
: Signature compilation logic is well-implemented.The method correctly handles the conversion of hex-encoded signatures and public keys to byte arrays and properly utilizes the DataVector for compilation.
buildNearUnsignedTx(jsonStr: string): Uint8Array { | ||
const req = JSON.parse(jsonStr); | ||
|
||
const action = this.TW.NEAR.Proto.Action.create({ | ||
transfer: this.TW.NEAR.Proto.Transfer.create({ | ||
deposit: this.HexCoding.decode(req.transferAmount) | ||
}) | ||
}) | ||
|
||
const input = { | ||
signerId: req.signerId, | ||
nonce: Long.fromString(String(req.nonce)), | ||
receiverId: req.receiverId, | ||
blockHash: this.HexCoding.decode(req.blockHash), | ||
actions: [action], | ||
publicKey: this.HexCoding.decode(req.publicKey), | ||
}; | ||
return this.TW.NEAR.Proto.SigningInput.encode(input).finish(); | ||
} |
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.
🛠️ Refactor suggestion
Add error handling for JSON parsing and input validation.
Consistent error handling is needed across all blockchain methods.
buildNearUnsignedTx(jsonStr: string): Uint8Array {
- const req = JSON.parse(jsonStr);
+ let req;
+ try {
+ req = JSON.parse(jsonStr);
+ } catch (error) {
+ throw new Error(`Invalid JSON input: ${error.message}`);
+ }
+
+ // Validate required fields
+ const requiredFields = ['signerId', 'nonce', 'receiverId', 'blockHash', 'transferAmount', 'publicKey'];
+ for (const field of requiredFields) {
+ if (req[field] === undefined || req[field] === null) {
+ throw new Error(`Missing required field: ${field}`);
+ }
+ }
📝 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.
buildNearUnsignedTx(jsonStr: string): Uint8Array { | |
const req = JSON.parse(jsonStr); | |
const action = this.TW.NEAR.Proto.Action.create({ | |
transfer: this.TW.NEAR.Proto.Transfer.create({ | |
deposit: this.HexCoding.decode(req.transferAmount) | |
}) | |
}) | |
const input = { | |
signerId: req.signerId, | |
nonce: Long.fromString(String(req.nonce)), | |
receiverId: req.receiverId, | |
blockHash: this.HexCoding.decode(req.blockHash), | |
actions: [action], | |
publicKey: this.HexCoding.decode(req.publicKey), | |
}; | |
return this.TW.NEAR.Proto.SigningInput.encode(input).finish(); | |
} | |
buildNearUnsignedTx(jsonStr: string): Uint8Array { | |
- const req = JSON.parse(jsonStr); | |
+ let req; | |
+ try { | |
+ req = JSON.parse(jsonStr); | |
+ } catch (error) { | |
+ throw new Error(`Invalid JSON input: ${error.message}`); | |
+ } | |
+ | |
+ // Validate required fields | |
+ const requiredFields = ['signerId', 'nonce', 'receiverId', 'blockHash', 'transferAmount', 'publicKey']; | |
+ for (const field of requiredFields) { | |
+ if (req[field] === undefined || req[field] === null) { | |
+ throw new Error(`Missing required field: ${field}`); | |
+ } | |
+ } | |
const action = this.TW.NEAR.Proto.Action.create({ | |
transfer: this.TW.NEAR.Proto.Transfer.create({ | |
deposit: this.HexCoding.decode(req.transferAmount) | |
}) | |
}) | |
const input = { | |
signerId: req.signerId, | |
nonce: Long.fromString(String(req.nonce)), | |
receiverId: req.receiverId, | |
blockHash: this.HexCoding.decode(req.blockHash), | |
actions: [action], | |
publicKey: this.HexCoding.decode(req.publicKey), | |
}; | |
return this.TW.NEAR.Proto.SigningInput.encode(input).finish(); | |
} |
🤖 Prompt for AI Agents
In wasmjs/src/index.ts around lines 186 to 204, the buildNearUnsignedTx method
lacks error handling for JSON parsing and input validation. Wrap the JSON.parse
call in a try-catch block to catch and handle parsing errors gracefully.
Additionally, validate the required fields in the parsed object before
proceeding, throwing descriptive errors if any are missing or invalid. This will
ensure consistent error handling and robustness across blockchain methods.
buildAptosUnsignedTx(jsonStr: string): Uint8Array { | ||
const req = JSON.parse(jsonStr); | ||
const input = { | ||
chainId: req.chainId, | ||
sender: req.sender, | ||
sequenceNumber: Long.fromString(String(req.sequenceNumber)), | ||
expirationTimestampSecs: Long.fromString(String(req.ttl)), | ||
gasUnitPrice: Long.fromString(String(req.gasUnitPrice)), | ||
maxGasAmount: Long.fromString(String(req.maxGasAmount)), | ||
transfer: this.TW.Aptos.Proto.TransferMessage.create({ | ||
to: req.toAddress, | ||
amount: Long.fromString(String(req.amount)), | ||
}), | ||
}; | ||
return this.TW.Aptos.Proto.SigningInput.encode(input).finish(); | ||
} |
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.
🛠️ Refactor suggestion
Add error handling for JSON parsing and input validation.
The method lacks error handling for JSON.parse() and doesn't validate required fields, which could lead to runtime errors.
buildAptosUnsignedTx(jsonStr: string): Uint8Array {
- const req = JSON.parse(jsonStr);
+ let req;
+ try {
+ req = JSON.parse(jsonStr);
+ } catch (error) {
+ throw new Error(`Invalid JSON input: ${error.message}`);
+ }
+
+ // Validate required fields
+ const requiredFields = ['chainId', 'sender', 'sequenceNumber', 'ttl', 'gasUnitPrice', 'maxGasAmount', 'toAddress', 'amount'];
+ for (const field of requiredFields) {
+ if (req[field] === undefined || req[field] === null) {
+ throw new Error(`Missing required field: ${field}`);
+ }
+ }
+
const input = {
chainId: req.chainId,
sender: req.sender,
sequenceNumber: Long.fromString(String(req.sequenceNumber)),
expirationTimestampSecs: Long.fromString(String(req.ttl)),
gasUnitPrice: Long.fromString(String(req.gasUnitPrice)),
maxGasAmount: Long.fromString(String(req.maxGasAmount)),
transfer: this.TW.Aptos.Proto.TransferMessage.create({
to: req.toAddress,
amount: Long.fromString(String(req.amount)),
}),
};
return this.TW.Aptos.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.
buildAptosUnsignedTx(jsonStr: string): Uint8Array { | |
const req = JSON.parse(jsonStr); | |
const input = { | |
chainId: req.chainId, | |
sender: req.sender, | |
sequenceNumber: Long.fromString(String(req.sequenceNumber)), | |
expirationTimestampSecs: Long.fromString(String(req.ttl)), | |
gasUnitPrice: Long.fromString(String(req.gasUnitPrice)), | |
maxGasAmount: Long.fromString(String(req.maxGasAmount)), | |
transfer: this.TW.Aptos.Proto.TransferMessage.create({ | |
to: req.toAddress, | |
amount: Long.fromString(String(req.amount)), | |
}), | |
}; | |
return this.TW.Aptos.Proto.SigningInput.encode(input).finish(); | |
} | |
buildAptosUnsignedTx(jsonStr: string): Uint8Array { | |
let req; | |
try { | |
req = JSON.parse(jsonStr); | |
} catch (error) { | |
throw new Error(`Invalid JSON input: ${error.message}`); | |
} | |
// Validate required fields | |
const requiredFields = [ | |
'chainId', | |
'sender', | |
'sequenceNumber', | |
'ttl', | |
'gasUnitPrice', | |
'maxGasAmount', | |
'toAddress', | |
'amount', | |
]; | |
for (const field of requiredFields) { | |
if (req[field] === undefined || req[field] === null) { | |
throw new Error(`Missing required field: ${field}`); | |
} | |
} | |
const input = { | |
chainId: req.chainId, | |
sender: req.sender, | |
sequenceNumber: Long.fromString(String(req.sequenceNumber)), | |
expirationTimestampSecs: Long.fromString(String(req.ttl)), | |
gasUnitPrice: Long.fromString(String(req.gasUnitPrice)), | |
maxGasAmount: Long.fromString(String(req.maxGasAmount)), | |
transfer: this.TW.Aptos.Proto.TransferMessage.create({ | |
to: req.toAddress, | |
amount: Long.fromString(String(req.amount)), | |
}), | |
}; | |
return this.TW.Aptos.Proto.SigningInput.encode(input).finish(); | |
} |
🤖 Prompt for AI Agents
In wasmjs/src/index.ts around lines 85 to 100, the buildAptosUnsignedTx method
currently parses JSON input without error handling and does not validate
required fields, risking runtime errors. Add a try-catch block around JSON.parse
to catch and handle parsing errors gracefully. After parsing, validate that all
required fields (chainId, sender, sequenceNumber, ttl, gasUnitPrice,
maxGasAmount, toAddress, amount) are present and of the expected type, throwing
or returning an error if validation fails before proceeding with transaction
construction.
buildCardanoUnsignedTx(jsonStr: string): Uint8Array { | ||
const req = JSON.parse(jsonStr); | ||
|
||
const transfer = this.TW.Cardano.Proto.Transfer.create({ | ||
toAddress: req.toAddress, | ||
changeAddress: req.changeAddress, | ||
amount: req.amount, | ||
useMaxAmount: false, | ||
}); | ||
|
||
const utxos = req.utxos.map((utxo: any) => | ||
this.TW.Cardano.Proto.TxInput.create({ | ||
outPoint: this.TW.Cardano.Proto.OutPoint.create({ | ||
txHash: utxo.tx_hash, | ||
outputIndex: utxo.output_index, | ||
}), | ||
address: utxo.address, | ||
amount: utxo.amount, | ||
tokenAmount: null | ||
}) | ||
); | ||
|
||
const input = { | ||
transferMessage: transfer, | ||
utxos: utxos, | ||
ttl: new Long(req.ttl), | ||
}; | ||
return this.TW.Cardano.Proto.SigningInput.encode(input).finish(); | ||
} |
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.
🛠️ Refactor suggestion
Add error handling for JSON parsing and input validation.
Similar to the Aptos method, this lacks error handling and input validation.
buildCardanoUnsignedTx(jsonStr: string): Uint8Array {
- const req = JSON.parse(jsonStr);
+ let req;
+ try {
+ req = JSON.parse(jsonStr);
+ } catch (error) {
+ throw new Error(`Invalid JSON input: ${error.message}`);
+ }
+
+ // Validate required fields
+ const requiredFields = ['toAddress', 'changeAddress', 'amount', 'utxos', 'ttl'];
+ for (const field of requiredFields) {
+ if (req[field] === undefined || req[field] === null) {
+ throw new Error(`Missing required field: ${field}`);
+ }
+ }
+
+ if (!Array.isArray(req.utxos)) {
+ throw new Error('utxos must be an array');
+ }
📝 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.
buildCardanoUnsignedTx(jsonStr: string): Uint8Array { | |
const req = JSON.parse(jsonStr); | |
const transfer = this.TW.Cardano.Proto.Transfer.create({ | |
toAddress: req.toAddress, | |
changeAddress: req.changeAddress, | |
amount: req.amount, | |
useMaxAmount: false, | |
}); | |
const utxos = req.utxos.map((utxo: any) => | |
this.TW.Cardano.Proto.TxInput.create({ | |
outPoint: this.TW.Cardano.Proto.OutPoint.create({ | |
txHash: utxo.tx_hash, | |
outputIndex: utxo.output_index, | |
}), | |
address: utxo.address, | |
amount: utxo.amount, | |
tokenAmount: null | |
}) | |
); | |
const input = { | |
transferMessage: transfer, | |
utxos: utxos, | |
ttl: new Long(req.ttl), | |
}; | |
return this.TW.Cardano.Proto.SigningInput.encode(input).finish(); | |
} | |
buildCardanoUnsignedTx(jsonStr: string): Uint8Array { | |
let req; | |
try { | |
req = JSON.parse(jsonStr); | |
} catch (error) { | |
throw new Error(`Invalid JSON input: ${error.message}`); | |
} | |
// Validate required fields | |
const requiredFields = ['toAddress', 'changeAddress', 'amount', 'utxos', 'ttl']; | |
for (const field of requiredFields) { | |
if (req[field] === undefined || req[field] === null) { | |
throw new Error(`Missing required field: ${field}`); | |
} | |
} | |
if (!Array.isArray(req.utxos)) { | |
throw new Error('utxos must be an array'); | |
} | |
const transfer = this.TW.Cardano.Proto.Transfer.create({ | |
toAddress: req.toAddress, | |
changeAddress: req.changeAddress, | |
amount: req.amount, | |
useMaxAmount: false, | |
}); | |
const utxos = req.utxos.map((utxo: any) => | |
this.TW.Cardano.Proto.TxInput.create({ | |
outPoint: this.TW.Cardano.Proto.OutPoint.create({ | |
txHash: utxo.tx_hash, | |
outputIndex: utxo.output_index, | |
}), | |
address: utxo.address, | |
amount: utxo.amount, | |
tokenAmount: null | |
}) | |
); | |
const input = { | |
transferMessage: transfer, | |
utxos: utxos, | |
ttl: new Long(req.ttl), | |
}; | |
return this.TW.Cardano.Proto.SigningInput.encode(input).finish(); | |
} |
🤖 Prompt for AI Agents
In wasmjs/src/index.ts around lines 108 to 136, the buildCardanoUnsignedTx
method lacks error handling for JSON parsing and input validation. Wrap the
JSON.parse call in a try-catch block to handle invalid JSON input gracefully.
Add checks to validate required fields in the parsed object before proceeding,
and throw or return meaningful errors if validation fails to prevent runtime
exceptions.
buildCosmosUnsignedTx(jsonStr: string): Uint8Array { | ||
const req = JSON.parse(jsonStr); | ||
|
||
const message = this.TW.Cosmos.Proto.Message.create({ | ||
sendCoinsMessage: this.TW.Cosmos.Proto.Message.Send.create({ | ||
fromAddress: req.fromAddress, | ||
toAddress: req.toAddress, | ||
amounts: [ | ||
this.TW.Cosmos.Proto.Amount.create({ | ||
denom: req.denom, | ||
amount: req.amount, | ||
}), | ||
] | ||
}) | ||
}); | ||
|
||
const input = { | ||
chainId: req.chainId, | ||
accountNumber: req.accountNumber, | ||
sequence: req.sequence, | ||
messages: [message], | ||
memo: req.memo, | ||
publicKey: this.HexCoding.decode(req.publicKey), | ||
fee: this.TW.Cosmos.Proto.Fee.create({ | ||
gas: req.gas, | ||
amounts: [ | ||
this.TW.Cosmos.Proto.Amount.create({ | ||
denom: req.gasDenom, | ||
amount: req.gasAmount, | ||
}) | ||
] | ||
}) | ||
}; | ||
return this.TW.Cosmos.Proto.SigningInput.encode(input).finish(); | ||
} |
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.
🛠️ Refactor suggestion
Add error handling for JSON parsing and input validation.
This method also needs error handling and input validation for robustness.
buildCosmosUnsignedTx(jsonStr: string): Uint8Array {
- const req = JSON.parse(jsonStr);
+ let req;
+ try {
+ req = JSON.parse(jsonStr);
+ } catch (error) {
+ throw new Error(`Invalid JSON input: ${error.message}`);
+ }
+
+ // Validate required fields
+ const requiredFields = ['chainId', 'accountNumber', 'sequence', 'fromAddress', 'toAddress', 'denom', 'amount', 'memo', 'publicKey', 'gas', 'gasDenom', 'gasAmount'];
+ for (const field of requiredFields) {
+ if (req[field] === undefined || req[field] === null) {
+ throw new Error(`Missing required field: ${field}`);
+ }
+ }
📝 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.
buildCosmosUnsignedTx(jsonStr: string): Uint8Array { | |
const req = JSON.parse(jsonStr); | |
const message = this.TW.Cosmos.Proto.Message.create({ | |
sendCoinsMessage: this.TW.Cosmos.Proto.Message.Send.create({ | |
fromAddress: req.fromAddress, | |
toAddress: req.toAddress, | |
amounts: [ | |
this.TW.Cosmos.Proto.Amount.create({ | |
denom: req.denom, | |
amount: req.amount, | |
}), | |
] | |
}) | |
}); | |
const input = { | |
chainId: req.chainId, | |
accountNumber: req.accountNumber, | |
sequence: req.sequence, | |
messages: [message], | |
memo: req.memo, | |
publicKey: this.HexCoding.decode(req.publicKey), | |
fee: this.TW.Cosmos.Proto.Fee.create({ | |
gas: req.gas, | |
amounts: [ | |
this.TW.Cosmos.Proto.Amount.create({ | |
denom: req.gasDenom, | |
amount: req.gasAmount, | |
}) | |
] | |
}) | |
}; | |
return this.TW.Cosmos.Proto.SigningInput.encode(input).finish(); | |
} | |
buildCosmosUnsignedTx(jsonStr: string): Uint8Array { | |
let req; | |
try { | |
req = JSON.parse(jsonStr); | |
} catch (error) { | |
throw new Error(`Invalid JSON input: ${error.message}`); | |
} | |
// Validate required fields | |
const requiredFields = [ | |
'chainId', | |
'accountNumber', | |
'sequence', | |
'fromAddress', | |
'toAddress', | |
'denom', | |
'amount', | |
'memo', | |
'publicKey', | |
'gas', | |
'gasDenom', | |
'gasAmount' | |
]; | |
for (const field of requiredFields) { | |
if (req[field] === undefined || req[field] === null) { | |
throw new Error(`Missing required field: ${field}`); | |
} | |
} | |
const message = this.TW.Cosmos.Proto.Message.create({ | |
sendCoinsMessage: this.TW.Cosmos.Proto.Message.Send.create({ | |
fromAddress: req.fromAddress, | |
toAddress: req.toAddress, | |
amounts: [ | |
this.TW.Cosmos.Proto.Amount.create({ | |
denom: req.denom, | |
amount: req.amount, | |
}), | |
], | |
}), | |
}); | |
const input = { | |
chainId: req.chainId, | |
accountNumber: req.accountNumber, | |
sequence: req.sequence, | |
messages: [message], | |
memo: req.memo, | |
publicKey: this.HexCoding.decode(req.publicKey), | |
fee: this.TW.Cosmos.Proto.Fee.create({ | |
gas: req.gas, | |
amounts: [ | |
this.TW.Cosmos.Proto.Amount.create({ | |
denom: req.gasDenom, | |
amount: req.gasAmount, | |
}), | |
], | |
}), | |
}; | |
return this.TW.Cosmos.Proto.SigningInput.encode(input).finish(); | |
} |
🤖 Prompt for AI Agents
In wasmjs/src/index.ts around lines 144 to 178, the buildCosmosUnsignedTx method
lacks error handling for JSON parsing and input validation. Add a try-catch
block around JSON.parse to catch and handle parsing errors gracefully. Validate
the required fields in the parsed object before proceeding, throwing or
returning an error if any required field is missing or invalid. This will
improve the method's robustness and prevent runtime exceptions.
buildStellarUnsignedTx(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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling and consider extracting the hardcoded passphrase.
The method needs error handling and the hardcoded Stellar passphrase should be configurable.
buildStellarUnsignedTx(jsonStr: string): Uint8Array {
- const req = JSON.parse(jsonStr);
+ let req;
+ try {
+ req = JSON.parse(jsonStr);
+ } catch (error) {
+ throw new Error(`Invalid JSON input: ${error.message}`);
+ }
+
+ // Validate required fields
+ const requiredFields = ['destination', 'amount', 'fee', 'account', 'sequence', 'memoText'];
+ for (const field of requiredFields) {
+ if (req[field] === undefined || req[field] === null) {
+ throw new Error(`Missing required field: ${field}`);
+ }
+ }
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
})
+
+ // Consider making this configurable
+ const STELLAR_MAINNET_PASSPHRASE = 'Public Global Stellar Network ; September 2015';
+
const input = {
fee : req.fee,
account: req.account,
- passphrase: 'Public Global Stellar Network ; September 2015',
+ passphrase: req.passphrase || STELLAR_MAINNET_PASSPHRASE,
sequence: Long.fromString(String(req.sequence)),
opPayment: opPayment,
memoText: memoText
};
return this.TW.Stellar.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.
buildStellarUnsignedTx(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(); | |
} | |
buildStellarUnsignedTx(jsonStr: string): Uint8Array { | |
let req; | |
try { | |
req = JSON.parse(jsonStr); | |
} catch (error) { | |
throw new Error(`Invalid JSON input: ${error.message}`); | |
} | |
// Validate required fields | |
const requiredFields = ['destination', 'amount', 'fee', 'account', 'sequence', 'memoText']; | |
for (const field of requiredFields) { | |
if (req[field] === undefined || req[field] === null) { | |
throw new Error(`Missing required field: ${field}`); | |
} | |
} | |
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 | |
}) | |
// Consider making this configurable | |
const STELLAR_MAINNET_PASSPHRASE = 'Public Global Stellar Network ; September 2015'; | |
const input = { | |
fee: req.fee, | |
account: req.account, | |
passphrase: req.passphrase || STELLAR_MAINNET_PASSPHRASE, | |
sequence: Long.fromString(String(req.sequence)), | |
opPayment: opPayment, | |
memoText: memoText | |
}; | |
return this.TW.Stellar.Proto.SigningInput.encode(input).finish(); | |
} |
🤖 Prompt for AI Agents
In wasmjs/src/index.ts around lines 212 to 231, the buildStellarUnsignedTx
method lacks error handling and uses a hardcoded Stellar passphrase. Add
try-catch blocks to handle JSON parsing and other potential errors gracefully.
Extract the hardcoded passphrase into a configurable constant or parameter to
improve flexibility and maintainability.
Description
How to test
Types of changes
Checklist
If you're adding a new blockchain
Summary by CodeRabbit
New Features
Chores
Style
Tests