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

fix(x/accounts/lockup): fix proto path #22319

Merged
merged 6 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion tests/e2e/accounts/lockup/periodic_lockup_test_suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"cosmossdk.io/core/header"
"cosmossdk.io/math"
lockupaccount "cosmossdk.io/x/accounts/defaults/lockup"
"cosmossdk.io/x/accounts/defaults/lockup/types"
types "cosmossdk.io/x/accounts/defaults/lockup/v1"

"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/accounts/lockup/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (

"cosmossdk.io/core/transaction"
"cosmossdk.io/simapp"
"cosmossdk.io/x/accounts/defaults/lockup/types"
types "cosmossdk.io/x/accounts/defaults/lockup/v1"
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent import paths for types package.

The import path for the types package varies between cosmossdk.io/x/accounts/defaults/lockup/v1 and cosmossdk.io/x/accounts/defaults/lockup/types across different files. To maintain consistency and prevent potential issues, please standardize all import paths to use either v1 or types consistently throughout the codebase.

Affected files:

  • tests/e2e/accounts/lockup/continous_lockup_test_suite.go
  • tests/e2e/accounts/lockup/permanent_lockup_test_suite.go
  • tests/e2e/accounts/lockup/delayed_lockup_test_suite.go
🔗 Analysis chain

Import path updated correctly.

The import path for the types package has been updated from cosmossdk.io/x/accounts/defaults/lockup/types to cosmossdk.io/x/accounts/defaults/lockup/v1. This change aligns with the PR objective to fix the proto path and is consistent with similar changes in other files of this PR.

However, there's an ongoing discussion about whether to use "v1" or "types" in the import path. While "v1" is consistent with prior implementations, it might be worth considering using "types" for clarity if that's the preferred convention moving forward.

To ensure consistency across the codebase, let's verify the usage of this import path:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistency in import paths for lockup types
# Expected result: All imports should use the same path (either v1 or types)

echo "Checking import paths for lockup types:"
rg --type go 'cosmossdk\.io/x/accounts/defaults/lockup/(v1|types)'

Length of output: 1976

"cosmossdk.io/x/bank/testutil"

"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
Expand Down
34 changes: 17 additions & 17 deletions x/accounts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ It's crucial to understand that an account's state is isolated. This means:

For example, consider two accounts of type Counter:

- One located at address "cosmos123"
- Another at address "cosmos456"
* One located at address "cosmos123"
* Another at address "cosmos456"

These accounts do not share the same collections.Item instance. Instead, each maintains its own separate state.

