Skip to content

Conversation

@swift1337
Copy link
Contributor

@swift1337 swift1337 commented Mar 11, 2025

This PR decouples go-tss from being a Thor fork into an independent project. The reasoning is to contribute improvements and remove code that is not used by ZetaChain, which is challenging right now. Also, the original project is archived, so it makes no sense to try to keep the code similar.

  • Rename imports
  • Format code
  • Lint code
  • Update go.mod
  • Update CI
  • Update readme
  • Disable leader-less party tests
  • Test locally with node's repo to ensure there are no conflicts.
  • Improve Makefile
  • Update code owners

Related: zeta-chain/node#3383

Summary by CodeRabbit

  • New Features

    • Rebranded the service to Zetachain Threshold Signature Scheme with updated usage guidance.
  • Refactor

    • Streamlined build commands and standardized interfaces to enhance overall system clarity.
    • Enhanced readability of function signatures and error messages across various components.
  • Chores

    • Updated dependency management and CI configurations for more reliable performance.
    • Improved testing stability through dynamic port allocation.
  • Documentation

    • Overhauled introductory materials to reflect the new brand identity and project direction.

@swift1337 swift1337 requested review from a team and gartnera March 11, 2025 14:52
@swift1337 swift1337 self-assigned this Mar 11, 2025
@coderabbitai
Copy link

coderabbitai bot commented Mar 11, 2025

📝 Walkthrough

Walkthrough

The repository has undergone a large-scale refactoring and migration. Import paths have been switched from the old gitlab.com/thorchain/tss/go-tss namespace to github.com/zeta-chain/go-tss. New configuration files—such as a CODEOWNERS file—have been added and CI workflows updated, including changes to lint jobs, Go versions, and environment variables. The .gitignore, Makefile, and README have been revised for clarity and rebranding. In addition, many source files have been reformatted with consistent naming (e.g., renaming TssServer to Server and updating constructor names), method signatures, and logging improvements.

Changes

File(s) Change Summary
.github/CODEOWNERS Added new CODEOWNERS file assigning @zeta-chain/protocol-engineering to all files and @zeta-chain/devops to the .github directory.
.github/workflows/ci.yml, .gitlab-ci.yml, .golangci.yml Updated CI workflows and lint configurations: reorganized lint job steps, added a 15-minute timeout, set GO111MODULE to on, bumped Go version (1.22 → 1.23) and linter version (v1.56.2 → v1.63.4); removed GitLab CI config.
.gitignore Modified ignore rules: adjusted directory syntax (e.g., changed .vscode to .vscode/) and removed entries for vendor directories, build artifacts, and *.iml files.
Makefile, README.md Revised Makefile targets (added help, deps, fmt; removed obsolete targets) and rebranded README from Thorchain TSS to Zetachain TSS with updated project origins and usage details.
blame/*, cmd/*, common/*, conversion/*, keygen/*, keysign/*, p2p/*, storage/*, tss/*, messages/*.proto Extensive refactoring: updated all import paths from gitlab.com/thorchain/tss/go-tss to github.com/zeta-chain/go-tss; renamed functions and types (e.g., TssServerServer, NewTssHttpServerNewHTTPServer); improved method signatures, variable names, logging, and formatting; and updated protobuf go_package options.
p2p/test.go Introduced a new function, GetFreePorts, to dynamically allocate free TCP ports for testing.

Suggested reviewers

  • gartnera
  • fbac

Tip

⚡🧪 Multi-step agentic review comment chat (experimental)
  • We're introducing multi-step agentic chat in review comments. This experimental feature enhances review discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments.
    - To enable this feature, set early_access to true under in the settings.

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@swift1337 swift1337 marked this pull request as ready for review March 11, 2025 15:02
Copy link
Member

@lumtis lumtis left a comment

Choose a reason for hiding this comment

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

Looks good to me, I don't see changes to be deemed as sensitive

@swift1337
Copy link
Contributor Author

UPD: CI tests fail. Will investigate & fix (works on my local machine)

Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (6)
README.md (1)

1-5: README updated with project rebranding.

The README has been appropriately updated to reflect the project's new identity as a Zetachain component rather than a Thorchain component, which aligns with the PR objective.

Consider formatting the bare URLs as proper markdown links for better readability:

-# Zetachain Threshold Signature Scheme (TSS) P2P server
+# Zetachain Threshold Signature Scheme (TSS) P2P Server
 
-Forked from https://gitlab.com/thorchain/tss/go-tss
-Based on https://github.com/bnb-chain/tss-lib
-Used by `zetaclientd`: https://github.com/zeta-chain/node
+- Forked from [thorchain/tss/go-tss](https://gitlab.com/thorchain/tss/go-tss)
+- Based on [bnb-chain/tss-lib](https://github.com/bnb-chain/tss-lib)
+- Used by [`zetaclientd`](https://github.com/zeta-chain/node)
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)

3-3: Bare URL used
null

(MD034, no-bare-urls)


4-4: Bare URL used
null

(MD034, no-bare-urls)


5-5: Bare URL used
null

(MD034, no-bare-urls)

.github/workflows/ci.yml (1)

40-44: Updated linter configuration with missing newline

The golangci-lint update to v1.63.4 with skip-cache option is beneficial, but there's a missing newline at the end of the file.

Add a newline at the end of the file to satisfy YAML formatting standards:

   with:
     version: v1.63.4
     skip-cache: true
+
🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 44-44: no new line character at the end of file

(new-line-at-end-of-file)

tss/tss_4nodes_test.go (1)

80-84: Improved test reliability with dynamic port allocation.

Replaced hardcoded ports with dynamically allocated free ports using p2p.GetFreePorts(4), which prevents port conflicts during testing. The added logging of allocated ports enhances debugging capabilities.

Consider adding error handling in case port allocation fails, such as retrying with a different range or providing a more descriptive error message.

.golangci.yml (1)

49-53: Added exclude options for generated files and specific directories.

The configuration now excludes generated files and specific directories like .git and .github from linting. This prevents unnecessary linting of files that shouldn't be manually modified.

However, the file is missing a newline character at the end, which some tools flag as an issue.

Add a newline at the end of the file:

  exclude:
    - composite
-  exclude-dirs: [ ".git", ".github" ]
+  exclude-dirs: [ ".git", ".github" ]
+
🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 53-53: no new line character at the end of file

(new-line-at-end-of-file)

keysign/ecdsa/tss_keysign.go (1)

271-273: Minor typographical inconsistency in blameNodesMisingShare.

Consider renaming to maintain clarity:

- blameNodesMisingShare, isUnicast, err := blameMgr.TssMissingShareBlame(
+ blameNodesMissingShare, isUnicast, err := blameMgr.TssMissingShareBlame(
keysign/eddsa/tss_keysign.go (1)

264-267: Typo in blameNodesMisingShare.

Recommend renaming for clarity:

- blameNodesMisingShare, isUnicast, err := blameMgr.TssMissingShareBlame(
+ blameNodesMissingShare, isUnicast, err := blameMgr.TssMissingShareBlame(
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between be92b20 and 6628a87.

⛔ Files ignored due to path filters (3)
  • go.sum is excluded by !**/*.sum
  • messages/join_party.pb.go is excluded by !**/*.pb.go
  • messages/signature_notifier.pb.go is excluded by !**/*.pb.go
