Skip to content
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

wasm: sign wallet comm using correct signing key #148

Merged
merged 2 commits into from
Jan 4, 2025
Merged

Conversation

sehyunc
Copy link
Contributor

@sehyunc sehyunc commented Jan 4, 2025

Purpose

This PR fixes an issue where the rotated keychain was used to derive a signing key that was resulting in invalid wallet update signatures. To do so, the correct signing key is returned by handle_keychain_rotation, and is used as the signing key for internal wallets.

Testing

  • Tested locally
  • Tested in testnet

@sehyunc sehyunc force-pushed the sehyun/fix-sign-comm branch from 540a0c8 to 8ea513f Compare January 4, 2025 20:40
@sehyunc sehyunc force-pushed the sehyun/fix-sign-comm branch from 8ea513f to 2187ea7 Compare January 4, 2025 20:41
@sehyunc sehyunc requested a review from akirillo January 4, 2025 20:54
Copy link
Contributor

@akirillo akirillo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lacking context on why we don't do key rotation for update_order, o/w lgtm - approving to unblock

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't we do a key rotation in update_order? and if we should be doing this, we'd also need to replicate these changes to use the proper old signing key there

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was omitted from the original external key implementation PR since no external wallets will use that task. Currently on the backlog to bring it up to speed with the rest of the tasks.

@sehyunc sehyunc merged commit dea5932 into main Jan 4, 2025
1 check passed
@sehyunc sehyunc deleted the sehyun/fix-sign-comm branch January 4, 2025 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants