-
Notifications
You must be signed in to change notification settings - Fork 10
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
feat(oracle): register new token and set up feeder through assets-module #159
feat(oracle): register new token and set up feeder through assets-module #159
Conversation
WalkthroughThe recent changes refine the asset management system by restructuring the processes for token registration and updates. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Assets
participant OracleKeeper
participant Keeper
User->>Assets: Request Token Registration
Assets->>Assets: Validate Token Metadata
Assets->>OracleKeeper: Check if Token Registered
alt Token Exists
OracleKeeper->>Assets: Update Token Information
else New Token
Assets->>Assets: Create New Token Info
Assets->>OracleKeeper: Register New Token
end
OracleKeeper->>Keeper: Register Token and Feeder
Keeper->>Keeper: Store Updated Parameters
Keeper->>User: Confirmation of Registration
Possibly related issues
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? TipsChatThere are 3 ways to chat with CodeRabbit:
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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, codebase verification and nitpick comments (3)
x/assets/types/expected_keepers.go (1)
10-10
: Implement the new method in OracleKeeper implementationsThe method
RegisterNewTokenAndSetTokenFeeder
has been added to theOracleKeeper
interface but is not implemented in any of its current implementations. Please ensure that all structs implementing this interface include this method.
- Interface definition found in
x/assets/types/expected_keepers.go
.Analysis chain
Ensure the new method is implemented.
The method
RegisterNewTokenAndSetTokenFeeder
has been added to theOracleKeeper
interface. Verify that all implementations of this interface have been updated to include this method.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all implementations of OracleKeeper have the new method implemented. # Test: Search for implementations of the OracleKeeper interface. Expect: All implementations include the new method. ast-grep --lang go --pattern $'type $_ OracleKeeper interface { $$$ RegisterNewTokenAndSetTokenFeeder($_, $_, $_, $_, $_, $_, $_) error $$$ }'Length of output: 688
Script:
#!/bin/bash # Step 1: Find the definition of the OracleKeeper interface to ensure it is correctly identified. ast-grep --lang go --pattern 'type OracleKeeper interface { $$$ }' # Step 2: Find all types that implement the OracleKeeper interface. rg --type go 'OracleKeeper' -A 5 # Step 3: Check if these types have the RegisterNewTokenAndSetTokenFeeder method implemented. ast-grep --lang go --pattern 'func ($_).RegisterNewTokenAndSetTokenFeeder($_, $_, $_, $_, $_, $_, $_) error { $$$ }'Length of output: 21159
x/oracle/keeper/params.go (1)
12-15
: Define constants clearly.The constants
startAfterBlocks
anddefaultInterval
are defined to manage token feeder configurations. Ensure these values are appropriate for the intended functionality and consider documenting their purpose.+ // startAfterBlocks defines the number of blocks after which the token feeder starts. + // defaultInterval is the default interval for token feeder updates. const ( startAfterBlocks = 10 defaultInterval = 30 )precompiles/assets/tx.go (1)
143-169
: Ensure robust error handling inRegisterOrUpdateTokens
.The function
RegisterOrUpdateTokens
includes logic for parsing oracle information and registering tokens. Ensure that:
- The regex match count (
len(oracleInfo)
) is correct for all expected cases.- The error message for missing oracle information is clear and actionable.
- Consider adding comments to explain the registration logic for better readability.
+ // Ensure that the oracle_info is correctly described in the meta_info. if len(oracleInfo) != 5 { return nil, errors.New("register new asset fail, oracle_info must be described in meta_info") }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- precompiles/assets/tx.go (3 hunks)
- x/assets/types/expected_keepers.go (1 hunks)
- x/oracle/keeper/params.go (2 hunks)
- x/oracle/types/params.go (2 hunks)
Additional comments not posted (2)
x/oracle/keeper/params.go (1)
34-102
: Review the logic inRegisterNewTokenAndSetTokenFeeder
.The function
RegisterNewTokenAndSetTokenFeeder
is well-structured, handling parameter retrieval, error checking, and token registration. However, ensure that:
- The error messages are consistent and informative.
- The logic for updating
TokenFeeders
is correct and aligns with the intended feeder configuration.- Consider adding comments to explain complex logic blocks for better readability.
+ // Check if the assetID already exists to prevent duplicate registrations. if p.GetTokenIDFromAssetID(assetID) > 0 { return fmt.Errorf("assetID exists:%s", assetID) }
Verification successful
Function usage is correct and error handling is appropriate.
The function
RegisterNewTokenAndSetTokenFeeder
is used inprecompiles/assets/tx.go
with proper error handling and parameter validation. The code ensures thatoracleInfo
is correctly parsed and checked before calling the function, and any errors from the function are properly propagated. This aligns with the intended usage and error management.
precompiles/assets/tx.go
: The function is called after parsing and validatingoracleInfo
, with errors being handled correctly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct behavior of RegisterNewTokenAndSetTokenFeeder. # Test: Search for usages of RegisterNewTokenAndSetTokenFeeder to ensure correct error handling and parameter usage. rg --type go 'RegisterNewTokenAndSetTokenFeeder'Length of output: 549
Script:
#!/bin/bash # Extract the context around the usage of RegisterNewTokenAndSetTokenFeeder in precompiles/assets/tx.go rg --type go -A 10 -B 10 'RegisterNewTokenAndSetTokenFeeder' precompiles/assets/tx.goLength of output: 875
precompiles/assets/tx.go (1)
28-28
: Verify regex correctness.The
oracleInfoMatcher
regex is designed to extract oracle information from metadata. Ensure that it correctly matches the expected format and consider edge cases that might not be covered.Verification successful
Regex pattern correctly matches expected format.
The
oracleInfoMatcher
regex pattern successfully matches the expected format of oracle information metadata in the codebase. The pattern correctly handles both required and optional fields. No issues found with the regex.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the oracleInfoMatcher regex. # Test: Search for all occurrences of oracle_info in the codebase to ensure they match the regex pattern. rg --type go 'oracle_info:{chain:.+,token:.+,decimal:\d+(,interval:.+)?(,contract:.+)?}'Length of output: 259
Script:
#!/bin/bash # Corrected regex pattern to search for oracle_info occurrences in Go files. rg --type go 'oracle_info:\{chain:[^,]+,token:[^,]+,decimal:\d+(,interval:[^,]+)?(,contract:[^,]+)?\}'Length of output: 224
assetIDs := strings.Split(token.AssetID, ",") | ||
for _, aID := range assetIDs { | ||
if aID == assetID { | ||
return id | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider returning a sentinel value or error for no match.
The current implementation returns 0 if no asset ID matches. This could be ambiguous if 0 is a valid token ID. Consider returning a sentinel value or an error to indicate no match found.
func (p Params) GetTokenIDFromAssetID(assetID string) (int, error) {
for id, token := range p.Tokens {
assetIDs := strings.Split(token.AssetID, ",")
for _, aID := range assetIDs {
if aID == assetID {
return id, nil
}
}
}
return -1, errors.New("asset ID not found")
}
Optimize string splitting for performance.
If GetTokenIDFromAssetID
is called frequently, consider optimizing the splitting operation to improve performance, such as caching the split results if the token data is static.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, since we're going to change the proto-definition, this will be updated to map
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leonz789, updating the proto-definition to use a map sounds like a great approach. This change should effectively address both the ambiguity issue and the performance concerns raised in the review. Let me know if there's anything else you'd like assistance with!
There was a problem hiding this 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
Files selected for processing (9)
- precompiles/assets/IAssets.sol (1 hunks)
- precompiles/assets/abi.json (1 hunks)
- precompiles/assets/assets.go (1 hunks)
- precompiles/assets/tx.go (2 hunks)
- precompiles/assets/types.go (3 hunks)
- precompiles/common/error.go (1 hunks)
- x/assets/types/expected_keepers.go (1 hunks)
- x/oracle/keeper/params.go (2 hunks)
- x/oracle/types/types.go (1 hunks)
Files skipped from review due to trivial changes (1)
- precompiles/assets/assets.go
Files skipped from review as they are similar to previous changes (1)
- precompiles/assets/tx.go
Additional comments not posted (29)
x/assets/types/expected_keepers.go (1)
10-10
: Ensure consistent interface implementation.The addition of
RegisterNewTokenAndSetTokenFeeder
enhances theOracleKeeper
interface. Verify that all types implementing this interface are updated to include this new method.x/oracle/types/types.go (1)
9-27
: Review struct design for completeness and correctness.The
OracleInfo
struct encapsulates detailed information about an oracle's token and feeder attributes. Ensure that:
- Field types are appropriate for their intended use.
- JSON tags are consistent with naming conventions and correctly reflect the field names.
- Consider whether additional validation is needed for fields like
Decimal
andInterval
.Verification successful
OracleInfo struct validation is complete and correct.
The
OracleInfo
struct fieldsDecimal
andInterval
are appropriately validated in the codebase. TheDecimal
field is parsed and checked for negativity, while theInterval
field is validated to ensure it is non-zero and logically consistent with other parameters. No additional validation is necessary.
x/oracle/keeper/params.go
: ValidatesDecimal
andInterval
.precompiles/assets/types.go
: Checks for non-emptyDecimal
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify usage and validation of OracleInfo struct fields. # Test: Search for OracleInfo struct usage. Expect: Proper validation and usage of fields. rg --type go 'OracleInfo' -A 10Length of output: 3084
Script:
#!/bin/bash # Description: Search for specific validation of OracleInfo fields, especially Decimal and Interval. # Check for validation of Decimal field. rg --type go 'Decimal' -A 5 # Check for validation of Interval field. rg --type go 'Interval' -A 5Length of output: 69043
precompiles/common/error.go (1)
19-19
: Ensure error message clarity and alignment with validation logic.The
ErrInvalidOracleInfo
constant provides a specific error message for oracle information validation. Verify that this error message is used consistently with the validation logic and that it clearly communicates the validation requirements.Verification successful
Validation Logic Aligns with Error Message
The usage of
ErrInvalidOracleInfo
in the validation logic correctly aligns with the error message. The logic checks thattoken.Name
,token.Chain.Name
, andtoken.Decimal
are not empty, consistent with the message requirements. No changes are needed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify usage of ErrInvalidOracleInfo in validation logic. # Test: Search for ErrInvalidOracleInfo usage. Expect: Consistent use in validation logic. rg --type go 'ErrInvalidOracleInfo'Length of output: 271
Script:
#!/bin/bash # Search for the validation logic related to ErrInvalidOracleInfo in precompiles/assets/types.go rg --type go -A 10 'ErrInvalidOracleInfo' precompiles/assets/types.goLength of output: 447
Script:
#!/bin/bash # Search for function definitions and usages of ErrInvalidOracleInfo in precompiles/assets/types.go to understand the validation logic rg --type go -A 15 'func ' precompiles/assets/types.go | rg -B 5 'ErrInvalidOracleInfo'Length of output: 87
Script:
#!/bin/bash # Search for the usage of ErrInvalidOracleInfo in precompiles/assets/types.go with more context to capture the validation logic rg --type go -A 20 -B 10 'ErrInvalidOracleInfo' precompiles/assets/types.goLength of output: 925
x/oracle/keeper/params.go (9)
12-15
: Define constants for configuration.The constants
startAfterBlocks
anddefaultInterval
are defined for configuration purposes. This is a good practice as it centralizes configuration values, making them easier to manage and change if needed.
34-38
: Check for existing asset ID.The function correctly checks if the asset ID already exists, preventing duplicate registrations. The error message is clear and informative.
39-53
: Handle new chain addition.The logic for adding a new chain if it doesn't exist is well-implemented. The use of a loop to check existing chains and append a new one if necessary is efficient.
54-60
: Validate decimal value.The code correctly parses and validates the decimal value, ensuring it is non-negative. The error handling is appropriate.
61-67
: Default interval handling.The interval is parsed and defaults to
defaultInterval
if zero. This ensures a valid interval is always set, which is a good practice.
80-88
: Add a new token.The function correctly adds a new token with all necessary information. The use of structured data ensures clarity and consistency.
90-96
: Calculate start block for feeder.The calculation of the start block for the token feeder is logical and ensures the feeder starts after a predefined number of blocks.
98-108
: Set token feeder configuration.The setup of the token feeder is well-structured, with clear parameters. The decision to not end feeders for version 1 is noted.
110-111
: Finalize parameter updates.The function concludes by setting the updated parameters, ensuring the state reflects all changes made. This is a crucial step for maintaining consistency.
precompiles/assets/IAssets.sol (1)
76-77
: EnhancecreateAsset
function withoracleInfo
.The addition of the
oracleInfo
parameter to thecreateAsset
function allows for more comprehensive asset definitions, enhancing the interface's utility. Ensure that all calls to this function are updated to include the new parameter.precompiles/assets/abi.json (1)
162-165
: Update ABI to includeoracleInfo
.The ABI now includes the
oracleInfo
parameter for thewithdrawPrincipal
function, aligning with the contract's updated signature. This ensures that interactions with the contract are correctly handled.precompiles/assets/types.go (15)
114-118
: Update function signature and error handling.The function signature has been updated to return
oracletypes.OracleInfo
along withtypes.AssetInfo
and an error. This change enhances the function's capability to provide additional oracle information.The error handling pattern has been improved by using a consistent approach of assigning errors to a variable and returning initialized variables. This enhances clarity and consistency in the function's return structure.
138-139
: Validate address length check.The address length check ensures that the provided address is not shorter than expected. This validation is essential for preventing errors related to address handling.
163-164
: Validate name length check.The name length check ensures that the provided name is within acceptable limits. This validation is crucial for preventing errors related to name handling.
174-175
: Validate metaInfo length check.The metaInfo length check ensures that the provided metaInfo is within acceptable limits. This validation is crucial for preventing errors related to metaInfo handling.
188-192
: Validate oracle information fields.The checks for critical fields in
oInfo
ensure that the oracle information is complete. This validation is essential for preventing errors related to incomplete oracle data.
195-195
: Consistent return structure.The function returns a consistent structure, maintaining clarity and integrity in its return types.
128-128
: Handle potential errors fromGetClientChainInfoByIndex
.The function correctly handles errors from
GetClientChainInfoByIndex
. Ensure that this function is robust and handles all edge cases, as its failure could impact theTokenFromInputs
function.
134-135
: Ensure correct type assertion forassetAddr
.The error handling for type assertion of
assetAddr
is consistent with the new pattern. Ensure thatargs[1]
is always a[]byte
in the calling code to prevent runtime errors.
152-153
: Ensure correct type assertion fortvlLimit
.The error handling for type assertion of
tvlLimit
is consistent with the new pattern. Ensure thatargs[3]
is always a*big.Int
in the calling code to prevent runtime errors.
185-187
: Validate JSON unmarshalling ofoInfoStr
.The JSON unmarshalling of
oInfoStr
intooInfo
is correctly handled. Ensure thatoInfoStr
is valid JSON in the calling code to prevent unmarshalling errors.
170-171
: Ensure correct type assertion formetaInfo
.The error handling for type assertion of
metaInfo
is consistent with the new pattern. Ensure thatargs[5]
is always astring
in the calling code to prevent runtime errors.
159-160
: Ensure correct type assertion forname
.The error handling for type assertion of
name
is consistent with the new pattern. Ensure thatargs[4]
is always astring
in the calling code to prevent runtime errors.
145-146
: Ensure correct type assertion fordecimal
.The error handling for type assertion of
decimal
is consistent with the new pattern. Ensure thatargs[2]
is always auint8
in the calling code to prevent runtime errors.
122-123
: Ensure correct type assertion forclientChainID
.The error handling for type assertion of
clientChainID
is consistent with the new pattern. Ensure thatargs[0]
is always auint32
in the calling code to prevent runtime errors.
179-182
: Ensure correct type assertion foroInfoStr
.The error handling for type assertion of
oInfoStr
is consistent with the new pattern. Ensure thatargs[6]
is always astring
in the calling code to prevent runtime errors.
for _, t := range p.Tokens { | ||
// token exists, bind assetID for this token | ||
// it's possible for one price bonded with multiple assetID, like ETHUSDT from sepolia/mainnet | ||
if t.Name == oInfo.Token.Name && t.ChainID == chainID { | ||
t.AssetID = strings.Join([]string{t.AssetID, oInfo.AssetID}, ",") | ||
k.SetParams(ctx, p) | ||
// there should have been existing tokenFeeder running(currently we register tokens from assets-module and with infinite endBlock) | ||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bind asset ID to existing token.
The logic for binding a new asset ID to an existing token is clear. The use of string concatenation to handle multiple asset IDs is straightforward, but consider using a more robust data structure if the list grows.
Consider using a slice or a set-like structure for managing multiple asset IDs to improve scalability and maintainability.
There was a problem hiding this 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
Files selected for processing (5)
- precompiles/assets/tx.go (1 hunks)
- precompiles/assets/types.go (2 hunks)
- precompiles/common/error.go (1 hunks)
- x/oracle/keeper/params.go (2 hunks)
- x/oracle/types/types.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- precompiles/common/error.go
- x/oracle/keeper/params.go
Additional context used
GitHub Check: build
precompiles/assets/types.go
[failure] 125-125:
undefined: types.OracleInfo
GitHub Check: test-unit-cover
precompiles/assets/types.go
[failure] 125-125:
undefined: types.OracleInfo
[failure] 125-125:
undefined: types.OracleInfo
[failure] 125-125:
undefined: types.OracleInfo
GitHub Check: Run golangci-lint
precompiles/assets/types.go
[failure] 125-125:
undefined: types.OracleInfo (typecheck)
[failure] 125-125:
undefined: types.OracleInfo) (typecheck)
Additional comments not posted (3)
x/oracle/types/types.go (1)
9-32
: Ensure completeness and consistency ofOracleInfo
struct.The
OracleInfo
struct is well-structured for encapsulating oracle-related data. However, consider the following:
- Ensure that all necessary fields are included and initialized correctly.
- Review the commented-out fields to determine if they are needed or should be removed for clarity.
- Verify that the
AssetID
field is correctly used and updated throughout the codebase.Verification successful
OracleInfo Struct is Complete and Consistent
The
OracleInfo
struct is appropriately utilized across the codebase. TheAssetID
field is used correctly in conditional checks and validations. The commented-out fields in the struct definition are not used elsewhere, indicating they might not be necessary. The current implementation appears consistent and complete.
x/oracle/keeper/params.go
:AssetID
is checked for existence.precompiles/assets/types.go
: Fields are initialized and validated.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `OracleInfo` and ensure all fields are utilized correctly. # Test: Search for `OracleInfo` usage. Expect: Proper initialization and usage of all fields. rg --type go 'OracleInfo'Length of output: 2611
Script:
#!/bin/bash # Extract lines where OracleInfo is initialized and used to verify field utilization. rg --type go 'OracleInfo' -A 5 -B 2Length of output: 6620
precompiles/assets/tx.go (1)
132-159
: Review handling ofOracleInfo
inRegisterOrUpdateTokens
.The function now incorporates
OracleInfo
for enhanced asset management. Consider the following:
- Ensure that
oInfo
is correctly populated and used throughout the function.- Verify error handling when registering new tokens and setting token feeders.
- Confirm that the logic for updating
stakingAsset
is robust and handles all edge cases.precompiles/assets/types.go (1)
119-208
: Address undefinedtypes.OracleInfo
issue.The static analysis tools have flagged an issue with
types.OracleInfo
being undefined. This needs to be resolved to ensure the code compiles and functions correctly.- oracleInfo := types.OracleInfo{} + oracleInfo := oracletypes.OracleInfo{}Enhance error handling in
TokenFromInputs
.The function now checks for valid oracle information. Ensure that:
- All necessary fields in
oracleInfo
are validated.- Error messages are clear and informative for debugging purposes.
- Consider logging errors for better traceability.
Verification successful
Correct Usage of
oracletypes.OracleInfo
ConfirmedThe review comment correctly identifies that
types.OracleInfo
should be replaced withoracletypes.OracleInfo
. TheOracleInfo
struct is defined in thex/oracle/types
package, confirming the validity of this change.
- Ensure that the import statement for
oracletypes
is correctly added to the file if not already present.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the initialization and usage of `oracleInfo` in `TokenFromInputs`. # Test: Search for `TokenFromInputs` usage. Expect: Correct initialization and error handling. rg --type go 'TokenFromInputs'Length of output: 258
Script:
#!/bin/bash # Search for the definition of `OracleInfo` in the codebase to verify its package and usage. ast-grep --lang go --pattern 'type OracleInfo struct { $$$ }'Length of output: 1287
Tools
GitHub Check: build
[failure] 125-125:
undefined: types.OracleInfoGitHub Check: test-unit-cover
[failure] 125-125:
undefined: types.OracleInfo
[failure] 125-125:
undefined: types.OracleInfo
[failure] 125-125:
undefined: types.OracleInfoGitHub Check: Run golangci-lint
[failure] 125-125:
undefined: types.OracleInfo (typecheck)
[failure] 125-125:
undefined: types.OracleInfo) (typecheck)
There was a problem hiding this 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
Outside diff range, codebase verification and nitpick comments (1)
precompiles/assets/types.go (1)
22-22
: Address the formatting issue flagged by static analysis.The file is not
gofumpt
-ed. Ensure the code is formatted according togofumpt
standards.- var tokenDescMatcher = regexp.MustCompile(`TokenDesc:{(.+?)}`) +var tokenDescMatcher = regexp.MustCompile(`TokenDesc:{(.+?)}`)Tools
GitHub Check: Run golangci-lint
[failure] 22-22:
File is notgofumpt
-ed (gofumpt)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- precompiles/assets/types.go (2 hunks)
Additional context used
GitHub Check: Run golangci-lint
precompiles/assets/types.go
[failure] 22-22:
File is notgofumpt
-ed (gofumpt)
precompiles/assets/types.go
Outdated
oracleInfoStr, ok := args[6].(string) | ||
if !ok { | ||
return types.AssetInfo{}, oracletypes.OracleInfo{}, fmt.Errorf(exocmn.ErrContractInputParaOrType, 6, "string", args[6]) | ||
} | ||
parsed := strings.Split(oracleInfoStr, ",") | ||
l := len(parsed) | ||
switch { | ||
case l > 5: | ||
joined := strings.Join(parsed[5:], "") | ||
tokenDesc := tokenDescMatcher.FindStringSubmatch(joined) | ||
chainDesc := chainDescMatcher.FindStringSubmatch(joined) | ||
if len(tokenDesc) == 2 { | ||
oracleInfo.Token.Desc = tokenDesc[1] | ||
} | ||
if len(chainDesc) == 2 { | ||
oracleInfo.Chain.Desc = chainDesc[1] | ||
} | ||
case l >= 5: | ||
oracleInfo.Token.Contract = parsed[4] | ||
fallthrough | ||
case l >= 4: | ||
oracleInfo.Feeder.Interval = parsed[3] | ||
fallthrough | ||
case l >= 3: | ||
oracleInfo.Token.Name = parsed[0] | ||
oracleInfo.Chain.Name = parsed[1] | ||
oracleInfo.Token.Decimal = parsed[2] | ||
default: | ||
return types.AssetInfo{}, oracletypes.OracleInfo{}, errors.New(exocmn.ErrInvalidOracleInfo) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider simplifying the parsing logic for clarity.
The current switch-case structure for parsing oracleInfoStr
can be simplified for better readability. Consider using a more straightforward approach to handle different lengths of parsed data.
parsed := strings.Split(oracleInfoStr, ",")
if len(parsed) < 3 {
return types.AssetInfo{}, oracletypes.OracleInfo{}, errors.New(exocmn.ErrInvalidOracleInfo)
}
oracleInfo.Token.Name = parsed[0]
oracleInfo.Chain.Name = parsed[1]
oracleInfo.Token.Decimal = parsed[2]
if len(parsed) >= 4 {
oracleInfo.Feeder.Interval = parsed[3]
}
if len(parsed) >= 5 {
oracleInfo.Token.Contract = parsed[4]
}
if len(parsed) > 5 {
joined := strings.Join(parsed[5:], "")
tokenDesc := tokenDescMatcher.FindStringSubmatch(joined)
chainDesc := chainDescMatcher.FindStringSubmatch(joined)
if len(tokenDesc) == 2 {
oracleInfo.Token.Desc = tokenDesc[1]
}
if len(chainDesc) == 2 {
oracleInfo.Chain.Desc = chainDesc[1]
}
}
There was a problem hiding this 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
Files selected for processing (1)
- precompiles/assets/types.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- precompiles/assets/types.go
There was a problem hiding this 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
Files selected for processing (2)
- precompiles/assets/types.go (2 hunks)
- x/oracle/keeper/params.go (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- precompiles/assets/types.go
- x/oracle/keeper/params.go
Testing failed, oracle feed price fail. Steps:
|
This failure was due to the new commit of price feeder: ExocoreNetwork/price-feeder@da450eb With https://github.com/ExocoreNetwork/price-feeder/releases/tag/v0.1.3 there is no such failure. But the price update failed when register a new token. |
There was a problem hiding this 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
Outside diff range, codebase verification and nitpick comments (3)
x/oracle/keeper/params.go (3)
Line range hint
17-33
:
Add validation logic inSetParams
.The
SetParams
function has a TODO comment for validation checks, which is crucial for ensuring data integrity when setting parameters.Would you like me to help implement the validation logic or open a GitHub issue to track this task?
35-39
: Enhance error message for existing asset ID.The error message when an asset ID already exists could provide more context for debugging.
- return fmt.Errorf("assetID exists:%s", oInfo.AssetID) + return fmt.Errorf("assetID %s already exists in the system", oInfo.AssetID)
55-64
: Improve error handling for parsing failures.Error messages for parsing decimal and interval values could be more descriptive to aid debugging.
- return err + return fmt.Errorf("failed to parse decimal value for token %s: %w", oInfo.Token.Name, err)- return err + return fmt.Errorf("failed to parse interval value for feeder %s: %w", oInfo.Feeder.Name, err)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- x/oracle/keeper/params.go (2 hunks)
Additional comments not posted (3)
x/oracle/keeper/params.go (3)
13-16
: Constants for feeder configuration are appropriate.The constants
startAfterBlocks
anddefaultInterval
are well-defined and enhance readability by clarifying the purpose of these values in the feeder setup.
70-78
: Consider using a more robust data structure for asset IDs.The logic for binding asset IDs is clear, but consider using a slice or set-like structure for scalability and maintainability.
81-107
: Token registration and feeder setup logic is clear.The logic for adding new tokens and setting up feeders is well-implemented and aligns with the PR objectives.
There was a problem hiding this 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
Files selected for processing (6)
- app/ante/cosmos/authz.go (2 hunks)
- precompiles/avs/types.go (1 hunks)
- testutil/abci.go (3 hunks)
- x/dogfood/types/genesis.go (1 hunks)
- x/oracle/keeper/aggregator/context.go (1 hunks)
- x/oracle/types/params.go (3 hunks)
Files skipped from review due to trivial changes (2)
- precompiles/avs/types.go
- x/dogfood/types/genesis.go
Files skipped from review as they are similar to previous changes (1)
- x/oracle/types/params.go
Additional comments not posted (5)
app/ante/cosmos/authz.go (1)
30-30
: Simplify error handling withErrUnauthorized.Wrap
.The change from
errorsmod.Wrapf
toerrortypes.ErrUnauthorized.Wrap
streamlines error handling by using a more specific error type. This improves clarity and consistency in error reporting.testutil/abci.go (2)
193-193
: Enhance error handling withErrInvalidRequest.Wrap
.The change from
errorsmod.Wrapf
toerrortypes.ErrInvalidRequest.Wrap
inBroadcastTxBytes
improves clarity by directly using a specific error type, aligning with the error handling conventions.
240-240
: Enhance error handling withErrInvalidRequest.Wrap
.Similarly, the change in
checkTxBytes
to useerrortypes.ErrInvalidRequest.Wrap
enhances clarity and consistency in error reporting.x/oracle/keeper/aggregator/context.go (2)
93-93
: Simplify validation logic by removing nil check.The removal of the nil check for
msg.Prices
assumes that prior validation ensures non-nil values, simplifying the condition and improving efficiency.
98-98
: Streamline validation by removing nil check forpSource.Prices
.By assuming
pSource.Prices
is not nil, the logic is simplified. Ensure that upstream validation guarantees this assumption to prevent runtime errors.
fixed by #160 |
Test passed. Input of oracleInfo field.
|
Tried adding this to the contract in Solidity but it complains. Looking for a workaround in Solidity first; otherwise we might have to make some changes here. |
ExocoreNetwork/exocore#159 adds a new `oracleInfo` parameter to the `IAssets.sol` precompile when registering tokens. This PR follows that change. Previously, multiple tokens could be registered with a single call to ExocoreGateway. However, that resulted in too many variables (and too much gas) for Solidity to handle within a single function. To that end, and keeping in mind that addition of new tokens isn't likely to be a frequent occurrence, the function's capabilities have been tempered to support only a single token within the transaction. As a side effect, this fixes ExocoreNetwork#80
There was a problem hiding this 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
Outside diff range, codebase verification and nitpick comments (3)
precompiles/assets/tx.go (2)
Line range hint
120-160
: Consider refactoring the function into smaller functions for better readability and maintainability.The
RegisterToken
function is performing multiple operations:
- Input parsing
- Asset existence check
- Token registration
- Staking asset info update
Consider refactoring these operations into separate smaller functions. This will improve the readability and maintainability of the code.
162-200
: Consider refactoring the function into smaller functions for better readability and maintainability.The
UpdateToken
function is performing multiple operations:
- Input parsing
- Asset existence check
- Asset info update
Consider refactoring these operations into separate smaller functions. This will improve the readability and maintainability of the code.
precompiles/assets/types.go (1)
184-214
: Consider simplifying the parsing logic for clarity.The current switch-case structure for parsing
oracleInfoStr
can be simplified for better readability. Consider using a more straightforward approach to handle different lengths of parsed data.parsed := strings.Split(oracleInfoStr, ",") if len(parsed) < 3 { return types.AssetInfo{}, oracletypes.OracleInfo{}, errors.New(exocmn.ErrInvalidOracleInfo) } oracleInfo.Token.Name = parsed[0] oracleInfo.Chain.Name = parsed[1] oracleInfo.Token.Decimal = parsed[2] if len(parsed) >= 4 { oracleInfo.Feeder.Interval = parsed[3] } if len(parsed) >= 5 { oracleInfo.Token.Contract = parsed[4] } if len(parsed) > 5 { joined := strings.Join(parsed[5:], "") tokenDesc := tokenDescMatcher.FindStringSubmatch(joined) chainDesc := chainDescMatcher.FindStringSubmatch(joined) if len(tokenDesc) == 2 { oracleInfo.Token.Desc = tokenDesc[1] } if len(chainDesc) == 2 { oracleInfo.Chain.Desc = chainDesc[1] } }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- precompiles/assets/IAssets.sol (1 hunks)
- precompiles/assets/abi.json (2 hunks)
- precompiles/assets/assets.go (3 hunks)
- precompiles/assets/tx.go (3 hunks)
- precompiles/assets/types.go (2 hunks)
- x/oracle/types/params.go (1 hunks)
Additional comments not posted (15)
precompiles/assets/IAssets.sol (2)
58-79
: LGTM! The changes to theregisterToken
function improve clarity and functionality.The key improvements include:
- Separating the registration and update functionality into separate functions.
- Adding the
oracleInfo
parameter to provide additional information about the token's oracle during registration.- Renaming
clientChainID
toclientChainId
to maintain consistent casing.- Simplifying the return value to focus on the success status of token registration.
These changes enhance the clarity and functionality of the token registration process.
80-92
: LGTM! The introduction of theupdateToken
function enhances the token management process.The benefits of the new function include:
- Improving clarity by separating the update functionality from the registration functionality.
- Allowing for updating specific token parameters, such as the TVL limit and metadata, without affecting other token details.
- Providing clear documentation on how to use the function effectively.
The
updateToken
function strengthens the flexibility and maintainability of the token management system.precompiles/assets/abi.json (2)
129-129
: Approve the changes to theregisterToken
function.The following changes have been made:
- The function has been renamed from
registerOrUpdateTokens
toregisterToken
, indicating a separation of concerns for token registration and updates.- The
clientChainID
parameter has been renamed toclientChainId
, following a consistent naming convention.- A new
oracleInfo
parameter of type string has been added, allowing additional metadata related to oracle services to be provided during token registration.These changes enhance clarity and functionality.
Also applies to: 132-132, 160-164, 172-175
Line range hint
177-209
: Approve the addition of theupdateToken
function.The
updateToken
function has been introduced to handle token updates separately from token registration. This change aligns with the separation of concerns and improves the clarity of the contract's functionality.The function accepts the following parameters:
clientChainId
: The ID of the client chain.token
: The token bytes.tvlLimit
: The TVL (Total Value Locked) limit for the token.metaData
: Additional metadata associated with the token.The function returns a boolean value indicating the success of the token update operation.
This addition enhances the contract's ability to manage token updates independently and provides flexibility in modifying token details.
precompiles/assets/assets.go (6)
78-78
: LGTM!The comment change improves the accuracy of the description.
105-105
: LGTM!The error handling has been refined to consistently pack outputs with a single boolean value. The removal of the workaround comment suggests that the underlying issue may have been resolved.
107-112
: LGTM!The addition of
MethodRegisterToken
enhances the functionality of theRun
method by introducing a new method for registering tokens. The error handling is consistent with the other methods.
113-118
: LGTM!The addition of
MethodUpdateToken
enhances the functionality of theRun
method by introducing a new method for updating tokens. The error handling is consistent with the other methods.
122-125
: LGTM!The error handling for query methods has been refined to consistently pack outputs with a single boolean value indicating the success status, along with the query result. This improves the clarity and consistency of the error handling.
Also applies to: 128-131
153-153
: LGTM!The addition of
MethodRegisterToken
andMethodUpdateToken
to theIsTransaction
method ensures that these methods are correctly identified as transaction methods. This enhances the clarity and functionality of the transaction identification logic.precompiles/assets/tx.go (2)
189-192
: Good practice of selectively updating the metadata.The function is updating the metadata only if it's provided in the input. This is a good practice to avoid overwriting existing metadata with empty values.
150-152
: Verify the usage ofRegisterNewTokenAndSetTokenFeeder
.Ensure that the
RegisterNewTokenAndSetTokenFeeder
function is defined and does the necessary operations.Run the following script to verify the function usage:
Verification successful
Function
RegisterNewTokenAndSetTokenFeeder
is properly defined and used.
- The function is defined in
x/oracle/keeper/params.go
.- It is used in
precompiles/assets/tx.go
.- It is part of an interface in
x/assets/types/expected_keepers.go
.The function is integrated into the codebase as expected.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the definition and usage of `RegisterNewTokenAndSetTokenFeeder`. # Test: Search for the function definition. Expect: The function definition. rg --type go $'func \(.*\) RegisterNewTokenAndSetTokenFeeder\(' # Test: Search for the function usage. Expect: The usage in `RegisterToken`. rg --type go -A 5 $'RegisterNewTokenAndSetTokenFeeder'Length of output: 1194
precompiles/assets/types.go (2)
124-217
: LGTM!The code changes in the
TokenFromInputs
function are approved.
219-262
: LGTM!The new
UpdateTokenFromInputs
function looks good with proper validation and error handling.x/oracle/types/params.go (1)
489-493
: Enhancement: Support for multiple asset IDs per token.The changes to the
GetTokenIDFromAssetID
function enable it to handle scenarios where a token'sAssetID
field contains multiple asset IDs separated by commas. This enhances flexibility and allows for more diverse token configurations.The updated logic correctly splits the
AssetID
string by commas and iterates over the individual asset IDs to find a match with the providedassetID
. If a match is found, the corresponding token ID is returned.
There was a problem hiding this 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
Files selected for processing (2)
- app/ante/cosmos/txsize_gas.go (3 hunks)
- precompiles/testutil/logs.go (1 hunks)
Files skipped from review due to trivial changes (1)
- precompiles/testutil/logs.go
Additional comments not posted (2)
app/ante/cosmos/txsize_gas.go (2)
42-43
: LGTM!The code changes are approved. The change in the size limit constant from
utils.TxSizeLimit
toanteutils.TxSizeLimit
aligns with the reorganization of utility functions mentioned in the summary.
73-73
: LGTM!The code changes are approved. The added comment directive for gitleaks enhances the clarity of the code regarding security practices without altering the logic.
Test object: 3c2f9bb
|
There was a problem hiding this 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
Files selected for processing (1)
- x/oracle/keeper/params.go (2 hunks)
Additional comments not posted (1)
x/oracle/keeper/params.go (1)
35-111
: Review ofRegisterNewTokenAndSetTokenFeeder
FunctionThis function is central to the PR's functionality, facilitating the registration of new tokens and setting up token feeders. Here are several observations and suggestions:
Error Handling and Input Validation:
- The function begins by retrieving existing parameters and checks if the asset ID already exists (lines 36-39). This is a good practice as it prevents duplicate entries.
- Error messages should provide more context to aid debugging. For example, including the attempted asset ID in the error message at line 38 could be more informative.
Handling of Chains and Tokens:
- The logic to check if a chain exists and potentially add a new one (lines 40-54) is clear. However, consider encapsulating this logic into a separate method to improve readability and reusability.
- When parsing the decimal and interval values (lines 55-68), the function defaults to predefined constants if necessary. This fallback mechanism is prudent, but it's essential to ensure that these defaults are documented clearly in the function's comments or external documentation.
Token and Feeder Registration:
- The loop from lines 70-78 handles the scenario where a token exists and needs to bind a new asset ID. The use of string concatenation for asset IDs (line 74) is straightforward but might not scale well with a large number of asset IDs. Consider using a more structured data format if scalability becomes an issue.
- Adding a new token (lines 81-89) and setting up a new token feeder (lines 91-101) are well-handled. The use of constants and clear struct initialization makes these blocks easy to understand.
Performance Considerations:
- The function updates parameters and potentially modifies the cache based on the transaction context (lines 103-109). It's crucial to ensure that these operations are optimized for performance, especially given the issues reported with gas limits and transaction failures. Profiling and optimizing the database operations and cache interactions could help mitigate these problems.
General Code Quality:
- Overall, the function is structured logically. However, breaking down this large function into smaller, more focused sub-functions could enhance maintainability and testability.
Consider the following improvements:
- Enhance error messages for better debugging.
- Refactor chain and token handling into separate methods.
- Optimize database operations and cache interactions to address performance issues.
Would you like assistance in implementing these suggestions or further testing to ensure compatibility with existing systems?
* feat(exo-gateway): add oracleInfo ExocoreNetwork/exocore#159 adds a new `oracleInfo` parameter to the `IAssets.sol` precompile when registering tokens. This PR follows that change. Previously, multiple tokens could be registered with a single call to ExocoreGateway. However, that resulted in too many variables (and too much gas) for Solidity to handle within a single function. To that end, and keeping in mind that addition of new tokens isn't likely to be a frequent occurrence, the function's capabilities have been tempered to support only a single token within the transaction. As a side effect, this fixes #80 * chore(fmt): forge fmt with newer nightly * doc: update comments * fix(test): update test from merge * forge fmt * feat(assets): split precompile into add/update Since only certain parameters can be updated (TVL limit and metadata), it does not make sense to use the same function for token additions and updates * fix: accept empty metadata * fix: remove superfluous check * forge fmt * doc: fix outdated comments
Test passed on 9cc32ee. |
* feat(exo-gateway): add oracleInfo ExocoreNetwork/exocore#159 adds a new `oracleInfo` parameter to the `IAssets.sol` precompile when registering tokens. This PR follows that change. Previously, multiple tokens could be registered with a single call to ExocoreGateway. However, that resulted in too many variables (and too much gas) for Solidity to handle within a single function. To that end, and keeping in mind that addition of new tokens isn't likely to be a frequent occurrence, the function's capabilities have been tempered to support only a single token within the transaction. As a side effect, this fixes #80 * chore(fmt): forge fmt with newer nightly * doc: update comments * fix(test): update test from merge * forge fmt * feat(assets): split precompile into add/update Since only certain parameters can be updated (TVL limit and metadata), it does not make sense to use the same function for token additions and updates * fix: accept empty metadata * fix: remove superfluous check * forge fmt * doc: fix outdated comments
* feat(exo-gateway): add oracleInfo ExocoreNetwork/exocore#159 adds a new `oracleInfo` parameter to the `IAssets.sol` precompile when registering tokens. This PR follows that change. Previously, multiple tokens could be registered with a single call to ExocoreGateway. However, that resulted in too many variables (and too much gas) for Solidity to handle within a single function. To that end, and keeping in mind that addition of new tokens isn't likely to be a frequent occurrence, the function's capabilities have been tempered to support only a single token within the transaction. As a side effect, this fixes #80 * chore(fmt): forge fmt with newer nightly * doc: update comments * fix(test): update test from merge * forge fmt * feat(assets): split precompile into add/update Since only certain parameters can be updated (TVL limit and metadata), it does not make sense to use the same function for token additions and updates * fix: accept empty metadata * fix: remove superfluous check * forge fmt * doc: fix outdated comments * feat(tvl-limit): impose tvl limit on client chain We can consider the TVL limits to be of two types: one is the logical TVL limit which disallows deposits from exceeding total supply and the other is the operational TVL limit which is used for risk management during the launch phase of the network. The former is imposed by Exocore's precompiles while the latter is imposed by the client chain's gateway contracts. Both fields are editable by the contract owners: the total supply in response to minting and burning and the deposit limits with time to larger values. As a consequence of the introduction of a limit on the client chain, it must now be provided at the time of token registration. It can be updated or reduced unilaterally on the client chain. While editing the `totalSupply` on Exocore, it is best to be fully accurate. Reducing the `totalSupply` to lower than the TVL limit configured on the client chain will result in failed deposits which may produce a system halt. Even if the TVL limit is reduced on the client chain after this, it can impact the messages that are already in-flight. * fix(doc): update IAssets comment * feat: prevent misconfig of supply and tvl limits The objective of this change is to prevent deposit failures by ensuring that the tvlLimit (as enforced on the client chain) <= totalSupply (as enforced by Exocore). Note that, with time, the totalSupply stored on Exocore may become outdated as well; however, it is a secondary check which occurs only after tokens are transferred into the Vault on the client chain. Since the addition of tokens to the whitelist happens through Exocore, it can be enforced easily that the tvlLimit of said token is less than the total supply (as known to Exocore). Whenever a transaction to update the tvlLimit on a client chain is received, it is handled on a case-by-case basis. - A decrease in tvl limit is always allowed. - An increase in tvl limit is forwarded to Exocore for validation. Exocore receives this validation request, and responds in the affirmative if the new tvl limit <= supply of the token (again, according to Exocore). It responds in the negative, otherwise. Reciprocally, whenever an attempt is made to change the recorded value of total supply corresponding to a token on Exocore, here's what happens. - An increase in the total supply is always allowed. - A decrease in the total supply is forwarded to the client chain for validation. when the client chain receives this validation request, it responds in the affirmative if the tvl limit <= new total supply (which may be totally different from the actual total supply due to changes in the number when the message was in-flight). Otherwise, it responds in the negative. One caveat for both of these validations is that if a validation request arising from the validating chain is already in-flight, the validation is rejected. This is to ensure that in-flight messages cannot result in a misconfiguration. For example, if the TVL limit is increased from 400m to 500m and the total supply is 600m, it will be approved. However, if, at the same time, there is a validation request sent to the client chain by Exocore to verify if the total supply can be reduced to 450m, it could cause inconsistency. In this event, the TVL limit increase is rejected and the total supply decrease will be approved. * test: validate nonces once done * feat(bootstrap): reject if tvl limit > supply * fix: use a count of in-flight messages, not bool * fix: native restake limit + non whitelist check * fix: index requests by chainId and nonce Mistakenly, only the nonce was used but that is not accurate * fix: simplify TVL limit and total supply limit Based on the discussion, it was decided to remove the total supply limit enforced by Exocore. Instead, the TVL limit will be imposed on the client chain, which will also enforce that any transfers more than the total supply will fail. It is possible to set a value of infinite TVL limit by using `type(uin256).max` * fix(script): print ProxyAdmin address to JSON * feat(ci): fail compilation if code size is large Within Foundry's configuration, the `deny_warnings` boolean may be set to `true` to force it to report a compilation failure in the case of warnings. With the `code-size` warning enabled, this will cause a build failure and report to the developer. * fix: remove unused variables * fix: remove some more unused fields * feat: use `uint128` for tvl limit on exo gateway
* feat(exo-gateway): add oracleInfo ExocoreNetwork/exocore#159 adds a new `oracleInfo` parameter to the `IAssets.sol` precompile when registering tokens. This PR follows that change. Previously, multiple tokens could be registered with a single call to ExocoreGateway. However, that resulted in too many variables (and too much gas) for Solidity to handle within a single function. To that end, and keeping in mind that addition of new tokens isn't likely to be a frequent occurrence, the function's capabilities have been tempered to support only a single token within the transaction. As a side effect, this fixes #80 * chore(fmt): forge fmt with newer nightly * doc: update comments * fix(test): update test from merge * forge fmt * feat(assets): split precompile into add/update Since only certain parameters can be updated (TVL limit and metadata), it does not make sense to use the same function for token additions and updates * fix: accept empty metadata * fix: remove superfluous check * forge fmt * doc: fix outdated comments * feat(tvl-limit): impose tvl limit on client chain We can consider the TVL limits to be of two types: one is the logical TVL limit which disallows deposits from exceeding total supply and the other is the operational TVL limit which is used for risk management during the launch phase of the network. The former is imposed by Exocore's precompiles while the latter is imposed by the client chain's gateway contracts. Both fields are editable by the contract owners: the total supply in response to minting and burning and the deposit limits with time to larger values. As a consequence of the introduction of a limit on the client chain, it must now be provided at the time of token registration. It can be updated or reduced unilaterally on the client chain. While editing the `totalSupply` on Exocore, it is best to be fully accurate. Reducing the `totalSupply` to lower than the TVL limit configured on the client chain will result in failed deposits which may produce a system halt. Even if the TVL limit is reduced on the client chain after this, it can impact the messages that are already in-flight. * fix(doc): update IAssets comment * feat: prevent misconfig of supply and tvl limits The objective of this change is to prevent deposit failures by ensuring that the tvlLimit (as enforced on the client chain) <= totalSupply (as enforced by Exocore). Note that, with time, the totalSupply stored on Exocore may become outdated as well; however, it is a secondary check which occurs only after tokens are transferred into the Vault on the client chain. Since the addition of tokens to the whitelist happens through Exocore, it can be enforced easily that the tvlLimit of said token is less than the total supply (as known to Exocore). Whenever a transaction to update the tvlLimit on a client chain is received, it is handled on a case-by-case basis. - A decrease in tvl limit is always allowed. - An increase in tvl limit is forwarded to Exocore for validation. Exocore receives this validation request, and responds in the affirmative if the new tvl limit <= supply of the token (again, according to Exocore). It responds in the negative, otherwise. Reciprocally, whenever an attempt is made to change the recorded value of total supply corresponding to a token on Exocore, here's what happens. - An increase in the total supply is always allowed. - A decrease in the total supply is forwarded to the client chain for validation. when the client chain receives this validation request, it responds in the affirmative if the tvl limit <= new total supply (which may be totally different from the actual total supply due to changes in the number when the message was in-flight). Otherwise, it responds in the negative. One caveat for both of these validations is that if a validation request arising from the validating chain is already in-flight, the validation is rejected. This is to ensure that in-flight messages cannot result in a misconfiguration. For example, if the TVL limit is increased from 400m to 500m and the total supply is 600m, it will be approved. However, if, at the same time, there is a validation request sent to the client chain by Exocore to verify if the total supply can be reduced to 450m, it could cause inconsistency. In this event, the TVL limit increase is rejected and the total supply decrease will be approved. * test: validate nonces once done * feat(bootstrap): reject if tvl limit > supply * fix: use a count of in-flight messages, not bool * fix: native restake limit + non whitelist check * fix: index requests by chainId and nonce Mistakenly, only the nonce was used but that is not accurate * fix: simplify TVL limit and total supply limit Based on the discussion, it was decided to remove the total supply limit enforced by Exocore. Instead, the TVL limit will be imposed on the client chain, which will also enforce that any transfers more than the total supply will fail. It is possible to set a value of infinite TVL limit by using `type(uin256).max` * fix(script): print ProxyAdmin address to JSON * feat(ci): fail compilation if code size is large Within Foundry's configuration, the `deny_warnings` boolean may be set to `true` to force it to report a compilation failure in the case of warnings. With the `code-size` warning enabled, this will cause a build failure and report to the developer. * fix: remove unused variables * fix: remove some more unused fields * feat: use `uint128` for tvl limit on exo gateway
Description
When register a new token through assets-module, set and bonded it to a token in oracle module, and set up a tokenFeeder for that token to update price.
Notable Changes
RegisterOrUpdateClientToken
, only allow: TotalSupply, MetaInfo to be updatedCloses #XXX
Summary by CodeRabbit
New Features
Bug Fixes