Update IDL and twap computation #397
Conversation
WalkthroughThe changes introduce new classes and error types to handle autodrift rebalancing and staking rate sources. They also include updates to serialization and deserialization methods, and error handling. A utility function to filter out zero values from TWAP chains has been added, and test cases have been updated to validate the new functionality. Changes
TipsChat with CodeRabbit Bot (
|
alittlezz
force-pushed
the
vali/update-idl-and-scope-ids
branch
from
d57fd60 to
756dd39
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files selected for processing (9)
- packages/kamino-sdk/src/Kamino.ts (3} hunks)
- packages/kamino-sdk/src/kamino-client/errors/custom.ts (6} hunks)
- packages/kamino-sdk/src/kamino-client/types/CollateralInfo.ts (9} hunks)
- packages/kamino-sdk/src/kamino-client/types/RebalanceAutodriftStep.ts (1} hunks)
- packages/kamino-sdk/src/kamino-client/types/RebalanceType.ts (4} hunks)
- packages/kamino-sdk/src/kamino-client/types/StakingRateSource.ts (1} hunks)
- packages/kamino-sdk/src/kamino-client/types/UpdateCollateralInfoMode.ts (4} hunks)
- packages/kamino-sdk/src/kamino-client/types/index.ts (6} hunks)
- packages/kamino-sdk/src/utils/utils.ts (2} hunks)
Files skipped from review due to trivial changes (1)
- packages/kamino-sdk/src/kamino-client/types/index.ts
Additional comments: 31
packages/kamino-sdk/src/Kamino.ts (3)
115-115: The
stripTwapZerosfunction is imported but there is no context about what it does. Ensure that it is implemented correctly and that it indeed strips zero values from TWAP price chains as expected.1632-1637: The new hunk has removed the lines where
tokenATwapandtokenBTwapwere being assigned. This change is reflected in the new hunk at lines 1647-1650 wheretokenATwapandtokenBTwapare now assigned the result ofstripTwapZerosfunction. This change seems to be in line with the PR summary, which mentions the introduction ofstripTwapZerosfunction to improve the accuracy of price fetching.1647-1650: The
stripTwapZerosfunction is used to filter out zero values from the TWAP price chains oftokenAandtokenB. This is followed by fetching the price from the non-zero TWAP chains. If the TWAP chain is empty after stripping zeros, the price is set tonull. This seems to be a logical and efficient way to handle zero values in the TWAP chains.packages/kamino-sdk/src/kamino-client/types/StakingRateSource.ts (5)
6-26: The
Constantclass and its methods seem to be well implemented. ThetoJSONandtoEncodablemethods return the correct JSON and encodable objects respectively. The static and instance properties are also correctly defined.29-49: The
Scopeclass and its methods are also well implemented, similar to theConstantclass. ThetoJSONandtoEncodablemethods return the correct JSON and encodable objects respectively. The static and instance properties are also correctly defined.52-66: The
fromDecodedfunction correctly checks if the input object is of type "object" and throws an error if not. It then checks if the object contains either "Constant" or "Scope" and returns a new instance of the corresponding class. If neither is found, it throws an error. This function seems to be correctly implemented.68-79: The
fromJSONfunction correctly switches on thekindproperty of the input object and returns a new instance of the corresponding class. This function seems to be correctly implemented.81-90: The
layoutfunction correctly creates a borsh schema for theConstantandScopeclasses and replicates it if a property is provided. This function seems to be correctly implemented.packages/kamino-sdk/src/utils/utils.ts (2)
147-148: The
numberToRebalanceTypefunction has been updated to handle a new rebalance type,Autodrift. Ensure that all calls to this function throughout the codebase have been updated to handle this new type.232-236: The
stripTwapZerosfunction is introduced to filter out zero values from TWAP price chains. This function improves the accuracy of price fetching by removing invalid data points. However, consider the implications of this change on the rest of the system. If there are parts of the system that rely on the length of the TWAP chain or its indices, this could potentially cause issues.+ // Zero is not a valid TWAP component as that indicates the SOL price + export function stripTwapZeros(chain: number[]): number[] { + return chain.filter((component) => component > 0); + }Committable suggestion (Beta)
// Zero is not a valid TWAP component as that indicates the SOL price export function stripTwapZeros(chain: number[]): number[] { return chain.filter((component) => component > 0); }packages/kamino-sdk/src/kamino-client/types/UpdateCollateralInfoMode.ts (3)
259-280: The
UpdateStakingRateChainclass is introduced with a static and instancediscriminatorandkindproperties. ThetoJSONandtoEncodablemethods are implemented for JSON conversion and encoding respectively. This class seems to be a new type of update mode for collateral information. Ensure that this new update mode is handled appropriately in the rest of the codebase.321-323: The
fromDecodedfunction is updated to handle theUpdateStakingRateChainobject. This function now returns an instance ofUpdateStakingRateChainwhen the input object has a propertyUpdateStakingRateChain. This change is consistent with the handling of other update modes in this function.365-367: The
fromJSONfunction is updated to handle theUpdateStakingRateChainobject. This function now returns an instance ofUpdateStakingRateChainwhen thekindproperty of the input JSON object isUpdateStakingRateChain. This change is consistent with the handling of other update modes in this function.384:
Thelayoutfunction is updated to includeUpdateStakingRateChainin the layout representation. This change is consistent with the handling of other update modes in this function.packages/kamino-sdk/src/kamino-client/types/RebalanceType.ts (3)
167-188: The
Autodriftclass and its corresponding interfaceAutodriftJSONhave been added. This class represents a new rebalance type. It includestoJSON()andtoEncodable()methods for serialization and deserialization. Thediscriminatorproperty is set to7, which should be unique among all rebalance types. Ensure that this new rebalance type is handled correctly in all parts of the codebase where rebalance types are used.217-219: The
fromDecoded()function has been updated to handle the newAutodriftrebalance type. If the decoded object contains anAutodriftproperty, a newAutodriftinstance is returned. This change is consistent with the handling of other rebalance types in this function.249-251: The
fromJSON()function has been updated to handle the newAutodriftrebalance type. If the JSON object contains anAutodriftproperty, a newAutodriftinstance is returned. This change is consistent with the handling of other rebalance types in this function.264:
Thelayout()function has been updated to include the newAutodriftrebalance type in theborshlayout. This change allows for efficient encoding and decoding of theAutodrifttype. Ensure that this new rebalance type is handled correctly in all parts of the codebase where thelayout()function is used.packages/kamino-sdk/src/kamino-client/errors/custom.ts (6)
106-106: The
InvalidTwapIderror type has been replaced withInvalidScopeTWAPChain. Ensure that this change is reflected in all parts of the codebase where this error type is used.132-136: New error types
InvalidScopeStakingRateChain,StakingRateNotValid,DecimalToU128ConversionFailed,DecimalNegativeSqrtRoot, andDriftingOppositeDirectionhave been added. Make sure these new error types are handled appropriately in the codebase.1294-1301: The
InvalidScopeTWAPChainerror class has been added, replacing theInvalidTwapIderror class. Ensure that this change is reflected in all parts of the codebase where this error class is used.1595-1647: New error classes
InvalidScopeStakingRateChain,StakingRateNotValid,DecimalToU128ConversionFailed,DecimalNegativeSqrtRoot, andDriftingOppositeDirectionhave been added. Make sure these new error classes are used and handled appropriately in the codebase.1861-1861: The
InvalidTwapIderror class has been replaced withInvalidScopeTWAPChainin thefromCodefunction. Ensure that this change is reflected in all parts of the codebase where this function is used.1912-1921: New error classes
InvalidScopeStakingRateChain,StakingRateNotValid,DecimalToU128ConversionFailed,DecimalNegativeSqrtRoot, andDriftingOppositeDirectionhave been added to thefromCodefunction. Make sure these new error classes are used and handled appropriately in the codebase.packages/kamino-sdk/src/kamino-client/types/CollateralInfo.ts (9)
12-20: The
scopePriceIdTwapproperty has been replaced withscopeTwapPriceChainandscopeStakingRateChainproperties. Ensure that all references toscopePriceIdTwapin the codebase have been updated to use these new properties. Also, verify that the new properties are being correctly populated and used in the application logic.30-38: The
scopePriceIdTwapproperty has been replaced withscopeTwapPriceChainandscopeStakingRateChainproperties in theCollateralInfoFieldsinterface. Ensure that all references toscopePriceIdTwapin the codebase have been updated to use these new properties. Also, verify that the new properties are being correctly populated and used in the application logic.48-56: The
scopePriceIdTwapproperty has been replaced withscopeTwapPriceChainandscopeStakingRateChainproperties in theCollateralInfoclass. Ensure that all references toscopePriceIdTwapin the codebase have been updated to use these new properties. Also, verify that the new properties are being correctly populated and used in the application logic.65-73: The
scopePriceIdTwapproperty has been replaced withscopeTwapPriceChainandscopeStakingRateChainproperties in theCollateralInfoclass constructor. Ensure that all references toscopePriceIdTwapin the codebase have been updated to use these new properties. Also, verify that the new properties are being correctly populated and used in the application logic.85-94: The
scopePriceIdTwapproperty has been replaced withscopeTwapPriceChainandscopeStakingRateChainproperties in thelayoutfunction. Ensure that all references toscopePriceIdTwapin the codebase have been updated to use these new properties. Also, verify that the new properties are being correctly populated and used in the application logic.108-116: The
scopePriceIdTwapproperty has been replaced withscopeTwapPriceChainandscopeStakingRateChainproperties in thefromDecodedfunction. Ensure that all references toscopePriceIdTwapin the codebase have been updated to use these new properties. Also, verify that the new properties are being correctly populated and used in the application logic.128-136: The
scopePriceIdTwapproperty has been replaced withscopeTwapPriceChainandscopeStakingRateChainproperties in thefromJSONfunction. Ensure that all references toscopePriceIdTwapin the codebase have been updated to use these new properties. Also, verify that the new properties are being correctly populated and used in the application logic.148-156: The
scopePriceIdTwapproperty has been replaced withscopeTwapPriceChainandscopeStakingRateChainproperties in thetoJSONfunction. Ensure that all references toscopePriceIdTwapin the codebase have been updated to use these new properties. Also, verify that the new properties are being correctly populated and used in the application logic.168-176: The
scopePriceIdTwapproperty has been replaced withscopeTwapPriceChainandscopeStakingRateChainproperties in thefromJSONfunction. Ensure that all references toscopePriceIdTwapin the codebase have been updated to use these new properties. Also, verify that the new properties are being correctly populated and used in the application logic.
| import { PublicKey } from "@solana/web3.js" // eslint-disable-line @typescript-eslint/no-unused-vars | ||
| import BN from "bn.js" // eslint-disable-line @typescript-eslint/no-unused-vars | ||
| import * as types from "../types" // eslint-disable-line @typescript-eslint/no-unused-vars | ||
| import * as borsh from "@project-serum/borsh" | ||
|
|
There was a problem hiding this comment.
The import statements have eslint disable comments for unused vars. If these imports are not used in this file, consider removing them to clean up the code. If they are used in the code, remove the eslint disable comments.
| export function fromDecoded(obj: any): types.RebalanceAutodriftStepKind { | ||
| if (typeof obj !== "object") { | ||
| throw new Error("Invalid enum object") | ||
| } | ||
|
|
||
| if ("Uninitialized" in obj) { | ||
| return new Uninitialized() | ||
| } | ||
| if ("Autodrifting" in obj) { | ||
| return new Autodrifting() | ||
| } | ||
|
|
||
| throw new Error("Invalid enum object") | ||
| } |
There was a problem hiding this comment.
The fromDecoded function uses a loose type any for its parameter. This can lead to potential type errors. Consider using a more specific type or interface if possible. Also, the function throws a generic error "Invalid enum object" in two different places. It would be more helpful to throw more specific error messages to help with debugging.
| export function fromJSON( | ||
| obj: types.RebalanceAutodriftStepJSON | ||
| ): types.RebalanceAutodriftStepKind { | ||
| switch (obj.kind) { | ||
| case "Uninitialized": { | ||
| return new Uninitialized() | ||
| } | ||
| case "Autodrifting": { | ||
| return new Autodrifting() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The fromJSON function does not handle the case where obj.kind is neither "Uninitialized" nor "Autodrifting". It would be better to throw an error in the default case of the switch statement to handle unexpected values.
68:
export function fromJSON(
69:
obj: types.RebalanceAutodriftStepJSON
70:
): types.RebalanceAutodriftStepKind {
71:
switch (obj.kind) {
72:
case "Uninitialized": {
73:
return new Uninitialized()
74:
}
75:
case "Autodrifting": {
76:
return new Autodrifting()
77:
}
+ default: {
+ throw new Error(`Invalid kind: ${obj.kind}`);
+ }
78:
}
79:
}Committable suggestion (Beta)
| export function fromJSON( | |
| obj: types.RebalanceAutodriftStepJSON | |
| ): types.RebalanceAutodriftStepKind { | |
| switch (obj.kind) { | |
| case "Uninitialized": { | |
| return new Uninitialized() | |
| } | |
| case "Autodrifting": { | |
| return new Autodrifting() | |
| } | |
| } | |
| } | |
| export function fromJSON( | |
| obj: types.RebalanceAutodriftStepJSON | |
| ): types.RebalanceAutodriftStepKind { | |
| switch (obj.kind) { | |
| case "Uninitialized": { | |
| return new Uninitialized() | |
| } | |
| case "Autodrifting": { | |
| return new Autodrifting() | |
| } | |
| default: { | |
| throw new Error(`Invalid kind: ${obj.kind}`); | |
| } | |
| } | |
| } |
Summary by CodeRabbit
AutodriftandUpdateStakingRateChainfunctionalities to enhance the flexibility of rebalancing and updating staking rates.CollateralInfoclass with additional properties for better handling of TWAP price chains and staking rate chains.stripTwapZerosto filter out zero values from TWAP price chains, improving data quality.Kaminoclass and itsgetStrategiesShareDatamethod.These updates aim to improve the overall user experience by providing more accurate data and better error handling.