refactor: SetCctxAndNonceToCctxAndInboundHashToCctx to accept tssPubkey as an argument #2748
Conversation
…dInboundHashToCctx
kingpinXD
requested review from
brewmaster012,
lumtis,
ws4charlie,
skosito,
swift1337 and
fbac
as code owners
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 13340122 | Triggered | Generic High Entropy Secret | d6ea2df | zetaclient/chains/solana/signer/signer_test.go | View secret |
| 13340122 | Triggered | Generic High Entropy Secret | d6ea2df | zetaclient/chains/solana/signer/signer_test.go | View secret |
| 13392123 | Triggered | Generic High Entropy Secret | d6ea2df | zetaclient/chains/solana/signer/signer_test.go | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
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 WalkthroughWalkthroughThe changes enhance the cross-chain transaction handling by introducing a new parameter, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant msgServer
participant Keeper
User->>msgServer: Initiate Cross-Chain Transaction
msgServer->>Keeper: SetCctxAndNonceToCctxAndInboundHashToCctx(tssPubkey)
Keeper-->>msgServer: Acknowledge
msgServer-->>User: Transaction Successfully Processed
Assessment against linked issues
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 as PR comments)
Additionally, you can add Documentation and Community
|
kingpinXD
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2748 +/- ##
===========================================
- Coverage 66.83% 66.80% -0.04%
===========================================
Files 368 368
Lines 20662 20661 -1
===========================================
- Hits 13810 13803 -7
- Misses 6219 6223 +4
- Partials 633 635 +2
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (14)
- changelog.md (1 hunks)
- testutil/keeper/crosschain.go (2 hunks)
- x/crosschain/genesis.go (1 hunks)
- x/crosschain/keeper/cctx.go (2 hunks)
- x/crosschain/keeper/cctx_orchestrator_validate_inbound.go (1 hunks)
- x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go (1 hunks)
- x/crosschain/keeper/cctx_test.go (8 hunks)
- x/crosschain/keeper/grpc_query_inbound_hash_to_cctx_test.go (3 hunks)
- x/crosschain/keeper/msg_server_migrate_erc20_custody_funds.go (1 hunks)
- x/crosschain/keeper/msg_server_migrate_tss_funds.go (1 hunks)
- x/crosschain/keeper/msg_server_update_erc20_custody_pause_status.go (1 hunks)
- x/crosschain/keeper/msg_server_vote_outbound_tx.go (6 hunks)
- x/crosschain/keeper/msg_server_vote_outbound_tx_test.go (19 hunks)
- x/crosschain/keeper/msg_server_whitelist_erc20.go (1 hunks)
Additional context used
Path-based instructions (13)
x/crosschain/genesis.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/msg_server_update_erc20_custody_pause_status.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/msg_server_migrate_erc20_custody_funds.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/cctx_orchestrator_validate_inbound.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/cctx.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/msg_server_migrate_tss_funds.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/msg_server_whitelist_erc20.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/grpc_query_inbound_hash_to_cctx_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/cctx_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/msg_server_vote_outbound_tx.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.testutil/keeper/crosschain.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/msg_server_vote_outbound_tx_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
Learnings (2)
x/crosschain/keeper/msg_server_vote_outbound_tx.go (1)
Learnt from: kingpinXD PR: zeta-chain/node#2615 File: x/crosschain/keeper/msg_server_vote_outbound_tx_test.go:472-472 Timestamp: 2024-08-01T18:08:13.681Z Learning: The `SaveFailedOutbound` function in `x/crosschain/keeper/msg_server_vote_outbound_tx.go` requires a string argument for the error message.x/crosschain/keeper/msg_server_vote_outbound_tx_test.go (1)
Learnt from: kingpinXD PR: zeta-chain/node#2615 File: x/crosschain/keeper/msg_server_vote_outbound_tx_test.go:472-472 Timestamp: 2024-08-01T18:08:13.681Z Learning: The `SaveFailedOutbound` function in `x/crosschain/keeper/msg_server_vote_outbound_tx.go` requires a string argument for the error message.
GitHub Check: codecov/patch
x/crosschain/genesis.go
[warning] 48-50: x/crosschain/genesis.go#L48-L50
Added lines #L48 - L50 were not covered by tests
Additional comments not posted (21)
x/crosschain/genesis.go (1)
46-51: Ensure test coverage for new logic.The addition of the
tssPubkeyparameter in theInitGenesisfunction enhances security and traceability. However, lines 48-50 are not covered by tests, as indicated by the code coverage tool. Consider adding tests to ensure the new logic is validated.Tools
GitHub Check: codecov/patch
[warning] 48-50: x/crosschain/genesis.go#L48-L50
Added lines #L48 - L50 were not covered by testsx/crosschain/keeper/msg_server_update_erc20_custody_pause_status.go (1)
84-84: VerifytssPubkeyintegration.The integration of
tss.TssPubkeyinto theSetCctxAndNonceToCctxAndInboundHashToCctxcall aligns with the refactor's objectives. Ensure that this change is correctly reflected in all relevant parts of the system and that tests validate this integration.Verification successful
Integration of
tssPubkeyis consistent and correctly implemented.The
tssPubkeyis appropriately integrated into theSetCctxAndNonceToCctxAndInboundHashToCctxfunction across the codebase. This includes its usage in multiple files and the presence of test cases that validate this integration. No further action is required.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of `tssPubkey` in the `UpdateERC20CustodyPauseStatus` function. # Test: Check for references to `SetCctxAndNonceToCctxAndInboundHashToCctx` with `tssPubkey`. rg --type go "SetCctxAndNonceToCctxAndInboundHashToCctx" --context 5Length of output: 11593
x/crosschain/keeper/msg_server_migrate_erc20_custody_funds.go (1)
86-86: VerifytssPubkeyintegration.The addition of
tss.TssPubkeyto theSetCctxAndNonceToCctxAndInboundHashToCctxcall is consistent with the refactor's goals. Ensure that this integration is correctly implemented and tested across the system.Verification successful
Integration of
tssPubkeyis consistent and well-tested.The
tss.TssPubkeyparameter has been consistently integrated into theSetCctxAndNonceToCctxAndInboundHashToCctxfunction across the codebase. This change is reflected in both functional and test code, indicating thorough integration and coverage.
- Files with updates:
x/crosschain/genesis.gox/crosschain/keeper/msg_server_migrate_erc20_custody_funds.gox/crosschain/keeper/msg_server_update_erc20_custody_pause_status.gox/crosschain/keeper/msg_server_migrate_tss_funds.gox/crosschain/keeper/msg_server_whitelist_erc20.gox/crosschain/keeper/cctx_test.goThis suggests the integration is robust and should function as intended.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of `tssPubkey` in the `MigrateERC20CustodyFunds` function. # Test: Check for references to `SetCctxAndNonceToCctxAndInboundHashToCctx` with `tssPubkey`. rg --type go "SetCctxAndNonceToCctxAndInboundHashToCctx" --context 5Length of output: 11593
x/crosschain/keeper/cctx_orchestrator_validate_inbound.go (1)
54-54: VerifytssPubkeyintegration.The integration of
tss.TssPubkeyinto theSetCctxAndNonceToCctxAndInboundHashToCctxcall aligns with the refactor's objectives. Ensure that this change is correctly reflected in all relevant parts of the system and that tests validate this integration.Verification successful
Integration of
tss.TssPubkeyVerifiedThe integration of
tss.TssPubkeyinto theSetCctxAndNonceToCctxAndInboundHashToCctxfunction is consistent across the codebase. This change aligns with the refactor's objectives and is reflected in various files, ensuring comprehensive coverage. The function's documentation supports this integration, confirming its correctness.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of `tssPubkey` in the `ValidateInbound` function. # Test: Check for references to `SetCctxAndNonceToCctxAndInboundHashToCctx` with `tssPubkey`. rg --type go "SetCctxAndNonceToCctxAndInboundHashToCctx" --context 5Length of output: 11593
x/crosschain/keeper/cctx.go (1)
Line range hint
19-32: Confirm correct usage oftssPubkey.The addition of
tssPubkeyas a parameter inSetCctxAndNonceToCctxAndInboundHashToCctxsimplifies the function. Ensure that thetssPubkeyis correctly utilized throughout the function to maintain the intended functionality.x/crosschain/keeper/msg_server_migrate_tss_funds.go (1)
131-131: Ensure correct usage oftssPubkey.The addition of
tssPubkeyto theSetCctxAndNonceToCctxAndInboundHashToCctxfunction call is consistent with the refactor's objective. Ensure that all invocations of this function across the codebase are updated to match the new signature.Verification successful
All function calls updated to match new signature
The function
SetCctxAndNonceToCctxAndInboundHashToCctxhas been consistently updated across the codebase to include thetssPubkeyparameter. No discrepancies were found in the implementation.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `SetCctxAndNonceToCctxAndInboundHashToCctx` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'SetCctxAndNonceToCctxAndInboundHashToCctx'Length of output: 6379
x/crosschain/keeper/msg_server_whitelist_erc20.go (1)
157-157: Ensure correct usage oftssPubkey.The addition of
tssPubkeyto theSetCctxAndNonceToCctxAndInboundHashToCctxfunction call is consistent with the refactor's objective. Ensure that all invocations of this function across the codebase are updated to match the new signature.Verification successful
All instances of
SetCctxAndNonceToCctxAndInboundHashToCctxare updated correctly.The function
SetCctxAndNonceToCctxAndInboundHashToCctxhas been consistently updated across the codebase to include thetssPubkeyparameter. No discrepancies were found in its usage.
- The function calls in
x/crosschain/genesis.go,x/crosschain/keeper/grpc_query_inbound_hash_to_cctx_test.go,x/crosschain/keeper/msg_server_vote_outbound_tx.go, and other files correctly includetssPubkey.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `SetCctxAndNonceToCctxAndInboundHashToCctx` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'SetCctxAndNonceToCctxAndInboundHashToCctx'Length of output: 6379
x/crosschain/keeper/grpc_query_inbound_hash_to_cctx_test.go (2)
129-133: Ensure correct parameter order and usage.The function
createInTxHashToCctxWithCctxsnow acceptstssPubkeyas a parameter. Ensure that all calls to this function are updated to match the new signature and that thetssPubkeyis used correctly within the function.Verification successful
Function signature and usage are consistent.
The function
createInTxHashToCctxWithCctxshas been updated to includetssPubkeyas a parameter, and all calls to this function have been correctly updated to match the new signature. ThetssPubkeyis used appropriately within the function. No issues were found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `createInTxHashToCctxWithCctxs` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'createInTxHashToCctxWithCctxs'Length of output: 1202
143-143: Ensure correct usage oftssPubkey.The addition of
tssPubkeyto theSetCctxAndNonceToCctxAndInboundHashToCctxfunction call is consistent with the refactor's objective. Ensure that all invocations of this function across the codebase are updated to match the new signature.Verification successful
Consistent Usage of
tssPubkeyin Function Calls VerifiedAll instances of the function
SetCctxAndNonceToCctxAndInboundHashToCctxin the codebase have been updated to include thetssPubkeyparameter as intended. This ensures consistency with the refactor's objective. No further action is required.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `SetCctxAndNonceToCctxAndInboundHashToCctx` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'SetCctxAndNonceToCctxAndInboundHashToCctx'Length of output: 6379
x/crosschain/keeper/cctx_test.go (4)
27-27: Ensure correct parameter usage increateNCctxWithStatus.The function
createNCctxWithStatusnow acceptstssPubkeyas a parameter. Ensure that all calls to this function are updated to match the new signature and that thetssPubkeyis used correctly within the function.Verification successful
All calls to
createNCctxWithStatusare correctly updated.The function
createNCctxWithStatushas been correctly updated to include thetssPubkeyparameter across all its usages in thecctx_test.gofile. No issues were found with the parameter usage.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `createNCctxWithStatus` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'createNCctxWithStatus'Length of output: 2210
49-49: Ensure correct parameter usage increateNCctx.The function
createNCctxnow acceptstssPubkeyas a parameter. Ensure that all calls to this function are updated to match the new signature and that thetssPubkeyis used correctly within the function.Verification successful
Parameter usage in
createNCctxis correct.All calls to the
createNCctxfunction have been updated to match the new signature, including thetssPubkeyparameter. No further action is needed.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `createNCctx` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'createNCctx'Length of output: 4285
43-43: Ensure correct usage oftssPubkey.The addition of
tssPubkeyto theSetCctxAndNonceToCctxAndInboundHashToCctxfunction call is consistent with the refactor's objective. Ensure that all invocations of this function across the codebase are updated to match the new signature.Verification successful
All instances of
SetCctxAndNonceToCctxAndInboundHashToCctxare correctly updated withtssPubkey.The function
SetCctxAndNonceToCctxAndInboundHashToCctxis consistently called with thetssPubkeyparameter across the codebase, as per the refactor's objective. No further action is needed.
- Verified in files such as
genesis.go,msg_server_vote_outbound_tx.go,msg_server_whitelist_erc20.go, and others.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `SetCctxAndNonceToCctxAndInboundHashToCctx` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'SetCctxAndNonceToCctxAndInboundHashToCctx'Length of output: 6379
85-85: Ensure correct usage oftssPubkey.The addition of
tssPubkeyto theSetCctxAndNonceToCctxAndInboundHashToCctxfunction call is consistent with the refactor's objective. Ensure that all invocations of this function across the codebase are updated to match the new signature.Verification successful
All function calls to
SetCctxAndNonceToCctxAndInboundHashToCctxare correctly updated.The function
SetCctxAndNonceToCctxAndInboundHashToCctxis consistently invoked with thetssPubkeyargument across all relevant files, confirming the refactor's objective has been met. No further action is needed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `SetCctxAndNonceToCctxAndInboundHashToCctx` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'SetCctxAndNonceToCctxAndInboundHashToCctx'Length of output: 6379
x/crosschain/keeper/msg_server_vote_outbound_tx.go (4)
66-74: Ensure TSS existence check is robust.The addition of a TSS existence check at the start of the
VoteOutboundfunction is a good practice. Ensure that this check is performed consistently across all relevant functions to prevent errors related to missing TSS keys.
118-118: Ensure correct usage oftssPubkeyinSaveFailedOutbound.The addition of
tssPubkeyto theSaveFailedOutboundfunction call is consistent with the refactor's objective. Ensure that all invocations of this function across the codebase are updated to match the new signature.Verification successful
Verification Successful: All
SaveFailedOutboundCalls Match the New SignatureThe function
SaveFailedOutboundis consistently used with the updated signature across the codebase. No discrepancies were found in its invocation.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `SaveFailedOutbound` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'SaveFailedOutbound'Length of output: 3886
211-219: Ensure correct usage oftssPubkeyinSaveOutbound.The addition of
tssPubkeyto theSaveOutboundfunction call is consistent with the refactor's objective. Ensure that all invocations of this function across the codebase are updated to match the new signature.Verification successful
All calls to
SaveOutboundhave been updated withtssPubkey. The function's new signature has been correctly implemented across the codebase, ensuring consistency with the refactor's objective. No further updates are necessary.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `SaveOutbound` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'SaveOutbound'Length of output: 7273
121-121: Ensure correct usage oftssPubkeyinSaveSuccessfulOutbound.The addition of
tssPubkeyto theSaveSuccessfulOutboundfunction call is consistent with the refactor's objective. Ensure that all invocations of this function across the codebase are updated to match the new signature.Verification successful
Verification of
SaveSuccessfulOutboundUsage Across CodebaseAll instances of the
SaveSuccessfulOutboundfunction in the codebase have been updated to use the new signature with thetssPubkeyparameter. This includes the main implementation and test files. No discrepancies were found in the function's usage.
x/crosschain/keeper/msg_server_vote_outbound_tx.gox/crosschain/keeper/msg_server_vote_outbound_tx_test.goThe function is consistently used with the new signature across the examined files.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `SaveSuccessfulOutbound` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'SaveSuccessfulOutbound'Length of output: 5512
testutil/keeper/crosschain.go (1)
Line range hint
1-1: LGTM!The removal of the
MockGetOutboundfunction aligns with the updated testing strategy.x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go (1)
Line range hint
1-1: Consider reintroducing tests for failure scenarios.The removal of the test case for
SetCctxAndNonceToCctxAndInboundHashToCctxfailures reduces coverage of potential failure paths. It's important to ensure all critical scenarios are adequately tested.x/crosschain/keeper/msg_server_vote_outbound_tx_test.go (1)
176-212: LGTM! Ensure consistent testing of new parameters.The addition of a test case for missing TSS keys enhances the robustness of the tests. Ensure that all new parameters, like
tssPubkey, are consistently tested across relevant scenarios.changelog.md (1)
21-21: LGTM!The changelog entry accurately documents the refactor involving
SetCctxAndNonceToCctxAndInboundHashToCctx.
This pr refactors the SetCctxAndNonceToCctxAndInboundHashToCctx to accept a tssPubkey instead of trying to fetch it.
It does not modify any logic.
Closes #2719
https://github.com/zeta-chain/protocol-private/issues/72
How Has This Been Tested?
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests