[all] Communication #2234
Merged
[all] Communication #2234
Conversation
This file contains 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
* RM old IInterchainModule, delete Interchain.sol code ahead of DB Integration * rename to InterchainClient * interchainSend() * interchainReceive() and corresponding test * remove forge console.log * Add interfaces for SynapseModule & InterchainClient
* Implement SynapseModule w/ ECREcover Around InterchainDB contract + TEsts * fix typo in event name * Interchain + InterchainDB integration (#2046) * RM old IInterchainModule, delete Interchain.sol code ahead of DB Integration * rename to InterchainClient * interchainSend() * interchainReceive() and corresponding test * remove forge console.log * Add interfaces for SynapseModule & InterchainClient * add keccak'd datahash to event emit * change variable name * Fix event variabl ename
* Scaffold ThresholdECDSA library * Start working on the tests * Add some revert tests * Start working on verify hash tests * More tests * More revert tests * Implement `verifySignedHash` * Chore: test cleanup
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (2)
- .github/workflows/go.yml (1 hunks)
- .github/workflows/goreleaser-actions.yml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/go.yml
Additional comments: 1
.github/workflows/goreleaser-actions.yml (1)
- 179-183: Adding debug steps like
debuganddebug2can be helpful for troubleshooting, but consider the following:
- Ensure these steps are necessary for the main branch or production workflows. If they are primarily for debugging during development, you might want to conditionally include them or remove them before merging to main.
- Document the purpose of these debug steps in the workflow file to make their intent clear to other developers.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (1)
- ethergo/parser/tracely/README.md (1 hunks)
Additional comments: 1
ethergo/parser/tracely/README.md (1)
- 1-3: The introduction of
Tracelyin the README is clear and concise, effectively outlining its origin, purpose, and future plans. It's good to see the correction of the previously identified typo, enhancing the document's readability and accuracy.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (9)
- .golangci.yml (2 hunks)
- ethergo/backends/base/base.go (4 hunks)
- ethergo/backends/base/trace.go (1 hunks)
- ethergo/deployer/functional.go (1 hunks)
- ethergo/parser/tracely/README.md (1 hunks)
- ethergo/signer/signer/awssigner/signer.go (2 hunks)
- ethergo/signer/signer/awssigner/unsafe.go (1 hunks)
- ethergo/signer/signer/gcpsigner/google.go (6 hunks)
- ethergo/signer/signer/suite_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- ethergo/backends/base/base.go
- ethergo/backends/base/trace.go
- ethergo/deployer/functional.go
- ethergo/signer/signer/awssigner/unsafe.go
Additional comments: 9
ethergo/parser/tracely/README.md (1)
- 1-4: The README provides a clear overview of the Tracely tool, its current enhancements, and future intentions. The previously noted typo has been corrected.
ethergo/signer/signer/awssigner/signer.go (2)
- 16-16: The
NewKmsSignerfunction is well-implemented, correctly initializing the KMS signer with the provided AWS configuration.- 53-104: The enhancements to the
Signerstruct, including the implementation oflibp2p.PrivKeyand the addition of new methods, are well-executed. Ensure that the handling ofcontextin theSignmethod is revisited for better context propagation..golangci.yml (2)
- 8-8: Excluding
tracely/*from linting is a reasonable decision, likely to avoid linting issues in forked or external code.- 104-104: Disabling the
typechecklinter due to malfunctions with embedded structs is understandable. Consider revisiting this decision in future to check if newer versions of the linter address this issue.ethergo/signer/signer/suite_test.go (2)
- 43-61: The test suite setup is comprehensive, covering AWS, GCP, and local signer configurations, ensuring thorough testing across different environments.
- 63-89: The
NewSignerFromMockKMSfunction is correctly marked as unsafe for production use, which is good practice. Ensure this function is strictly used in test environments to avoid security risks.ethergo/signer/signer/gcpsigner/google.go (2)
- 5-19: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [8-34]
The structural changes, including the addition of the
pubKeyDatafield in themanagedKeystruct, and the new imports for cryptographic operations are well-integrated and necessary for the enhancements being introduced.
- 212-251: The new methods added to the
managedKeystruct for key comparison, raw key retrieval, key type retrieval, signing, and public key retrieval are well-implemented. Consider adding documentation for these methods, especially for unconventional implementations like thePrivKeymethod, to clarify their purpose and usage.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (4)
- .github/workflows/go.yml (3 hunks)
- .github/workflows/solidity.yml (4 hunks)
- committee/testutil/deployers.go (1 hunks)
- ethergo/listener/listener.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- .github/workflows/go.yml
- committee/testutil/deployers.go
Additional comments: 4
ethergo/listener/listener.go (1)
- 188-189: The error handling here uses both
errors.Isand a string comparison witherr.Error(). While this approach might be necessary in some cases, relying on string comparison for error handling can be fragile and error-prone due to potential changes in error messages. If possible, consider defining custom error types or using sentinel errors for more robust and maintainable error handling..github/workflows/solidity.yml (3)
- 9-9: Adding
'packages/contracts-communication'to the list of paths that trigger the workflow is a good practice. It ensures that any changes to the contracts-communication package will trigger the appropriate CI/CD actions, maintaining the integrity and reliability of the codebase.- 42-42: Including
contracts-communicationin the job configuration for secrets and paths is crucial for ensuring that the CI/CD pipeline correctly handles this package. This addition helps maintain consistency and ensures that all necessary steps are taken for the contracts-communication package during the CI/CD process.- 171-173: The note about concurrency needing to be 1 due to potential "text file busy" errors when multiple instances of forge try to install the same version of solc simultaneously is important. This limitation should be documented clearly in the workflow file or an associated README to ensure that future modifications to the workflow take this constraint into account. Consider exploring ways to mitigate this limitation to improve the efficiency of the CI/CD pipeline.
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.
Description
Communication big pr
Summary by CodeRabbit
New Features
LibP2PManagerfor enhanced peer-to-peer communication and discovery.FunctionalDeployerfor deploying blockchain contracts.Bug Fixes
Documentation
go.modfiles with the latest dependency versions.Refactor
Style
Tests
UnsafeSignerfor testing purposes, facilitating the creation of signers with AWS KMS service.Chores
Revert