feat：EOS and Cosmos applications migrated to the secp256k1 applet functionality[IBIZ-647] #163
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
|
可以看一下下面的 Review 结果 |
- Fix trans_multi_signs.push() bug in pubkey loops - Add separate version constants for EOS (0.0.1) and Cosmos (1.0.00) - Extract common code to reduce duplication - Improve error handling and documentation - Remove unused constants and fix compiler warnings - Update version to 2.8.2
- Fix signature verification by using sign_source_val[2..130] instead of sign_result[2..130] - sign_result contains DER format signature (128 bytes) which is incompatible with Signature::from_compact() - sign_source_val contains compact signature format (64 bytes) expected by secp256k1 library - Align with CKB/Tron implementation pattern for consistent signature handling - Remove debug println statement for cleaner code - Add signature assertions in tests to verify correct signature generation This resolves the issue where EOS signature verification would fail in the new secp256k1 workflow due to incorrect signature format being passed to Signature::from_compact().
XuNeal
approved these changes
Sep 18, 2025
Member
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
Comment on lines
244
to
263
| let select_apdu = Apdu::select_applet(EOS_AID); | ||
| let select_result = send_apdu(select_apdu)?; | ||
| ApduCheck::check_response(&select_result)?; | ||
|
|
||
| let key_manager_obj = KEY_MANAGER.lock(); | ||
| let path_signature = | ||
| secp256k1_sign(&key_manager_obj.pri_key, &sign_param.path.as_bytes())?; | ||
| let mut path_pack: Vec<u8> = vec![]; | ||
| path_pack.push(0x00); | ||
| path_pack.push(path_signature.len() as u8); | ||
| path_pack.extend(path_signature.as_slice()); | ||
| path_pack.push(0x01); | ||
| path_pack.push(sign_param.path.as_bytes().len() as u8); | ||
| path_pack.extend(sign_param.path.as_bytes()); | ||
|
|
||
| let msg_pubkey = Secp256k1Apdu::get_xpub(&path_pack); | ||
| let res_msg_pubkey = send_apdu(msg_pubkey)?; | ||
| let pubkey_raw = hex::decode(&res_msg_pubkey[..130]).unwrap(); | ||
| let comprs_pubkey = utility::uncompress_pubkey_2_compress(&res_msg_pubkey); | ||
| let mut comprs_pubkey_slice = hex::decode(comprs_pubkey)?; |
There was a problem hiding this comment.
[P1] Check EOS xpub response before decoding
The secp256k1 signing path fetches the device public key and immediately slices res_msg_pubkey[..130] without validating the APDU response. If the call to Secp256k1Apdu::get_xpub fails (e.g. invalid derivation path or transport error), the response length may be shorter or contain only a status word, causing hex::decode to panic or decode garbage. The legacy implementation checked ApduCheck::check_response before decoding. Add a status check on res_msg_pubkey and surface a proper error before decoding or using the bytes.
Useful? React with 👍 / 👎.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of Changes
EOS and Cosmos applications migrated to the secp256k1 applet
Motivation and Context
How Has This Been Tested? (Test Plan)
Other information
Screenshots (if appropriate):
Final checklist
Security checklist (only for leader check)