Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(testutil): remove staking from testutil/sims #21059

Closed
wants to merge 15 commits into from

Conversation

julienrbrt
Copy link
Member

@julienrbrt julienrbrt commented Jul 24, 2024

Description

Extracts testutil/sims changes from #20887.
Integration tests should pass.


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title, you can find examples of the prefixes below:
  • confirmed ! in the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • reviewed "Files changed" and left comments if necessary
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • updated the relevant documentation or specification, including comments for documenting Go code
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

Summary by CodeRabbit

  • Bug Fixes
    • Improved clarity in end-to-end testing logs for out-of-gas scenarios.
  • New Features
    • Enhanced staking state handling with new data structures for improved clarity and functionality.
    • Introduced comprehensive staking structures and serialization methods to streamline staking operations.
  • Chores
    • Removed redundant configuration function to streamline codebase.
  • Refactor
    • Reorganized import statements for clarity without changing functionality.

@julienrbrt julienrbrt added the backport/v0.52.x PR scheduled for inclusion in the v0.52's next stable release label Jul 24, 2024
@julienrbrt julienrbrt requested a review from a team as a code owner July 24, 2024 14:05
Copy link
Contributor

coderabbitai bot commented Jul 24, 2024

Walkthrough

Walkthrough

The changes primarily focus on renaming and restructuring test suites to reflect a transition from integration tests to end-to-end (E2E) tests, enhancing clarity in the test suite's purpose. Additionally, significant updates were made to the staking management code, including new data structures and improved state handling through protobuf serialization. These modifications aim to enhance code maintainability and functionality while streamlining configuration management.

Changes

Files Change Summary
tests/e2e/server/grpc/out_of_gas_test.go
tests/e2e/server/grpc/server_test.go
Renamed test suites from IntegrationTest to E2ETest, updated logging messages, and ensured all methods reflect the new naming without altering logic.
testutil/sims/app_helpers.go Refactored GenesisStateWithValSet to use new types for validators and delegations, improving clarity and adding new data structures for managing staking state.
testutil/sims/staking.go Introduced new types and methods for staking management, including serialization functions, enhancing the functionality and organization of the staking module.
testutil/sims/state_helpers.go Updated appStateFnWithExtendedCbs to use protobuf for staking state representation, simplifying the logic for validator status checks and enhancing state handling.
x/genutil/client/testutil/helpers.go Removed the TrackCometConfig function to streamline configuration management, consolidating functionality for better maintainability.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant TestSuite
    participant StakingModule

    User->>TestSuite: Run E2E Tests
    TestSuite->>StakingModule: Initialize Staking State
    StakingModule->>StakingModule: Serialize State to Proto
    StakingModule->>TestSuite: Return Staking State
    TestSuite->>User: Test Results
Loading

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.

@github-actions github-actions bot added C:CLI C:x/genutil genutil module issues labels Jul 24, 2024
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: 4

Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 339e26e and 414ea26.

Files selected for processing (6)
  • tests/e2e/server/grpc/out_of_gas_test.go (3 hunks)
  • tests/e2e/server/grpc/server_test.go (10 hunks)
  • testutil/sims/app_helpers.go (5 hunks)
  • testutil/sims/staking.go (1 hunks)
  • testutil/sims/state_helpers.go (4 hunks)
  • x/genutil/client/testutil/helpers.go (1 hunks)
Files skipped from review due to trivial changes (2)
  • tests/e2e/server/grpc/server_test.go
  • x/genutil/client/testutil/helpers.go
Additional context used
Path-based instructions (4)
tests/e2e/server/grpc/out_of_gas_test.go (3)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.


Pattern tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"


Pattern **/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"

testutil/sims/state_helpers.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

testutil/sims/app_helpers.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

testutil/sims/staking.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

Additional comments not posted (29)
tests/e2e/server/grpc/out_of_gas_test.go (6)

27-33: Renaming is consistent.

The struct E2ETestOutOfGasSuite is correctly renamed from IntegrationTestOutOfGasSuite. The struct fields remain unchanged.


Line range hint 35-64:
Renaming and log message update are consistent.

The method SetupSuite is correctly renamed to E2ETestOutOfGasSuite.SetupSuite. The log message correctly reflects the E2E test suite.


66-70: Renaming and log message update are consistent.

The method TearDownSuite is correctly renamed to E2ETestOutOfGasSuite.TearDownSuite. The log message correctly reflects the E2E test suite.


72-78: Renaming is consistent.

