feat: Sui supports the secp256k1/secp256r1 algorithms #2476
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hi @lispking! Welcome to the elizaOS community. Thanks for submitting your first pull request; your efforts are helping us accelerate towards AGI. We'll review it shortly. You are now an elizaOS contributor!
📝 WalkthroughWalkthroughThe changes enhance the Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🪛 Biome (1.9.4)packages/plugin-sui/src/utils.ts[error] 25-25: Unnecessary continue statement Unsafe fix: Delete the unnecessary continue statement (lint/correctness/noUnnecessaryContinue) [error] 41-41: Unnecessary continue statement Unsafe fix: Delete the unnecessary continue statement (lint/correctness/noUnnecessaryContinue) 🔇 Additional comments (5)
Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/plugin-sui/src/utils.ts(1 hunks)
🔇 Additional comments (1)
packages/plugin-sui/src/utils.ts (1)
6-8: Verify that all callers ofparseAccounthandle the new return typesThe return type of
parseAccountnow includes multiple keypair types. Ensure that all functions usingparseAccountcorrectly handleEd25519Keypair,Secp256k1Keypair, andSecp256r1Keypair.Run this script to identify usages of
parseAccount:✅ Verification successful
All keypair types are safely handled
The code only uses the
toSuiAddress()method which is standard across all keypair types from the official @mysten/sui SDK. Test coverage inwallet.test.tsconfirms the compatibility.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all usages of `parseAccount` and check for type handling. # Search for function calls to `parseAccount` rg 'parseAccount\(' -A 5 --glob '!packages/plugin-sui/src/utils.ts'Length of output: 1056
Script:
#!/bin/bash # Get full context of functions using suiAccount ast-grep --pattern 'const suiAccount = parseAccount($_); $$$' # Also search for any usage of suiAccount variable rg 'suiAccount\.' -A 2 packages/plugin-sui/src/actions/transfer.ts packages/plugin-sui/src/providers/wallet.tsLength of output: 405
Script:
#!/bin/bash # Find keypair type definitions and imports rg "import.*\{.*(Ed25519Keypair|Secp256k1Keypair|Secp256r1Keypair).*\}" packages/plugin-sui/src/ # Look for toSuiAddress method usage or definition rg "toSuiAddress" packages/plugin-sui/src/Length of output: 810
lispking
force-pushed
the
feat-sui-more-key
branch
from
066290b to
c0543b5
Compare
|
@coderabbitai final review |
|
@wtfsayo the implementation looks clean and well-structured. the code successfully adds support for secp256k1 and secp256r1 algorithms alongside the existing ed25519. while the implementation is solid, here are a few suggestions for future improvements:
would you like me to create a github issue to track these improvements for a future pr? despite these minor suggestions, the current implementation is production-ready. i'll approve the changes. 🔗 analysis chain 🏁 scripts executed the following scripts were executed for the analysis: script: length of output: 1717 ✏️ learnings added
✅ Actions performedComments resolved. |
…o develop * 'develop' of https://github.com/mgunnin/virtualDegen: (41 commits) Update README.md (elizaOS#2495) Update README_CN.md (elizaOS#2489) Update README_JA.md (elizaOS#2490) feat: introduce Dependency Injection to enhance developer experience (elizaOS#2115) chore: corrected the link to the banner (elizaOS#2491) chore: update createToken.ts (elizaOS#2493) feat: Solana plugin improvement for flawless transfers (elizaOS#2340) Update README_DE.md (elizaOS#2483) feat: Sui supports the secp256k1/secp256r1 algorithms (elizaOS#2476) chore: remove eslint, prettier, tslint and replace with biome (elizaOS#2439) chore: let -> const fix: lint command fix: unused import and type errors chore: console -> elizaLogger fix: error logging and remove unused import fix: remove unused error var chore: pnpm lock file add openai var Revert "Revert "refactor: dockerize smoke tests (elizaOS#2420)" (elizaOS#2459)" fix swaps evm plugin (elizaOS#2332) ...
Relates to
Risks
Low
Background
What does this PR do?
Sui supports the secp256k1/secp256r1 algorithms. When the wallet is loaded, it needs to support both of these algorithms simultaneously.
docs: https://docs.sui.io/guides/developer/cryptography/signing
What kind of change is this?
Features: Sui supports the secp256k1/secp256r1 algorithms
Documentation changes needed?
My changes do not require a change to the project documentation.
Testing
Where should a reviewer start?
Detailed testing steps