-
Notifications
You must be signed in to change notification settings - Fork 108
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
refactor(zetaclient)!: remove hsm signer #3118
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe pull request implements significant changes by removing all references to Hardware Security Module (HSM) functionality across multiple files. This includes the deletion of HSM-related commands, parameters, and configurations in the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
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: 0
🧹 Outside diff range and nitpick comments (1)
go.mod (1)
Line range hint
3-5
: Fix Go toolchain version inconsistency.The module declaration specifies
go 1.22.7
but the toolchain directive specifiesgo1.22.8
. This inconsistency should be resolved.Apply this diff to align the versions:
-go 1.22.7 +go 1.22.8 toolchain go1.22.8
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
📒 Files selected for processing (12)
Makefile
(0 hunks)cmd/zetaclientd/debug.go
(0 hunks)cmd/zetaclientd/hsm.go
(0 hunks)cmd/zetaclientd/init.go
(0 hunks)cmd/zetaclientd/utils.go
(1 hunks)go.mod
(1 hunks)zetaclient/config/types.go
(0 hunks)zetaclient/hsm/hsm_signer.go
(0 hunks)zetaclient/hsm/hsm_signer_test.go
(0 hunks)zetaclient/zetacore/broadcast.go
(0 hunks)zetaclient/zetacore/client.go
(0 hunks)zetaclient/zetacore/client_test.go
(0 hunks)
💤 Files with no reviewable changes (10)
- Makefile
- cmd/zetaclientd/debug.go
- cmd/zetaclientd/hsm.go
- cmd/zetaclientd/init.go
- zetaclient/config/types.go
- zetaclient/hsm/hsm_signer.go
- zetaclient/hsm/hsm_signer_test.go
- zetaclient/zetacore/broadcast.go
- zetaclient/zetacore/client.go
- zetaclient/zetacore/client_test.go
🧰 Additional context used
📓 Path-based instructions (1)
cmd/zetaclientd/utils.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
🔇 Additional comments (3)
cmd/zetaclientd/utils.go (2)
34-34
: LGTM! Changes align with HSM removal objective.
The simplified client initialization without HSM mode parameter is consistent with the PR's goal of removing HSM support.
34-34
: Verify complete removal of HSM references.
Let's ensure all HSM-related code has been consistently removed across the codebase.
✅ Verification successful
HSM references have been successfully removed from the codebase
Our comprehensive analysis shows no remaining traces of HSM-related code, including imports, struct definitions, configuration fields, function parameters, variables, or comments. The codebase appears to be clean of any HSM references.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any remaining HSM references in the codebase
# Expect: No matches as HSM support should be completely removed
echo "Searching for HSM-related code..."
rg -i "hsm" --type go
echo "Searching for Hardware Security Module references..."
rg -i "hardware.?security.?module" --type go
echo "Checking for any remaining HSM-related configuration..."
rg -i "hsmmode|hsmhotkey" --type go
Length of output: 419
Script:
#!/bin/bash
# Let's try a more comprehensive search for HSM-related code
# Search for specific HSM-related terms and patterns
# Search for any HSM-related imports
echo "Checking for HSM-related imports..."
rg -i "import.*hsm" --type go -A 5
# Search for HSM-related struct fields and type definitions
echo "Checking for HSM-related structs and types..."
ast-grep --pattern 'type $_ struct {
$$$
HSM$_
$$$
}'
# Search for HSM-related configuration fields
echo "Checking for HSM configuration..."
rg -i "hsm.*=|hsm.*:" --type go
# Search for HSM-related function parameters and variables
echo "Checking for HSM-related functions and variables..."
ast-grep --pattern 'func $_($_ HSM$_)'
# Check for any remaining HSM-related comments
echo "Checking for HSM-related comments..."
rg -i "//.*hsm|/\*.*hsm" --type go
Length of output: 665
go.mod (1)
336-344
: LGTM: Dependencies align with project objectives.
The changes appropriately remove HSM-related dependencies while adding necessary new ones. The modifications align well with the PR objective of removing HSM signer mode.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3118 +/- ##
===========================================
+ Coverage 63.30% 63.54% +0.24%
===========================================
Files 422 421 -1
Lines 29954 29838 -116
===========================================
- Hits 18963 18962 -1
+ Misses 10150 10036 -114
+ Partials 841 840 -1
|
0081a4c
to
ca4d250
Compare
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.
LGTM
@gartnera shouldn't we regenerate cli docs? |
I don't think we generate zetaclient cli docs do we? Only zetacored? |
* Consolidate files and logic within zetaclientd cli * Update `init` command * Consolidate TSS commands * Drop HSM cli (#3118) * Move p2p diagnostic to tss package * Refactor relayer commands * Rename init config * Restructure inbound cmd * Update changelog * Drop p2p diagnostics * lint & pr comments [1] * Address pr comments [2]
Description
Remove zetaclient hsm signer mode as it is not used by any of the observer signers.
Closes #3111
How Has This Been Tested?
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores
Documentation