Skip to content

refactor(observer): replace chainName by chainID for ChainNonces indexing#2515

Merged
lumtis merged 21 commits intodevelopfrom
refactor/remove-chain-name-indexing
Jul 23, 2024
Merged

refactor(observer): replace chainName by chainID for ChainNonces indexing#2515
lumtis merged 21 commits intodevelopfrom
refactor/remove-chain-name-indexing

Conversation

@lumtis
Copy link
Contributor

@lumtis lumtis commented Jul 19, 2024

Description

Closes: #1490

Replace the indexing for the ChainNonces object.

While this is a logical change since chain ID is the unique identifier for a chain and we should remove the chain name field over time, this change is necessary for #2005 because ChainName is a field for static chain information and it is a enum representing hardcoded values. This PR remove the dependency to this ChainName attribute, makes the whole chain information indexed by the chain ID that can be dynamically added.

Over time we will completely remove chain name enum field.

  • Change the indexing from chain name to chain ID
  • Add a migration script for observer module updating all the indexes for chainNonces object

How Has This Been Tested?

  • Unit test updated
  • Migration script with unit test implemented
  • Upgrade test passed, meaning the nonces are used with the previous indexing in test, then used with the new indexing

Summary by CodeRabbit

  • New Features

    • Introduced an observer module that enhances the application’s capabilities for monitoring events.
    • The API endpoints for querying chain nonces have been updated to use chain_id instead of index.
    • Added migration functionality to transition data from version 7 to version 8, focusing on chain ID indexing.
  • Bug Fixes

    • Corrected handling of chain identifiers in various functions to ensure consistency and reliability.
  • Documentation

    • Updated command descriptions and usage examples for querying chain nonces to reflect the new parameter naming conventions.
  • Chores

    • Cleaned up deprecated references to index across the codebase to streamline functionality.

@lumtis lumtis added the UPGRADE_TESTS Run make start-upgrade-tests label Jul 19, 2024
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 19, 2024

Caution

Review failed

The head commit changed during the review from eefb41a to 93cc623.

Walkthrough

The changes focus on transitioning from using chain names to chain IDs as the primary identifier for chain nonces within the crosschain module. This ensures more reliable and consistent identification of chains, enhancing the integrity of the codebase. The modifications span various components, including handlers, tests, and documentation, reflecting a holistic approach to this key update.

Changes

Files Summary
app/setup_handlers.go, x/crosschain/types/expected_keepers.go Added import for observertypes and updated module integration with chainID.
docs/cli/..., docs/openapi/... Updated command and API documentation to reflect the change from index to chain-id.
proto/zetachain/... Modified proto definitions to replace index with chain_id in relevant messages.
testutil/keeper/..., testutil/sample/... Adjusted tests to utilize chainID instead of index, aligning with new identifier.
x/crosschain/keeper/..., x/observer/keeper/... Changed methods to accept chainID instead of index, enhancing type consistency.
x/observer/module.go, x/observer/migrations/... Added migration support for changes in nonce indexing and updated consensus version.

Assessment against linked issues

Objective Addressed Explanation
Change crosschain.ChainNonce to index by chain ID instead of chain name (#1490)
Ensure all related components reflect this identifier shift
Maintain backward compatibility in data handling It's unclear if legacy support is retained.

🐰 In the meadow, where the bunnies hop,
Chain IDs now lead, no more names to swap.
With each nonce stored, in a clearer way,
Our code grows strong, come what may!
Hopping for changes, with joy we sing,
A brighter tomorrow, our project takes wing! 🌸✨


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?

Share
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>.
    • 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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @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 as 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 resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

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.

@codecov
Copy link

codecov bot commented Jul 19, 2024

Codecov Report

Attention: Patch coverage is 96.15385% with 1 line in your changes missing coverage. Please review.

Project coverage is 46.65%. Comparing base (59515eb) to head (93cc623).
Report is 10 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2515      +/-   ##
===========================================
- Coverage    46.88%   46.65%   -0.23%     
===========================================
  Files          445      460      +15     
  Lines        29636    30596     +960     
===========================================
+ Hits         13894    14275     +381     
- Misses       14936    15474     +538     
- Partials       806      847      +41     
Files Coverage Δ
x/authority/migrations/v2/migrate.go 100.00% <ø> (ø)
x/crosschain/keeper/cctx_utils.go 100.00% <100.00%> (ø)
x/crosschain/migrations/v5/migrate.go 83.07% <100.00%> (ø)
x/observer/keeper/chain_nonces.go 100.00% <100.00%> (ø)
x/observer/keeper/grpc_query_nonces.go 87.50% <100.00%> (ø)
x/observer/keeper/msg_server_reset_chain_nonces.go 100.00% <100.00%> (ø)
x/observer/keeper/pending_nonces.go 92.85% <ø> (-0.13%) ⬇️
x/observer/migrations/v8/migrate.go 100.00% <100.00%> (ø)
x/observer/types/genesis.go 76.66% <100.00%> (ø)
zetaclient/zetacore/client_query_observer.go 68.31% <100.00%> (ø)
... and 1 more

... and 42 files with indirect coverage changes

@lumtis lumtis marked this pull request as ready for review July 19, 2024 16:29
Copy link
Contributor

@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: 3

Outside diff range, codebase verification and nitpick comments (6)
docs/openapi/openapi.swagger.yaml (1)

Parameter name change inconsistency:

The parameter name change from index to chain_id has not been applied consistently across the API documentation. The following instances of index still exist and should be reviewed and updated accordingly:

  • docs/openapi/openapi.swagger.yaml (multiple occurrences)

Please ensure that all instances of the parameter index are replaced with chain_id to maintain consistency.

Analysis chain

Line range hint 14-14:
Parameter name change: Ensure consistency.

The parameter name has been changed from index to chain_id. Verify that this change is consistent across the entire API documentation.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the parameter name change from `index` to `chain_id` is consistent across the API documentation.

# Test: Search for the old parameter name `index`. Expect: No occurrences of `index` as a parameter name.
rg --type yaml -A 5 $'name: index'

Length of output: 1652


Script:

#!/bin/bash
# Description: Verify the usage of the new parameter name `chain_id` in the API documentation.

# Test: Search for the new parameter name `chain_id`. Expect: Occurrences of `chain_id` if the change was partially implemented.
rg --type yaml -A 5 $'name: chain_id'

Length of output: 5444

typescript/zetachain/zetacore/observer/chain_nonces_pb.d.ts (1)

19-20: Ensure complete deprecation of index and replacement by chain_id.

The search results show multiple instances of index across various TypeScript files, some of which are not accompanied by deprecation comments. This suggests that the deprecation might not be fully implemented.

  • typescript/zetachain/zetacore/observer/chain_nonces_pb.d.ts:22
  • typescript/zetachain/zetacore/observer/tss_funds_migrator_pb.d.ts:22
  • typescript/zetachain/zetacore/lightclient/query_pb.d.ts:22
  • typescript/zetachain/zetacore/observer/blame_pb.d.ts:22
  • typescript/zetachain/zetacore/observer/ballot_pb.d.ts:22
  • typescript/zetachain/zetacore/pkg/proofs/bitcoin/bitcoin_pb.d.ts:22
  • typescript/zetachain/zetacore/crosschain/outbound_tracker_pb.d.ts:22
  • typescript/zetachain/zetacore/crosschain/tx_pb.d.ts:22
  • typescript/zetachain/zetacore/crosschain/query_pb.d.ts:22
  • typescript/zetachain/zetacore/fungible/query_pb.d.ts:22
  • typescript/zetachain/zetacore/fungible/foreign_coins_pb.d.ts:22
  • typescript/zetachain/zetacore/crosschain/gas_price_pb.d.ts:22
  • typescript/zetachain/zetacore/observer/query_pb.d.ts:22
  • typescript/zetachain/zetacore/crosschain/cross_chain_tx_pb.d.ts:22
  • typescript/zetachain/zetacore/crosschain/events_pb.d.ts:22
  • typescript/zetachain/zetacore/crosschain/last_block_height_pb.d.ts:22
  • typescript/zetachain/zetacore/crosschain/inbound_hash_to_cctx_pb.d.ts:22

Please ensure that all instances of index are properly deprecated and replaced by chain_id where applicable.

Analysis chain

LGTM! But verify the deprecation of index.

The deprecation comment indicates that the index field has been replaced by chain_id for unique identification purposes. This aligns with the objective of using chainID for indexing.

However, ensure that all instances of index are properly deprecated and replaced by chain_id.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances of `index` are properly deprecated and replaced by `chain_id`.

# Test: Search for the usage of `index`. Expect: Only occurrences with deprecation comments.
rg --type ts -A 5 $'index'

Length of output: 19510

x/observer/migrations/v8/migrate_test.go (1)

15-66: Consider renaming variables for clarity.

The variable chainNoncesArray could be renamed to legacyChainNoncesArray to make it clear that these are the legacy chain nonces being migrated.

- var chainNoncesArray = []types.ChainNonces{
+ var legacyChainNoncesArray = []types.ChainNonces{
x/observer/client/querytests/chain_nonces.go (1)

Line range hint 47-92:
Ensure comprehensive test coverage for edge cases.

The test covers the basic pagination scenarios well. However, consider adding edge cases such as:

  • Listing chain nonces with an empty store.
  • Listing chain nonces with a very large limit.
  • Handling potential errors during listing.

Add additional test cases to cover these scenarios.

x/observer/keeper/msg_server_reset_chain_nonces_test.go (1)

Line range hint 74-118:
Ensure comprehensive test coverage for edge cases.

The test covers the basic scenarios well. However, consider adding edge cases such as:

  • Resetting chain nonces with a very large chain ID.
  • Resetting chain nonces with a negative chain ID.
  • Handling potential errors during resetting.

Add additional test cases to cover these scenarios.

x/crosschain/keeper/evm_hooks_test.go (1)

Inconsistent Removal of Index Field in ChainNonces Struct

The Index field is still present in the following files:

  • x/observer/types/chain_nonces.pb.go
  • x/observer/migrations/v8/migrate.go

Please ensure that the Index field is removed from these files to maintain consistency across the codebase.

Analysis chain

Line range hint 317-319:
LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to SetChainNonces match the new struct definition.


Let's verify if the Index field has been removed from the ChainNonces struct across the codebase.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `SetChainNonces` match the new struct definition.

# Test: Search for the function usage. Expect: Only occurrences of the new struct definition.
rg --type go -A 5 $'SetChainNonces'

Length of output: 14531


Script:

#!/bin/bash
# Description: Verify if the `Index` field has been removed from the `ChainNonces` struct across the codebase.

# Test: Search for the `Index` field in the `ChainNonces` struct.
rg --type go 'ChainNonces.*Index'

Length of output: 666

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 8019ea2 and b1d4bbf.

Files ignored due to path filters (3)
  • x/observer/types/chain_nonces.pb.go is excluded by !**/*.pb.go, !**/*.pb.go
  • x/observer/types/query.pb.go is excluded by !**/*.pb.go, !**/*.pb.go
  • x/observer/types/query.pb.gw.go is excluded by !**/*.pb.gw.go, !**/*.pb.gw.go
Files selected for processing (42)
  • app/setup_handlers.go (2 hunks)
  • changelog.md (1 hunks)
  • docs/cli/zetacored/zetacored_query_observer_show-chain-nonces.md (1 hunks)
  • docs/openapi/openapi.swagger.yaml (3 hunks)
  • proto/zetachain/zetacore/observer/chain_nonces.proto (1 hunks)
  • proto/zetachain/zetacore/observer/query.proto (2 hunks)
  • testutil/keeper/crosschain.go (1 hunks)
  • testutil/keeper/mocks/crosschain/observer.go (1 hunks)
  • testutil/network/genesis_state.go (2 hunks)
  • testutil/sample/observer.go (1 hunks)
  • typescript/zetachain/zetacore/observer/chain_nonces_pb.d.ts (1 hunks)
  • typescript/zetachain/zetacore/observer/query_pb.d.ts (1 hunks)
  • x/authority/migrations/v2/migrate.go (1 hunks)
  • x/crosschain/keeper/cctx_orchestrator_validate_outbound_test.go (1 hunks)
  • x/crosschain/keeper/cctx_utils.go (1 hunks)
  • x/crosschain/keeper/evm_hooks_test.go (3 hunks)
  • x/crosschain/keeper/initiate_outbound_test.go (3 hunks)
  • x/crosschain/keeper/msg_server_migrate_tss_funds_test.go (1 hunks)
  • x/crosschain/keeper/msg_server_vote_outbound_tx_test.go (1 hunks)
  • x/crosschain/migrations/v4/migrate_test.go (2 hunks)
  • x/crosschain/migrations/v5/migrate.go (1 hunks)
  • x/crosschain/migrations/v5/migrate_test.go (6 hunks)
  • x/crosschain/types/expected_keepers.go (1 hunks)
  • x/observer/client/cli/query_chain_nonce.go (2 hunks)
  • x/observer/client/querytests/chain_nonces.go (1 hunks)
  • x/observer/genesis_test.go (1 hunks)
  • x/observer/keeper/chain_nonces.go (3 hunks)
  • x/observer/keeper/chain_nonces_test.go (1 hunks)
  • x/observer/keeper/grpc_query_nonces.go (1 hunks)
  • x/observer/keeper/grpc_query_nonces_test.go (3 hunks)
  • x/observer/keeper/migrator.go (2 hunks)
  • x/observer/keeper/msg_server_reset_chain_nonces.go (3 hunks)
  • x/observer/keeper/msg_server_reset_chain_nonces_test.go (4 hunks)
  • x/observer/keeper/nonces_test.go (1 hunks)
  • x/observer/keeper/pending_nonces.go (1 hunks)
  • x/observer/migrations/v8/migrate.go (1 hunks)
  • x/observer/migrations/v8/migrate_test.go (1 hunks)
  • x/observer/module.go (2 hunks)
  • x/observer/types/genesis.go (1 hunks)
  • x/observer/types/genesis_test.go (1 hunks)
  • zetaclient/zetacore/client_query_observer.go (1 hunks)
  • zetaclient/zetacore/client_query_test.go (1 hunks)
Files skipped from review due to trivial changes (2)
  • changelog.md
  • x/authority/migrations/v2/migrate.go
Additional context used
Path-based instructions (36)
proto/zetachain/zetacore/observer/chain_nonces.proto (1)

Pattern **/*.proto: Review the Protobuf definitions, point out issues relative to compatibility, and expressiveness.

x/observer/keeper/nonces_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/observer/keeper/migrator.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/observer/keeper/msg_server_reset_chain_nonces.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/observer/keeper/chain_nonces_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/observer/client/cli/query_chain_nonce.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/observer/types/genesis_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/observer/types/genesis.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/observer/migrations/v8/migrate.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/observer/keeper/chain_nonces.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/observer/keeper/grpc_query_nonces.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/observer/keeper/pending_nonces.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/observer/migrations/v8/migrate_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/observer/client/querytests/chain_nonces.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/observer/keeper/msg_server_reset_chain_nonces_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/observer/genesis_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_utils.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/crosschain/migrations/v5/migrate.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/observer/keeper/grpc_query_nonces_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/observer/module.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

app/setup_handlers.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/zetacore/client_query_observer.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/crosschain/migrations/v4/migrate_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

testutil/sample/observer.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

testutil/network/genesis_state.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/crosschain/types/expected_keepers.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/crosschain/migrations/v5/migrate_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

proto/zetachain/zetacore/observer/query.proto (1)

Pattern **/*.proto: Review the Protobuf definitions, point out issues relative to compatibility, and expressiveness.

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/msg_server_migrate_tss_funds_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/crosschain/keeper/initiate_outbound_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_orchestrator_validate_outbound_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.

testutil/keeper/mocks/crosschain/observer.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/zetacore/client_query_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

x/crosschain/keeper/evm_hooks_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

GitHub Check: codecov/patch
app/setup_handlers.go

[warning] 72-72: app/setup_handlers.go#L72
Added line #L72 was not covered by tests

Additional comments not posted (62)
docs/openapi/openapi.swagger.yaml (1)

Line range hint 15-15:
Deprecation note: Ensure clarity.

The deprecation note indicates that index has been replaced by chain_id. Ensure that this note is clear and visible to users.

proto/zetachain/zetacore/observer/chain_nonces.proto (2)

11-11: Deprecation comment: Ensure clarity.

The deprecation comment indicates that index has been replaced by chain_id. Ensure that this comment is clear and visible to developers.


13-13: New field definition: Verify correctness.

The chain_id field is introduced as an int64. Ensure that this definition is correct and consistent with the intended usage.

x/observer/keeper/nonces_test.go (3)

19-19: Test function update: Verify correctness.

The GetChainNonces function now uses chain_id instead of index. Ensure that this change is correctly reflected and that the test still covers the necessary scenarios.


31-32: Test function update: Verify correctness.

The RemoveChainNonces function now uses chain_id instead of index. Ensure that this change is correctly reflected and that the test still covers the necessary scenarios.


39-39: Test function update: Verify correctness.

The GetAllChainNonces function now uses chain_id instead of index. Ensure that this change is correctly reflected and that the test still covers the necessary scenarios.

docs/cli/zetacored/zetacored_query_observer_show-chain-nonces.md (1)

6-6: LGTM!

The change from [index] to [chain-id] improves clarity and aligns the documentation with the updated functionality.

x/observer/keeper/migrator.go (2)

5-6: LGTM!

The import of v8 is necessary for the new migration function Migrate7to8.


48-51: LGTM!

The new migration function Migrate7to8 is correctly implemented and follows the pattern of the existing migration functions.

x/observer/keeper/msg_server_reset_chain_nonces.go (2)

44-44: LGTM!

The change to directly use msg.ChainId in the PendingNonces structure is consistent with the overall simplification of the function.


31-31: LGTM! But verify the function's behavior with invalid chain IDs.

The change simplifies the function and improves efficiency by directly using msg.ChainId. However, ensure that the function handles invalid chain IDs correctly.

x/observer/keeper/chain_nonces_test.go (3)

15-20: Ensure ChainId is correctly used and validated.

The change from n.Index to n.ChainId aligns with the objective of using chain IDs for indexing. Ensure that ChainId is correctly set in the sample.ChainNoncesList function.


27-31: Ensure ChainId is correctly used and validated for non-existent entries.

The change to use a hardcoded value of 1000 for testing non-existent entries is appropriate. Ensure that 1000 is a valid test case for a non-existent ChainId.


36-37: Ensure ChainId is correctly used and validated for retrieving all chain nonces.

The change ensures that all chain nonces are retrieved and validated using ChainId. Ensure that the sample.ChainNoncesList function correctly sets the ChainId.

x/observer/client/cli/query_chain_nonce.go (2)

49-49: Update command usage to chain-id.

The command usage update from [index] to [chain-id] enhances clarity regarding the expected input.


57-63: Parse input as integer for chain-id.

The input parsing change ensures that the chain-id is treated as an integer, improving type safety and aligning with the expected input format.

x/observer/types/genesis_test.go (1)

22-22: Initialize chainNonce with an integer argument.

The change to use an integer argument for chainNonce initialization enhances type safety and aligns with the expected data type.

x/observer/types/genesis.go (1)

41-47: LGTM! But verify the usage of ChainId for indexing.

The change from elem.Index to elem.ChainId ensures unique identification of ChainNonces entries by ChainId. This aligns with the objective of using chainID for indexing.

However, ensure that all instances of ChainNonces are updated accordingly to use ChainId.

Verification successful

Verification successful!

The change from elem.Index to elem.ChainId for indexing ChainNonces has been consistently applied across the codebase. All instances of ChainNonces now use ChainId for indexing, ensuring unique identification by ChainId.

  • Verified in various files including test files, migration scripts, keeper methods, and protobuf definitions.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances of `ChainNonces` are updated to use `ChainId`.

# Test: Search for the usage of `ChainNonces`. Expect: Only occurrences of `ChainId`.
rg --type go -A 5 $'ChainNonces'

Length of output: 167561

x/observer/migrations/v8/migrate.go (2)

24-37: LGTM! But verify the implementation of removeChainNoncesLegacy.

The updateChainNonceIndexing function iterates over all ChainNonces objects and updates the indexing to use chainID. This ensures that all ChainNonces objects are updated correctly.

However, ensure that the implementation of removeChainNoncesLegacy is correct.

Verification successful

LGTM!

The updateChainNonceIndexing function iterates over all ChainNonces objects and updates the indexing to use chainID. This ensures that all ChainNonces objects are updated correctly.

The implementation of removeChainNoncesLegacy correctly removes a chainNonces entry from the store using its index.

  • removeChainNoncesLegacy is correctly implemented in x/observer/migrations/v8/migrate.go:
    func removeChainNoncesLegacy(ctx sdk.Context, observerKeeper obsserverKeeper, index string) {
        store := prefix.NewStore(ctx.KVStore(observerKeeper.StoreKey()), types.KeyPrefix(types.ChainNoncesKey))
        store.Delete(types.KeyPrefix(index))
    }
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation of `removeChainNoncesLegacy`.

# Test: Search for the implementation of `removeChainNoncesLegacy`. Expect: Correct implementation.
rg --type go -A 5 $'removeChainNoncesLegacy'

Length of output: 752


19-21: LGTM! But verify the implementation of updateChainNonceIndexing.

The MigrateStore function calls updateChainNonceIndexing to update the indexing for ChainNonces to use chainID. This ensures that the migration is performed correctly.

However, ensure that the implementation of updateChainNonceIndexing is correct.

Verification successful

LGTM! But verify the implementation of updateChainNonceIndexing.

The MigrateStore function calls updateChainNonceIndexing to update the indexing for ChainNonces to use chainID. This ensures that the migration is performed correctly.

The implementation of updateChainNonceIndexing is correct and logically sound. It iterates over all chain nonces, updates their indexing, and removes the old objects.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation of `updateChainNonceIndexing`.

# Test: Search for the implementation of `updateChainNonceIndexing`. Expect: Correct implementation.
rg --type go -A 5 $'updateChainNonceIndexing'

Length of output: 789


Script:

#!/bin/bash
# Description: Retrieve the complete implementation of `updateChainNonceIndexing`.

# Test: Search for the complete implementation of `updateChainNonceIndexing`. Expect: Entire function code.
rg --type go -A 20 $'func updateChainNonceIndexing('

Length of output: 164


Script:

#!/bin/bash
# Description: Retrieve the complete implementation of `updateChainNonceIndexing`.

# Test: Search for the complete implementation of `updateChainNonceIndexing`. Expect: Entire function code.
rg --type go -A 50 'func updateChainNonceIndexing'

Length of output: 1566

x/observer/keeper/chain_nonces.go (3)

19-19: LGTM!

The usage of strconv.FormatInt to convert chainID to a string ensures type safety and consistency.


Line range hint 23-30:
LGTM!

The conversion of chainID to a string and the retrieval logic are correct and ensure type safety.


36-38: LGTM!

The conversion of chainID to a string and the deletion logic are correct and ensure type safety.

x/observer/keeper/grpc_query_nonces.go (1)

58-61: LGTM!

The change to use req.ChainId instead of req.Index aligns with the transition to using chain IDs and ensures consistency.

x/observer/keeper/pending_nonces.go (1)

Line range hint 47-54:
LGTM!

The removal of the Index assignment simplifies the instantiation of ChainNonces and aligns with the transition to using chain IDs.

x/observer/genesis_test.go (1)

38-40: Enhancement: Improved type safety and clarity.

The change from string arguments to integer literals for ChainNonces instantiation enhances type safety and clarity.

x/crosschain/keeper/cctx_utils.go (1)

27-27: Enhancement: Improved reliability of nonce retrieval.

The change from chain.ChainName.String() to receiveChainID enhances the reliability of nonce retrieval by using a direct identifier.

x/crosschain/migrations/v5/migrate.go (1)

50-50: Enhancement: Improved reliability of nonce retrieval.

The change from chainNonce.chain.ChainName.String() to chainNonce.chain.ChainId enhances the reliability of nonce retrieval by using a direct identifier.

x/observer/keeper/grpc_query_nonces_test.go (4)

32-32: Ensure the correctness of the ChainId field.

The change from Index to ChainId in the request structure aligns with the new indexing mechanism. Verify that the ChainId values used in the tests are correct and consistent with the data model.


37-37: Ensure the correctness of the ChainId field.

The change from Index to ChainId in the request structure aligns with the new indexing mechanism. Verify that the ChainId values used in the tests are correct and consistent with the data model.


42-42: Verify the hardcoded ChainId value.

The hardcoded ChainId value of 1000 is used to test the KeyNotFound case. Ensure that this value is appropriate and does not conflict with any valid ChainId values.


65-65: Ensure the correctness of the ChainId field.

The change from Index to ChainId in the nonce generation aligns with the new indexing mechanism. Verify that the ChainId values used in the tests are correct and consistent with the data model.

x/observer/module.go (2)

145-147: Verify the new migration registration.

The new migration from version 7 to version 8 is correctly registered. Ensure that the Migrate7to8 function is properly implemented and tested.


172-172: Verify the updated consensus version.

The updated consensus version of 8 aligns with the new migration. Ensure that this change is consistent across the codebase and does not introduce any issues.

app/setup_handlers.go (2)

30-30: Verify the import statement for observertypes.

The import statement for observertypes is added. Ensure that this import is necessary and correctly used in the file.


72-72: Verify the addition of observertypes.ModuleName.

The addition of observertypes.ModuleName to the list of module names is correct. Ensure that this change is necessary and does not introduce any issues.

Tools
GitHub Check: codecov/patch

[warning] 72-72: app/setup_handlers.go#L72
Added line #L72 was not covered by tests

zetaclient/zetacore/client_query_observer.go (1)

87-87: LGTM! But verify the function usage in the codebase.

The change to use ChainId instead of ChainName aligns with the PR objectives.

However, ensure that all function calls to GetNonceByChain match the new parameter usage.

Verification successful

Function usage verified successfully.

The chain object used in the test file has the ChainId property, ensuring compatibility with the updated function signature in GetNonceByChain.

  • zetaclient/zetacore/client_query_test.go: Verified that chain object has ChainId property.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `GetNonceByChain` match the new parameter usage.

# Test: Search for the function usage. Expect: Only occurrences of the new parameter usage.
rg --type go -A 5 $'GetNonceByChain'

Length of output: 1606


Script:

#!/bin/bash
# Description: Verify that the chain object used in the test file has the ChainId property.

# Test: Search for the definition and structure of the chain object in the test file.
rg --type go -A 10 'chain := ' zetaclient/zetacore/client_query_test.go

Length of output: 1036

x/crosschain/migrations/v4/migrate_test.go (2)

63-63: LGTM! But verify the function behavior.

The change to use ChainId instead of Index aligns with the PR objectives.

However, ensure that the function behavior is correctly tested with the new parameter usage.

Verification successful

LGTM!

The change to use ChainId instead of Index aligns with the PR objectives. The function behavior is correctly tested with the new parameter usage.

  • x/crosschain/migrations/v4/migrate_test.go: The TestMigrateStore function includes comprehensive tests for verifying the migration process, including the handling of chain nonces.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the function behavior with the new parameter usage.

# Test: Run the tests to ensure correct behavior.
go test ./x/crosschain/migrations/v4/...

Length of output: 112


Script:

#!/bin/bash
# Retrieve the content of the TestMigrateStore function for analysis
ast-grep --lang go --pattern 'func TestMigrateStore($_) $_' --json

Length of output: 15985


68-68: LGTM! But verify the function behavior.

The change to use ChainId instead of Index aligns with the PR objectives.

However, ensure that the function behavior is correctly tested with the new parameter usage.

Verification successful

Function behavior verified

The test functions TestMigrateStore and TestSetBitcoinFinalizedInbound in migrate_test.go include scenarios that test the usage of ChainId, ensuring that the function behavior is correctly tested with the new parameter usage.

  • TestMigrateStore: Includes a test case for moving nonces using ChainId.
  • TestSetBitcoinFinalizedInbound: Tests setting finalized inbound transactions for Bitcoin using ChainId.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the function behavior with the new parameter usage.

# Test: Run the tests to ensure correct behavior.
go test ./x/crosschain/migrations/v4/...

Length of output: 112


Script:

#!/bin/bash
# Description: Search for test functions in the migrate_test.go file and review their content.

# Find all test functions in the migrate_test.go file.
ast-grep --lang go --pattern 'func Test$_($_) $_' --json | jq '.[] | .range.start.line, .range.end.line'

Length of output: 106


Script:

#!/bin/bash
# Description: Verify the presence of migrate_test.go and search for test functions.

# List contents of the directory to confirm the presence of migrate_test.go
ls x/crosschain/migrations/v4/

# Search for test functions in migrate_test.go
rg 'func Test' x/crosschain/migrations/v4/migrate_test.go -A 5

Length of output: 682


Script:

#!/bin/bash
# Description: Extract the full content of test functions in migrate_test.go for detailed review.

# Extract the full content of TestMigrateStore function
rg -A 50 'func TestMigrateStore' x/crosschain/migrations/v4/migrate_test.go

# Extract the full content of TestSetBitcoinFinalizedInbound function
rg -A 50 'func TestSetBitcoinFinalizedInbound' x/crosschain/migrations/v4/migrate_test.go

Length of output: 4183

testutil/sample/observer.go (2)

173-177: LGTM! But verify the function behavior.

The change to use chainID instead of index and remove the dependency on the testing context aligns with the PR objectives.

However, ensure that the function behavior is correctly tested with the new parameter usage.

Verification successful

LGTM! But verify the function behavior.

The change to use chainID instead of index and remove the dependency on the testing context aligns with the PR objectives.

  • Implementation found in testutil/sample/observer.go (lines 173-182).
  • Multiple references in test files indicate that the function behavior is covered by tests.

However, ensure that the function behavior is correctly tested with the new parameter usage.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the function behavior with the new parameter usage.

# Test: Run the tests to ensure correct behavior.
go test ./testutil/sample/...

Length of output: 101


Script:

#!/bin/bash
# Search for the implementation of the ChainNonces function
ast-grep --lang go --pattern 'func ChainNonces($_) $_' > chain_nonces_impl.txt

# Search for test cases that reference the ChainNonces function
rg 'ChainNonces' > chain_nonces_tests.txt

Length of output: 73


Script:

#!/bin/bash
# Search for the implementation of the ChainNonces function in Go files
ast-grep --lang go --pattern 'func ChainNonces(chainID int64) $_'

# Search for references to the ChainNonces function in test files
rg 'ChainNonces' --glob '*_test.go'

Length of output: 15419


184-187: LGTM! But verify the function behavior.

The change to use chainID instead of index and remove the dependency on the testing context aligns with the PR objectives.

However, ensure that the function behavior is correctly tested with the new parameter usage.

testutil/network/genesis_state.go (2)

Line range hint 44-55:
LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to SetupZetaGenesisState are updated accordingly.

Verification successful

Verification Successful: No references to the removed Index field found.

The function SetupZetaGenesisState has been correctly updated, and no references to the old Index field were found in x/crosschain/client/querytests/suite.go and x/observer/client/querytests/suite.go.

  • x/crosschain/client/querytests/suite.go
  • x/observer/client/querytests/suite.go
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `SetupZetaGenesisState` match the new structure.

# Test: Search for the function usage. Expect: No references to the old `Index` field.
rg --type go -A 5 $'SetupZetaGenesisState'

Length of output: 1606


Script:

#!/bin/bash
# Description: Verify no references to the old `Index` field in the relevant files.

# Test: Search for the `Index` field in `x/crosschain/client/querytests/suite.go` and `x/observer/client/querytests/suite.go`.
rg --type go 'Index' x/crosschain/client/querytests/suite.go
rg --type go 'Index' x/observer/client/querytests/suite.go

Length of output: 119


181-181: LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to AddObserverData are updated accordingly.

Verification successful

Verification successful: No issues found with the AddObserverData function usage.

The Index field is not present in the identified files, and the usage of AddObserverData appears consistent with the intended update to use ChainId.

  • x/observer/client/querytests/suite.go
  • x/crosschain/client/querytests/suite.go
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `AddObserverData` match the new structure.

# Test: Search for the function usage. Expect: No references to the old `Index` field.
rg --type go -A 5 $'AddObserverData'

Length of output: 1350


Script:

#!/bin/bash
# Description: Verify the absence of the `Index` field and the correct usage of the `ChainId` field in the identified files.

# Check for the presence of the `Index` field in the identified files.
rg --type go 'Index' x/observer/client/querytests/suite.go x/crosschain/client/querytests/suite.go

# Check for the usage of the `ChainId` field in the identified files.
rg --type go 'ChainId' x/observer/client/querytests/suite.go x/crosschain/client/querytests/suite.go

Length of output: 199


Script:

#!/bin/bash
# Description: Extract and review the context around the `AddObserverData` function calls in the identified files.

# Extract 10 lines before and after the `AddObserverData` function calls in the identified files.
rg --type go -C 10 'AddObserverData' x/observer/client/querytests/suite.go x/crosschain/client/querytests/suite.go

Length of output: 3473

x/crosschain/types/expected_keepers.go (1)

78-78: LGTM! But verify the method usage in the codebase.

The code changes are approved.

However, ensure that all calls to GetChainNonces are updated to use chainID instead of index.

Verification successful

Verification Complete: All calls to GetChainNonces use chainID instead of index.

  • The search results indicate that all instances of GetChainNonces are correctly using the chainID parameter.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all calls to `GetChainNonces` use `chainID` instead of `index`.

# Test: Search for the method usage. Expect: No references to the old `index` parameter.
rg --type go -A 5 $'GetChainNonces'

Length of output: 38646

x/crosschain/migrations/v5/migrate_test.go (4)

177-177: LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all calls to GetChainNonces and SetChainNonces are updated to use chainID instead of index.

Verification successful

Verification successful: All calls to GetChainNonces and SetChainNonces use chainID instead of index.

  • No references to the old index parameter were found.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all calls to `GetChainNonces` and `SetChainNonces` use `chainID` instead of `index`.

# Test: Search for the function usage. Expect: No references to the old `index` parameter.
rg --type go -A 5 $'GetChainNonces'
rg --type go -A 5 $'SetChainNonces'

Length of output: 53178


124-124: LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all calls to GetChainNonces and SetChainNonces are updated to use chainID instead of index.

Verification successful

Verification successful: All calls to GetChainNonces and SetChainNonces are using chainID instead of index.

  • Verified that all instances of GetChainNonces and SetChainNonces in the codebase use chainID as the parameter.
  • No references to the old index parameter were found.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all calls to `GetChainNonces` and `SetChainNonces` use `chainID` instead of `index`.

# Test: Search for the function usage. Expect: No references to the old `index` parameter.
rg --type go -A 5 $'GetChainNonces'
rg --type go -A 5 $'SetChainNonces'

Length of output: 53178


133-133: LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all calls to GetChainNonces and SetChainNonces are updated to use chainID instead of index.

Verification successful

Verification successful: All instances of GetChainNonces and SetChainNonces use chainID instead of index.

  • All references to GetChainNonces and SetChainNonces in the codebase are using chainID as verified by the search results.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all calls to `GetChainNonces` and `SetChainNonces` use `chainID` instead of `index`.

# Test: Search for the function usage. Expect: No references to the old `index` parameter.
rg --type go -A 5 $'GetChainNonces'
rg --type go -A 5 $'SetChainNonces'

Length of output: 53178


170-170: LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all calls to GetChainNonces and SetChainNonces are updated to use chainID instead of index.

Verification successful

Verification successful: All instances of GetChainNonces and SetChainNonces use chainID instead of index.

  • The search results confirm that the codebase has been updated to use chainID for both GetChainNonces and SetChainNonces.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all calls to `GetChainNonces` and `SetChainNonces` use `chainID` instead of `index`.

# Test: Search for the function usage. Expect: No references to the old `index` parameter.
rg --type go -A 5 $'GetChainNonces'
rg --type go -A 5 $'SetChainNonces'

Length of output: 53178

proto/zetachain/zetacore/observer/query.proto (2)

146-147: LGTM! Ensure endpoint consistency.

The update to use chain_id instead of index in the HTTP GET option enhances clarity and specificity. Verify that the new endpoint format is consistent with other endpoints.


181-181: LGTM! Verify data type consistency.

The update to use int64 chain_id instead of string index aligns with the shift to using chain_id for indexing. Ensure that int64 is the appropriate data type and consistent with other similar fields.

testutil/keeper/crosschain.go (1)

334-334: LGTM! Verify identifier consistency.

The update to use senderChain.ChainId instead of senderChain.ChainName.String() aligns with the shift to using chain_id for indexing. Ensure that chain_id is consistently used across similar changes.

x/crosschain/keeper/msg_server_migrate_tss_funds_test.go (1)

Line range hint 98-98:
LGTM! Verify impact of Index removal.

The removal of the Index field simplifies the data structure and aligns with the shift to using chain_id for identification. Ensure that this removal does not affect any operations or conditions that rely on the Index.

x/crosschain/keeper/initiate_outbound_test.go (3)

391-391: LGTM! Verify GetChainNonces handles chain IDs correctly.

The change to use receiverChain.ChainId instead of receiverChain.ChainName.String() aligns with the PR objective. Ensure that the GetChainNonces method correctly handles chain IDs.

Verification successful

LGTM! Verify GetChainNonces handles chain IDs correctly.

The change to use receiverChain.ChainId instead of receiverChain.ChainName.String() aligns with the PR objective. The GetChainNonces method correctly handles chain IDs as confirmed by its usage across the codebase.

  • x/crosschain/types/expected_keepers.go: GetChainNonces(ctx sdk.Context, chainID int64) (val observertypes.ChainNonces, found bool)
  • x/observer/keeper/chain_nonces.go: func (k Keeper) GetChainNonces(ctx sdk.Context, chainID int64) (val types.ChainNonces, found bool)
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the `GetChainNonces` method handles chain IDs correctly.

# Test: Search for the `GetChainNonces` method implementation. Expect: It handles chain IDs.
rg --type go -A 10 'func GetChainNonces'

Length of output: 42


Script:

#!/bin/bash
# Description: Broaden the search to include interface definitions and mock implementations for `GetChainNonces`.

# Search for interface definitions that include `GetChainNonces`.
rg --type go -A 5 'GetChainNonces()'

# Search for mock implementations that include `GetChainNonces`.
rg --type go -A 5 'On("GetChainNonces"'

# Trace method calls to `GetChainNonces` to confirm it handles chain IDs.
rg --type go -A 5 'GetChainNonces('

Length of output: 38884


450-450: LGTM! Verify GetChainNonces handles chain IDs correctly.

The change to use receiverChain.ChainId instead of receiverChain.ChainName.String() aligns with the PR objective. Ensure that the GetChainNonces method correctly handles chain IDs.

Verification successful

LGTM! Verify GetChainNonces handles chain IDs correctly.

The change to use receiverChain.ChainId instead of receiverChain.ChainName.String() aligns with the PR objective. The GetChainNonces method correctly handles chainId as evidenced by its consistent usage across the codebase.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the `GetChainNonces` method handles chain IDs correctly.

# Test: Search for the `GetChainNonces` method implementation. Expect: It handles chain IDs.
rg --type go -A 10 'func GetChainNonces'

Length of output: 42


Script:

#!/bin/bash
# Description: Verify that the `GetChainNonces` method handles chain IDs correctly.

# Test: Search for the `GetChainNonces` method implementation or interface definition. Expect: It handles chain IDs.
rg --type go -A 10 'GetChainNonces'

Length of output: 60266


277-277: LGTM! Verify GetChainNonces handles chain IDs correctly.

The change to use senderChain.ChainId instead of senderChain.ChainName.String() aligns with the PR objective. Ensure that the GetChainNonces method correctly handles chain IDs.

Verification successful

LGTM! The GetChainNonces method correctly handles chain IDs.

The change to use senderChain.ChainId instead of senderChain.ChainName.String() aligns with the PR objective. The GetChainNonces method is confirmed to handle chainID appropriately.

  • x/observer/keeper/chain_nonces.go: func (k Keeper) GetChainNonces(ctx sdk.Context, chainID int64) (val types.ChainNonces, found bool)
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the `GetChainNonces` method handles chain IDs correctly.

# Test: Search for the `GetChainNonces` method implementation. Expect: It handles chain IDs.
rg --type go -A 10 'func GetChainNonces'

Length of output: 42


Script:

#!/bin/bash
# Description: Broadly search for `GetChainNonces` across the codebase, including interfaces and mock definitions.

# Search for `GetChainNonces` in Go files to find its definition or usage.
rg --type go 'GetChainNonces'

# Search for `GetChainNonces` in mock files to find its mock setup.
rg --type go 'GetChainNonces' --glob '*mock*'

Length of output: 11879

x/crosschain/keeper/cctx_orchestrator_validate_outbound_test.go (1)

334-334: LGTM! Verify GetChainNonces handles chain IDs correctly.

The change to use senderChain.ChainId instead of senderChain.ChainName.String() aligns with the PR objective. Ensure that the GetChainNonces method correctly handles chain IDs.

Verification successful

LGTM! Verify GetChainNonces handles chain IDs correctly.

The change to use senderChain.ChainId instead of senderChain.ChainName.String() aligns with the PR objective. The GetChainNonces method correctly handles chain IDs as confirmed by its implementation and usage across the codebase.

  • x/observer/keeper/chain_nonces.go: The method GetChainNonces accepts chainID as an argument and processes it correctly.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the `GetChainNonces` method handles chain IDs correctly.

# Test: Search for the `GetChainNonces` method implementation. Expect: It handles chain IDs.
rg --type go -A 10 'func GetChainNonces'

Length of output: 42


Script:

#!/bin/bash
# Description: Verify that the `GetChainNonces` method handles chain IDs correctly.

# Test: Search for the `GetChainNonces` method implementation across the codebase.
# We will search for the method definition and its usage to ensure it handles chain IDs.
rg 'GetChainNonces' --type go -A 10

Length of output: 60266

x/crosschain/keeper/msg_server_vote_outbound_tx_test.go (1)

265-265: LGTM! Verify GetChainNonces handles chain IDs correctly.

The change to use senderChain.ChainId instead of senderChain.ChainName.String() aligns with the PR objective. Ensure that the GetChainNonces method correctly handles chain IDs.

Verification successful

LGTM! The GetChainNonces method handles chain IDs correctly.

The change to use senderChain.ChainId instead of senderChain.ChainName.String() aligns with the PR objective and is correctly handled by the GetChainNonces method.

  • x/observer/keeper/chain_nonces.go: Implementation of GetChainNonces confirms it handles chain IDs.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the `GetChainNonces` method handles chain IDs correctly.

# Test: Search for the `GetChainNonces` method implementation. Expect: It handles chain IDs.
rg --type go -A 10 'func GetChainNonces'

Length of output: 42


Script:

#!/bin/bash
# Description: Search for similar method names and references to `GetChainNonces` in the codebase.

# Search for methods with similar names to `GetChainNonces`
rg --type go -A 10 'func.*ChainNonce'

# Search for references to `GetChainNonces` in the codebase
rg --type go 'GetChainNonces'

Length of output: 93064

testutil/keeper/mocks/crosschain/observer.go (1)

305-325: LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to GetChainNonces match the new signature.

Verification successful

LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to GetChainNonces match the new signature.

The search results show that all instances of GetChainNonces are using the new signature with chainID int64. Here are some examples:

  • x/observer/migrations/v8/migrate_test.go: _, found := k.GetChainNonces(ctx, chainNonces.ChainId)
  • x/crosschain/types/expected_keepers.go: GetChainNonces(ctx sdk.Context, chainID int64) (val observertypes.ChainNonces, found bool)
  • x/crosschain/migrations/v5/migrate.go: cn, found := observerKeeper.GetChainNonces(ctx, chainNonce.chain.ChainId)

All the function calls to GetChainNonces have been updated to use the new signature with chainID int64.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `GetChainNonces` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type go -A 5 $'GetChainNonces'

Length of output: 38646

zetaclient/zetacore/client_query_test.go (1)

577-577: LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to GetNonceByChain match the new signature.

Verification successful

Verified: All function calls to GetNonceByChain match the new signature.

The code changes are confirmed to be consistent across the codebase.

  • zetaclient/zetacore/client_query_test.go
  • zetaclient/zetacore/client_query_observer.go
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `GetNonceByChain` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type go -A 5 $'GetNonceByChain'

Length of output: 1606

x/crosschain/keeper/evm_hooks_test.go (2)

Line range hint 596-598:
LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to SetChainNonces match the new struct definition.

Verification successful

All instances of SetChainNonces are consistent with the new struct definition.

The search results confirm that the SetChainNonces function is used correctly with the new struct definition across the codebase. There are no references to the removed Index field, and the necessary fields ChainId and Nonce are appropriately used.

  • x/crosschain/types/expected_keepers.go
  • x/crosschain/keeper/msg_server_migrate_tss_funds_test.go
  • x/crosschain/keeper/initiate_outbound_test.go
  • x/crosschain/keeper/evm_hooks_test.go
  • x/crosschain/keeper/cctx_utils_test.go
  • x/crosschain/keeper/cctx_utils.go
  • x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go
  • x/crosschain/migrations/v4/migrate.go
  • x/crosschain/migrations/v5/migrate_test.go
  • x/crosschain/migrations/v5/migrate.go
  • x/observer/migrations/v8/migrate.go
  • x/observer/genesis.go
  • x/observer/keeper/pending_nonces.go
  • x/observer/keeper/chain_nonces_test.go
  • x/observer/keeper/nonces_test.go
  • x/observer/keeper/msg_server_reset_chain_nonces.go
  • x/observer/keeper/chain_nonces.go
  • x/observer/keeper/grpc_query_nonces_test.go
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `SetChainNonces` match the new struct definition.

# Test: Search for the function usage. Expect: Only occurrences of the new struct definition.
rg --type go -A 5 $'SetChainNonces'

Length of output: 14531


Line range hint 50-55:
LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to SetChainNonces match the new struct definition.

Verification successful

Verification successful: All instances of SetChainNonces match the new struct definition.

The function calls to SetChainNonces have been updated to use the new ChainNonces struct without the Index field. The changes are consistent across the codebase.

  • x/observer/genesis.go
  • x/observer/migrations/v8/migrate.go
  • x/observer/keeper/chain_nonces.go
  • x/observer/keeper/msg_server_reset_chain_nonces.go
  • x/observer/keeper/pending_nonces.go
  • x/observer/keeper/grpc_query_nonces_test.go
  • x/observer/keeper/chain_nonces_test.go
  • x/observer/keeper/nonces_test.go
  • x/crosschain/types/expected_keepers.go
  • x/crosschain/keeper/cctx_utils.go
  • x/crosschain/keeper/evm_hooks_test.go
  • x/crosschain/keeper/cctx_utils_test.go
  • x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go
  • x/crosschain/migrations/v5/migrate_test.go
  • x/crosschain/migrations/v4/migrate.go
  • x/crosschain/keeper/msg_server_migrate_tss_funds_test.go
  • x/crosschain/keeper/initiate_outbound_test.go
  • testutil/keeper/crosschain.go
  • testutil/keeper/mocks/crosschain/observer.go
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `SetChainNonces` match the new struct definition.

# Test: Search for the function usage. Expect: Only occurrences of the new struct definition.
rg --type go -A 5 $'SetChainNonces'

Length of output: 14531

typescript/zetachain/zetacore/observer/query_pb.d.ts (1)

118-120: LGTM! But verify the consistency of the new field name and type.

The change from index to chainId of type bigint improves clarity and functionality. Ensure that the new field name and type are consistent across the codebase.

Verification successful

Consistent usage of chainId of type bigint verified.

The change from index to chainId of type bigint is consistent across the codebase.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the consistency of the new field name and type across the codebase.

# Test: Search for the usage of `chainId` in the codebase. Expect: Consistent usage of `chainId` of type `bigint`.
rg --type-add 'd.ts' --type d.ts 'chainId: bigint'

Length of output: 110


Script:

#!/bin/bash
# Description: Verify the consistency of the new field name and type across the codebase.

# Test: Search for the usage of `chainId` in the codebase. Expect: Consistent usage of `chainId` of type `bigint`.
rg 'chainId: bigint' --glob '*.d.ts'

Length of output: 2820

Copy link
Contributor

@swift1337 swift1337 left a comment

Choose a reason for hiding this comment

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

LGTM

lumtis and others added 4 commits July 22, 2024 13:58
Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com>
Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com>
Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com>
Copy link
Contributor

@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

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b1d4bbf and 85eef7c.

Files ignored due to path filters (1)
  • x/observer/types/chain_nonces.pb.go is excluded by !**/*.pb.go, !**/*.pb.go
Files selected for processing (5)
  • proto/zetachain/zetacore/observer/chain_nonces.proto (1 hunks)
  • typescript/zetachain/zetacore/observer/chain_nonces_pb.d.ts (1 hunks)
  • x/observer/client/cli/query_chain_nonce.go (2 hunks)
  • x/observer/migrations/v8/migrate.go (1 hunks)
  • x/observer/migrations/v8/migrate_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (5)
  • proto/zetachain/zetacore/observer/chain_nonces.proto
  • typescript/zetachain/zetacore/observer/chain_nonces_pb.d.ts
  • x/observer/client/cli/query_chain_nonce.go
  • x/observer/migrations/v8/migrate.go
  • x/observer/migrations/v8/migrate_test.go

lumtis and others added 2 commits July 23, 2024 11:02
Co-authored-by: Tanmay <tanmay@zetachain.com>
Co-authored-by: Tanmay <tanmay@zetachain.com>
Copy link
Contributor

@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

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 85eef7c and 829477c.

Files selected for processing (1)
  • proto/zetachain/zetacore/observer/chain_nonces.proto (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • proto/zetachain/zetacore/observer/chain_nonces.proto

@lumtis lumtis requested a review from kingpinXD July 23, 2024 09:11
@lumtis lumtis added this pull request to the merge queue Jul 23, 2024
Merged via the queue into develop with commit 24ccd85 Jul 23, 2024
@lumtis lumtis deleted the refactor/remove-chain-name-indexing branch July 23, 2024 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

crosschain.ChainNonce: index by chain ID instead of chain name

3 participants