📒 Files selected for processing (70)
  • .github/CODEOWNERS (1 hunks)
  • .github/workflows/ci.yml (1 hunks)
  • .gitignore (1 hunks)
  • .gitlab-ci.yml (0 hunks)
  • .golangci.yml (1 hunks)
  • Makefile (1 hunks)
  • README.md (1 hunks)
  • blame/manager.go (1 hunks)
  • blame/policy.go (2 hunks)
  • blame/policy_helper.go (1 hunks)
  • blame/policy_test.go (1 hunks)
  • blame/round_mgr.go (1 hunks)
  • blame/round_mgr_test.go (1 hunks)
  • cmd/tss-benchsign/main.go (3 hunks)
  • cmd/tss-recovery/key.go (2 hunks)
  • cmd/tss-recovery/tss-recovery.go (2 hunks)
  • cmd/tss/main.go (4 hunks)
  • cmd/tss/mock_tss_server.go (3 hunks)
  • cmd/tss/tss_http.go (8 hunks)
  • cmd/tss/tss_http_test.go (6 hunks)
  • common/local_cache_item.go (1 hunks)
  • common/tss.go (13 hunks)
  • common/tss_helper.go (1 hunks)
  • common/tss_helper_test.go (3 hunks)
  • common/tss_test.go (10 hunks)
  • conversion/conversion.go (1 hunks)
  • conversion/conversion_test.go (2 hunks)
  • conversion/key_provider.go (0 hunks)
  • conversion/tss_helper.go (1 hunks)
  • go.mod (3 hunks)
  • keygen/ecdsa/keygen_test.go (3 hunks)
  • keygen/ecdsa/tss_keygen.go (3 hunks)
  • keygen/eddsa/keygen_test.go (5 hunks)
  • keygen/eddsa/tss_keygen.go (6 hunks)
  • keygen/keygen.go (1 hunks)
  • keygen/request.go (1 hunks)
  • keygen/response.go (1 hunks)
  • keysign/ecdsa/keysign_old_test.go (9 hunks)
  • keysign/ecdsa/keysign_test.go (7 hunks)
  • keysign/ecdsa/tss_keysign.go (9 hunks)
  • keysign/eddsa/keysign_test.go (6 hunks)
  • keysign/eddsa/tss_keysign.go (8 hunks)
  • keysign/keysign.go (1 hunks)
  • keysign/notifier.go (1 hunks)
  • keysign/notifier_test.go (1 hunks)
  • keysign/response.go (1 hunks)
  • keysign/signature_notifier.go (4 hunks)
  • keysign/signature_notifier_test.go (1 hunks)
  • messages/join_party.proto (1 hunks)
  • messages/signature_notifier.proto (1 hunks)
  • p2p/communication.go (4 hunks)
  • p2p/communication_test.go (1 hunks)
  • p2p/metric_reporter.go (3 hunks)
  • p2p/party_coordinator.go (7 hunks)
  • p2p/party_coordinator_test.go (1 hunks)
  • p2p/party_coordinator_test_with_leader_test.go (1 hunks)
  • p2p/peer_status.go (2 hunks)
  • p2p/stream_helper.go (1 hunks)
  • p2p/test.go (1 hunks)
  • p2p/whitelist_connection_gater.go (2 hunks)
  • storage/localstate_backwards_test.go (1 hunks)
  • storage/localstate_mgr.go (3 hunks)
  • storage/localstate_mgr_test.go (2 hunks)
  • storage/mock_localstatemanager.go (1 hunks)
  • tss/keygen.go (8 hunks)
  • tss/keysign.go (11 hunks)
  • tss/server.go (0 hunks)
  • tss/tss.go (8 hunks)
  • tss/tss_4nodes_test.go (7 hunks)
  • tss/tss_4nodes_zeta_test.go (4 hunks)
💤 Files with no reviewable changes (3)
  • conversion/key_provider.go
  • tss/server.go
  • .gitlab-ci.yml
🧰 Additional context used
🧠 Learnings (1)
tss/tss_4nodes_zeta_test.go (1)
Learnt from: gartnera
PR: zeta-chain/go-tss#22
File: tss/tss_4nodes_zeta_test.go:40-78
Timestamp: 2025-03-12T16:55:11.103Z
Learning: In the `SetUpSuite` function of `FourNodeScaleZetaSuite`, directories are cleaned up before the setup starts.
🪛 markdownlint-cli2 (0.17.2)
README.md

3-3: Bare URL used
null

(MD034, no-bare-urls)


4-4: Bare URL used
null

(MD034, no-bare-urls)


5-5: Bare URL used
null

(MD034, no-bare-urls)

🪛 YAMLlint (1.35.1)
.github/workflows/ci.yml

[error] 44-44: no new line character at the end of file

(new-line-at-end-of-file)

.golangci.yml

[error] 53-53: no new line character at the end of file

(new-line-at-end-of-file)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: test go 1.22
🔇 Additional comments (218)
messages/join_party.proto (1)

3-3: Update go_package Namespace

The go_package option has been updated to "github.com/zeta-chain/go-tss/messages", which is fully in line with the migration from the legacy GitLab namespace to the new GitHub organization. This change supports the decoupling objective and rebranding of the project. Please ensure that corresponding generated code and any related proto references throughout the codebase are updated accordingly.

messages/signature_notifier.proto (1)

3-3: Revised go_package for Consistency

The go_package option now correctly targets "github.com/zeta-chain/go-tss/messages" in accordance with the overall repository transition. This update fosters uniformity across proto files and aligns with the PR’s objective to establish an independent project. Verify that the associated Go imports and generated code reflect this change to prevent potential namespace mismatches.

.github/CODEOWNERS (1)

