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.

Sign up for GitHub

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

Jump to bottom

refactor: reimplement BridgeTokens gRPC query #748

Merged
merged 1 commit into from
Oct 17, 2024
Merged

refactor: reimplement BridgeTokens gRPC query #748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
473 changes: 160 additions & 313 deletions api/fx/gravity/crosschain/v1/genesis.pulsar.go
View file
Open in desktop

Large diffs are not rendered by default.

3,089 changes: 3,059 additions & 30 deletions api/fx/gravity/crosschain/v1/legacy.pulsar.go
View file
Open in desktop

Large diffs are not rendered by default.

3,325 changes: 1,077 additions & 2,248 deletions api/fx/gravity/crosschain/v1/query.pulsar.go
View file
Open in desktop

Large diffs are not rendered by default.

4,180 changes: 1,165 additions & 3,015 deletions api/fx/gravity/crosschain/v1/types.pulsar.go
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 0 additions & 8 deletions app/genesis.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
},
"oracles": [],
"oracle_sets": [],
"bridge_tokens": [],
"unbatched_transfers": [],
"batches": [],
"oracle_set_confirms": [],
Expand Down Expand Up @@ -86,7 +85,6 @@
},
"oracles": [],
"oracle_sets": [],
"bridge_tokens": [],
"unbatched_transfers": [],
"batches": [],
"oracle_set_confirms": [],
Expand Down Expand Up @@ -173,7 +171,6 @@
},
"oracles": [],
"oracle_sets": [],
"bridge_tokens": [],
"unbatched_transfers": [],
"batches": [],
"oracle_set_confirms": [],
Expand Down Expand Up @@ -254,7 +251,6 @@
},
"oracles": [],
"oracle_sets": [],
"bridge_tokens": [],
"unbatched_transfers": [],
"batches": [],
"oracle_set_confirms": [],
Expand Down Expand Up @@ -431,7 +427,6 @@
},
"oracles": [],
"oracle_sets": [],
"bridge_tokens": [],
"unbatched_transfers": [],
"batches": [],
"oracle_set_confirms": [],
Expand Down Expand Up @@ -491,7 +486,6 @@
},
"oracles": [],
"oracle_sets": [],
"bridge_tokens": [],
"unbatched_transfers": [],
"batches": [],
"oracle_set_confirms": [],
Expand Down Expand Up @@ -537,7 +531,6 @@
},
"oracles": [],
"oracle_sets": [],
"bridge_tokens": [],
"unbatched_transfers": [],
"batches": [],
"oracle_set_confirms": [],
Expand Down Expand Up @@ -619,7 +612,6 @@
},
"oracles": [],
"oracle_sets": [],
"bridge_tokens": [],
"unbatched_transfers": [],
"batches": [],
"oracle_set_confirms": [],
Expand Down
2 changes: 1 addition & 1 deletion docs/statik/statik.go
View file
Open in desktop

Large diffs are not rendered by default.

3 changes: 0 additions & 3 deletions docs/swagger-ui/swagger.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7314,7 +7314,6 @@ paths:
channel_ibc:
type: string
title: 'Deprecated: after upgrade v3'
title: BridgeToken
default:
description: An unexpected error response.
schema:
Expand Down Expand Up @@ -41567,7 +41566,6 @@ definitions:
channel_ibc:
type: string
title: 'Deprecated: after upgrade v3'
title: BridgeToken
fx.gravity.crosschain.v1.BridgeValidator:
type: object
properties:
Expand Down Expand Up @@ -42093,7 +42091,6 @@ definitions:
channel_ibc:
type: string
title: 'Deprecated: after upgrade v3'
title: BridgeToken
fx.gravity.crosschain.v1.QueryCurrentOracleSetResponse:
type: object
properties:
Expand Down
19 changes: 9 additions & 10 deletions proto/fx/gravity/crosschain/v1/genesis.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,15 @@ message GenesisState {
[ (gogoproto.nullable) = false ];
repeated Oracle oracles = 4 [ (gogoproto.nullable) = false ];
repeated OracleSet oracle_sets = 5 [ (gogoproto.nullable) = false ];
repeated BridgeToken bridge_tokens = 6 [ (gogoproto.nullable) = false ];
repeated OutgoingTransferTx unbatched_transfers = 7
repeated OutgoingTransferTx unbatched_transfers = 6
[ (gogoproto.nullable) = false ];
repeated OutgoingTxBatch batches = 8 [ (gogoproto.nullable) = false ];
repeated MsgOracleSetConfirm oracle_set_confirms = 9
repeated OutgoingTxBatch batches = 7 [ (gogoproto.nullable) = false ];
repeated MsgOracleSetConfirm oracle_set_confirms = 8
[ (gogoproto.nullable) = false ];
repeated MsgConfirmBatch batch_confirms = 10 [ (gogoproto.nullable) = false ];
repeated Attestation attestations = 11 [ (gogoproto.nullable) = false ];
ProposalOracle proposal_oracle = 12 [ (gogoproto.nullable) = false ];
OracleSet last_observed_oracle_set = 13 [ (gogoproto.nullable) = false ];
uint64 last_slashed_batch_block = 14;
uint64 last_slashed_oracle_set_nonce = 15;
repeated MsgConfirmBatch batch_confirms = 9 [ (gogoproto.nullable) = false ];
repeated Attestation attestations = 10 [ (gogoproto.nullable) = false ];
ProposalOracle proposal_oracle = 11 [ (gogoproto.nullable) = false ];
OracleSet last_observed_oracle_set = 12 [ (gogoproto.nullable) = false ];
uint64 last_slashed_batch_block = 13;
uint64 last_slashed_oracle_set_nonce = 14;
zakir-code marked this conversation as resolved.
Show resolved Hide resolved
}
47 changes: 46 additions & 1 deletion proto/fx/gravity/crosschain/v1/legacy.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,4 +123,49 @@ message MsgOracleSetConfirmResponse { option deprecated = true; }

message MsgConfirmBatchResponse { option deprecated = true; }

message MsgBridgeCallConfirmResponse { option deprecated = true; }
message MsgBridgeCallConfirmResponse { option deprecated = true; }
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Deprecated MsgBridgeCallConfirmResponse is Still Referenced

  • Found reference in proto/fx/gravity/crosschain/v1/tx.proto
🔗 Analysis chain

Verify usage of deprecated MsgBridgeCallConfirmResponse

The MsgBridgeCallConfirmResponse message is marked as deprecated. Please confirm that all references to this message in the codebase have been properly updated or handled to prevent unintended usage.

Run the following script to identify any remaining usages:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find all references to MsgBridgeCallConfirmResponse in .proto files

rg --type proto 'MsgBridgeCallConfirmResponse'

Length of output: 76

Script:

#!/bin/bash
# Description: Find all references to MsgBridgeCallConfirmResponse in .proto files

rg 'MsgBridgeCallConfirmResponse' --glob '*.proto'

Length of output: 247


message BridgeToken {
option deprecated = true;
string token = 1;
string denom = 2;
// Deprecated: after upgrade v3
string channel_ibc = 3;
}
Comment on lines +128 to +134
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid adding new messages that are immediately deprecated

The BridgeToken message is added and immediately marked as deprecated. This could cause confusion for developers and users of the API. If these messages are no longer intended for use, consider not adding them at all or providing a clear rationale for their inclusion and immediate deprecation.


message BatchFees {
option deprecated = true;
string token_contract = 1;
string total_fees = 2 [
(cosmos_proto.scalar) = "cosmos.Int",
(gogoproto.customtype) = "cosmossdk.io/math.Int",
(gogoproto.nullable) = false
];
uint64 total_txs = 3;
string total_amount = 4 [
(cosmos_proto.scalar) = "cosmos.Int",
(gogoproto.customtype) = "cosmossdk.io/math.Int",
(gogoproto.nullable) = false
];
}
Comment on lines +136 to +150
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Reevaluate the addition of deprecated messages

Similarly, the BatchFees message is introduced with a deprecation notice. Adding new messages that are deprecated from the outset may lead to maintenance challenges and confusion. Ensure that adding these messages is necessary and that there is documentation explaining their purpose despite being deprecated.


message MinBatchFee {
option deprecated = true;
string token_contract = 1;
string base_fee = 2 [
(cosmos_proto.scalar) = "cosmos.Int",
(gogoproto.customtype) = "cosmossdk.io/math.Int",
(gogoproto.nullable) = false
];
}
Comment on lines +152 to +160
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Clarify the necessity of deprecated MinBatchFee message

The MinBatchFee message is also added and marked as deprecated immediately. Please verify whether this message is required. If it is included for backward compatibility or future use, provide appropriate documentation.


message QueryBatchFeeRequest {
option deprecated = true;
string chain_name = 1;
repeated MinBatchFee min_batch_fees = 2 [ (gogoproto.nullable) = false ];
}

Comment on lines +162 to +167
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Check the use of nullable option on repeated fields

In the QueryBatchFeeRequest message, the field min_batch_fees is defined as:

repeated MinBatchFee min_batch_fees = 2 [ (gogoproto.nullable) = false ];

Setting (gogoproto.nullable) = false on a repeated field may be unnecessary, as repeated fields are lists and are not nullable by default in Protobuf. Consider removing this option to avoid potential issues with code generation and serialization.

message QueryBatchFeeResponse {
option deprecated = true;
repeated BatchFees batch_fees = 1;
}
Comment on lines +168 to +171
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Assess the immediate deprecation of QueryBatchFeeResponse

The QueryBatchFeeResponse message is added and marked as deprecated. If this message is no longer needed, consider excluding it entirely. If it must be included, ensure that its deprecation status and purpose are clearly documented to aid developers.

7 changes: 1 addition & 6 deletions proto/fx/gravity/crosschain/v1/query.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import "cosmos/base/v1beta1/coin.proto";
import "cosmos_proto/cosmos.proto";
import "fx/gravity/crosschain/v1/tx.proto";
import "fx/gravity/crosschain/v1/types.proto";
import "fx/gravity/crosschain/v1/legacy.proto";
import "gogoproto/gogo.proto";
import "google/api/annotations.proto";
import "google/protobuf/any.proto";
Expand Down Expand Up @@ -187,12 +188,6 @@ message QueryLastPendingOracleSetRequestByAddrResponse {
repeated OracleSet oracle_sets = 1;
}

message QueryBatchFeeRequest {
string chain_name = 1;
repeated MinBatchFee min_batch_fees = 2 [ (gogoproto.nullable) = false ];
}
message QueryBatchFeeResponse { repeated BatchFees batch_fees = 1; }

message QueryLastPendingBatchRequestByAddrRequest {
string chain_name = 1;
string bridger_address = 2;
Expand Down
32 changes: 0 additions & 32 deletions proto/fx/gravity/crosschain/v1/types.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,6 @@ message LastObservedBlockHeight {
uint64 block_height = 2;
}

// BridgeToken
message BridgeToken {
string token = 1;
string denom = 2;
// Deprecated: after upgrade v3
string channel_ibc = 3;
}

// ClaimType is the cosmos type of an event from the counterpart chain that can
// be handled
enum ClaimType {
Expand Down Expand Up @@ -124,30 +116,6 @@ message ERC20Token {
];
}

message BatchFees {
string token_contract = 1;
string total_fees = 2 [
(cosmos_proto.scalar) = "cosmos.Int",
(gogoproto.customtype) = "cosmossdk.io/math.Int",
(gogoproto.nullable) = false
];
uint64 total_txs = 3;
string total_amount = 4 [
(cosmos_proto.scalar) = "cosmos.Int",
(gogoproto.customtype) = "cosmossdk.io/math.Int",
(gogoproto.nullable) = false
];
}

message MinBatchFee {
string token_contract = 1;
string base_fee = 2 [
(cosmos_proto.scalar) = "cosmos.Int",
(gogoproto.customtype) = "cosmossdk.io/math.Int",
(gogoproto.nullable) = false
];
}

// Params represent the Gravity genesis and store parameters
// gravity_id:
// a random 32 byte value to prevent signature reuse, for example if the
Expand Down
6 changes: 3 additions & 3 deletions x/crosschain/keeper/bridge_token.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func (k Keeper) AddBridgeTokenExecuted(ctx sdk.Context, claim *types.MsgBridgeTo
return types.ErrInvalid.Wrapf("%s denom decimals not match %d, expect %d",
fxtypes.DefaultDenom, claim.Decimals, fxtypes.DenomUnit)
}
bridgeToken, err := k.erc20Keeper.GetBridgeToken(ctx, claim.Symbol, k.moduleName)
bridgeToken, err := k.erc20Keeper.GetBridgeToken(ctx, k.moduleName, claim.Symbol)
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent GetBridgeToken Parameter Order Detected

The verification script identified multiple calls to GetBridgeToken with varying second parameters. While some instances correctly use moduleName, others use chainName, which may lead to inconsistencies and potential runtime errors. Please ensure that all calls pass moduleName as the second parameter to maintain consistency across the codebase.

🔗 Analysis chain

LGTM. Verify consistent application of parameter reordering.

The change in the order of parameters for k.erc20Keeper.GetBridgeToken looks good and aligns with the reported modifications. However, it's crucial to ensure this change has been consistently applied across all calls to GetBridgeToken in the codebase to prevent potential runtime errors.

To verify the consistent application of this change, run the following script:

If the script returns any results, those might be instances where the parameter reordering hasn't been applied and should be addressed.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining calls to GetBridgeToken with the old parameter order

# Test: Search for GetBridgeToken calls. Expect: All calls should have moduleName as the second parameter.
rg --type go 'GetBridgeToken\s*\([^,]+,\s*[^,]+,\s*[^,]+\)' -g '!vendor/'

Length of output: 1685

if err != nil {
return err
}
Expand All @@ -49,13 +49,13 @@ func (k Keeper) BridgeCoinSupply(ctx context.Context, token, target string) (sdk
}
var targetDenom string
if fxTarget.IsIBC() {
ibcToken, err := k.erc20Keeper.GetIBCToken(ctx, baseDenom, fxTarget.IBCChannel)
ibcToken, err := k.erc20Keeper.GetIBCToken(ctx, fxTarget.IBCChannel, baseDenom)
if err != nil {
return sdk.Coin{}, err
}
targetDenom = ibcToken.IbcDenom
} else {
bridgeToken, err := k.erc20Keeper.GetBridgeToken(ctx, baseDenom, fxTarget.GetModuleName())
bridgeToken, err := k.erc20Keeper.GetBridgeToken(ctx, fxTarget.GetModuleName(), baseDenom)
if err != nil {
return sdk.Coin{}, err
}
Expand Down
19 changes: 14 additions & 5 deletions x/crosschain/keeper/grpc_query.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ func (k QueryServer) DenomToToken(c context.Context, req *types.QueryDenomToToke
return nil, status.Error(codes.InvalidArgument, "denom")
}

bridgeToken, err := k.erc20Keeper.GetBridgeToken(c, req.Denom, req.ChainName)
bridgeToken, err := k.erc20Keeper.GetBridgeToken(c, req.ChainName, req.Denom)
if err != nil {
return nil, status.Error(codes.NotFound, err.Error())
}
Expand All @@ -207,7 +207,7 @@ func (k QueryServer) TokenToDenom(c context.Context, req *types.QueryTokenToDeno
if err != nil {
return nil, status.Error(codes.NotFound, err.Error())
}
_, err = k.erc20Keeper.GetBridgeToken(c, baseDenom, req.ChainName)
_, err = k.erc20Keeper.GetBridgeToken(c, req.ChainName, baseDenom)
if err != nil {
return nil, status.Error(codes.NotFound, err.Error())
}
Expand Down Expand Up @@ -282,9 +282,18 @@ func (k QueryServer) ProjectedBatchTimeoutHeight(c context.Context, _ *types.Que
return &types.QueryProjectedBatchTimeoutHeightResponse{TimeoutHeight: timeout}, nil
}

func (k QueryServer) BridgeTokens(c context.Context, _ *types.QueryBridgeTokensRequest) (*types.QueryBridgeTokensResponse, error) {
bridgeTokens := make([]*types.BridgeToken, 0)
// todo: need implement
func (k QueryServer) BridgeTokens(c context.Context, req *types.QueryBridgeTokensRequest) (*types.QueryBridgeTokensResponse, error) {
tokens, err := k.erc20Keeper.GetBridgeTokens(c, req.ChainName)
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}
Comment on lines +286 to +289
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refine error handling for GetBridgeTokens to provide more specific gRPC error codes.

Currently, any error from k.erc20Keeper.GetBridgeTokens(c, req.ChainName) results in an Internal error code. Consider returning more specific error codes based on the nature of the error (e.g., codes.NotFound if the chain name is not found, or codes.InvalidArgument if the chain name is invalid) to offer clearer feedback to clients.

Apply this diff to enhance error handling:

 tokens, err := k.erc20Keeper.GetBridgeTokens(c, req.ChainName)
 if err != nil {
-	return nil, status.Error(codes.Internal, err.Error())
+	if errors.Is(err, types.ErrChainNotFound) {
+		return nil, status.Error(codes.NotFound, err.Error())
+	} else if errors.Is(err, types.ErrInvalidChainName) {
+		return nil, status.Error(codes.InvalidArgument, err.Error())
+	} else {
+		return nil, status.Error(codes.Internal, err.Error())
+	}
 }

Ensure that types.ErrChainNotFound and types.ErrInvalidChainName are the actual error variables used in your codebase to represent these error conditions.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
tokens, err := k.erc20Keeper.GetBridgeTokens(c, req.ChainName)
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}
tokens, err := k.erc20Keeper.GetBridgeTokens(c, req.ChainName)
if err != nil {
if errors.Is(err, types.ErrChainNotFound) {
return nil, status.Error(codes.NotFound, err.Error())
} else if errors.Is(err, types.ErrInvalidChainName) {
return nil, status.Error(codes.InvalidArgument, err.Error())
} else {
return nil, status.Error(codes.Internal, err.Error())
}
}

bridgeTokens := make([]*types.BridgeToken, 0, len(tokens))
for _, token := range tokens {
bridgeTokens = append(bridgeTokens, &types.BridgeToken{
Token: token.Contract,
Denom: token.BridgeDenom(),
})
}
return &types.QueryBridgeTokensResponse{BridgeTokens: bridgeTokens}, nil
}

Expand Down
6 changes: 3 additions & 3 deletions x/crosschain/keeper/many_to_one.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (k Keeper) BridgeTokenToBaseCoin(ctx context.Context, holder sdk.AccAddress
}

func (k Keeper) BaseCoinToBridgeToken(ctx context.Context, holder sdk.AccAddress, baseCoin sdk.Coin) (bridgeToken erc20types.BridgeToken, err error) {
bridgeToken, err = k.erc20Keeper.GetBridgeToken(ctx, baseCoin.Denom, k.moduleName)
bridgeToken, err = k.erc20Keeper.GetBridgeToken(ctx, k.moduleName, baseCoin.Denom)
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent Parameter Order in GetBridgeToken Calls

The following instances of GetBridgeToken still use the old parameter order and need to be updated:

  • x/crosschain/keeper/bridge_token.go:

    bridgeToken, err := k.erc20Keeper.GetBridgeToken(ctx, fxTarget.GetModuleName(), baseDenom)

  • x/crosschain/keeper/many_to_one.go:

    bridgeToken, err = k.GetBridgeToken(ctx, tokenAddr)
🔗 Analysis chain

LGTM. Verify consistency across the codebase.

The change in parameter order for the GetBridgeToken call looks good. This appears to be part of a larger refactoring effort to standardize parameter order.

To ensure consistency, please run the following script to check for any remaining instances of the old parameter order:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining instances of the old GetBridgeToken parameter order

# Test: Search for GetBridgeToken calls with the old parameter order
rg --type go 'GetBridgeToken\s*\([^,]+,\s*[^,]+\)'

Length of output: 2587

if err != nil {
return bridgeToken, err
}
Expand Down Expand Up @@ -106,7 +106,7 @@ func (k Keeper) IBCCoinToBaseCoin(ctx context.Context, holder sdk.AccAddress, ib
}

func (k Keeper) BaseCoinToIBCCoin(ctx context.Context, holder sdk.AccAddress, baseCoin sdk.Coin, channel string) (sdk.Coin, error) {
ibcToken, err := k.erc20Keeper.GetIBCToken(ctx, baseCoin.Denom, channel)
ibcToken, err := k.erc20Keeper.GetIBCToken(ctx, channel, baseCoin.Denom)
if err != nil {
// NOTE: if not found in IBCToken
return baseCoin, nil
Expand Down Expand Up @@ -168,5 +168,5 @@ func (k Keeper) GetBridgeToken(ctx context.Context, tokenAddr string) (erc20type
if err != nil {
return erc20types.BridgeToken{}, err
}
return k.erc20Keeper.GetBridgeToken(ctx, baseDenom, k.moduleName)
return k.erc20Keeper.GetBridgeToken(ctx, k.moduleName, baseDenom)
}
31 changes: 23 additions & 8 deletions x/crosschain/mock/expected_keepers_mocks.go
View file
Open in desktop

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

5 changes: 3 additions & 2 deletions x/crosschain/types/expected_keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,10 @@ type Erc20Keeper interface {

GetERC20Token(ctx context.Context, baseDenom string) (erc20types.ERC20Token, error)

GetBridgeToken(ctx context.Context, baseDenom, chainName string) (erc20types.BridgeToken, error)
GetBridgeToken(ctx context.Context, chainName, baseDenom string) (erc20types.BridgeToken, error)
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent GetBridgeToken Method Signatures Detected

Multiple instances of GetBridgeToken have been found with differing parameter counts:

  • Two Parameters:

    • x/crosschain/keeper/many_to_one.go: GetBridgeToken(ctx, tokenAddr)
    • x/crosschain/keeper/many_to_one.go: func (k Keeper) GetBridgeToken(ctx context.Context, tokenAddr string) (erc20types.BridgeToken, error)

  • Three Parameters (Updated Order):

    • All other occurrences align with the updated signature (ctx context.Context, chainName, baseDenom string)

This inconsistency may lead to potential bugs or runtime errors.

🔗 Analysis chain

Approve parameter reordering for GetBridgeToken

The reordering of parameters from (ctx context.Context, baseDenom, chainName string) to (ctx context.Context, chainName, baseDenom string) improves readability and consistency. It follows a logical order from broader (chainName) to more specific (baseDenom).

Please ensure all calls to this method throughout the codebase are updated to reflect this change. Run the following script to identify potential places that need updating:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for calls to GetBridgeToken that might need updating
rg --type go 'GetBridgeToken\s*\([^)]*\)' -g '!x/crosschain/types/expected_keepers.go'

Length of output: 832

Script:

#!/bin/bash
# Search for calls to GetBridgeToken that might need updating
rg --type go 'GetBridgeToken\s*\([^)]*\)' -g '!x/crosschain/types/expected_keepers.go'

Length of output: 1449

Script:

#!/bin/bash
# Search for calls to GetBridgeToken that might need updating
rg --type go 'GetBridgeToken\s*\([^)]*\)' -g '!x/crosschain/types/expected_keepers.go'

Length of output: 1449

GetBridgeTokens(ctx context.Context, chainName string) ([]erc20types.BridgeToken, error)

GetIBCToken(ctx context.Context, baseDenom, channel string) (erc20types.IBCToken, error)
GetIBCToken(ctx context.Context, channel, baseDenom string) (erc20types.IBCToken, error)
}

// EVMKeeper defines the expected EVM keeper interface used on crosschain
Expand Down
Loading