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

refactor(x/participationrewards/keeper): combine GetProtocolData+UnmarshalProtocolData #1681

Conversation

odeke-em
Copy link
Contributor

@odeke-em odeke-em commented Jun 29, 2024

This change combines GetProtocolData and types.UnmarshalProtocolData into a generic function that unifies the functionality and the pattern.

Fixes #1631

Summary by CodeRabbit

  • Refactor

    • Improved data retrieval and unmarshalling process for various pool types to enhance code maintainability and simplify error handling.
  • Bug Fixes

    • Corrected data assignments in callback functions to ensure accurate protocol data updates and enhance system reliability.

Copy link

vercel bot commented Jun 29, 2024

@odeke-em is attempting to deploy a commit to the quicksilver Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

coderabbitai bot commented Jun 29, 2024

Walkthrough

The changes streamlined data retrieval and unmarshalling across several callback functions in the participationrewards/keeper package. The primary improvement is the introduction of a unified function, GetAndUnmarshalProtocolData, which centralizes and simplifies the process of handling protocol data. This refactor enhances code maintainability and reduces redundancy.

Changes

File Path Change Summary
x/participationrewards/keeper/callbacks.go Renamed variables for protocol data and refactored error handling and data assignments.
x/participationrewards/keeper/callbacks_test.go Refactored to use GetAndUnmarshalProtocolData for data retrieval and unmarshalling.
x/participationrewards/keeper/distribution.go Updated CalcTokenValues to use GetAndUnmarshalProtocolData.
x/participationrewards/keeper/protocol_data.go Added GetAndUnmarshalProtocolData to centralize protocol data handling.

Sequence Diagram(s)

sequenceDiagram
    participant SDK.Context as SDK.Context
    participant Keeper as Keeper
    participant ProtocolData as ProtocolData
    participant GetAndUnmarshal as GetAndUnmarshalProtocolData
    
    SDK.Context ->> Keeper: Call callback function
    Keeper ->> GetAndUnmarshal: Retrieve and unmarshal protocol data
    GetAndUnmarshal ->> ProtocolData: Fetch data
    ProtocolData -->> GetAndUnmarshal: Return data
    GetAndUnmarshal -->> Keeper: Return unmarshalled data
    Keeper -->> SDK.Context: Use protocol data in logic
Loading

Assessment against linked issues