1-3: Ownership Configuration is Correct and Clear
The CODEOWNERS file correctly assigns overall repository ownership to the protocol engineering team and delegates ownership for the .github directory to the devops team. The pattern matching (using * and .github/**) is appropriate for the intended scope.

keygen/keygen.go (1)

6-7: Updated Import Paths Reflect the New Repository Structure
The import statements for common and p2p have been successfully updated from the old GitLab namespace to the new GitHub structure. This is inline with the decoupling objective and ensures consistency across the codebase.

keygen/response.go (1)

4-5: Refactored Import Paths are Implemented Correctly
The modifications in the import paths for both the blame and common packages align with the overall project migration to the new repository. The changes do not affect functionality and improve clarity.

keysign/response.go (1)

4-5: Consistent Update of Import Statements
The import paths in this file have been updated to reflect the new repository location for blame and common. The changes maintain consistency with the other parts of the codebase and are correct.

conversion/tss_helper.go (1)

65-69: Consistent Variable Naming Enhances Readability
The variable originally known as peerId has been renamed to peerID for consistency with naming conventions. The subsequent usage in constructing the multiaddress via peerID.String() is now uniform and clearer.

keygen/request.go (1)

3-3: Approved import path update

The import path has been correctly updated from the old Thor repository path to the new ZetaChain repository path, aligning with the hard-fork objective.

keysign/notifier_test.go (1)

11-12: Import paths correctly updated

The import paths have been properly updated from the Thor repository namespace to the ZetaChain repository namespace, which is consistent with the project decoupling strategy.

storage/localstate_backwards_test.go (1)

6-6: Import path correctly migrated

The import path for the conversion package has been successfully migrated from the Thor repository to the ZetaChain repository.

p2p/communication_test.go (1)

10-10: Import path correctly updated

The import path for the messages package has been properly updated to use the new ZetaChain repository path instead of the Thor repository path.

conversion/conversion.go (2)

22-22: Package import reordering

The import for the edwards/v2 package has been appropriately reordered within the import block.


26-26: Import path correctly updated

The import path for the messages package has been updated to use the ZetaChain repository path instead of the Thor repository path, which aligns with the hard-fork objective.

blame/manager.go (1)

101-101: Improved parameter naming in Range function

The change from key interface{} to _ any is appropriate as it:

  1. Explicitly indicates the key parameter is not being used (with underscore)
  2. Updates to the more modern any type (Go 1.18+) from interface{}

This improves code clarity and follows modern Go conventions.

common/tss_helper_test.go (2)

16-18: Import path updates align with repository migration

The import paths have been correctly updated from the Thor repository (gitlab.com/thorchain/tss/go-tss/...) to the ZetaChain repository (github.com/zeta-chain/go-tss/...), which aligns with the PR's objective of decoupling from Thor.


65-68: Improved function call formatting

The multi-line formatting of the sdk.UnmarshalPubKey function calls enhances readability by clearly separating the function parameters. This is particularly beneficial for function calls with multiple or lengthy parameters.

Also applies to: 79-82

p2p/stream_helper.go (1)

105-110: Enhanced error message formatting

The error message has been reformatted from a single line to multiple lines, improving readability while maintaining the same functionality. This change makes the code more maintainable, especially during debugging sessions when error messages need to be reviewed.

.gitignore (1)

8-8: Directory-specific syntax in .gitignore

The modification from .vscode to .vscode/ with the trailing slash explicitly indicates it's a directory. This is a more precise approach for ignoring directories in Git and follows best practices for .gitignore files.

keysign/keysign.go (2)

6-8: Import paths updated correctly.

The import paths have been properly updated from gitlab.com/thorchain/tss/go-tss to github.com/zeta-chain/go-tss for the common, p2p, and storage packages, which aligns with the hard-fork objective of decoupling from Thor.


14-18: Method signature formatting improvement.

The SignMessage method signature has been reformatted to span multiple lines, enhancing code readability without changing the functional signature. This follows better Go formatting practices for complex method signatures.

keygen/ecdsa/tss_keygen.go (3)

13-24: Import paths updated correctly.

The import paths have been properly updated from gitlab.com/thorchain/tss/go-tss to github.com/zeta-chain/go-tss for all relevant packages, maintaining consistency across the codebase as part of the hard-fork objective.


116-119: Function call formatting improvement.

The GetPeersID function call has been reformatted with arguments on separate lines, improving code readability while maintaining the same functionality.


202-205: Function call formatting improvement.

The TssMissingShareBlame function call has been reformatted with arguments on separate lines, enhancing code readability while preserving the function's behavior.

keysign/signature_notifier_test.go (1)

16-19: Import paths updated correctly.

The import paths have been properly updated from gitlab.com/thorchain/tss/go-tss to github.com/zeta-chain/go-tss, consistent with the repository migration effort.

common/tss_helper.go (1)

27-28: Import paths updated correctly.

The import paths for blame and messages packages have been properly updated from gitlab.com/thorchain/tss/go-tss to github.com/zeta-chain/go-tss, maintaining consistency with the hard-fork objective.

p2p/peer_status.go (3)

10-10: Import path updated correctly.

The import path has been properly updated from the old Thor repository to the new Zeta repository, which aligns with the PR objective of decoupling from Thor.


13-14: Good use of constant instead of magic string.

Introducing the NoLeader constant improves code maintainability by removing hardcoded string literals. The comment "will be dropped" suggests this might be temporary, but it's a good practice nonetheless.


100-100: Correctly using the newly defined constant.

The conditional statement now uses the defined constant instead of a magic string, which is consistent with best practices for maintainability.

keysign/notifier.go (1)

35-40: Improved function signature readability.

The refactoring of the function signature to a multi-line format improves readability, especially for functions with multiple parameters. This change follows modern Go formatting conventions.

storage/localstate_mgr.go (3)

19-19: Import statements correctly updated.

The import statements have been appropriately modified as part of the repository migration. Removing the direct dependency on ecdsa/keygen and replacing it with a local conversion package is a good architectural decision.

Also applies to: 23-23


154-158: Improved error message formatting.

The error message formatting has been enhanced for better readability using a multi-line approach, which is a good practice for long error messages.


166-170: Consistent error message formatting.

This error message has been formatted consistently with the previous one, maintaining a uniform style throughout the codebase.

conversion/conversion_test.go (2)

18-23: Improved code readability with multi-line formatting

The restructuring of the testPubKeys array initialization from a single line to a multi-line format enhances readability without altering functionality.


228-236: Enhanced readability of complex assertion

The JSON unmarshaling assertion has been reformatted into a multi-line structure, making it significantly easier to read and maintain without changing its behavior.

.github/workflows/ci.yml (3)

26-28: Appropriate CI job constraints added

Adding a timeout limit and explicitly setting GO111MODULE to 'on' are good practices that prevent hanging jobs and ensure consistent module behavior.


30-35: Improved checkout step clarity

Naming the checkout step explicitly and setting fetch-depth to 0 ensures complete repository history is available, which is beneficial for tools that may need commit history.


35-39: Updated Go setup with latest version

Upgrading to Go 1.23 ensures the project uses the most recent stable version with security patches and performance improvements.

p2p/test.go (1)

9-35: Well-implemented utility for dynamic port allocation

The GetFreePorts function effectively solves the common testing issue of port conflicts by dynamically allocating available ports. The implementation correctly:

  1. Uses "localhost:0" to let the OS assign available ports
  2. Maintains open connections during allocation to prevent port reuse
  3. Provides clear error handling with contextual messages

This is a valuable addition that will make tests more reliable by eliminating port collision issues.

storage/localstate_mgr_test.go (2)

16-16: Updated import path to reflect new module path

The import path has been correctly updated from the old GitLab thorchain path to the new GitHub zeta-chain path, which aligns with the PR objective of decoupling from Thor.


45-49: Improved assertion readability with multi-line formatting

Reformatting the complex assertion to span multiple lines enhances readability and maintainability without changing the test's behavior.

p2p/communication.go (4)

25-25: Import path updated.

The import path has been correctly updated from the old gitlab.com path to the new github.com path, aligning with the repository move.


75-75: Simplified variable initialization.

The initialization has been improved to use Go's zero value initialization pattern rather than explicitly setting to nil. This is a cleaner approach.

-externalAddr := maddr.Multiaddr(nil)
+var externalAddr maddr.Multiaddr

189-190: Improved log formatting.

The log message has been formatted across multiple lines, enhancing readability while maintaining the same functionality.


292-296: Enhanced resource manager initialization formatting.

The resource manager initialization has been reformatted across multiple lines for better readability, making the parameter structure more clear.

p2p/whitelist_connection_gater.go (3)

8-8: Reorganized import statement.

The zerolog import has been properly relocated, maintaining the same functionality.


40-40: Explicit unused parameter.

The parameter name has been replaced with an underscore to clearly indicate it's not used within the function, following Go's best practices for unused parameters.


44-48: Improved method signature formatting and unused parameter marking.

The method signature has been reformatted across multiple lines for better readability, and the direction parameter has been marked as unused. This adheres to Go best practices while maintaining functionality.

go.mod (3)

9-11: Major dependency updates.

Critical dependencies have been updated:

  • Added btcsuite/btcd/btcec/v2 v2.3.4
  • Changed from tendermint to cometbft v0.38.17
  • Updated cosmos-sdk to v0.50.13

These changes align with the transition from Tendermint to CometBFT and maintain compatibility with the broader Cosmos ecosystem.


237-238: Explicit fork reference.

Excellent documentation in the replacement comments for the tss-lib dependency. The comment clearly indicates this is a fork of the threshold-network repository with updates, which helps with maintenance and understanding of dependency sources.


1-3:

✅ Verification successful

Module path and Go version updated.

The module path has been updated from GitLab to GitHub, and the Go version has been upgraded from 1.20 to 1.22.11. This is a significant version jump that enables newer language features.


🏁 Script executed:

#!/bin/bash
# Description: Verify Go version compatibility

# Check if the Go version specified exists
echo "Checking if Go 1.22.11 is a valid version..."
curl -s https://go.dev/dl/?mode=json | grep -o '"version":"go1.22.[0-9]*"' | sort -V

Length of output: 187


Final Approval - Module Path and Go Version Update
The changes in go.mod have been verified. The module path has been correctly updated from GitLab to GitHub, and the Go version upgrade to 1.22.11 is valid based on the version check. This upgrade will enable newer language features compared to the previous version.

  • Verified that the module path is now github.com/zeta-chain/go-tss.
  • Confirmed that Go version 1.22.11 is a valid release.
keygen/ecdsa/keygen_test.go (3)

18-18: Added structured logging.

Introduced zerolog for improved structured logging capabilities, which will enhance the diagnostic output quality.


29-34: Updated import paths.

Import paths have been correctly updated from the old GitLab repository to the new GitHub location, maintaining the same package structure.


107-109: Implemented dynamic port allocation.

Replaced hardcoded port assignments with dynamic port allocation using p2p.GetFreePorts(4). This is a significant improvement that prevents port conflicts during testing, especially in CI environments where ports might be in use.

The addition of explicit error checking and log output for the allocated ports also improves test debugging capabilities.

storage/mock_localstatemanager.go (3)

12-12: Appropriate use of unnamed parameters in mock implementation.

Using underscore _ for parameters that aren't referenced in the method body follows Go conventions and helps avoid linter warnings about unused variables. This change clarifies that these parameters are intentionally unused in this mock implementation.


14-16: Consistent use of unnamed parameters.

The same pattern of using underscore for unused parameters is consistently applied here, which improves readability across the entire mock interface.


18-20: Appropriate use of unnamed parameters in mock implementation.

Following the same pattern as previous methods, this improves code clarity by explicitly indicating that the address parameter is not used in this mock implementation.

keysign/ecdsa/keysign_test.go (8)

23-25: Properly updated import paths.

Import paths have been correctly updated from the old GitLab repository to the new GitHub repository, which aligns with the project's migration.


32-36: Updated import paths align with repository migration.

The remaining import paths have been systematically updated to reflect the project's new location on GitHub.


133-136: Improved formatting for better readability.

The code has been reformatted to use proper indentation and whitespace, improving readability while maintaining functionality.


180-186: Enhanced code readability through multi-line formatting.

Breaking the keysign.NewRequest call into multiple lines with proper indentation improves readability, especially for function calls with multiple parameters.


284-290: Consistent formatting style applied.

The same multi-line formatting style has been consistently applied to this function call, maintaining a uniform code style throughout the file.


338-340: Improved logging statement formatting.

The logging statement has been reformatted to use method chaining, which enhances readability while maintaining the same functionality.


382-388: Consistent application of formatting improvements.

The same formatting style has been applied to this function call, ensuring consistency across the entire file.


431-432: Enhanced logging statement readability.

The logging statement has been reformatted to improve readability while maintaining the same functionality.

tss/keygen.go (14)

9-18: Systematic update of import paths.

The import paths have been systematically updated from the old GitLab repository to the new GitHub repository, which aligns with the project's migration.


20-20: Updated method receiver type for consistent type naming.

The receiver type has been changed from *TssServer to *Server, which is part of a consistent naming scheme across the codebase. This simplifies the API by removing redundant prefixes.


24-24: Improved naming convention for acronym usage.

Changed requestToMsgId to requestToMsgID to follow Go's convention where acronyms in identifiers should be capitalized (ID instead of Id).


44-54: Simplified constructor call.

The constructor has been renamed from eddsa.NewTssKeyGen to the more concise eddsa.New, maintaining a consistent pattern for constructors across the codebase.


76-83: Improved readability through multi-line formatting.

The joinParty method call has been reformatted to spread parameters across multiple lines, enhancing readability for this complex function call.


86-88: Enhanced error logging formatting.

The error logging statement has been reformatted to use method chaining for improved readability.


93-93: Replaced magic string with named constant.

Changed a hardcoded string "NONE" to a named constant p2p.NoLeader, which improves code maintainability and reduces the risk of typos or inconsistencies.


161-162: Added line break for better code formatting.

Added a line break after the method call, which improves readability by visually separating different code blocks.


189-189: Updated method receiver type for consistent type naming.

Consistent with earlier changes, the receiver type has been changed from *TssServer to *Server.


195-195: Consistent naming convention for acronym usage.

Similar to previous change, updated from requestToMsgId to requestToMsgID to follow Go conventions.


212-222: Simplified constructor call for consistency.

The constructor has been renamed from a verbose name to the more concise eddsa.New, maintaining a consistent pattern for constructors.


250-257: Enhanced readability through multi-line formatting.

The joinParty method call has been reformatted to spread parameters across multiple lines, improving readability for this complex function call.


263-263: Replaced magic string with named constant.

Changed a hardcoded string to a named constant p2p.NoLeader, which improves code maintainability.


336-337: Added line break for better code structure.

Added a line break after the method call for improved visual separation between code blocks.

tss/tss_4nodes_test.go (7)

20-28: Added logging import and updated package paths.

Added the zerolog logging package and updated import paths to reflect the new repository location, which is consistent with the project-wide migration.


66-66: Updated type to reflect renamed Server struct.

Changed the type of servers from []*TssServer to []*Server to align with the renaming of the core server type throughout the codebase.


88-88: Updated type to reflect renamed Server struct.

Changed the type of servers array from []*TssServer to []*Server to maintain consistency with the type renaming throughout the codebase.


126-127: Focus on current version testing.

Commented out testing of the old join party version, focusing only on the new version. This streamlines testing to concentrate on currently supported functionality.


181-187: Enhanced readability through multi-line formatting.

The keysign.NewRequest call has been reformatted to spread parameters across multiple lines, improving readability for this complex function call.


370-370: Updated method signature to reflect renamed Server struct.

Changed the return type from *TssServer to *Server to align with the renaming of the core server type.


384-394: Updated constructor call and improved formatting.

Changed from NewTss to New, following a more concise naming convention for constructors. The multi-line formatting enhances readability for this complex constructor call.

keysign/ecdsa/keysign_old_test.go (5)

23-24: Import paths updated to reflect new organization.

The import paths have been updated from gitlab.com/thorchain/tss/go-tss to github.com/zeta-chain/go-tss, which aligns with the decoupling from Thor to become an independent project.

Also applies to: 32-35


61-64: Error messages reformatted for improved readability.

The multi-line formatting for error messages enhances readability, especially for lengthy error messages. This adheres to Go best practices for structured and clear error reporting.

Also applies to: 73-76


127-129: Dynamic port allocation improves test reliability.

Replacing hardcoded port numbers with p2p.GetFreePorts(4) is an excellent improvement. This prevents port conflicts when running tests in parallel or on different environments, making the tests more reliable and robust.


173-179: Request object construction reformatted for clarity.

The multi-line format for keysign.NewRequest calls improves code readability by clearly delineating each parameter, making it easier to understand the request configuration.

Also applies to: 253-259, 327-332


307-308: Log formatting improved with structured logging.

The log statements have been reformatted using structured logging patterns with the zerolog library, which enhances readability and provides better organization of log information.

Also applies to: 375-376

.golangci.yml (3)

1-4: Updated Go version and timeout settings for linting.

The configuration now explicitly specifies Go version 1.22.11 and sets a 5-minute timeout for linting operations. This ensures compatibility with the specific Go version used in the project and prevents linting operations from running indefinitely.


7-28: Enhanced linter configuration with additional code quality tools.

The linter configuration has been significantly expanded to include numerous additional linters such as revive, goimports, stylecheck, typecheck, etc. This comprehensive set of linters will help maintain a higher standard of code quality.


29-47: Updated linter settings with improved exclusions and configurations.

The linter settings have been refined with better exclusion lists and new settings for various linters. The addition of gci settings for import order is particularly valuable as it establishes a clear, consistent ordering of imports throughout the codebase.

keygen/eddsa/keygen_test.go (3)

25-30: Import paths updated to reflect new organization.

The import paths have been updated from gitlab.com/thorchain/tss/go-tss to github.com/zeta-chain/go-tss, which aligns with the migration effort to decouple from Thor.


94-96: Dynamic port allocation improves test reliability.

The implementation of p2p.GetFreePorts(4) replaces hardcoded port values, preventing potential port conflicts when running tests in different environments or in parallel. The addition of structured logging for the allocated ports enhances debugging capabilities.


166-166: Constructor renamed for API simplification.

The constructor has been renamed from NewTssKeyGen to New, which follows Go's idiomatic pattern of using New as a constructor name when the package and type names already provide sufficient context. This simplifies the API and improves readability.

Also applies to: 204-204, 213-213

keysign/eddsa/keysign_test.go (4)

19-20: Import paths updated to reflect new organization.

The import paths have been updated from gitlab.com/thorchain/tss/go-tss to github.com/zeta-chain/go-tss, which aligns with the migration effort to decouple from Thor.

Also applies to: 29-32


132-134: Dynamic port allocation improves test reliability.

Similar to changes in other test files, the implementation of p2p.GetFreePorts(4) with appropriate error checking and logging enhances test reliability by preventing port conflicts.


177-183: Request object construction reformatted for clarity.

The multi-line format for keysign.NewRequest improves code readability by clearly delineating each parameter, making the request configuration more understandable.


207-207: Constructor renamed for API simplification.

The constructor has been renamed to New, which follows Go's idiomatic pattern of using New as a constructor name when the package and type names already provide sufficient context. This simplifies the API and improves code consistency across the codebase.

Also applies to: 264-264

p2p/metric_reporter.go (6)

22-22: Improved parameter naming for unused parameters.

The use of blank identifiers (_) for unused parameters in the AllowConn method follows Go best practices, making it explicit that these parameters are intentionally unused in the implementation.


30-30: Proper use of blank identifiers for unused parameters.

Good practice to use blank identifiers for the peer ID and direction parameters, which are unused in this method implementation.


38-38: Appropriate use of blank identifier.

Correctly updated the parameter to use a blank identifier since it's not referenced in the function body.


46-46: Consistent use of blank identifiers for unused parameters.

This maintains consistency with the other methods in this file by using blank identifiers for unused parameters.


59-59: Correctly marked unused parameter.

Using the blank identifier here improves code clarity and prevents linter warnings about unused variables.


72-72: Properly updated parameter name for consistency.

This change aligns with Go's best practice of using blank identifiers for unused parameters, completing the pattern established in the other methods.

tss/tss_4nodes_zeta_test.go (6)

19-20: Correctly updated import paths and added zerolog logger.

The import path update aligns with the project's goal to decouple from Thor, and the explicit alias for zerolog improves code clarity.


23-27: Updated import paths to reflect new organization.

Systematically replaced thorchain imports with zeta-chain, maintaining consistent dependency structure.


30-31: Updated server type to use the new Server type.

This change is part of the broader renaming to make types more concise and follows good Go naming conventions.


48-52: Implemented dynamic port allocation for tests.

Using p2p.GetFreePorts(4) instead of hardcoded values is a significant improvement that prevents port conflicts during test execution, especially in CI environments.


57-57: Updated variable initialization to match new type.

This change correctly aligns with the type change from TssServer to Server.


228-255: Updated method to use new constructor and parameter ordering.

The change from NewTss to New follows Go's convention for constructor naming, improving code clarity and consistency.

keysign/signature_notifier.go (5)

18-20: Updated import paths for the new organization.

Properly updated the import paths to use the new project namespace, maintaining the correct package structure.


188-194: Improved function signature formatting for readability.

Breaking the function signature across multiple lines improves readability, especially for functions with multiple parameters. This is a good stylistic change.


196-223: Consistent multi-line formatting applied to method signature.

Consistently applied the same multi-line formatting style to the broadcastCommon method, maintaining a uniform code style throughout the file.


244-274: Enhanced readability through improved function signature formatting.

The multi-line formatting makes the complex signature of createOrUpdateNotifier much easier to read and understand.


277-305: Consistent formatting applied to WaitForSignature method.

The multi-line formatting has been consistently applied to all complex method signatures in the file, greatly improving overall readability.

keygen/eddsa/tss_keygen.go (6)

17-24: Updated import paths to the new organization.

All imports have been correctly updated to use the new namespace, maintaining the appropriate package structure.


26-35: Renamed struct to more concise KeyGen.

Simplified the type name from EDDSAKeyGen to just KeyGen, which is more concise while still maintaining clarity about its purpose.


37-58: Improved constructor naming and formatting.

Renamed the constructor from NewTssKeyGen to simply New, which follows Go's idiomatic pattern for constructors. Also improved readability with proper multi-line formatting.


60-66: Updated method receiver to use new type.

Properly updated the method receiver to reflect the type name change.


68-142: Updated method receiver and improved function call formatting.

The method receiver has been updated to use the new type name, and function calls have been reformatted for better readability.


144-243: Consistently updated method receiver and function call formatting.

Method receiver has been updated to match the new type name, and multi-line function call formatting has been applied consistently throughout the method.

keysign/ecdsa/tss_keysign.go (9)

14-14: Updated import references.

No issues identified with the updated packages.

Also applies to: 17-17


23-28: Transition to new TSS imports.

These changes align with the renamed namespace and appear correctly updated.


41-51: Multi-line constructor parameters for NewTssKeySign.

The changes improve readability. No functional concerns.


75-75: Use of atomic.NewBool and any in sync.Map iteration.

This approach leverages Go 1.18+ features effectively. No issues found.

Also applies to: 77-77


85-86: Refined logging statements.

The multiline logging style is clear and concise.


95-99: Refactored SignMessage function signature.

Splitting parameters across lines enhances maintainability.


163-167: Assigning tKeySign.tssCommonStruct.P2PPeers.

No logical issues observed with setting the P2P peers list.


208-213: Multi-line signature for processKeySign.

The changes are straightforward and maintain consistency.


249-250: Blame setting with SetBlame.

No further concerns; the blame logic here appears properly integrated.

tss/keysign.go (8)

15-16: Updated imports to the new repository structure.

All references point to the correct modules.

Also applies to: 18-26


29-33: Refined signature for waitForSignatures.

Multi-line parameters improve clarity.


47-57: Reorganized generateSignature method signature.

Adopts a consistent layout with minimal functional impact.


97-104: Parameterized call to joinParty.

Well-structured approach for passing arguments.


151-153: Enhanced multi-line error logging.

No notable issues; the revised formatting is clear.


292-296: Multi-line error construction.

Clearer usage of formatted error strings; no functional changes.


316-327: Validation of signer keys against threshold.

Prevents insufficient signers scenario. This is correct.


364-374: Multi-line invocation of generateSignature.

Consistent with the rest of the code's style and no issues found.

keysign/eddsa/tss_keysign.go (12)

20-20: Updated imports for EDDSA keysign.

Migration to the new modules is correct and consistent.

Also applies to: 22-27


30-30: Renamed type to KeySign.

Improves naming consistency across the codebase.


40-50: New constructor function New.

Multi-line signature aligns with best practices.


63-63: Method rename for retrieving TSS KeySign channels.

No functionality concerns; the rename looks appropriate.


67-67: GetTssCommonStruct method.

No issues identified.


71-71: Refined startBatchSigning signature.

Properly matches the new naming conventions.


76-76: Use of any instead of interface{} in Range.

Consistent with Go 1.18 or newer.


94-99: Refactored SignMessage method parameters.

Enhanced readability with multi-line format.


128-128: Refined error message.

No concerns; error explanation is clear.


135-135: Improved error feedback for JSON unmarshal issues.

Code logic remains correct.


159-162: Setting P2P peers.

No detected anomalies; usage of GetPeersID is consistent.


206-210: Multi-line signature for processKeySign.

This structure fosters better maintainability.

tss/tss.go (8)

20-27: Updated import references.

Properly switched to the github.com/zeta-chain/go-tss namespace.


30-39: Introduction of IServer interface.

Defining a clear contract for TSS server functionality.


41-42: Renamed TssServer to Server.

Ensures naming consistency and clarity throughout the project.


63-64: Refined signature of New.

Parameters are now neatly structured.


74-74: Returning (*Server, error) from New.

Reflects the updated struct usage.


151-152: Constructing the Server instance.

Fields appear correctly assigned with no regressions.


272-273: Refined GetKnownPeers method.

Implementation logic remains straightforward.


295-296: Introduced GetP2PHost method.

Approach is consistent with the rest of the codebase.

common/tss_test.go (6)

24-27: Import paths updated correctly.

The import paths have been updated from the Thor repository to the ZetaChain repository, which aligns with the PR's objective to decouple the project and transform it into an independent project.


33-50: Improved array declarations for better readability.

The array declarations for testPubKeys and testBlamePubKeys have been reformatted from single-line to multi-line format, making the code more readable and easier to maintain without changing functionality.


144-150: Function signature reformatted for improved readability.

The fabricateTssMsg function signature has been reformatted to use multiple lines, which improves readability and aligns with modern Go style conventions.


197-203: Consistent function signature reformatting.

Several function signatures (testVerMsgDuplication, setupProcessVerMsgEnv, testDropMsgOwner, testVerMsgAndUpdateFromPeer, testVerMsgAndUpdate) have been reformatted to use a consistent multi-line style, improving code consistency and readability.

Also applies to: 226-231, 265-271, 384-389, 407-412


473-476: Function call reformatted for improved readability.

The UnmarshalPubKey function call has been reformatted to use multiple lines, maintaining consistency with other reformatted function signatures in the file.


492-500: Consistent function call reformatting.

The calls to fabricateTssMsg have been reformatted to follow the same multi-line pattern as the function declaration, maintaining consistent code style throughout the file.

Also applies to: 524-532

common/tss.go (5)

18-21: Import paths updated correctly.

The import paths have been updated from the Thor repository to the ZetaChain repository, which is consistent with the changes in other files and aligns with the PR objective to establish the project as independent.


56-63: Constructor function signature reformatted for improved readability.

The NewTssCommon function signature has been reformatted to use multiple lines, which improves readability and makes future parameter additions more manageable.


278-279: Improved error logging format.

The error logging statements have been reformatted to use the chain method pattern, which improves readability and is consistent with the style used elsewhere in the codebase.

Also applies to: 314-315


420-421: Debug logging format improved.

The debug logging statement has been reformatted to use the chain method pattern, maintaining consistent logging style throughout the file.


461-465: Method signatures reformatted for consistency.

Several method signatures (sendBulkMsg, applyShare, requestShareFromPeer, processVerMsg, broadcastHashToPeers, receiverBroadcastHashToPeers, processTSSMsg) have been reformatted to use multiple lines, which provides better readability and consistent code style across the codebase.

Also applies to: 583-588, 625-630, 671-674, 707-711, 742-745, 775-779

blame/policy_helper.go (1)

3-3: Import path updated correctly.

The import path for the conversion package has been updated from the Thor repository to the ZetaChain repository, which is consistent with the changes in other files and necessary for the project to function independently.

blame/round_mgr.go (1)

6-6: Import path updated correctly.

The import path for the messages package has been updated from the Thor repository to the ZetaChain repository, which is consistent with the changes in other files and necessary for the project to function independently.

blame/round_mgr_test.go (1)

6-6: Import path updated to reflect new repository location.

The update from gitlab.com/thorchain/tss/go-tss/messages to github.com/zeta-chain/go-tss/messages aligns with the decoupling of this project from Thor as mentioned in the PR objectives.

p2p/party_coordinator_test_with_leader_test.go (1)

15-15: Import path updated correctly.

The change from gitlab.com/thorchain/tss/go-tss/conversion to github.com/zeta-chain/go-tss/conversion properly reflects the repository migration.

p2p/party_coordinator_test.go (1)

15-15: Repository path update complete.

Import path has been successfully updated to use the new ZetaChain repository location.

common/local_cache_item.go (1)

6-6: Import path correctly updated.

The import has been properly migrated to the new repository path, maintaining consistency with the rest of the codebase.

cmd/tss-recovery/key.go (2)

27-27: Field name improved to follow Go conventions.

Changing from Id to ID follows Go's naming convention for acronyms, which should be in consistent case (all uppercase in this instance).


79-79: Struct initialization updated to match field name change.

The struct initialization has been properly updated to use ID instead of Id to match the field name change.

blame/policy_test.go (2)

14-15: Import paths updated correctly

The import paths have been properly updated to reflect the new repository structure under the zeta-chain organization.


19-24: Well-formatted array declaration

Good formatting improvement for the testPubKeys array. The multi-line format significantly improves readability.

cmd/tss/tss_http_test.go (2)

14-14: Import path updated correctly

The import path has been appropriately updated to reference the new repository structure.


26-26: Function references properly updated to use new naming convention

All occurrences of NewTssHttpServer have been consistently renamed to NewHTTPServer, which follows Go's standard naming convention (HTTP should be capitalized). This ensures consistency with the implementation file changes.

Also applies to: 43-43, 53-53, 117-117, 194-194

blame/policy.go (2)

11-12: Import paths updated correctly

The import paths have been properly updated to reflect the new repository structure under the zeta-chain organization.


185-186: Improved error message formatting

Good restructuring of the error message with proper indentation using the fluent logging API. This improves code readability while maintaining the same functionality.

cmd/tss-recovery/tss-recovery.go (2)

12-12: Improved import clarity by replacing dot import

Excellent change replacing the dot import with a named import. This follows Go best practices by making the source of imported functions explicitly visible in the code.


49-49: Updated function call with explicit package reference

The function call has been correctly updated to use the explicitly named package, which improves code clarity and maintainability.

cmd/tss-benchsign/main.go (3)

70-73: Enhanced error message formatting improves readability.

The error message for insufficient shares has been reformatted to use a multi-line style, which improves code readability while maintaining the same functionality.


234-238: Improved function signature formatting.

The loadKeyGenData function signature has been formatted with parameters on separate lines, improving readability for this multi-parameter function.


249-250: Consistent error message style.

The error message has been adjusted to use a consistent fragment style by removing the period at the end, which aligns with Go's idiomatic error message formatting.

cmd/tss/main.go (4)

17-20: Import paths properly updated for project decoupling.

The import paths have been correctly updated from gitlab.com/thorchain/tss/go-tss to github.com/zeta-chain/go-tss, which aligns with the primary objective of decoupling this project from Thor.


57-67: Constructor renamed to follow Go conventions.

The constructor has been renamed from tss.NewTss to tss.New which follows Go's idiomatic naming conventions for constructors and avoids redundancy.


71-71: HTTP server initialization updated to use proper Go casing.

Updated from NewTssHttpServer to NewHTTPServer to follow Go's convention of using uppercase for acronyms like HTTP.


90-95: Improved flag declaration formatting.

The flag.BoolVar call has been reformatted to a multi-line style, enhancing readability for flag declarations with longer descriptions.

cmd/tss/mock_tss_server.go (5)

8-12: Import paths correctly updated for project decoupling.

The import paths have been properly updated from Thor's namespace to ZetaChain's namespace, consistent with the overall project decoupling objective.


39-39: Unused parameter properly annotated.

The parameter in the Keygen method has been replaced with an underscore (_), explicitly indicating that it's not used within the function body. This follows Go's convention for unused parameters.


43-49: Improved return statement formatting.

The return statement in the Keygen function has been reformatted to spread arguments across multiple lines, which enhances readability, especially for functions with multiple parameters.


52-52: Unused parameter properly annotated in KeygenAllAlgo.

Similar to the Keygen method, the unused parameter in KeygenAllAlgo has been replaced with an underscore, following Go's convention for unused parameters.


62-62: Unused parameter properly annotated in KeySign.

Consistent with the other methods, the unused parameter in KeySign has been replaced with an underscore, maintaining a consistent approach throughout the codebase.

Makefile (11)

1-1: Module path updated to reflect the project's new identity.

The module path has been correctly updated to use the ZetaChain namespace, consistent with the overall project decoupling objective.


3-4: Added helpful target to improve developer experience.

The new help target provides a self-documenting Makefile by listing available commands, which is a valuable addition for new developers working with this project.


6-8: Added dependency management for development tools.

The new deps target streamlines the installation of required development tooling, ensuring consistent linting and formatting across the project.


10-10: Improved target documentation.

Added a descriptive comment for the tools target, enhancing the Makefile's self-documentation capabilities.


15-15: Enhanced install target clarity.

Added a comment to clarify the purpose of the install target, improving developer understanding of the build process.


22-22: Improved test target documentation.

Added a descriptive comment for the test target, providing clarity on its functionality.


25-30: Added comprehensive code formatting target.

The new fmt target ensures consistent code formatting through multiple steps - fixing long lines with golines and using golangci-lint for additional formatting checks, which will help maintain code quality.


32-34: Streamlined pre-lint checks.

The lint-pre target has been simplified while maintaining its core functionality, making the build process more efficient.


36-36: Improved lint target documentation.

Added a descriptive comment for the lint target, enhancing the Makefile's self-documentation.


39-39: Renamed target to better reflect its purpose.

The protob target has been renamed to codegen, which more accurately describes its purpose of generating code from protocol buffer definitions.


42-42: Updated .PHONY declaration for current targets.

The .PHONY declaration has been updated to reflect the current set of targets, ensuring that Make correctly handles targets that don't correspond to files.

cmd/tss/tss_http.go (5)

16-18: Import paths successfully updated to the new repository location.

The import paths have been correctly updated from the original Thor repository to the Zeta-Chain repository, aligning with the PR objective to decouple this project.


21-22: Enhanced struct naming convention.

Renaming from TssHttpServer to HTTPServer follows Go's standard naming conventions more closely, as HTTP is an acronym and should be capitalized uniformly.


24-24: Interface-based dependency improves design.

Changing the tssServer field type from tss.Server to tss.IServer introduces better abstraction through an interface, facilitating more maintainable code and easier testing.


28-29: Constructor name updated for consistency.

The constructor has been appropriately renamed to match the new struct name and updated to accept the interface type, maintaining consistency throughout the codebase.


43-43: Method receivers consistently updated.

All method receivers have been uniformly updated to use the new type name, ensuring consistency across the codebase.

Also applies to: 55-55, 91-91, 127-127, 166-166, 196-196, 207-207, 211-211

p2p/party_coordinator.go (3)

18-19: Import paths successfully updated to the new repository location.

The import paths have been correctly updated from the original Thor repository to the Zeta-Chain repository, aligning with the PR objective to decouple this project.


191-196: Function signatures reformatted for improved readability.

The function signatures have been reformatted to span multiple lines, which significantly improves readability for functions with numerous parameters. This follows Go style conventions for complex function signatures.

Also applies to: 272-278, 310-314, 400-404, 450-456


502-502: Magic string replaced with a named constant.

The hardcoded string "NONE" has been replaced with the constant NoLeader, eliminating a magic string and improving code readability and maintainability. This is a beneficial change that makes the code more self-documenting.

Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (2)
.github/workflows/ci.yml (1)

44-44: Newline at EOF
The file currently lacks a newline at the end. Please add a newline character at the end to comply with YAML linting standards and common best practices.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 44-44: no new line character at the end of file

(new-line-at-end-of-file)

Makefile (1)

10-14: Tools Target for TSS Utilities
The tools target installs tss-specific utilities. The commands are straightforward; however, please verify that these installations cover all necessary tools for your workflows.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6628a87 and 4c87c60.

📒 Files selected for processing (3)
  • .github/workflows/ci.yml (1 hunks)
  • Makefile (1 hunks)
  • keysign/signature_notifier_test.go (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • keysign/signature_notifier_test.go
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/ci.yml

[error] 44-44: no new line character at the end of file

(new-line-at-end-of-file)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: test go 1.22
🔇 Additional comments (16)
.github/workflows/ci.yml (5)

23-23: Test Command Update – Verify -race Flag Removal
The test step now runs as go test -timeout 25m ./..., which omits the -race flag. Removing the race detector may reduce the detection of race conditions. Please confirm that this change is intentional and its impact on test coverage is acceptable.


26-28: Lint Job Timeout and Environment Settings
The lint job now specifies a timeout-minutes: 15 and sets GO111MODULE: on via the environment. These additions ensure that the job doesn’t run indefinitely and that module mode is enforced. This is a good update provided it aligns with your build environment requirements.


30-33: Checkout Source Update
Renaming the checkout step to "Checkout Source" and setting fetch-depth: 0 ensures full repository history is available when needed. This change improves traceability for linting and other analysis tasks.


35-38: Go Setup Step Version Bump
The "Set up Go" step now uses Go version '1.23'. This update aligns with the updated dependency requirements. Please verify that all components are compatible with this Go version.


43-44: GolangCI-Lint Version Update and Cache Settings
Updating the linter version to v1.63.4 and enabling skip-cache: true ensures the latest linting checks are applied and avoids potential issues from stale cached results. Ensure that the performance impact of skipping the cache is acceptable for your CI environment.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 44-44: no new line character at the end of file

(new-line-at-end-of-file)

Makefile (11)

1-1: Module Path Update
The module directive has been updated to github.com/zeta-chain/go-tss, which correctly reflects the new repository structure.


3-4: Help Target Addition
The new help target provides users with a list of available commands in a clear and concise format. This addition enhances usability and documentation directly within the Makefile.


6-9: Dependencies Target – Tooling Installation
The deps target installs essential development tools with fixed versions for reproducibility ([email protected] and [email protected]). This approach ensures that tooling remains consistent across environments.


15-17: Install Target Clarity
The install target is clearly documented as “Compiles & install go-tss binary” and uses go.sum as a dependency to ensure integrity. This target meets expectations for a production-grade build script.


18-21: go.sum Verification Target
The go.sum target confirms that dependencies remain unaltered by echoing a message and running go mod verify, which is a solid practice to ensure module integrity.


22-24: Test Target Consistency Check
The test target runs tests using @go test -race -timeout 30m ./.... Note that while the CI workflow removed the -race flag, this target still includes it. Please confirm whether this discrepancy is deliberate or if both should be aligned for consistency across environments.


25-31: Code Formatting Target
The fmt target provides a two-step formatting operation: first, by fixing long lines using golines with specific parameters, and then by running golangci-lint for formatting checks. This structured approach enhances code consistency and readability.


32-35: Pre-Lint Target Validation
The lint-pre target effectively checks for formatting issues via gofmt -l . and verifies modules with go mod verify. This ensures that code adheres to styling guidelines before running the full linter.


36-38: Linter Target
The lint target simply invokes the linter after running pre-checks, which is a clean and modular approach for code quality validation.


39-41: Proto Generation Target
The codegen target is designated for generating protocol buffers using protoc with the current module variable. Ensure that all proto files in the ./messages directory are maintained in accordance with this command.


42-43: PHONY Declaration
The updated .PHONY declaration lists all the current targets accurately, ensuring that Make does not confuse filenames with target names.

@swift1337
Copy link
Contributor Author

UPD: CI tests fail. Will investigate & fix (works on my local machine)

#45

@swift1337 swift1337 merged commit 776a5e3 into master Mar 17, 2025
6 checks passed
@swift1337 swift1337 deleted the feat/hard-fork branch March 17, 2025 11:57
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.

4 participants