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

BE-676 | Implement InGivenOut APIs for CL pool #604

Merged
merged 9 commits into from
Jan 28, 2025
Prev Previous commit
Next Next commit
BE-676 | Implement ChargeTakerFeeExactOut
  • Loading branch information
deividaspetraitis committed Jan 21, 2025
commit acf6e4e6e25d2004850d94298b10dfa62494fce2
7 changes: 4 additions & 3 deletions router/usecase/pools/routable_concentrated_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@
currentSqrtPrice = concentratedPool.GetCurrentSqrtPrice()

amountRemainingOut = tokenOut.Amount.ToLegacyDec()
amountInTotal = osmomath.ZeroDec()

Check failure on line 260 in router/usecase/pools/routable_concentrated_pool.go

View workflow job for this annotation

GitHub Actions / Run linter

File is not `gofmt`-ed with `-s` (gofmt)
)

if currentSqrtPrice.IsZero() {
Expand Down Expand Up @@ -328,17 +328,18 @@
return fmt.Sprintf("pool (%d), pool type (%d), pool denoms (%v), token out (%s)", concentratedPool.Id, poolmanagertypes.Concentrated, concentratedPool.GetPoolDenoms(sdk.Context{}), r.TokenOutDenom)
}

// ChargeTakerFee implements domain.RoutablePool.
// ChargeTakerFeeExactIn implements domain.RoutablePool.
// Charges the taker fee for the given token in and returns the token in after the fee has been charged.
func (r *routableConcentratedPoolImpl) ChargeTakerFeeExactIn(tokenIn sdk.Coin) (tokenInAfterFee sdk.Coin) {
tokenInAfterTakerFee, _ := poolmanager.CalcTakerFeeExactIn(tokenIn, r.GetTakerFee())
Comment on lines +333 to 336
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle potential errors returned by CalcTakerFeeExactIn.

In ChargeTakerFeeExactIn, the error returned by CalcTakerFeeExactIn is being ignored. Ignoring errors can lead to unintended consequences or silent failures. Consider handling the error to ensure robust error management.

Apply this diff to handle the error:

 func (r *routableConcentratedPoolImpl) ChargeTakerFeeExactIn(tokenIn sdk.Coin) (tokenInAfterFee sdk.Coin, err error) {
-	tokenInAfterTakerFee, _ := poolmanager.CalcTakerFeeExactIn(tokenIn, r.GetTakerFee())
+	tokenInAfterTakerFee, err := poolmanager.CalcTakerFeeExactIn(tokenIn, r.GetTakerFee())
+	if err != nil {
+		return sdk.Coin{}, err
+	}
 	return tokenInAfterTakerFee, nil
 }
📝 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
// ChargeTakerFeeExactIn implements domain.RoutablePool.
// Charges the taker fee for the given token in and returns the token in after the fee has been charged.
func (r *routableConcentratedPoolImpl) ChargeTakerFeeExactIn(tokenIn sdk.Coin) (tokenInAfterFee sdk.Coin) {
tokenInAfterTakerFee, _ := poolmanager.CalcTakerFeeExactIn(tokenIn, r.GetTakerFee())
// ChargeTakerFeeExactIn implements domain.RoutablePool.
// Charges the taker fee for the given token in and returns the token in after the fee has been charged.
func (r *routableConcentratedPoolImpl) ChargeTakerFeeExactIn(tokenIn sdk.Coin) (tokenInAfterFee sdk.Coin, err error) {
tokenInAfterTakerFee, err := poolmanager.CalcTakerFeeExactIn(tokenIn, r.GetTakerFee())
if err != nil {
return sdk.Coin{}, err
}
return tokenInAfterTakerFee, nil
}

return tokenInAfterTakerFee
}

// ChargeTakerFee implements domain.RoutablePool.
// ChargeTakerFeeExactOut implements domain.RoutablePool.
// Charges the taker fee for the given token out and returns the token out after the fee has been charged.
func (r *routableConcentratedPoolImpl) ChargeTakerFeeExactOut(tokenOut sdk.Coin) (tokenOutAfterFee sdk.Coin) {
return sdk.Coin{}
tokenOutAfterTakerFee, _ := poolmanager.CalcTakerFeeExactOut(tokenOut, r.GetTakerFee())
return tokenOutAfterTakerFee
Comment on lines +340 to +344
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle potential errors returned by CalcTakerFeeExactOut.

Similarly, in ChargeTakerFeeExactOut, the error returned by CalcTakerFeeExactOut is being ignored. Proper error handling is crucial for reliable software. Consider modifying the function to handle the error appropriately.

Apply this diff to handle the error:

 func (r *routableConcentratedPoolImpl) ChargeTakerFeeExactOut(tokenOut sdk.Coin) (tokenOutAfterFee sdk.Coin, err error) {
-	tokenOutAfterTakerFee, _ := poolmanager.CalcTakerFeeExactOut(tokenOut, r.GetTakerFee())
+	tokenOutAfterTakerFee, err := poolmanager.CalcTakerFeeExactOut(tokenOut, r.GetTakerFee())
+	if err != nil {
+		return sdk.Coin{}, err
+	}
 	return tokenOutAfterTakerFee, nil
 }
📝 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
// ChargeTakerFeeExactOut implements domain.RoutablePool.
// Charges the taker fee for the given token out and returns the token out after the fee has been charged.
func (r *routableConcentratedPoolImpl) ChargeTakerFeeExactOut(tokenOut sdk.Coin) (tokenOutAfterFee sdk.Coin) {
return sdk.Coin{}
tokenOutAfterTakerFee, _ := poolmanager.CalcTakerFeeExactOut(tokenOut, r.GetTakerFee())
return tokenOutAfterTakerFee
// ChargeTakerFeeExactOut implements domain.RoutablePool.
// Charges the taker fee for the given token out and returns the token out after the fee has been charged.
func (r *routableConcentratedPoolImpl) ChargeTakerFeeExactOut(tokenOut sdk.Coin) (tokenOutAfterFee sdk.Coin, err error) {
tokenOutAfterTakerFee, err := poolmanager.CalcTakerFeeExactOut(tokenOut, r.GetTakerFee())
if err != nil {
return sdk.Coin{}, err
}
return tokenOutAfterTakerFee, nil
}

}

// SetTokenInDenom implements domain.RoutablePool.
Expand Down
Loading