Objective (Issue #1631) Addressed Explanation
Refactor data retrieval with GetAndUnmarshalProtocolData
Simplify protocol data handling logic

Poem

Amid the code of bytes and lines,
A rabbit hopped with bright designs,
To simplify and make it right,
With functions new, in day and night.
Protocols now clear and clean,
Data flows in a unified stream.
🐇✨


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
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Outside diff range and nitpick comments (2)
x/participationrewards/keeper/callbacks.go (2)

Line range hint 302-309: Fix syntax errors

The function UmeeUTokenSupplyUpdateCallback has several issues:

  • The variable err is used but not defined.
- data, supply, err := GetAndUnmarshalProtocolData[*types.UmeeUTokenSupplyProtocolData](ctx, k, denom, types.ProtocolDataTypeUmeeUTokenSupply)
+ data, supply, err := GetAndUnmarshalProtocolData[*types.UmeeUTokenSupplyProtocolData](ctx, k, denom, types.ProtocolDataTypeUmeeUTokenSupply)
+ if err != nil {
+     return err
+ }

Line range hint 338-345: Fix syntax errors

The function UmeeLeverageModuleBalanceUpdateCallback has several issues:

  • The variable err is used but not defined.
- data, balance, err := GetAndUnmarshalProtocolData[*types.UmeeLeverageModuleBalanceProtocolData](ctx, k, denom, types.ProtocolDataTypeUmeeLeverageModuleBalance)
+ data, balance, err := GetAndUnmarshalProtocolData[*types.UmeeLeverageModuleBalanceProtocolData](ctx, k, denom, types.ProtocolDataTypeUmeeLeverageModuleBalance)
+ if err != nil {
+     return err
+ }
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 5fd3a24 and a3376df.

Files selected for processing (2)
  • x/participationrewards/keeper/callbacks.go (8 hunks)
  • x/participationrewards/keeper/protocol_data.go (2 hunks)
Additional context used
GitHub Check: build quicksilver (darwin, arm64)
x/participationrewards/keeper/callbacks.go

[failure] 143-143:
ok declared and not used


[failure] 143-143:
not enough arguments in call to GetAndUnmarshalProtocolData[*types.OsmosisPoolProtocolData]


[failure] 144-144:
undefined: err


[failure] 145-145:
undefined: err


[failure] 148-148:
undefined: err


[failure] 149-149:
undefined: err


[failure] 150-150:
undefined: err


[failure] 153-153:
undefined: err


[failure] 154-154:
undefined: err


[failure] 155-155:
undefined: err

GitHub Check: test quicksilver (darwin, arm64)
x/participationrewards/keeper/callbacks.go

[failure] 143-143:
ok declared and not used


[failure] 143-143:
not enough arguments in call to GetAndUnmarshalProtocolData[*types.OsmosisPoolProtocolData]


[failure] 144-144:
undefined: err


[failure] 145-145:
undefined: err


[failure] 148-148:
undefined: err


[failure] 149-149:
undefined: err


[failure] 150-150:
undefined: err


[failure] 153-153:
undefined: err


[failure] 154-154:
undefined: err


[failure] 155-155:
undefined: err

GitHub Check: build quicksilver (amd64, darwin)
x/participationrewards/keeper/callbacks.go

[failure] 143-143:
ok declared and not used


[failure] 143-143:
not enough arguments in call to GetAndUnmarshalProtocolData[*types.OsmosisPoolProtocolData]


[failure] 144-144:
undefined: err


[failure] 145-145:
undefined: err


[failure] 148-148:
undefined: err


[failure] 149-149:
undefined: err


[failure] 150-150:
undefined: err


[failure] 153-153:
undefined: err


[failure] 154-154:
undefined: err


[failure] 155-155:
undefined: err

GitHub Check: test quicksilver (amd64, windows)
x/participationrewards/keeper/callbacks.go

[failure] 143-143:
ok declared and not used


[failure] 143-143:
not enough arguments in call to GetAndUnmarshalProtocolData[*types.OsmosisPoolProtocolData]


[failure] 144-144:
undefined: err


[failure] 145-145:
undefined: err


[failure] 148-148:
undefined: err


[failure] 149-149:
undefined: err


[failure] 150-150:
undefined: err


[failure] 153-153:
undefined: err


[failure] 154-154:
undefined: err


[failure] 155-155:
undefined: err

GitHub Check: build quicksilver (amd64, windows)
x/participationrewards/keeper/callbacks.go

[failure] 143-143:
ok declared and not used


[failure] 143-143:
not enough arguments in call to GetAndUnmarshalProtocolData[*types.OsmosisPoolProtocolData]


[failure] 144-144:
undefined: err


[failure] 145-145:
undefined: err


[failure] 148-148:
undefined: err


[failure] 149-149:
undefined: err


[failure] 150-150:
undefined: err


[failure] 153-153:
undefined: err


[failure] 154-154:
undefined: err


[failure] 155-155:
undefined: err

GitHub Check: build quicksilver (amd64, linux)
x/participationrewards/keeper/callbacks.go

[failure] 143-143:
ok declared and not used


[failure] 143-143:
not enough arguments in call to GetAndUnmarshalProtocolData[*types.OsmosisPoolProtocolData]


[failure] 144-144:
undefined: err


[failure] 145-145:
undefined: err


[failure] 148-148:
undefined: err


[failure] 149-149:
undefined: err


[failure] 150-150:
undefined: err


[failure] 153-153:
undefined: err


[failure] 154-154:
undefined: err


[failure] 155-155:
undefined: err

GitHub Check: lint
x/participationrewards/keeper/callbacks.go

[failure] 143-143:
ok declared and not used


[failure] 143-143:
not enough arguments in call to GetAndUnmarshalProtocolData[*types.OsmosisPoolProtocolData]


[failure] 144-144:
undefined: err


[failure] 145-145:
undefined: err


[failure] 148-148:
undefined: err


[failure] 149-149:
undefined: err


[failure] 150-150:
undefined: err


[failure] 153-153:
undefined: err


[failure] 154-154:
undefined: err


[failure] 155-155:
undefined: err

GitHub Check: test quicksilver (amd64, linux)
x/participationrewards/keeper/callbacks.go

[failure] 143-143:
ok declared and not used


[failure] 143-143:
not enough arguments in call to GetAndUnmarshalProtocolData[*types.OsmosisPoolProtocolData]


[failure] 144-144:
undefined: err


[failure] 145-145:
undefined: err


[failure] 148-148:
undefined: err


[failure] 149-149:
undefined: err


[failure] 150-150:
undefined: err


[failure] 153-153:
undefined: err


[failure] 154-154:
undefined: err


[failure] 155-155:
undefined: err

Additional comments not posted (5)
x/participationrewards/keeper/protocol_data.go (1)

43-57: LGTM!

The new function GetAndUnmarshalProtocolData correctly combines retrieval and unmarshaling of protocol data, handling error cases and type assertions effectively.

x/participationrewards/keeper/callbacks.go (4)

242-246: Fix syntax errors

The function UmeeTotalBorrowsUpdateCallback has several issues:

  • The variable err is used but not defined.
- data, borrows, err := GetAndUnmarshalProtocolData[*types.UmeeTotalBorrowsProtocolData](ctx, k, denom, types.ProtocolDataTypeUmeeTotalBorrows)
+ data, borrows, err := GetAndUnmarshalProtocolData[*types.UmeeTotalBorrowsProtocolData](ctx, k, denom, types.ProtocolDataTypeUmeeTotalBorrows)
+ if err != nil {
+     return err
+ }

Likely invalid or redundant comment.


212-216: Fix syntax errors

The function UmeeReservesUpdateCallback has several issues:

  • The variable err is used but not defined.
- data, reserves, err := GetAndUnmarshalProtocolData[*types.UmeeReservesProtocolData](ctx, k, denom, types.ProtocolDataTypeUmeeReserves)
+ data, reserves, err := GetAndUnmarshalProtocolData[*types.UmeeReservesProtocolData](ctx, k, denom, types.ProtocolDataTypeUmeeReserves)
+ if err != nil {
+     return err
+ }

Likely invalid or redundant comment.


182-186: Fix syntax errors

The function OsmosisClPoolUpdateCallback has several issues:

  • The variable err is used but not defined.
- data, pool, err := GetAndUnmarshalProtocolData[*types.OsmosisClPoolProtocolData](ctx, k, fmt.Sprintf("%d", poolID), types.ProtocolDataTypeOsmosisCLPool)
+ data, pool, err := GetAndUnmarshalProtocolData[*types.OsmosisClPoolProtocolData](ctx, k, fmt.Sprintf("%d", poolID), types.ProtocolDataTypeOsmosisCLPool)
+ if err != nil {
+     return err
+ }

Likely invalid or redundant comment.


272-276: Fix syntax errors

The function UmeeInterestScalarUpdateCallback has several issues:

  • The variable err is used but not defined.
- data, interest, err := GetAndUnmarshalProtocolData[*types.UmeeInterestScalarProtocolData](ctx, k, denom, types.ProtocolDataTypeUmeeInterestScalar)
+ data, interest, err := GetAndUnmarshalProtocolData[*types.UmeeInterestScalarProtocolData](ctx, k, denom, types.ProtocolDataTypeUmeeInterestScalar)
+ if err != nil {
+     return err
+ }

Likely invalid or redundant comment.

x/participationrewards/keeper/callbacks.go Show resolved Hide resolved
x/participationrewards/keeper/callbacks.go Outdated Show resolved Hide resolved
@odeke-em odeke-em force-pushed the x-participationrewards-centralize-GetProtocolData+UnmarshalProtocolData branch 3 times, most recently from b028e69 to 3d4753c Compare June 29, 2024 15:10
@odeke-em odeke-em changed the title refactor((x/participationrewards/keeper): combine GetProtocolData+UnmarshalProtocolData refactor(x/participationrewards/keeper): combine GetProtocolData+UnmarshalProtocolData Jun 29, 2024
@odeke-em odeke-em force-pushed the x-participationrewards-centralize-GetProtocolData+UnmarshalProtocolData branch 2 times, most recently from bbdbc3f to 79a0d1b Compare June 29, 2024 15:17
joe-bowman
joe-bowman previously approved these changes Jun 29, 2024
Copy link
Contributor

@joe-bowman joe-bowman left a comment

Choose a reason for hiding this comment

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

looks great, thank you!

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between a3376df and aba5a49.

Files selected for processing (4)
  • x/participationrewards/keeper/callbacks.go (8 hunks)
  • x/participationrewards/keeper/callbacks_test.go (8 hunks)
  • x/participationrewards/keeper/distribution.go (1 hunks)
  • x/participationrewards/keeper/protocol_data.go (2 hunks)
Files not reviewed due to errors (1)
  • x/participationrewards/keeper/callbacks_test.go (no review received)
Files skipped from review as they are similar to previous changes (2)
  • x/participationrewards/keeper/callbacks.go
  • x/participationrewards/keeper/protocol_data.go
Additional comments not posted (3)
x/participationrewards/keeper/distribution.go (3)

Line range hint 134-134: Verify error aggregation logic.

Ensure that the error aggregation logic correctly captures and reports all errors.

Verification successful

Line range hint 134-134: Verified error aggregation logic.

The error aggregation logic correctly captures and reports all errors in the CalcTokenValues function.

  • x/participationrewards/keeper/distribution.go:
    • Lines 170-172: if len(errs) > 0 { return nil, multierror.New(errs) }
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify error aggregation logic in the `CalcTokenValues` function.

# Test: Search for error aggregation logic in the `CalcTokenValues` function.
rg --type python $'if len(errs) > 0 {\n\treturn nil, multierror.New(errs)\n}'

Length of output: 377



Script:

#!/bin/bash
# Description: Verify error aggregation logic in the `CalcTokenValues` function.

# Test: Search for error aggregation logic in the `CalcTokenValues` function.
ast-grep --lang go --pattern $'if len(errs) > 0 {\n\treturn nil, multierror.New(errs)\n}'

Length of output: 301


Line range hint 76-76: Ensure consistency in type assertions and error handling.

Verify that the type assertion for ipool and the error handling logic are consistent with the first iteration.

Verification successful

Consistency in type assertions and error handling verified.

The type assertions and error handling logic for ipool in the second iteration are consistent with the first iteration.

  • Both iterations capture errors in the errs map.
  • Type assertions are performed similarly without additional error checking.
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify type assertions and error handling logic for `ipool` in the second iteration.

# Test: Search for type assertions and error handling in the second iteration of the `CalcTokenValues` function.
rg --type python $'ipool, err := types.UnmarshalProtocolData(types.ProtocolDataTypeOsmosisCLPool, data.Data)\npool, _ := ipool.(*types.OsmosisClPoolProtocolData)'

Length of output: 558



Script:

#!/bin/bash
# Description: Verify type assertions and error handling logic for `ipool` in the second iteration.

# Test: Search for type assertions and error handling in the second iteration of the `CalcTokenValues` function.
rg --type go 'ipool, err := types.UnmarshalProtocolData(types.ProtocolDataTypeOsmosisCLPool, data.Data)\npool, _ := ipool.(*types.OsmosisClPoolProtocolData)'

Length of output: 486



Script:

#!/bin/bash
# Description: Verify type assertions and error handling logic for `ipool` in the second iteration.

# Test: Search for type assertions and error handling in the second iteration of the `CalcTokenValues` function.
rg --type go 'ipool, err := types.UnmarshalProtocolData(types.ProtocolDataTypeOsmosisCLPool, data.Data).*pool, _ := ipool\.\(\*types\.OsmosisClPoolProtocolData\)'

Length of output: 164



Script:

#!/bin/bash
# Description: Verify type assertions and error handling logic for `ipool` in the `CalcTokenValues` function.

# Test: Search for all instances of `UnmarshalProtocolData` to review their context.
rg 'UnmarshalProtocolData'

Length of output: 8064



Script:

#!/bin/bash
# Description: Extract relevant lines from `x/participationrewards/keeper/distribution.go` to review type assertions and error handling logic.

# Extract lines containing `UnmarshalProtocolData` and their surrounding context.
rg --context 5 'UnmarshalProtocolData' x/participationrewards/keeper/distribution.go

Length of output: 1347


45-45: Ensure correct type assertion and error handling.

Verify that the type assertion for ipool and the error handling logic are correct.

Verification successful

Ensure correct type assertion and error handling.

The type assertion for ipool and the error handling logic in the CalcTokenValues function appear to be implemented correctly. The function checks for errors after unmarshalling protocol data and handles type assertions appropriately.

  • Lines 18-20: Error handling after unmarshalling protocol data.
  • Lines 21-22: Type assertion for ipool.

No further issues were found with the type assertion and error handling logic.

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify type assertions and error handling logic for `ipool`.

# Test: Search for type assertions and error handling in the `CalcTokenValues` function.
rg --type python $'ipool, err := types.UnmarshalProtocolData(types.ProtocolDataTypeOsmosisPool, data.Data)\npool, _ := ipool.(*types.OsmosisPoolProtocolData)'

Length of output: 550



Script:

#!/bin/bash
# Description: Verify type assertions and error handling logic for `ipool` in the `CalcTokenValues` function.

# Test: Search for type assertions and error handling in the `CalcTokenValues` function.
rg --type go $'ipool, err := types.UnmarshalProtocolData(types.ProtocolDataTypeOsmosisPool, data.Data)\npool, _ := ipool.\\(*types.OsmosisPoolProtocolData\\)'

Length of output: 348



Script:

#!/bin/bash
# Description: Verify type assertions and error handling logic for `ipool` in the `CalcTokenValues` function.

# Test: Search for type assertions and error handling in the `CalcTokenValues` function using multiline mode.
rg --type go -U $'ipool, err := types.UnmarshalProtocolData(types.ProtocolDataTypeOsmosisPool, data.Data)\npool, _ := ipool.\\(*types.OsmosisPoolProtocolData\\)'

Length of output: 159



Script:

#!/bin/bash
# Description: Locate the `CalcTokenValues` function and inspect its contents for type assertions and error handling.

# Step 1: Locate the `CalcTokenValues` function in the codebase.
rg --type go 'func CalcTokenValues'

# Step 2: If found, inspect the contents of the located function for type assertions and error handling.

Length of output: 37



Script:

#!/bin/bash
# Description: Locate all function definitions in the `x/participationrewards/keeper/distribution.go` file.

# Step 1: Search for all function definitions in the specified file.
rg --type go --line-number 'func ' x/participationrewards/keeper/distribution.go

Length of output: 767



Script:

#!/bin/bash
# Description: Extract and inspect the contents of the `CalcTokenValues` function for type assertions and error handling.

# Step 1: Extract the contents of the `CalcTokenValues` function.
rg --type go --context 50 --line-number '^func (k \*Keeper) CalcTokenValues' x/participationrewards/keeper/distribution.go

Length of output: 124



Script:

#!/bin/bash
# Description: Extract and inspect the contents of the `CalcTokenValues` function for type assertions and error handling.

# Step 1: Extract the contents of the `CalcTokenValues` function using `sed`.
sed -n '/^func (k \*Keeper) CalcTokenValues/,/^}/p' x/participationrewards/keeper/distribution.go

Length of output: 4108

x/participationrewards/keeper/distribution.go Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between aba5a49 and 493c0e8.

Files selected for processing (1)
  • x/participationrewards/keeper/distribution.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • x/participationrewards/keeper/distribution.go

@tuantran1702
Copy link
Contributor

Missing keeper argument also exists on several places, similar to the ‘distribution.go’ file, please fix @odeke-em

@odeke-em odeke-em force-pushed the x-participationrewards-centralize-GetProtocolData+UnmarshalProtocolData branch from 493c0e8 to f790f7f Compare June 30, 2024 14:35
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 493c0e8 and f790f7f.

Files selected for processing (4)
  • x/participationrewards/keeper/callbacks.go (8 hunks)
  • x/participationrewards/keeper/callbacks_test.go (8 hunks)
  • x/participationrewards/keeper/distribution.go (1 hunks)
  • x/participationrewards/keeper/protocol_data.go (2 hunks)
Files skipped from review as they are similar to previous changes (4)
  • x/participationrewards/keeper/callbacks.go
  • x/participationrewards/keeper/callbacks_test.go
  • x/participationrewards/keeper/distribution.go
  • x/participationrewards/keeper/protocol_data.go

@odeke-em
Copy link
Contributor Author

Missing keeper argument also exists on several places, similar to the ‘distribution.go’ file, please fix @odeke-em

Done, thanks for the ping @tropicaldog

@tuantran1702 tuantran1702 self-requested a review June 30, 2024 16:11
tuantran1702
tuantran1702 previously approved these changes Jun 30, 2024
Copy link
Contributor

@tuantran1702 tuantran1702 left a comment

Choose a reason for hiding this comment

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

LGTM. failing ci is pending fix on #1680

@odeke-em odeke-em requested a review from joe-bowman July 1, 2024 14:36
@odeke-em odeke-em force-pushed the x-participationrewards-centralize-GetProtocolData+UnmarshalProtocolData branch from f790f7f to 7062aa7 Compare July 13, 2024 13:50
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f790f7f and 7062aa7.

Files selected for processing (4)
  • x/participationrewards/keeper/callbacks.go (8 hunks)
  • x/participationrewards/keeper/callbacks_test.go (8 hunks)
  • x/participationrewards/keeper/distribution.go (1 hunks)
  • x/participationrewards/keeper/protocol_data.go (2 hunks)
Additional comments not posted (18)
x/participationrewards/keeper/protocol_data.go (1)

43-57: Ensure correct type assertion in GetAndUnmarshalProtocolData.

The type assertion on line 52 may fail if the type T does not match the expected type. Consider adding a more robust type check or logging for debugging purposes.

x/participationrewards/keeper/distribution.go (1)

40-45: Remove the unused variable data.

The variable data is assigned but never used.

- _, osmoParams, err := GetAndUnmarshalProtocolData[*types.OsmosisParamsProtocolData](ctx, k, "osmosisparams", types.ProtocolDataTypeOsmosisParams)
+ _, osmoParams, err := GetAndUnmarshalProtocolData[*types.OsmosisParamsProtocolData](ctx, k, "osmosisparams", types.ProtocolDataTypeOsmosisParams)

Likely invalid or redundant comment.

x/participationrewards/keeper/callbacks.go (8)

142-147: Ensure proper error handling in OsmosisPoolUpdateCallback.

The function correctly handles errors from GetAndUnmarshalProtocolData and json.Marshal.


182-186: Ensure proper error handling in OsmosisClPoolUpdateCallback.

The function correctly handles errors from GetAndUnmarshalProtocolData and json.Marshal.


212-216: Ensure proper error handling in UmeeReservesUpdateCallback.

The function correctly handles errors from GetAndUnmarshalProtocolData and json.Marshal.


242-246: Ensure proper error handling in UmeeTotalBorrowsUpdateCallback.

The function correctly handles errors from GetAndUnmarshalProtocolData and json.Marshal.


272-276: Ensure proper error handling in UmeeInterestScalarUpdateCallback.

The function correctly handles errors from GetAndUnmarshalProtocolData and json.Marshal.


302-307: Ensure proper error handling in UmeeUTokenSupplyUpdateCallback.

The function correctly handles errors from GetAndUnmarshalProtocolData and json.Marshal.


338-343: Ensure proper error handling in UmeeLeverageModuleBalanceUpdateCallback.

The function correctly handles errors from GetAndUnmarshalProtocolData and json.Marshal.


Line range hint 359-364:
Ensure proper error handling in SetEpochBlockCallback.

The function correctly handles errors from GetAndUnmarshalProtocolData and json.Marshal.

x/participationrewards/keeper/callbacks_test.go (8)

76-80: Ensure proper error handling in TestOsmosisPoolUpdateCallback.

The test function correctly handles errors from GetAndUnmarshalProtocolData.


134-138: Ensure proper error handling in TestOsmosisClPoolUpdateCallback.

The test function correctly handles errors from GetAndUnmarshalProtocolData.


189-193: Ensure proper error handling in executeOsmosisPoolUpdateCallback.

The test function correctly handles errors from GetAndUnmarshalProtocolData.


308-312: Ensure proper error handling in executeUmeeReservesUpdateCallback.

The test function correctly handles errors from GetAndUnmarshalProtocolData.


348-352: Ensure proper error handling in executeUmeeLeverageModuleBalanceUpdateCallback.

The test function correctly handles errors from GetAndUnmarshalProtocolData.


386-390: Ensure proper error handling in executeUmeeUTokenSupplyUpdateCallback.

The test function correctly handles errors from GetAndUnmarshalProtocolData.


424-428: Ensure proper error handling in executeUmeeTotalBorrowsUpdateCallback.

The test function correctly handles errors from GetAndUnmarshalProtocolData.


Line range hint 462-466:
Ensure proper error handling in executeUmeeInterestScalarUpdateCallback.

The test function correctly handles errors from GetAndUnmarshalProtocolData.

x/participationrewards/keeper/protocol_data.go Outdated Show resolved Hide resolved
…rshalProtocolData

This change combines GetProtocolData and types.UnmarshalProtocolData
into a generic function that unifies the functionality and the pattern.

Fixes quicksilver-zone#1631
@odeke-em odeke-em force-pushed the x-participationrewards-centralize-GetProtocolData+UnmarshalProtocolData branch from 7062aa7 to 4612067 Compare July 13, 2024 14:11
@odeke-em odeke-em requested a review from tuantran1702 July 13, 2024 14:12
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7062aa7 and 4612067.

Files selected for processing (4)
  • x/participationrewards/keeper/callbacks.go (8 hunks)
  • x/participationrewards/keeper/callbacks_test.go (8 hunks)
  • x/participationrewards/keeper/distribution.go (1 hunks)
  • x/participationrewards/keeper/protocol_data.go (2 hunks)
Files skipped from review as they are similar to previous changes (3)
  • x/participationrewards/keeper/callbacks.go
  • x/participationrewards/keeper/callbacks_test.go
  • x/participationrewards/keeper/distribution.go
Additional context used
golangci-lint
x/participationrewards/keeper/protocol_data.go

54-54: File is not gci-ed with --skip-generated -s standard -s default -s blank -s dot -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) -s prefix(github.com/cometbft/cometbft) -s prefix(github.com/cosmos/ibc-go) -s prefix(github.com/quicksilver-zone/quicksilver) --custom-order

(gci)

Additional comments not posted (2)
x/participationrewards/keeper/protocol_data.go (2)

43-57: Improve error message for type assertion failure.

The error message on line 54 could be more descriptive to aid in debugging.

- return dt, tt, fmt.Errorf("could not retrieve type of %T", (*T)(nil))
+ return dt, tt, fmt.Errorf("could not retrieve type of %T, actual type: %T", (*T)(nil), pd)
Tools
golangci-lint

54-54: File is not gci-ed with --skip-generated -s standard -s default -s blank -s dot -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) -s prefix(github.com/cometbft/cometbft) -s prefix(github.com/cosmos/ibc-go) -s prefix(github.com/quicksilver-zone/quicksilver) --custom-order