The method TestGRPCServer_TestService is correctly renamed to E2ETestOutOfGasSuite.TestGRPCServer_TestService. The method functionality remains unchanged.


Line range hint 80-94:
Renaming is consistent.

The method TestGRPCServer_BankBalance_OutOfGas is correctly renamed to E2ETestOutOfGasSuite.TestGRPCServer_BankBalance_OutOfGas. The method functionality remains unchanged.


96-98: Renaming is consistent.

The method TestE2ETestOutOfGasSuite is correctly renamed from TestIntegrationTestOutOfGasSuite. The method functionality remains unchanged.

testutil/sims/state_helpers.go (6)

150-153: Verify the status value for unbonded validators.

The status value 1 for unbonded validators should be verified to ensure it is correctly used.


142-145: Ensure ToProto method is correctly implemented.

The ToProto method of the staking state should be verified to ensure it correctly converts the staking state to protobuf.

Verification successful

Ensure ToProto method is correctly implemented.

The ToProto method for StakingGenesisState correctly converts the staking state to protobuf format. It handles the conversion of parameters and other necessary fields, and appropriately manages error cases.

  • File: testutil/sims/staking.go
  • Method: func (s *StakingGenesisState) ToProto() (*anypb.Any, error)
Scripts executed

The following scripts were executed for the analysis:

Script:

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

# Test: Search for the implementation of `ToProto` method. Expect: Correct implementation.
rg --type go --pattern 'ToProto'

Length of output: 444


Script:

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

# Test: Search for the implementation of `ToProto` method. Expect: Correct implementation.
rg 'func (.*) ToProto' --type go -A 10

Length of output: 4801


127-128: Ensure testutil.StakingModuleName is correctly defined.

The usage of testutil.StakingModuleName should be verified to ensure it is correctly defined and used.

Verification successful

The StakingModuleName is correctly defined and used.

  • testutil/types.go: StakingModuleName = "staking"
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the definition of `StakingModuleName` in `testutil`.

# Test: Search for the definition of `StakingModuleName`. Expect: Correct definition.
rg --type go --pattern 'StakingModuleName'

Length of output: 454


Script:

#!/bin/bash
# Description: Verify the definition of `StakingModuleName` in `testutil`.

# Test: Search for the definition of `StakingModuleName`. Expect: Correct definition.
rg 'StakingModuleName' --type go

Length of output: 1072


183-189: Ensure module state callback is correctly implemented.

The module state callback function should be verified to ensure it is correctly implemented and used.

Verification successful

Module state callback is correctly implemented.

The moduleStateCb function is correctly defined and used within the testutil/sims/state_helpers.go file. It is conditionally invoked to handle module states, ensuring safe execution.

  • testutil/sims/state_helpers.go: Lines 183-189, 197-202
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation of module state callback function.

# Test: Search for the implementation of module state callback function. Expect: Correct implementation.
rg --type go --pattern 'moduleStateCb'

Length of output: 450


Script:

#!/bin/bash
# Description: Verify the implementation of module state callback function.

# Test: Search for the implementation of module state callback function. Expect: Correct implementation.
rg 'moduleStateCb'

Length of output: 856


Script:

#!/bin/bash
# Description: Extract context around the implementation and usage of moduleStateCb function.

# Test: Extract context around the lines where moduleStateCb is defined and used.
rg 'moduleStateCb' -A 5 -B 5 testutil/sims/state_helpers.go

Length of output: 1668


132-133: Ensure anypb.Any is correctly used for unmarshaling.

The usage of anypb.Any for unmarshaling the staking state should be verified to ensure it is correctly implemented.

Verification successful

Usage of anypb.Any for unmarshaling is correctly implemented.

The code correctly unmarshals JSON into an anypb.Any object and then converts it to a specific staking genesis state.

  • testutil/sims/state_helpers.go:
    • Lines 132-133: Correct usage of anypb.Any for unmarshaling
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `anypb.Any` for unmarshaling.

# Test: Search for the usage of `anypb.Any`. Expect: Correct usage.
rg --type go --pattern 'anypb.Any'

Length of output: 446


Script:

#!/bin/bash
# Description: Verify the usage of `anypb.Any` for unmarshaling.

# Test: Search for the usage of `anypb.Any`. Expect: Correct usage.
rg 'anypb.Any' --type go

Length of output: 76203


Script:

#!/bin/bash
# Description: Extract lines around the usage of `anypb.Any` in `testutil/sims/state_helpers.go` to verify correct implementation for unmarshaling.

# Extract 10 lines before and after the usage of `anypb.Any` in `testutil/sims/state_helpers.go`
rg 'anypb.Any' -A 10 -B 10 testutil/sims/state_helpers.go

Length of output: 547


137-140: Ensure ProtoToStakingGenesisState function is correctly implemented.

The function ProtoToStakingGenesisState should be verified to ensure it correctly converts the protobuf representation to the staking genesis state.

testutil/sims/app_helpers.go (7)

244-248: Ensure LegacyNewDecFromStr method is correctly used for commission rate.

The method LegacyNewDecFromStr should be verified to ensure it correctly sets the commission rate.

Verification successful

Verified: The LegacyNewDecFromStr method is correctly used for setting the commission rate.

The method is used to convert a string representation of a decimal to a LegacyDec type, and the error handling is properly implemented.

  • testutil/sims/app_helpers.go line 244: commission, err := sdkmath.LegacyNewDecFromStr("0.5")
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `LegacyNewDecFromStr` method for commission rate.

# Test: Search for the usage of `LegacyNewDecFromStr`. Expect: Correct usage.
rg --type go --pattern 'LegacyNewDecFromStr'

Length of output: 456


Script:

#!/bin/bash
# Description: Verify the usage of `LegacyNewDecFromStr` method for commission rate.

# Test: Search for the usage of `LegacyNewDecFromStr`. Expect: Correct usage.
rg 'LegacyNewDecFromStr'

Length of output: 9581


277-282: Ensure StakingDelegation fields are correctly set.

The fields of StakingDelegation should be verified to ensure they are correctly set, including DelegatorAddress, ValidatorAddress, and Shares.

Verification successful

The fields of StakingDelegation are correctly set.

The fields DelegatorAddress, ValidatorAddress, and Shares in the StakingDelegation struct match the fields being set in the provided code snippet.

  • DelegatorAddress is set to genAccs[0].GetAddress().String()
  • ValidatorAddress is set to sdk.ValAddress(val.Address).String()
  • Shares is set to sdkmath.LegacyOneDec()
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the fields of `StakingDelegation` are correctly set.

# Test: Search for the definition of `StakingDelegation` and verify the fields. Expect: Correct fields.
rg --type go --pattern 'type StakingDelegation'

Length of output: 461


Script:

#!/bin/bash
# Description: Verify the fields of `StakingDelegation` are correctly set.

# Test: Search for the definition of `StakingDelegation` and verify the fields. Expect: Correct fields.
rg 'type StakingDelegation' --type go -A 10

Length of output: 1030


226-228: Ensure StakingValidator, LastValidatorPower, and StakingDelegation are correctly defined.

The new types StakingValidator, LastValidatorPower, and StakingDelegation should be verified to ensure they are correctly defined and used.

Verification successful

Ensure StakingValidator, LastValidatorPower, and StakingDelegation are correctly defined.

The new types StakingValidator, LastValidatorPower, and StakingDelegation are defined as follows:

  • StakingValidator is defined with fields for operator address, consensus public key, jailed status, validator status, and tokens.
  • LastValidatorPower is defined with fields for address and power.
  • StakingDelegation is defined with fields for delegator address, validator address, and shares.

These definitions appear to be correctly structured and include the necessary fields for their respective purposes.

  • testutil/sims/staking.go:
    • type StakingValidator struct
    • type LastValidatorPower struct
    • type StakingDelegation struct
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the definition of new types `StakingValidator`, `LastValidatorPower`, and `StakingDelegation`.

# Test: Search for the definition of new types. Expect: Correct definition.
rg --type go --pattern 'type StakingValidator'
rg --type go --pattern 'type LastValidatorPower'
rg --type go --pattern 'type StakingDelegation'

Length of output: 1385


Script:

#!/bin/bash
# Description: Verify the definition of new types `StakingValidator`, `LastValidatorPower`, and `StakingDelegation`.

# Test: Search for the definition of new types. Expect: Correct definition.
rg 'type StakingValidator' --type go
rg 'type LastValidatorPower' --type go
rg 'type StakingDelegation' --type go

Length of output: 497


Script:

#!/bin/bash
# Description: Retrieve the definitions of `StakingValidator`, `LastValidatorPower`, and `StakingDelegation` from `testutil/sims/staking.go`.

# Test: Extract the definitions.
rg 'type StakingValidator struct' -A 10 testutil/sims/staking.go
rg 'type LastValidatorPower struct' -A 10 testutil/sims/staking.go
rg 'type StakingDelegation struct' -A 10 testutil/sims/staking.go

Length of output: 2370


272-275: Ensure LastValidatorPower fields are correctly set.

The fields of LastValidatorPower should be verified to ensure they are correctly set, including Address and Power.

Verification successful

Verification successful: LastValidatorPower fields are correctly set.

The fields Address and Power are correctly defined in the LastValidatorPower struct.

  • x/staking/types/genesis.pb.go
  • testutil/sims/staking.go
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the fields of `LastValidatorPower` are correctly set.

# Test: Search for the definition of `LastValidatorPower` and verify the fields. Expect: Correct fields.
rg --type go --pattern 'type LastValidatorPower'

Length of output: 462


Script:

#!/bin/bash
# Description: Verify the fields of `LastValidatorPower` are correctly set.

# Test: Search for the definition of `LastValidatorPower` and verify the fields. Expect: Correct fields.
rg 'type LastValidatorPower' -A 5

Length of output: 1416


286-301: Ensure StakingGenesisState is correctly defined and used.

The new type StakingGenesisState should be verified to ensure it is correctly defined and used, including its fields Params, LastTotalPower, LastValidatorPowers, Validators, Delegations, and Exported.

Verification successful

StakingGenesisState is correctly defined and used.

The definition of StakingGenesisState in testutil/sims/staking.go includes all the fields used in the provided snippet, ensuring its correctness.

  • Params
  • LastTotalPower
  • LastValidatorPowers
  • Validators
  • Delegations
  • Exported
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the definition and usage of `StakingGenesisState`.

# Test: Search for the definition of `StakingGenesisState` and verify the fields. Expect: Correct definition and usage.
rg --type go --pattern 'type StakingGenesisState'

Length of output: 463


Script:

#!/bin/bash
# Description: Verify the definition and usage of `StakingGenesisState`.

# Test: Search for the definition of `StakingGenesisState` and verify the fields. Expect: Correct definition and usage.
rg 'type StakingGenesisState' --type go -A 10

Length of output: 1185


Script:

#!/bin/bash
# Description: Verify the definition and usage of `StakingGenesisState`.

# Test: Search for the remaining fields `Delegations` and `Exported` in the `StakingGenesisState` definition.
rg 'type StakingGenesisState' --type go -A 20

Length of output: 1925


249-267: Ensure StakingValidator fields are correctly set.

The fields of StakingValidator should be verified to ensure they are correctly set, including OperatorAddress, ConsensusPubkey, Status, Tokens, DelegatorShares, Description, Commission, and MinSelfDelegation.

Verification successful

Verified: The fields of StakingValidator are correctly set.

The struct StakingValidator includes all the necessary fields: OperatorAddress, ConsensusPubkey, Status, Tokens, DelegatorShares, Description, Commission, and MinSelfDelegation.

  • OperatorAddress
  • ConsensusPubkey
  • Status
  • Tokens
  • DelegatorShares
  • Description
  • Commission
  • MinSelfDelegation
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the fields of `StakingValidator` are correctly set.

# Test: Search for the definition of `StakingValidator` and verify the fields. Expect: Correct fields.
rg --type go --pattern 'type StakingValidator'

Length of output: 460


Script:

#!/bin/bash
# Description: Verify the fields of `StakingValidator` are correctly set.

# Test: Search for the definition of `StakingValidator` and verify the fields. Expect: Correct fields.
rg 'type StakingValidator' --type go

Length of output: 160


Script:

#!/bin/bash
# Description: Extract the definition of `StakingValidator` struct to verify the fields.

# Extract the definition of the `StakingValidator` struct from the file.
rg 'type StakingValidator struct' -A 20 testutil/sims/staking.go

Length of output: 2002


Script:

#!/bin/bash
# Description: Extract more lines from the definition of `StakingValidator` struct to verify the remaining fields.

# Extract the definition of the `StakingValidator` struct from the file, including more lines.
rg 'type StakingValidator struct' -A 40 testutil/sims/staking.go

Length of output: 3222


308-316: Ensure marshaling and extracting staking genesis proto is correctly implemented.

The marshaling and extracting of staking genesis proto should be verified to ensure it is correctly implemented.

testutil/sims/staking.go (10)

23-29: LGTM!

The LastValidatorPower struct is well-defined.


31-46: LGTM!

The fields in StakingGenesisState are well-defined and follow protobuf conventions.


182-198: LGTM!

The fields in StakingParams are well-defined and follow protobuf conventions.


200-219: LGTM!

The ToProto method in StakingParams is well-structured and does not have any issues.


222-258: LGTM!

The ProtoToStakingParams method in StakingParams is well-structured and does not have any issues.


260-287: LGTM!

The fields in StakingValidator are well-defined and follow protobuf conventions.


290-292: LGTM!

The SetPubKeyContrete method is simple and does not have any issues.


428-439: LGTM!

The fields in StakingDescription are well-defined and follow protobuf conventions.


442-456: LGTM!

The ToProto method in StakingDescription is well-structured and does not have any issues.


459-472: LGTM!

The ProtoToStakingDescription method in StakingDescription is well-structured and does not have any issues.

testutil/sims/staking.go Show resolved Hide resolved
Comment on lines +109 to +179
func ProtoToStakingGenesisState(protoMsg *anypb.Any) (*StakingGenesisState, error) {
var s structpb.Struct
if err := protoMsg.UnmarshalTo(&s); err != nil {
return nil, err
}

genesisState := &StakingGenesisState{}

// Parse Params
paramsAny, err := anypb.New(s.Fields["params"].GetStructValue())
if err != nil {
return nil, err
}
params, err := ProtoToStakingParams(paramsAny)
if err != nil {
return nil, err
}
genesisState.Params = *params

// Parse LastTotalPower
lastTotalPower, ok := math.NewIntFromString(s.Fields["last_total_power"].GetStringValue())
if !ok {
return nil, fmt.Errorf("failed to parse last_total_power")
}
genesisState.LastTotalPower = lastTotalPower

// Parse LastValidatorPowers
lastValidatorPowersValue := s.Fields["last_validator_powers"].GetListValue()
genesisState.LastValidatorPowers = make([]LastValidatorPower, len(lastValidatorPowersValue.Values))
for i, v := range lastValidatorPowersValue.Values {
lvpStruct := v.GetStructValue()
genesisState.LastValidatorPowers[i] = LastValidatorPower{
Address: lvpStruct.Fields["address"].GetStringValue(),
Power: int64(lvpStruct.Fields["power"].GetNumberValue()),
}
}

// Parse Validators
validatorsValue := s.Fields["validators"].GetListValue()
genesisState.Validators = make([]StakingValidator, len(validatorsValue.Values))
for i, v := range validatorsValue.Values {
validatorAny, err := anypb.New(v.GetStructValue())
if err != nil {
return nil, err
}
validator, err := ProtoToStakingValidator(validatorAny)
if err != nil {
return nil, err
}
genesisState.Validators[i] = *validator
}

// Parse Delegations
delegationsValue := s.Fields["delegations"].GetListValue()
genesisState.Delegations = make([]StakingDelegation, len(delegationsValue.Values))
for i, v := range delegationsValue.Values {
delegationAny, err := anypb.New(v.GetStructValue())
if err != nil {
return nil, err
}
delegation, err := ProtoToStakingDelegation(delegationAny)
if err != nil {
return nil, err
}
genesisState.Delegations[i] = *delegation
}

// Parse Exported
genesisState.Exported = s.Fields["exported"].GetBoolValue()

return genesisState, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Refactor repetitive code in ProtoToStakingGenesisState method.

The method has repetitive code blocks for parsing fields from protobuf maps. Consider extracting this logic into a helper function to improve maintainability.

func ProtoToStakingGenesisState(protoMsg *anypb.Any) (*StakingGenesisState, error) {
	var s structpb.Struct
	if err := protoMsg.UnmarshalTo(&s); err != nil {
		return nil, err
	}

	genesisState := &StakingGenesisState{}

	params, err := parseProtoField(s.Fields["params"])
	if err != nil {
		return nil, err
	}
	genesisState.Params = *params.(*StakingParams)

	lastTotalPower, err := parseProtoField(s.Fields["last_total_power"])
	if err != nil {
		return nil, err
	}
	genesisState.LastTotalPower = lastTotalPower.(math.Int)

	lastValidatorPowers, err := parseProtoListField(s.Fields["last_validator_powers"])
	if err != nil {
		return nil, err
	}
	genesisState.LastValidatorPowers = lastValidatorPowers.([]LastValidatorPower)

	validators, err := parseProtoListField(s.Fields["validators"])
	if err != nil {
		return nil, err
	}
	genesisState.Validators = validators.([]StakingValidator)

	delegations, err := parseProtoListField(s.Fields["delegations"])
	if err != nil {
		return nil, err
	}
	genesisState.Delegations = delegations.([]StakingDelegation)

	genesisState.Exported = s.Fields["exported"].GetBoolValue()

	return genesisState, nil
}

func parseProtoField(field *structpb.Value) (interface{}, error) {
	// Logic to parse a single field from protobuf map
}

func parseProtoListField(field *structpb.Value) (interface{}, error) {
	// Logic to parse a list of fields from protobuf map
}

Comment on lines +294 to +349
func (s *StakingValidator) ToProto() (*anypb.Any, error) {
var consensusPubkeyProto map[string]interface{}
if s.ConsensusPubkey != nil {
consensusPubkeyProto = map[string]interface{}{
"@type": s.ConsensusPubkey.TypeUrl,
"key": s.consensusPubKeyConcrete.Bytes(),
}
}

descriptionAny, err := s.Description.ToProto()
if err != nil {
return nil, err
}

descriptionProto, err := structpbToMap(descriptionAny)
if err != nil {
return nil, err
}

commissionAny, err := s.Commission.ToProto()
if err != nil {
return nil, err
}

commissionProto, err := structpbToMap(commissionAny)
if err != nil {
return nil, err
}

unbondingIds := make([]interface{}, len(s.UnbondingIds))
for i, id := range s.UnbondingIds {
unbondingIds[i] = id
}

fields := map[string]interface{}{
"operator_address": s.OperatorAddress,
"consensus_pubkey": consensusPubkeyProto,
"jailed": s.Jailed,
"status": s.Status,
"tokens": s.Tokens.String(),
"delegator_shares": s.DelegatorShares.String(),
"description": descriptionProto,
"unbonding_height": s.UnbondingHeight,
"unbonding_time": s.UnbondingTime.Format(time.RFC3339),
"commission": commissionProto,
"min_self_delegation": s.MinSelfDelegation.String(),
"unbonding_on_hold_ref_count": s.UnbondingOnHoldRefCount,
"unbonding_ids": unbondingIds,
}

pbStruct, err := structpb.NewStruct(fields)
if err != nil {
return nil, err
}

return anypb.New(pbStruct)
Copy link
Contributor

Choose a reason for hiding this comment

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

Refactor repetitive code in ToProto method.

The method has repetitive code blocks for converting fields to protobuf maps. Consider extracting this logic into a helper function to improve maintainability.

func (s *StakingValidator) ToProto() (*anypb.Any, error) {
	consensusPubkeyProto, err := convertToProtoMap(s.ConsensusPubkey)
	if err != nil {
		return nil, err
	}

	descriptionProto, err := convertToProtoMap(s.Description)
	if err != nil {
		return nil, err
	}

	commissionProto, err := convertToProtoMap(s.Commission)
	if err != nil {
		return nil, err
	}

	unbondingIds, err := convertListToProtoMap(s.UnbondingIds)
	if err != nil {
		return nil, err
	}

	fields := map[string]interface{}{
		"operator_address":            s.OperatorAddress,
		"consensus_pubkey":            consensusPubkeyProto,
		"jailed":                      s.Jailed,
		"status":                      s.Status,
		"tokens":                      s.Tokens.String(),
		"delegator_shares":            s.DelegatorShares.String(),
		"description":                 descriptionProto,
		"unbonding_height":            s.UnbondingHeight,
		"unbonding_time":              s.UnbondingTime.Format(time.RFC3339),
		"commission":                  commissionProto,
		"min_self_delegation":         s.MinSelfDelegation.String(),
		"unbonding_on_hold_ref_count": s.UnbondingOnHoldRefCount,
		"unbonding_ids":               unbondingIds,
	}

	pbStruct, err := structpb.NewStruct(fields)
	if err != nil {
		return nil, err
	}

	return anypb.New(pbStruct)
}

testutil/sims/staking.go Show resolved Hide resolved
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.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 414ea26 and 2703219.

Files selected for processing (1)
  • server/v2/cometbft/client/grpc/cmtservice/service.go (2 hunks)
Files skipped from review due to trivial changes (1)
  • server/v2/cometbft/client/grpc/cmtservice/service.go

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.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 2703219 and d31db70.

Files selected for processing (1)
  • testutil/sims/staking.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • testutil/sims/staking.go

@julienrbrt
Copy link
Member Author

Closing this and won't extract #20887 as more changes need to happen at the same place, so might as well do it there.

@julienrbrt julienrbrt closed this Jul 24, 2024
@julienrbrt julienrbrt deleted the julien/delstakingtestutil-integration branch July 24, 2024 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/v0.52.x PR scheduled for inclusion in the v0.52's next stable release C:server/v2 cometbft
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants