[Supplier] Refactor/non custodial staking tests

[red-0ne]

Summary

This PR is a follow-up to #716.

• It refactors the existing unit/integration tests to add Supplier.OwnerAddress
• Adapts the Supplier (un)staking and tests (staking config, (un)staking CLI and keeper methods) to comply with the Supplier non-custodial staking.

Issue

• [Utility][Morse Parity] Introduce non-custodial Staking #493

Type of change

Select one or more:

• New feature, functionality or library
• Bug fix
• Code health or cleanup
• Documentation
• Other (specify)

Testing

Documentation changes (only if making doc changes)

• make docusaurus_start; only needed if you make doc changes

Local Testing (only if making code changes)

• Unit Tests: make go_develop_and_test
• LocalNet E2E Tests: make test_e2e
• See quickstart guide for instructions

PR Testing (only if making code changes)

• DevNet E2E Tests: Add the devnet-test-e2e label to the PR.
  • THIS IS VERY EXPENSIVE, so only do it after all the reviews are complete.
  • Optionally run make trigger_ci if you want to re-trigger tests without any code changes
  • If tests fail, try re-running failed tests only using the GitHub UI as shown here

Sanity Checklist

• I have tested my changes using the available tooling
• I have commented my code
• I have performed a self-review of my own code; both comments & source code
• I create and reference any new tickets, if applicable
• I have left TODOs throughout the codebase, if applicable

Summary by CodeRabbit

• New Features

  • Enhanced readability of output messages in the Makefile.
  • Added OwnerAddress to the Supplier data model for better ownership tracking.
  • Introduced support for both owner and operator addresses in various staking and unstaking processes.

• Bug Fixes

  • Updated tests for MsgUnstakeSupplier to correctly validate signer addresses and enhance robustness.

• Documentation

  • Improved test coverage and clarity for supplier-related functionality, reflecting the new data model changes.

[github-actions]

The CI will now also run the e2e tests on devnet, which increases the time it takes to complete all CI checks.

You may need to run make trigger_ci to submit an empty commit that'll trigger the tests.

GCP workloads (requires changing the namespace to 718)
Grafana network dashboard for devnet-issue-718

[Olshansk]

Just an OPTIONAL minor NIT, but otherwise looks g2g!

[coderabbitai]

Walkthrough

The recent changes enhance the structure and functionality of supplier-related entities across multiple files. Key modifications include the addition of an OwnerAddress field to the Supplier struct, which clarifies ownership distinctions. Updates to message definitions in Protocol Buffers and corresponding test cases improve data handling and validation logic, ensuring robust supplier management and interactions. Overall, these enhancements foster better clarity and efficiency in the supplier management system.

Changes

Files	Change Summary
Makefile	Minor formatting adjustments to output messages for improved readability.
api/poktroll/supplier/tx.pulsar.go, proto/poktroll/supplier/tx.proto	Modified MsgUnstakeSupplier struct to update field identifiers for Address and Signer, affecting protobuf serialization.
config.yml	Updated supplierList structure to include owner_address alongside address for clearer differentiation of supplier ownership.
e2e/tests/init_test.go	Enhanced getConfigFileContent method to accept ownerAddress and operatorAddress, modifying its logic and output format.
testutil/integration/app.go, testutil/keeper/supplier.go, testutil/keeper/tokenomics.go	Added OwnerAddress field to Supplier struct, updating related logic in the integration app and keeper tests.
testutil/keeper/supplier.go	Introduced SupplierUnstakedFundsMap to track funds returned to suppliers.
testutil/network/network.go	Updated Supplier struct initialization to include OwnerAddress.
x/supplier/config/supplier_configs_reader_test.go	Enhanced test cases to dynamically generate supplier configuration strings, improving coverage for various scenarios.
x/supplier/keeper/msg_server_stake_supplier_test.go, x/supplier/keeper/msg_server_unstake_supplier_test.go	Updated staking/unstaking tests to involve both ownerAddr and operatorAddr, improving test clarity and coverage.
x/supplier/module/tx_stake_supplier_test.go, x/supplier/module/tx_unstake_supplier_test.go	Renamed variables and adjusted test structures to clarify roles of owner and operator accounts in staking and unstaking processes.
x/supplier/types/message_stake_supplier_test.go, x/supplier/types/message_unstake_supplier_test.go	Enhanced validation logic for MsgStakeSupplier and MsgUnstakeSupplier by adding test cases for various address scenarios.
x/tokenomics/keeper/keeper_settle_pending_claims_test.go	Updated Supplier struct to include OwnerAddress, enhancing clarity in ownership representation.
x/tokenomics/keeper/token_logic_modules_test.go	Modified tests to utilize OwnerAddress in balance checks, emphasizing supplier ownership in assertions.

---

Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between d296b17 and e52e1d9.

Files ignored due to path filters (1)

• x/supplier/types/tx.pb.go is excluded by !**/*.pb.go

Files selected for processing (18)

• Makefile (2 hunks)
• api/poktroll/supplier/tx.pulsar.go (4 hunks)
• config.yml (1 hunks)
• e2e/tests/init_test.go (4 hunks)
• proto/poktroll/supplier/tx.proto (1 hunks)
• testutil/integration/app.go (1 hunks)
• testutil/keeper/supplier.go (3 hunks)
• testutil/keeper/tokenomics.go (1 hunks)
• testutil/network/network.go (2 hunks)
• x/supplier/config/supplier_configs_reader_test.go (19 hunks)
• x/supplier/keeper/msg_server_stake_supplier_test.go (7 hunks)
• x/supplier/keeper/msg_server_unstake_supplier_test.go (5 hunks)
• x/supplier/module/tx_stake_supplier_test.go (6 hunks)
• x/supplier/module/tx_unstake_supplier_test.go (3 hunks)
• x/supplier/types/message_stake_supplier_test.go (10 hunks)
• x/supplier/types/message_unstake_supplier_test.go (1 hunks)
• x/tokenomics/keeper/keeper_settle_pending_claims_test.go (1 hunks)
• x/tokenomics/keeper/token_logic_modules_test.go (5 hunks)

Files skipped from review due to trivial changes (1)

• Makefile

Additional context used

GitHub Check: misspell

x/supplier/config/supplier_configs_reader_test.go

[warning] 493-493: [misspell] x/supplier/config/supplier_configs_reader_test.go#L493
"explictly" is a misspelling of "explicitly"

Additional comments not posted (42)

x/supplier/types/message_unstake_supplier_test.go (1)

50-55: Ensure valid message test case coverage.

The "valid message" test case checks for no error, which is appropriate. Ensure that all valid scenarios are covered in other parts of the test suite.

proto/poktroll/supplier/tx.proto (1)

63-63: Ensure backward compatibility with field number change.

Changing the field number for address from 3 to 2 in MsgUnstakeSupplier can affect serialization. Ensure that this change is compatible with existing data and clients.

Verification successful

---

No dependencies found for field number change in MsgUnstakeSupplier.

The search did not reveal any dependencies on the previous field numbering for MsgUnstakeSupplier in the .proto files. Ensure that this change is compatible with any external systems or data that might rely on the previous field number.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify usage of MsgUnstakeSupplier to ensure compatibility with field number change.

# Test: Search for MsgUnstakeSupplier usage. Expect: No dependencies on previous field numbering.
rg --type proto -A 5 'message MsgUnstakeSupplier'

Length of output: 81

---

Script:

#!/bin/bash
# Description: Verify usage of MsgUnstakeSupplier to ensure compatibility with field number change.

# Test: Search for MsgUnstakeSupplier usage in .proto files.
rg 'message MsgUnstakeSupplier' --glob '*.proto' -A 5

Length of output: 716

testutil/keeper/supplier.go (2)

35-36: Good addition for tracking unbonded funds.

The introduction of SupplierUnstakedFundsMap improves the tracking of funds returned to suppliers. This enhances testability and state management.

---

64-67: Verify correctness of unbonded funds tracking.

The SendCoinsFromModuleToAccount method now updates supplierUnstakedFundsMap. Verify that this logic correctly reflects all unbonding scenarios.

x/supplier/module/tx_unstake_supplier_test.go (2)

28-30: Improved clarity with separate account roles.

The introduction of operatorAccount and ownerAccount enhances the clarity of account roles in the test.

---

48-51: Comprehensive test scenarios.

The test cases now cover a wide range of scenarios, including valid and invalid addresses for both signer and operator roles.

config.yml (1)

172-173: Enhanced data model with owner_address.

The addition of owner_address alongside address in the supplierList improves the clarity and structure of the data model, allowing for better management of supplier information.

x/supplier/keeper/msg_server_unstake_supplier_test.go (3)

57-60: Clarity improved with Signer in MsgUnstakeSupplier.

The addition of the Signer field enhances the clarity of the message structure, ensuring explicit specification of the signer in the unstaking process.

---

197-239: Comprehensive test for operator unstaking.

The TestMsgServer_UnstakeSupplier_OperatorCanUnstake test case thoroughly verifies the operator's ability to initiate unstaking and ensures that the initial stake is returned after the unbonding period.

---

244-246: Explicit handling of signer and owner in createStakeMsg.

The addition of Signer and OwnerAddress fields in createStakeMsg ensures a more explicit and secure handling of supplier-related messages.

x/supplier/module/tx_stake_supplier_test.go (5)

31-31: Variable Renaming: ownerAccount

The variable supplierAccount has been renamed to ownerAccount to better reflect its role. This improves clarity in the test.

---

46-55: Configuration String Update

The defaultConfig string now dynamically includes ownerAccount.Address and a sample address. This change ensures the configuration accurately reflects the expected state.

---

58-104: Test Case Updates

The test cases have been updated to use ownerAddress consistently. This enhances readability and maintains a consistent naming convention across scenarios.

---

Line range hint 150-202:
Stake and Service Configurations

The test cases for stake and service configurations have been expanded and clarified. Ensure that all edge cases are covered and that error handling is consistent.

Verification successful

---

Edge Cases for Stake and Service Configurations are Well-Covered

The test cases for stake and service configurations comprehensively cover a wide range of edge cases, including missing or invalid values, ensuring consistent error handling. No further action is needed.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify that all edge cases for stake and service configurations are covered in the tests.

rg --type go 'desc: "invalid stake' -A 5
rg --type go 'desc: "invalid service' -A 5

Length of output: 14986

---

34-34: Initialize Account with Funds

The network.InitAccount function is used to initialize the ownerAccount with funds. Ensure the function correctly handles this initialization.

Verification successful

---

Account Initialization Verified

The InitAccount function correctly initializes the ownerAccount by sending 200 "stake" tokens and verifying the transaction's success. No issues were found with this implementation.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the implementation of `InitAccount` function to ensure it initializes accounts with funds correctly.

ast-grep --lang go --pattern $'func InitAccount($_, $_, $_) {
  $$$
}'

Length of output: 1521

testutil/network/network.go (2)

143-144: Addition of OwnerAddress Field

The OwnerAddress field has been added to the Supplier struct. This change allows for the association of an owner with each supplier, potentially expanding functionality.

---

173-174: SupplierModuleGenesisStateWithAddresses Update

The SupplierModuleGenesisStateWithAddresses function now includes the OwnerAddress field. This ensures that each supplier is associated with an owner address.

x/supplier/types/message_stake_supplier_test.go (4)

34-35: Dynamic Address Generation

The introduction of ownerAddress and operatorAddress using sample.AccAddress() allows for dynamic generation of addresses, improving test flexibility.

---

44-104: Enhanced Address Validation Tests

The test cases for address validation have been expanded to cover various scenarios. This improves the robustness of the validation logic.

---

150-210: Stake Validation Tests

The stake validation tests have been enhanced to cover missing, zero, negative, and invalid denominations. Ensure these tests align with business logic requirements.

---

Line range hint 219-402:
Service Configuration Tests

The service configuration tests have been expanded to include multiple services and various invalid configurations. This ensures comprehensive validation of service-related logic.

testutil/keeper/tokenomics.go (1)

126-128: LGTM! Addition of OwnerAddress to Supplier.

The addition of the OwnerAddress field to the Supplier struct is consistent with the PR objectives and enhances the supplier's data model.

x/supplier/keeper/msg_server_stake_supplier_test.go (7)

22-61: LGTM! Successful creation and update tests.

The test effectively verifies the creation and update of a supplier with both owner and operator addresses, aligning with the new logic.

---

68-107: LGTM! Failure scenarios for invalid services.

The test cases effectively cover scenarios where restaking fails due to invalid services, enhancing robustness.

---

118-141: LGTM! Enforcing stake lowering restrictions.

The test accurately verifies that lowering the stake amount is not permitted, ensuring compliance with staking rules.

---

151-156: LGTM! Handling non-existing service scenarios.

The test appropriately checks that staking fails when attempting to use a non-existing service, ensuring data integrity.

---

163-261: LGTM! Comprehensive operator and owner authorization tests.

The test thoroughly validates the authorization logic for operators and owners, ensuring correct permissions in the staking process.

---

Line range hint 263-307:
LGTM! Supplier activation logic tests.

The test accurately checks the activation status of suppliers, ensuring they are active at the correct session height.

---

Line range hint 334-368:
LGTM! Helper functions for staking messages.

The helper functions improve code clarity and reusability in the tests, aligning with best practices.

x/supplier/config/supplier_configs_reader_test.go (2)

Line range hint 22-318:
LGTM! Comprehensive valid configuration tests.

The test cases effectively validate the parsing logic for a variety of valid configurations, ensuring robustness.

---

Line range hint 322-537:
LGTM! Thorough invalid configuration tests.

The test cases comprehensively cover various error scenarios, ensuring the robustness of the configuration parsing logic.

Tools

GitHub Check: misspell

[warning] 493-493: [misspell] x/supplier/config/supplier_configs_reader_test.go#L493
"explictly" is a misspelling of "explicitly"

x/tokenomics/keeper/token_logic_modules_test.go (6)

172-175: Validate expected supplier balance calculation.

Ensure that the calculation of expectedSupplierBalance accurately reflects the intended logic, particularly with the addition of OwnerAddress.

---

229-231: Ensure correct initialization of Supplier.

The Supplier struct now includes OwnerAddress. This change clarifies ownership but ensure that all instances of Supplier are correctly initialized with this new field.

---

235-237: Ensure correct usage of OwnerAddress in balance checks.

The test now checks the balance of supplier.GetOwnerAddress(). Verify that this logic aligns with the intended behavior of using OwnerAddress for balance assertions.

---

278-283: Validate expected supplier balance calculation.

Ensure that the calculation of expectedSupplierBalance accurately reflects the intended logic, particularly with the addition of OwnerAddress.

---

119-121: Ensure correct usage of OwnerAddress in balance checks.

The test now checks the balance of supplier.GetOwnerAddress(). Verify that this logic aligns with the intended behavior of using OwnerAddress for balance assertions.

Verification successful

---

Correct usage of OwnerAddress in balance checks confirmed.

The balance checks in the test file consistently use supplier.GetOwnerAddress(), aligning with the intended logic. No issues found.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all balance checks use `GetOwnerAddress()` where applicable.

# Test: Search for balance checks. Expect: Usage of `GetOwnerAddress()`.
rg --type go -A 5 $'getBalance\\(.*supplier.GetOwnerAddress\\(\\)'

Length of output: 1891

---

59-61: Ensure correct initialization of Supplier.

The Supplier struct now includes OwnerAddress. This change clarifies ownership but ensure that all instances of Supplier are correctly initialized with this new field.

Verification successful

Let's correct the regex pattern and re-run the search to verify the Supplier struct initializations.

---

Let's simplify the search pattern to avoid regex syntax issues and try again.

---

Let's adjust the search pattern to avoid regex parsing issues by looking for the keyword Supplier followed by a newline.

---

Verification Successful: Supplier Struct Initialization Includes OwnerAddress.

The Supplier struct initializations in x/tokenomics/keeper/token_logic_modules_test.go correctly include the OwnerAddress field, ensuring proper initialization as intended. No further action is needed.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all `Supplier` struct initializations include `OwnerAddress`.

# Test: Search for `Supplier` initializations. Expect: All initializations include `OwnerAddress`.
rg --type go -A 5 $'sharedtypes.Supplier{'

Length of output: 149

---

Script:

#!/bin/bash
# Description: Verify all `Supplier` struct initializations include `OwnerAddress`.

# Test: Search for `Supplier` initializations. Expect: All initializations include `OwnerAddress`.
rg --type go -A 5 'sharedtypes.Supplier{'

Length of output: 149

---

Script:

#!/bin/bash
# Description: Verify all `Supplier` struct initializations include `OwnerAddress`.

# Test: Search for `Supplier` initializations. Expect: All initializations include `OwnerAddress`.
rg --type go -A 5 'Supplier{'

Length of output: 113

---

Script:

#!/bin/bash
# Description: Verify all `Supplier` struct initializations include `OwnerAddress`.

# Test: Search for `Supplier` initializations. Expect: All initializations include `OwnerAddress`.
rg --type go -A 5 'Supplier\s*\{'

Length of output: 52199

e2e/tests/init_test.go (2)

Line range hint 311-336:
Verify correct usage of new parameters in configuration content.

Ensure that ownerAddress and operatorAddress are correctly incorporated into the configuration content for suppliers.

Verification successful

---

New parameters are correctly utilized in configuration content.

The ownerAddress and operatorAddress are correctly incorporated into the configuration content for suppliers, as verified by the sample output. No issues found.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that ownerAddress and operatorAddress are correctly used in the configuration content.

# Test: Print configuration content for a sample supplier. Expect: Correct inclusion of owner and operator addresses.
sampleOwnerAddress="sampleOwnerAddress"
sampleOperatorAddress="sampleOperatorAddress"
sampleConfigContent=$(cat <<EOF
owner_address: $sampleOwnerAddress
operator_address: $sampleOperatorAddress
stake_amount: 1000upokt
services:
  - service_id: sampleServiceId
    endpoints:
    - publicly_exposed_url: http://relayminer:8545
      rpc_type: json_rpc
EOF
)
echo "Sample Configuration Content:"
echo "$sampleConfigContent"

Length of output: 887

---

283-284: Verify address mapping correctness.

Ensure that accNameToAddrMap correctly maps account names to addresses. This is crucial for accurate address resolution.

Verification successful

---

Address mapping is correctly implemented and used.

The accNameToAddrMap is properly initialized and populated in e2e/tests/init_test.go, and its usage across the file aligns with its intended purpose. No issues found with the address mapping logic.

• File: e2e/tests/init_test.go
• Lines of Interest:
  • Initialization: accNameToAddrMap = make(map[string]string)
  • Population: accNameToAddrMap[name] = address

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the correctness of the accNameToAddrMap mappings.

# Test: Print all mappings. Expect: Correct mappings of account names to addresses.
echo "Account Name to Address Map:"
for name in "${!accNameToAddrMap[@]}"; do
  echo "$name -> ${accNameToAddrMap[$name]}"
done

Length of output: 66

---

Script:

#!/bin/bash
# Search for the definition and initialization of accNameToAddrMap in Go files.
ast-grep --lang go --pattern 'var accNameToAddrMap = $_' -A 5

Length of output: 63

---

Script:

#!/bin/bash
# Use ripgrep to search for occurrences of accNameToAddrMap in Go files.
rg 'accNameToAddrMap' --type go -A 5

Length of output: 3727

api/poktroll/supplier/tx.pulsar.go (1)

3024-3024: Verify the protobuf field identifier change for Address.

The protobuf identifier for the Address field in MsgUnstakeSupplier has been changed from 3 to 2. Ensure that this change is intentional and verify its impact on any systems or components that rely on this message format.

x/tokenomics/keeper/keeper_settle_pending_claims_test.go (1)

101-104: Addition of OwnerAddress enhances clarity.

The inclusion of the OwnerAddress field in the Supplier struct improves the distinction between the supplier's address and the owner's address, aligning with the non-custodial staking model.

testutil/integration/app.go (1)

532-534: Addition of OwnerAddress enhances clarity.

The inclusion of the OwnerAddress field in the Supplier struct improves the distinction between the supplier's address and the owner's address, aligning with the non-custodial staking model.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT
Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

• 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.

[coderabbitai]

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (7)

x/supplier/types/message_unstake_supplier_test.go (5)

20-26: Enhance error message clarity.

The test case description "invalid operator address" is clear, but consider providing more context in the expected error message to ensure clarity during test failures.

- expectedErr: ErrSupplierInvalidAddress,
+ expectedErr: fmt.Errorf("invalid operator address: %w", ErrSupplierInvalidAddress),

---

28-33: Clarify missing address error.

The test case for "missing operator address" could benefit from a more descriptive error message to clarify the cause of the error.

- expectedErr: ErrSupplierInvalidAddress,
+ expectedErr: fmt.Errorf("missing operator address: %w", ErrSupplierInvalidAddress),

---

35-41: Improve invalid signer address test case.

The test case for "invalid signer address" is well-structured. Ensure that the error message provides sufficient context for debugging.

- expectedErr: ErrSupplierInvalidAddress,
+ expectedErr: fmt.Errorf("invalid signer address: %w", ErrSupplierInvalidAddress),

---

43-48: Enhance missing signer address error message.

The "missing signer address" test case should include a more descriptive error message for better clarity.

- expectedErr: ErrSupplierInvalidAddress,
+ expectedErr: fmt.Errorf("missing signer address: %w", ErrSupplierInvalidAddress),

---

57-62: Explicit handling for same signer and operator addresses is missing.

The ValidateBasic method checks for valid Bech32 addresses but does not explicitly handle the case where the Signer and Address are the same. Consider adding logic to handle this scenario if it is required.

• x/supplier/types/message_unstake_supplier.go: ValidateBasic method

Analysis chain

Verify same signer and operator addresses scenario.

The test case for "valid message - same signer and operator addresses" is a good addition. Verify that this scenario is handled correctly in the implementation.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify handling of same signer and operator addresses in MsgUnstakeSupplier.

# Test: Search for handling logic in MsgUnstakeSupplier. Expect: Proper handling of same addresses.
rg --type go -A 5 'func (m MsgUnstakeSupplier) ValidateBasic' 

Length of output: 63

---

Script:

#!/bin/bash
# Search for the definition of MsgUnstakeSupplier and any associated methods.
ast-grep --lang go --pattern 'type MsgUnstakeSupplier struct { $$$ }'

# Search for any methods associated with MsgUnstakeSupplier.
rg --type go 'func (m MsgUnstakeSupplier)' -A 10

Length of output: 1225

---

Script:

#!/bin/bash
# Search for references to MsgUnstakeSupplier in the codebase to identify usage or validation.
rg --type go 'MsgUnstakeSupplier' -A 5

Length of output: 66190

x/supplier/module/tx_unstake_supplier_test.go (1)

64-67: Consider adding a test for missing operator address.

While there is a test for a missing signer address, consider adding a similar test for a missing operator address.

+		{
+			desc:          "unstake supplier: missing operator address",
+			signerAddress: ownerAccount.Address.String(),
+			expectedErr:   types.ErrSupplierInvalidAddress,
+		},

x/supplier/config/supplier_configs_reader_test.go (1)

493-493: Correct spelling error in comment.

The word "explictly" is misspelled. It should be "explicitly."

-				# explictly omitted owner address
+				# explicitly omitted owner address

Tools

GitHub Check: misspell

[warning] 493-493: [misspell] x/supplier/config/supplier_configs_reader_test.go#L493
"explictly" is a misspelling of "explicitly"