(gci)


43-57: Ensure proper type assertion for generics.

The type assertion on line 52 might fail if the type T is not compatible. Consider adding more robust type handling or validation.

Ensure that the type T is always compatible with the expected protocol data type.

Tools
golangci-lint

54-54: File is not gci-ed with --skip-generated -s standard -s default -s blank -s dot -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) -s prefix(github.com/cometbft/cometbft) -s prefix(github.com/cosmos/ibc-go) -s prefix(github.com/quicksilver-zone/quicksilver) --custom-order

(gci)

Copy link

codecov bot commented Jul 13, 2024

Codecov Report

Attention: Patch coverage is 81.81818% with 4 lines in your changes missing coverage. Please review.

Project coverage is 63.92%. Comparing base (de7a37c) to head (4612067).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1681      +/-   ##
==========================================
+ Coverage   63.82%   63.92%   +0.10%     
==========================================
  Files         196      196              
  Lines       13831    13794      -37     
==========================================
- Hits         8827     8818       -9     
+ Misses       4136     4122      -14     
+ Partials      868      854      -14     
Flag Coverage Δ
unittests 63.92% <81.81%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
x/participationrewards/keeper/callbacks.go 55.70% <100.00%> (+3.68%) ⬆️
x/participationrewards/keeper/distribution.go 68.98% <100.00%> (+0.66%) ⬆️
x/participationrewards/keeper/protocol_data.go 81.25% <63.63%> (-3.66%) ⬇️

Copy link
Contributor

@tuantran1702 tuantran1702 left a comment

Choose a reason for hiding this comment

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

lgtm. Thanks @odeke-em!

@joe-bowman
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you!

@joe-bowman joe-bowman added Stale and removed Stale labels Jul 29, 2024
Copy link
Contributor

@joe-bowman joe-bowman left a comment

Choose a reason for hiding this comment

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

lgtm

@joe-bowman joe-bowman merged commit 7419f3a into quicksilver-zone:main Aug 5, 2024
17 of 19 checks passed
@odeke-em odeke-em deleted the x-participationrewards-centralize-GetProtocolData+UnmarshalProtocolData branch August 5, 2024 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add GetAndUnmarshalProtocolData and MarshalAndSetProtocolData methods
3 participants