Expand All @@ -51,10 +51,10 @@ type Account struct {

Creating an account begins with defining its init message. This message is processed when an account is created, similar to:

- The `instantiate` method in a CosmWasm contract
- The `constructor` in an EVM contract
* The `instantiate` method in a CosmWasm contract
* The `constructor` in an EVM contract

For an account to be a valid `x/account` implementer, it must define both:
For an account to be a valid `x/accounts` implementer, it must define both:

1. An `Init` method
2. An init message
Expand Down Expand Up @@ -106,8 +106,8 @@ func (a Account) RegisterInitHandler(builder *accountstd.InitBuilder) {

Execute handlers are methods that an account can execute, defined as messages. These executions can be triggered:

- During block execution (not queries) through transactions
- During begin or end block
* During block execution (not queries) through transactions
* During begin or end block

To define an execute handler, we start by creating its proto message:

Expand Down Expand Up @@ -176,8 +176,8 @@ value and returning the new value in the response.

Query Handlers are read-only methods implemented by an account to expose information about itself. This information can be accessed by:

- External clients (e.g., CLI, wallets)
- Other modules and accounts within the system
* External clients (e.g., CLI, wallets)
* Other modules and accounts within the system

Query handlers can be invoked:

Expand Down Expand Up @@ -315,9 +315,9 @@ accountsKeeper, err := accounts.NewKeeper(

Choose the method that best fits your application structure.

### The accountsstd Package
### The accountstd Package

The `accountsstd` package provides utility functions for use within account init, execution, or query handlers. Key functions include:
The `accountstd` package provides utility functions for use within account init, execution, or query handlers. Key functions include:

1. `Whoami()`: Retrieves the address of the current account.
2. `Sender()`: Gets the address of the transaction sender (not available in queries).
Expand All @@ -340,9 +340,9 @@ This flexibility enables defining interfaces as common sets of messages and/or q

Example: Transaction Authentication

- We define a `MsgAuthenticate` message.
- Any account capable of handling `MsgAuthenticate` is considered to implement the `Authentication` interface.
- This approach allows for standardized interaction patterns across different account types.
* We define a `MsgAuthenticate` message.
* Any account capable of handling `MsgAuthenticate` is considered to implement the `Authentication` interface.
* This approach allows for standardized interaction patterns across different account types.

(Note: More details on the `Authentication` interface will be provided later.)

Expand Down Expand Up @@ -431,7 +431,7 @@ func (a Account) RegisterExecuteHandlers(builder *accountstd.ExecuteBuilder) {

1. **Sender Verification**: Always verify that the sender is the x/accounts module. This prevents unauthorized accounts from triggering authentication.
2. **Authentication Safety**: Ensure your authentication mechanism is secure:
- Prevent replay attacks by making it impossible to reuse the same action with the same signature.
* Prevent replay attacks by making it impossible to reuse the same action with the same signature.

##### Implementation example

Expand Down Expand Up @@ -491,8 +491,8 @@ func (a Account) AuthRetroCompatibility(ctx context.Context, _ *authtypes.QueryL

### Usage Notes

- Implement this handler only for account types you want to expose via x/auth gRPC methods.
- The `info` field in the response can be nil if your account doesn't fit the `BaseAccount` structure.
* Implement this handler only for account types you want to expose via x/auth gRPC methods.
* The `info` field in the response can be nil if your account doesn't fit the `BaseAccount` structure.

## Genesis

Expand Down
2 changes: 1 addition & 1 deletion x/accounts/defaults/lockup/continuous_locking_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
collcodec "cosmossdk.io/collections/codec"
"cosmossdk.io/math"
"cosmossdk.io/x/accounts/accountstd"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/types"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/v1"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"cosmossdk.io/core/store"
"cosmossdk.io/log"
"cosmossdk.io/math"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/types"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/v1"
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Action Required: Update Remaining Import Paths for lockuptypes

The old import path cosmossdk.io/x/accounts/defaults/lockup/types is still present in the following test files:

  • tests/e2e/accounts/lockup/continous_lockup_test_suite.go
  • tests/e2e/accounts/lockup/delayed_lockup_test_suite.go
  • tests/e2e/accounts/lockup/permanent_lockup_test_suite.go

Please update these import statements to use the new versioned package structure.

🔗 Analysis chain

LGTM: Import path updated correctly.

The import path for lockuptypes has been updated to use the new versioned package structure. This change aligns with the PR objective and maintains consistency with prior implementations.

To ensure consistency across the codebase, please run the following command to check for any remaining occurrences of the old import path:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining occurrences of the old import path
rg "cosmossdk.io/x/accounts/defaults/lockup/types"

Length of output: 366


sdk "github.com/cosmos/cosmos-sdk/types"
)
Expand Down
2 changes: 1 addition & 1 deletion x/accounts/defaults/lockup/delayed_locking_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (

"cosmossdk.io/math"
"cosmossdk.io/x/accounts/accountstd"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/types"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/v1"
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent Import Paths Detected

The old import path cosmossdk.io/x/accounts/defaults/lockup/types is still used in the following files:

  • tests/e2e/accounts/lockup/continous_lockup_test_suite.go
  • tests/e2e/accounts/lockup/permanent_lockup_test_suite.go
  • tests/e2e/accounts/lockup/delayed_lockup_test_suite.go

Please update these files to use cosmossdk.io/x/accounts/defaults/lockup/v1 to ensure consistency across the codebase.

🔗 Analysis chain

LGTM: Import path updated correctly.

The import path for lockuptypes has been updated from cosmossdk.io/x/accounts/defaults/lockup/types to cosmossdk.io/x/accounts/defaults/lockup/v1. This change aligns with the PR objectives and the broader restructuring of the lockup module to accommodate versioning.

To ensure consistency across the codebase, let's verify the usage of this new import path:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the consistency of the new import path across the codebase.

# Test: Search for the old import path. Expect: No results.
echo "Checking for old import path:"
rg "cosmossdk.io/x/accounts/defaults/lockup/types"

# Test: Search for the new import path. Expect: Consistent usage across relevant files.
echo "Checking for new import path:"
rg "cosmossdk.io/x/accounts/defaults/lockup/v1"

Length of output: 2443


sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"cosmossdk.io/core/store"
"cosmossdk.io/log"
"cosmossdk.io/math"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/types"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/v1"
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent Import Paths Detected

The following files still use the old import path cosmossdk.io/x/accounts/defaults/lockup/types and need to be updated to v1:

  • tests/e2e/accounts/lockup/permanent_lockup_test_suite.go
  • tests/e2e/accounts/lockup/continous_lockup_test_suite.go
  • tests/e2e/accounts/lockup/delayed_lockup_test_suite.go
🔗 Analysis chain

LGTM: Import path updated correctly.

The import path for lockuptypes has been updated from cosmossdk.io/x/accounts/defaults/lockup/types to cosmossdk.io/x/accounts/defaults/lockup/v1. This change is consistent with the PR objectives to address import naming inconsistency.

To ensure consistency across the codebase, please run the following command:

This will help identify any remaining instances of the old import path that may need updating.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify consistent usage of the new import path
rg -g '*.go' 'cosmossdk\.io/x/accounts/defaults/lockup/(types|v1)'

Length of output: 1887


sdk "github.com/cosmos/cosmos-sdk/types"
)
Expand Down
2 changes: 1 addition & 1 deletion x/accounts/defaults/lockup/lockup.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
errorsmod "cosmossdk.io/errors"
"cosmossdk.io/math"
"cosmossdk.io/x/accounts/accountstd"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/types"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/v1"
banktypes "cosmossdk.io/x/bank/types"
distrtypes "cosmossdk.io/x/distribution/types"
stakingtypes "cosmossdk.io/x/staking/types"
Expand Down
2 changes: 1 addition & 1 deletion x/accounts/defaults/lockup/lockup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (

"cosmossdk.io/core/store"
"cosmossdk.io/math"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/types"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/v1"
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent import paths found for lockuptypes

The following files are still using the old import path cosmossdk.io/x/accounts/defaults/lockup/types or cosmossdk.io/x/accounts/defaults/lockup/depinject instead of the updated v1 path:

  • tests/e2e/accounts/lockup/delayed_lockup_test_suite.go
  • tests/e2e/accounts/lockup/permanent_lockup_test_suite.go
  • tests/e2e/accounts/lockup/continous_lockup_test_suite.go
  • simapp/v2/app_di.go
  • simapp/app_di.go

Please update these import statements to ensure consistency across the codebase.

🔗 Analysis chain

LGTM: Import path updated to use versioned package.

The change from lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/types" to lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/v1" aligns with the PR objective of addressing import naming inconsistency. This shift to a versioned package structure is a good practice for managing API changes.

To ensure consistency across the codebase, please run the following command:

This command will search for any remaining imports that don't use the new v1 path. The results should be empty if the change has been applied consistently.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify that all imports of lockuptypes use the new v1 path
rg -g '*.go' 'cosmossdk\.io/x/accounts/defaults/lockup/(?!v1)' --stats

Length of output: 364


Script:

#!/bin/bash
# Find all .go files with imports of lockuptypes not using the v1 path
# Extract lines with the lockup import path
rg 'cosmossdk\.io/x/accounts/defaults/lockup/' -g '*.go' > all_lockup_imports.txt

# Exclude the v1 path
grep -v 'lockup/v1' all_lockup_imports.txt > invalid_imports.txt

# Output the result
if [ -s invalid_imports.txt ]; then
    echo "Found imports not using the v1 path:"
    cat invalid_imports.txt
else
    echo "All lockuptypes imports use the v1 path."
fi

Length of output: 734


sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
Expand Down
2 changes: 1 addition & 1 deletion x/accounts/defaults/lockup/periodic_locking_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
errorsmod "cosmossdk.io/errors"
"cosmossdk.io/math"
"cosmossdk.io/x/accounts/accountstd"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/types"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/v1"
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Old lockuptypes import paths still present in tests:

  • tests/e2e/accounts/lockup/continous_lockup_test_suite.go
  • tests/e2e/accounts/lockup/permanent_lockup_test_suite.go
  • tests/e2e/accounts/lockup/delayed_lockup_test_suite.go
🔗 Analysis chain

Verify the impact of the lockuptypes import change.

The import path for lockuptypes has been updated to use v1. This change might affect the usage of lockuptypes throughout the file. Please ensure that all references to lockuptypes are compatible with the new version.

To verify the impact, run the following script:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any incompatibilities or changes required due to the new lockuptypes import.

# Test: Search for all usages of lockuptypes in the file
rg 'lockuptypes\.' x/accounts/defaults/lockup/periodic_locking_account.go

# Test: Check if there are any compilation errors due to the import change
go build ./x/accounts/defaults/lockup/...

Length of output: 295


Script:

#!/bin/bash
# Description: Verify that the lockuptypes import path has been updated consistently and no old references remain.

# Test 1: Ensure the old import path does not exist anywhere in the codebase
echo "Checking for old import path 'cosmossdk.io/x/accounts/defaults/lockup/types'..."
rg 'cosmossdk.io/x/accounts/defaults/lockup/types' || echo "No occurrences of the old import path found."

# Test 2: Ensure the new import path is used consistently
echo "Checking for new import path 'cosmossdk.io/x/accounts/defaults/lockup/v1'..."
rg 'cosmossdk.io/x/accounts/defaults/lockup/v1' || echo "No occurrences of the new import path found."

# Test 3: Find all usages of lockuptypes to verify compatibility
echo "Finding all usages of 'lockuptypes'..."
rg 'lockuptypes\.' 

Length of output: 16958


"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"cosmossdk.io/core/store"
"cosmossdk.io/log"
"cosmossdk.io/math"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/types"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/v1"
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Remaining Old Import Paths Found in Test Files

The following test files still import cosmossdk.io/x/accounts/defaults/lockup/types. Please update these import paths to the new versioned path (v1):

  • tests/e2e/accounts/lockup/permanent_lockup_test_suite.go
  • tests/e2e/accounts/lockup/continous_lockup_test_suite.go
  • tests/e2e/accounts/lockup/delayed_lockup_test_suite.go
🔗 Analysis chain

LGTM: Import path updated correctly.

The import path for lockuptypes has been updated to use the new versioned path (v1). This change is consistent with the PR objective and doesn't affect the functionality of the tests.

To ensure consistency across the codebase, please run the following command to check for any remaining occurrences of the old import path:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining occurrences of the old import path
rg "cosmossdk.io/x/accounts/defaults/lockup/types" --type go

Length of output: 376


sdk "github.com/cosmos/cosmos-sdk/types"
)
Expand Down
2 changes: 1 addition & 1 deletion x/accounts/defaults/lockup/permanent_locking_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (

"cosmossdk.io/math"
"cosmossdk.io/x/accounts/accountstd"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/types"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/v1"

sdk "github.com/cosmos/cosmos-sdk/types"
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"cosmossdk.io/core/store"
"cosmossdk.io/log"
"cosmossdk.io/math"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/types"
lockuptypes "cosmossdk.io/x/accounts/defaults/lockup/v1"

sdk "github.com/cosmos/cosmos-sdk/types"
)
Expand Down
54 changes: 27 additions & 27 deletions x/accounts/defaults/lockup/types/lockup.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading