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

[TRA-78] Add function to retrieve collateral pool addr for a subaccount #1142

Merged
merged 4 commits into from
Mar 5, 2024
Merged

[TRA-78] Add function to retrieve collateral pool addr for a subaccount #1142

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9728b5a
Add function to retrieve collateral pool addr for a subaccount
shrenujb Mar 5, 2024
a793224
Merge branch 'main' into tra78
shrenujb Mar 5, 2024
571dee5
add tests and address comments
shrenujb Mar 5, 2024
9ac1711
fix lint
shrenujb Mar 5, 2024
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
53 changes: 53 additions & 0 deletions protocol/x/subaccounts/keeper/subaccount.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"math/rand"
Copy link
Contributor

Choose a reason for hiding this comment

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

The usage of math/rand is noted. For cryptographic operations or where security is a concern, it's recommended to use crypto/rand instead to ensure the randomness is secure. This change is crucial for functions that require secure random number generation.

- "math/rand"
+ "crypto/rand"
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.

Suggested change
"math/rand"
"crypto/rand"

"time"

authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"

"github.com/cosmos/gogoproto/proto"

storetypes "cosmossdk.io/store/types"
Expand Down Expand Up @@ -52,6 +54,57 @@ func (k Keeper) SetSubaccount(ctx sdk.Context, subaccount types.Subaccount) {
}
}

// GetCollateralPoolForSubaccount returns the collateral pool address for a subaccount
// based on the subaccount's perpetual positions. If the subaccount holds a position in an isolated
// market, the collateral pool address will be the isolated market's pool address. Otherwise, the
// collateral pool address will be the module's pool address.
func (k Keeper) GetCollateralPoolForSubaccount(ctx sdk.Context, subaccountId types.SubaccountId) (
sdk.AccAddress,
error,
) {
poolName, err := k.GetCollateralPoolNameForSubaccount(ctx, subaccountId)
if err != nil {
return nil, err
}

return authtypes.NewModuleAddress(poolName), nil
}

func (k Keeper) GetCollateralPoolNameForSubaccount(ctx sdk.Context, subaccountId types.SubaccountId) (string, error) {
subaccount := k.GetSubaccount(ctx, subaccountId)
if len(subaccount.PerpetualPositions) == 0 {
return types.ModuleName, nil
}

// Get the first perpetual position and return the collateral pool name.
perpetual, err := k.perpetualsKeeper.GetPerpetual(ctx, subaccount.PerpetualPositions[0].PerpetualId)
if err != nil {
panic(fmt.Sprintf("GetCollateralPoolNameForSubaccount: %v", err))
}

if perpetual.Params.MarketType == perptypes.PerpetualMarketType_PERPETUAL_MARKET_TYPE_ISOLATED {
return types.ModuleName + ":" + lib.UintToString(perpetual.GetId()), nil
}

return types.ModuleName, nil
}
Comment on lines +72 to +89
Copy link
Contributor

Choose a reason for hiding this comment

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

Using panic within GetCollateralPoolNameForSubaccount for error handling is not recommended as it can terminate the entire application. Consider returning the error to the caller to handle it more gracefully.

- panic(fmt.Sprintf("GetCollateralPoolNameForSubaccount: %v", err))
+ return "", fmt.Errorf("GetCollateralPoolNameForSubaccount: %w", err)
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.

Suggested change
func (k Keeper) GetCollateralPoolNameForSubaccount(ctx sdk.Context, subaccountId types.SubaccountId) (string, error) {
subaccount := k.GetSubaccount(ctx, subaccountId)
if len(subaccount.PerpetualPositions) == 0 {
return types.ModuleName, nil
}
// Get the first perpetual position and return the collateral pool name.
perpetual, err := k.perpetualsKeeper.GetPerpetual(ctx, subaccount.PerpetualPositions[0].PerpetualId)
if err != nil {
panic(fmt.Sprintf("GetCollateralPoolNameForSubaccount: %v", err))
}
if perpetual.Params.MarketType == perptypes.PerpetualMarketType_PERPETUAL_MARKET_TYPE_ISOLATED {
return types.ModuleName + ":" + lib.UintToString(perpetual.GetId()), nil
}
return types.ModuleName, nil
}
func (k Keeper) GetCollateralPoolNameForSubaccount(ctx sdk.Context, subaccountId types.SubaccountId) (string, error) {
subaccount := k.GetSubaccount(ctx, subaccountId)
if len(subaccount.PerpetualPositions) == 0 {
return types.ModuleName, nil
}
// Get the first perpetual position and return the collateral pool name.
perpetual, err := k.perpetualsKeeper.GetPerpetual(ctx, subaccount.PerpetualPositions[0].PerpetualId)
if err != nil {
return "", fmt.Errorf("GetCollateralPoolNameForSubaccount: %w", err)
}
if perpetual.Params.MarketType == perptypes.PerpetualMarketType_PERPETUAL_MARKET_TYPE_ISOLATED {
return types.ModuleName + ":" + lib.UintToString(perpetual.GetId()), nil
}
return types.ModuleName, nil
}


// IsIsolatedMarketSubaccount returns whether a subaccount is isolated to a specific market.
func (k Keeper) IsIsolatedMarketSubaccount(ctx sdk.Context, subaccountId types.SubaccountId) (bool, error) {
shrenujb marked this conversation as resolved.
Show resolved Hide resolved
subaccount := k.GetSubaccount(ctx, subaccountId)
if len(subaccount.PerpetualPositions) == 0 {
return false, nil
}

// Get the first perpetual position and return whether it is an isolated market.
perpetual, err := k.perpetualsKeeper.GetPerpetual(ctx, subaccount.PerpetualPositions[0].PerpetualId)
if err != nil {
return false, err
}

return perpetual.Params.MarketType == perptypes.PerpetualMarketType_PERPETUAL_MARKET_TYPE_ISOLATED, nil
}

// GetSubaccount returns a subaccount from its index.
//
// Note that this function is getting called very frequently; metrics in this function
Expand Down
Loading