feat(oracle): Add new DatedExchangeRate query with block timestamp and block height #2097
Conversation
WalkthroughThe changes in this pull request encompass updates to multiple files, primarily focusing on enhancing the functionality of the Nibiru EVM. Key modifications include the addition of a new RPC method for retrieving dated exchange rates, updates to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (13)
x/evm/embeds/artifacts/contracts/IOracle.sol/IOracle.json (1)
17-29: LGTM! The enhanced return type structure is well-designed.
The modification to return price with block metadata (time and height) is a good improvement that provides more context about the price data. The type choices are appropriate:
uint256for price accommodates large numbersuint64is sufficient for both block time and height
Consider documenting these changes in the contract's interface documentation (IOracle.sol) to help users understand:
- The units used for price (e.g., 6 or 18 decimal places)
- The format of blockTimeMs (Unix milliseconds timestamp)
x/evm/embeds/contracts/IOracle.sol (1)
6-13: Documentation looks good, consider adding validation details.
The documentation clearly explains the function's purpose and return values. However, it could be enhanced by including:
- Input validation requirements for the
pairparameter - Potential error cases or failure scenarios
Consider adding these details to the documentation:
/// @notice Queries the dated exchange rate for a given pair
/// @param pair The asset pair to query. For example, "ubtc:uusd" is the
/// USD price of BTC and "unibi:uusd" is the USD price of NIBI.
+/// @dev The pair must be in the format "base:quote" where both base and quote
+/// are valid denominations. Will revert if the pair is invalid or if the
+/// price data is not available.
/// @return price The exchange rate for the given pair
/// @return blockTimeMs The block time in milliseconds when the price was
/// last updated
/// @return blockHeight The block height when the price was last updated
/// @dev This function is view-only and does not modify state.x/evm/precompile/oracle_test.go (2)
60-62: Consider using more realistic test values.
While the test is functional, using more realistic values for block time and height would make the test more representative of production scenarios. Consider using recent timestamps and typical block heights.
-deps.Ctx = deps.Ctx.WithBlockTime(time.Unix(69, 420)).WithBlockHeight(69)
+// Use more realistic values
+deps.Ctx = deps.Ctx.WithBlockTime(time.Now()).WithBlockHeight(1000000)Add test coverage for error scenarios in Oracle precompile
The current test suite only covers the happy path and ABI packing errors, but misses important error scenarios from the precompile implementation:
- Invalid asset pair format (via
asset.TryNewPair) - Missing price data (via
GetDatedExchangeRate) - Invalid input argument type
Suggested test cases to add:
- Test with malformed asset pair (e.g., "unibi-uusd" or "unibi")
- Test querying a non-existent price
- Test with wrong argument type for the pair parameter
🔗 Analysis chain
Line range hint 60-77: Consider adding error test cases for the dated price functionality.
While the happy path is well tested, consider adding test cases for:
- Invalid block time
- Zero block height
- Missing price data
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are any error test cases for queryExchangeRate
rg -A 5 "TestOracle.*Error.*queryExchangeRate"
Length of output: 48
Script:
#!/bin/bash
# Let's search for all test cases in the oracle test file to understand the coverage
rg -A 10 "func.*Test.*Oracle" x/evm/precompile/oracle_test.go
# Also search for any error handling patterns in the oracle precompile
rg "Error|err" x/evm/precompile/oracle.go
Length of output: 1682
x/evm/precompile/oracle.go (1)
83-88: Implementation looks good but needs documentation
The implementation correctly retrieves and returns the dated exchange rate with block information. Consider adding a comment explaining the return values and their units (e.g., blockTime format).
Apply this documentation improvement:
func (p precompileOracle) queryExchangeRate(
ctx sdk.Context,
method *gethabi.Method,
args []interface{},
readOnly bool,
) (bz []byte, err error) {
+ // Returns:
+ // - price: exchange rate as a big.Int
+ // - blockTime: unix timestamp of the block when price was last updated
+ // - blockHeight: block height when price was last updatedx/oracle/keeper/querier.go (1)
82-83: Enhance function documentation.
The current comment could be more descriptive. Consider adding details about:
- The return values (price, timestamp, block height)
- Error conditions
- The relationship with
ExchangeRatevalidation
-// get the latest price snapshot from the oracle for a pair
+// DatedExchangeRate returns the latest price snapshot from the oracle for a given pair.
+// It includes the price value, timestamp (in milliseconds), and the block height.
+// Returns an error if the pair is invalid or if the exchange rate cannot be retrieved.x/oracle/keeper/keeper.go (1)
190-190: Fix return parameter naming convention.
The return parameter BlockHeight should be in camelCase as per Go conventions.
-func (k Keeper) GetDatedExchangeRate(ctx sdk.Context, pair asset.Pair) (price sdk.Dec, blockTimeMs int64, BlockHeight uint64, err error) {
+func (k Keeper) GetDatedExchangeRate(ctx sdk.Context, pair asset.Pair) (price sdk.Dec, blockTimeMs int64, blockHeight uint64, err error) {proto/nibiru/oracle/v1/query.proto (1)
124-137: Consider documentation and naming consistency improvements.
While the message structure is well-designed, there are two minor suggestions:
- The field name
pricediffers from theexchange_ratenaming used in other messages (e.g.,QueryExchangeRateResponse). Consider using consistent terminology across messages. - Add a comment explaining the
pricefield similar to the helpful comments ontimestamp_msandheight.
Apply this diff to improve documentation:
// QueryDatedExchangeRateResponse is the request type for the
// Query/DatedExchangeRate RPC method.
message QueryDatedExchangeRateResponse {
+ // price defines the exchange rate of the requested pair
string price = 1 [
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec",
(gogoproto.nullable) = false
];x/oracle/keeper/querier_test.go (3)
139-165: Extract time constants and enhance event verification.
Consider these improvements:
- Extract time constants to named variables for better readability
- Add verification for block height in events
+ const (
+ timeIncrement = 1 * time.Second
+ heightIncrement = 1
+ )
+
// --- Set first price for BTC/NUSD ---
input.OracleKeeper.SetPrice(input.Ctx, pairBTC, rateBTC1)
testutilevents.RequireContainsTypedEvent(
t,
input.Ctx,
&types.EventPriceUpdate{
Pair: pairBTC.String(),
Price: rateBTC1,
TimestampMs: input.Ctx.BlockTime().UnixMilli(),
+ BlockHeight: input.Ctx.BlockHeight(),
},
)198-218: Add block height verification in query responses.
The test verifies prices and timestamps but should also verify block heights in the responses to ensure complete data integrity.
require.NoError(t, err)
require.Equal(t, rateBTC2, resBTC.Price)
require.Equal(t, input.Ctx.BlockTime().UnixMilli(), resBTC.TimestampMs)
+ require.Equal(t, input.Ctx.BlockHeight(), resBTC.BlockHeight)121-121: Add test documentation.
Add a comment block explaining:
- The purpose of the test
- The scenarios being tested
- The expected behavior for each scenario
+// TestQueryDatedExchangeRate verifies the DatedExchangeRate query functionality:
+// - Tests price updates and retrieval for multiple pairs (BTC/NUSD, ETH/NUSD)
+// - Verifies correct timestamp and price tracking across multiple updates
+// - Ensures proper error handling for non-existent pairs
func TestQueryDatedExchangeRate(t *testing.T) {CHANGELOG.md (2)
151-151: Consider adding more details about the dated price query feature.
The changelog entry for PR #2097 could be more descriptive to help users understand:
- What information the dated price query provides
- Why this feature was added
- Any important usage details
Consider expanding the entry to something like:
-- [#2097](https://github.com/NibiruChain/nibiru/pull/2097) - feat(evm): Add new query to get dated price from the oracle precompile
++ [#2097](https://github.com/NibiruChain/nibiru/pull/2097) - feat(evm): Add new query to get dated price from the oracle precompile. This allows clients to fetch historical exchange rates along with their corresponding timestamps and block heights.66-68: Consider using proper Markdown link syntax for URLs.
Instead of using bare URLs, use proper Markdown link syntax for better readability and maintainability.
- - https://github.com/code-423n4/2024-10-nibiru-zenith/issues/43
- - https://github.com/code-423n4/2024-10-nibiru-zenith/issues/47
- - https://github.com/NibiruChain/nibiru/issues/2083
+ - [Issue #43](https://github.com/code-423n4/2024-10-nibiru-zenith/issues/43)
+ - [Issue #47](https://github.com/code-423n4/2024-10-nibiru-zenith/issues/47)
+ - [Issue #2083](https://github.com/NibiruChain/nibiru/issues/2083)🧰 Tools
🪛 Markdownlint
66-66: null
Bare URL used
(MD034, no-bare-urls)
67-67: null
Bare URL used
(MD034, no-bare-urls)
68-68: null
Bare URL used
(MD034, no-bare-urls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
x/oracle/types/query.pb.gois excluded by!**/*.pb.gox/oracle/types/query.pb.gw.gois excluded by!**/*.pb.gw.go
📒 Files selected for processing (10)
- CHANGELOG.md (2 hunks)
- app/wasmext/stargate_query.go (1 hunks)
- proto/nibiru/oracle/v1/query.proto (2 hunks)
- x/evm/embeds/artifacts/contracts/IOracle.sol/IOracle.json (1 hunks)
- x/evm/embeds/contracts/IOracle.sol (1 hunks)
- x/evm/precompile/oracle.go (1 hunks)
- x/evm/precompile/oracle_test.go (3 hunks)
- x/oracle/keeper/keeper.go (1 hunks)
- x/oracle/keeper/querier.go (1 hunks)
- x/oracle/keeper/querier_test.go (1 hunks)
🧰 Additional context used
🪛 Markdownlint
CHANGELOG.md
66-66: null
Bare URL used
(MD034, no-bare-urls)
67-67: null
Bare URL used
(MD034, no-bare-urls)
68-68: null
Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (12)
x/evm/embeds/artifacts/contracts/IOracle.sol/IOracle.json (1)
17-29: Verify integration with dependent components.
The change from returning a string to structured data requires updates in components that consume this interface.
x/evm/embeds/contracts/IOracle.sol (1)
14-19: Function signature changes improve type safety and context.
The new return types provide better type safety and additional context about the price data. However, this is a breaking change that requires updates to all consumers of this interface.
Let's verify the impact on consumers:
x/evm/precompile/oracle_test.go (1)
4-6: LGTM: Required imports added for new functionality.
The new imports support the enhanced price query functionality with timestamps and precise decimal handling.
x/evm/precompile/oracle.go (1)
83-88: Verify ABI compatibility with smart contracts
The change from returning a single price to returning (price, blockTime, blockHeight) could be a breaking change for existing smart contracts using this precompile.
Let's verify the ABI changes and usage:
✅ Verification successful
ABI changes are properly documented and aligned
The interface definition in IOracle.sol shows that the changes are well-documented and the contract interface explicitly defines the return values (price, blockTimeMs, blockHeight) with proper documentation. The precompile implementation matches exactly with the Solidity interface definition. This is not a breaking change as it appears to be a new interface version deployed at a specific precompile address.
Key observations:
- Interface is properly documented with detailed
@noticeand@devcomments - Return values are clearly specified in the interface
- Precompile address is defined as a constant at
0x0000000000000000000000000000000000000801
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential breaking changes in contract interfaces and usage
# Test 1: Look for contract interfaces defining or using queryExchangeRate
ast-grep --pattern 'interface $_ {
$$$
function queryExchangeRate($$$) $$$;
$$$
}'
# Test 2: Search for contract calls to queryExchangeRate
rg -A 2 'queryExchangeRate\('
Length of output: 693
Script:
#!/bin/bash
# Let's check the interface definition and its history to understand if this is a breaking change
# Check the current interface definition
cat x/evm/embeds/contracts/IOracle.sol
# Check if there are any existing implementations or tests
rg -l "IOracle"
# Look for any existing usage patterns
rg -A 5 "queryExchangeRate.*returns"
Length of output: 1190
x/oracle/keeper/querier.go (3)
84-92: Well-structured error handling.
Good practices observed:
- Reuses existing validation logic from
ExchangeRate - Consistent error handling pattern with other querier methods
- Proper empty response structure on error
88-89: Clean and well-structured implementation.
The implementation follows good practices:
- Proper context handling
- Clear separation of concerns with keeper delegation
- Single responsibility principle
93-93: Verify timestamp handling consistency.
The response includes a timestamp in milliseconds. Let's verify that the timestamp handling is consistent across the codebase.
✅ Verification successful
Timestamp handling is consistent across the codebase
The verification shows consistent use of milliseconds for timestamps throughout the oracle module:
- All proto definitions explicitly document timestamps as "milliseconds since unix epoch"
- All timestamp fields are consistently named
timestamp_msorTimestampMs - All timestamp values are consistently obtained using
UnixMilli()from the block time - The TWAP calculation in keeper.go correctly handles millisecond-based time differences
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other timestamp-related code to verify consistency in units (ms vs s)
rg -g '!{vendor,node_modules}/**' -A 2 -B 2 'TimestampMs|timestamp_ms'
Length of output: 13810
app/wasmext/stargate_query.go (1)
123-123: LGTM! The new query path is correctly integrated.
The addition follows the established patterns:
- Correct path format: "/nibiru.oracle.v1.Query/DatedExchangeRate"
- Consistent with other oracle queries
- Proper response type initialization
Let's verify the response type exists:
✅ Verification successful
✅ Response type exists and is properly integrated
The verification confirms that:
- The
QueryDatedExchangeRateResponsetype is properly defined inproto/nibiru/oracle/v1/query.proto - The generated Go type exists in
x/oracle/types/query.pb.go - The response type is properly used in the oracle keeper's implementation in
x/oracle/keeper/querier.go
The integration in app/wasmext/stargate_query.go is correct and consistent with the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify QueryDatedExchangeRateResponse type exists in the codebase
# Expected: Find the type definition in the oracle types package
# Search for the response type definition
ast-grep --pattern 'message QueryDatedExchangeRateResponse {
$$$
}'
Length of output: 71
Script:
#!/bin/bash
# Let's try different approaches to find the response type
# 1. Search for proto files containing DatedExchangeRate
fd -e proto | xargs rg "DatedExchangeRate"
# 2. Search for Go files containing the response type
rg "QueryDatedExchangeRateResponse" -A 2
# 3. Look specifically in oracle-related proto files
fd "oracle.*\.proto" | xargs cat
Length of output: 14265
x/oracle/keeper/keeper.go (1)
190-198: LGTM on the overall implementation.
The new method successfully implements the dated exchange rate query functionality, providing both the price and its temporal metadata (block time and height). This aligns well with the PR objective of adding dated price queries.
Let's verify the integration with the RPC endpoint:
✅ Verification successful
Integration verification complete - implementation is properly connected
The GetDatedExchangeRate method is correctly integrated across all required layers:
- Properly exposed via gRPC service in
proto/nibiru/oracle/v1/query.proto - Implemented in the QueryServer interface
- Connected through:
- Oracle keeper implementation (
x/oracle/keeper/keeper.go) - Querier layer (
x/oracle/keeper/querier.go) - EVM precompile integration (
x/evm/precompile/oracle.go)
- Oracle keeper implementation (
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the integration between GetDatedExchangeRate and RPC endpoints
# Test: Search for RPC handlers using this method
rg -A 5 "GetDatedExchangeRate"
# Test: Search for related protobuf definitions
rg "DatedExchangeRate" --type proto
Length of output: 1354
Script:
#!/bin/bash
# Let's search for proto files with a different approach and check the RPC integration
fd -e proto | xargs rg "DatedExchangeRate"
# Also check for any gRPC service definitions related to exchange rates
fd -e proto | xargs rg "service" -A 5 | rg -A 5 "Exchange"
# Let's also verify the querier implementation
ast-grep --pattern 'type QueryServer interface {
$$$
DatedExchangeRate($_, $_) $$$
$$$
}'
Length of output: 4710
proto/nibiru/oracle/v1/query.proto (2)
25-30: LGTM! Well-structured RPC method addition.
The new DatedExchangeRate RPC method follows the service's conventions and appropriately reuses the existing request type while introducing a new response type for dated exchange rates.
121-123: LGTM! Appropriate use of empty message.
The empty message definition is correct for a request that doesn't require any parameters, following Proto3 best practices.
CHANGELOG.md (1)
Line range hint 1-1000: LGTM! The changelog structure and categorization are well-maintained.
The changelog follows the Keep a Changelog format and properly categorizes changes under appropriate sections (Features, Bug Fixes, Dependencies, etc.). The entries are clear and include PR references.
🧰 Tools
🪛 Markdownlint
66-66: null
Bare URL used
(MD034, no-bare-urls)
67-67: null
Bare URL used
(MD034, no-bare-urls)
68-68: null
Bare URL used
(MD034, no-bare-urls)
| s.Equal(out[0].(*big.Int), big.NewInt(67000000000000000)) | ||
| s.Equal(out[1].(uint64), uint64(69000)) | ||
| s.Equal(out[2].(uint64), uint64(69)) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve test maintainability with explicit value calculations.
The test assertions would be clearer with explicit calculations of expected values. This would make the relationship between inputs and expected outputs more obvious.
+// Define expected values with clear calculations
+expectedPrice := big.NewInt(67000000000000000) // 0.067 in wei
+expectedBlockTimeMs := uint64(69 * 1000) // Unix seconds to milliseconds
+expectedBlockHeight := uint64(69)
-s.Equal(out[0].(*big.Int), big.NewInt(67000000000000000))
-s.Equal(out[1].(uint64), uint64(69000))
-s.Equal(out[2].(uint64), uint64(69))
+s.Equal(out[0].(*big.Int), expectedPrice)
+s.Equal(out[1].(uint64), expectedBlockTimeMs)
+s.Equal(out[2].(uint64), expectedBlockHeight)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| s.Equal(out[0].(*big.Int), big.NewInt(67000000000000000)) | |
| s.Equal(out[1].(uint64), uint64(69000)) | |
| s.Equal(out[2].(uint64), uint64(69)) | |
| // Define expected values with clear calculations | |
| expectedPrice := big.NewInt(67000000000000000) // 0.067 in wei | |
| expectedBlockTimeMs := uint64(69 * 1000) // Unix seconds to milliseconds | |
| expectedBlockHeight := uint64(69) | |
| s.Equal(out[0].(*big.Int), expectedPrice) | |
| s.Equal(out[1].(uint64), expectedBlockTimeMs) | |
| s.Equal(out[2].(uint64), expectedBlockHeight) |
| exchangeRate, err := k.ExchangeRates.Get(ctx, pair) | ||
| if err != nil { | ||
| return | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling with specific error types.
The current error handling doesn't provide specific information about why the exchange rate retrieval failed. Consider wrapping the error with additional context.
exchangeRate, err := k.ExchangeRates.Get(ctx, pair)
if err != nil {
- return
+ return sdk.Dec{}, 0, 0, sdkerrors.Wrapf(types.ErrNoValidPrice, "failed to get exchange rate for pair %s: %v", pair, err)
}Committable suggestion was skipped due to low confidence.
| func TestQueryDatedExchangeRate(t *testing.T) { | ||
| input := CreateTestFixture(t) | ||
| querier := NewQuerier(input.OracleKeeper) | ||
|
|
||
| // Set initial block time and height | ||
| initialTime := input.Ctx.BlockTime() | ||
| initialHeight := input.Ctx.BlockHeight() | ||
|
|
||
| // Pair 1: BTC/NUSD | ||
| pairBTC := asset.Registry.Pair(denoms.BTC, denoms.NUSD) | ||
| rateBTC1 := math.LegacyNewDec(1700) | ||
| rateBTC2 := math.LegacyNewDec(1800) | ||
|
|
||
| // Pair 2: ETH/NUSD | ||
| pairETH := asset.Registry.Pair(denoms.ETH, denoms.NUSD) | ||
| rateETH1 := math.LegacyNewDec(100) | ||
| rateETH2 := math.LegacyNewDec(110) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using table-driven tests for better coverage.
While the test setup is well-structured, consider converting it to a table-driven test to cover more edge cases such as:
- Invalid pairs
- Zero or negative prices
- Different time intervals
- Maximum/minimum values
|
|
||
| // Advance time and block height | ||
| input.Ctx = input.Ctx.WithBlockTime(initialTime.Add(2 * time.Second)). | ||
| WithBlockHeight(initialHeight + 2) | ||
|
|
||
| // --- Set second price for BTC/NUSD --- | ||
| input.OracleKeeper.SetPrice(input.Ctx, pairBTC, rateBTC2) | ||
| testutilevents.RequireContainsTypedEvent( | ||
| t, | ||
| input.Ctx, | ||
| &types.EventPriceUpdate{ | ||
| Pair: pairBTC.String(), | ||
| Price: rateBTC2, | ||
| TimestampMs: input.Ctx.BlockTime().UnixMilli(), | ||
| }, | ||
| ) | ||
|
|
||
| // Advance time and block height | ||
| input.Ctx = input.Ctx.WithBlockTime(initialTime.Add(3 * time.Second)). | ||
| WithBlockHeight(initialHeight + 3) | ||
|
|
||
| // --- Set second price for ETH/NUSD --- | ||
| input.OracleKeeper.SetPrice(input.Ctx, pairETH, rateETH2) | ||
| testutilevents.RequireContainsTypedEvent( | ||
| t, | ||
| input.Ctx, | ||
| &types.EventPriceUpdate{ | ||
| Pair: pairETH.String(), | ||
| Price: rateETH2, | ||
| TimestampMs: input.Ctx.BlockTime().UnixMilli(), | ||
| }, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract helper functions to reduce code duplication.
Consider creating helper functions to reduce repetition:
+ func setPriceAndVerifyEvent(
+ t *testing.T,
+ ctx sdk.Context,
+ k Keeper,
+ pair asset.Pair,
+ price math.LegacyDec,
+ ) {
+ k.SetPrice(ctx, pair, price)
+ testutilevents.RequireContainsTypedEvent(
+ t,
+ ctx,
+ &types.EventPriceUpdate{
+ Pair: pair.String(),
+ Price: price,
+ TimestampMs: ctx.BlockTime().UnixMilli(),
+ BlockHeight: ctx.BlockHeight(),
+ },
+ )
+ }Committable suggestion was skipped due to low confidence.
What would you use? @Unique-Divine |
Did |
Unique-Divine
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
x/evm/precompile/oracle.go (1)
84-89: Add documentation for return values.Consider adding a comment documenting the meaning and units of the returned values (price, blockTime, blockHeight) to improve code maintainability.
Apply this change above the method:
+// queryExchangeRate returns the exchange rate for a given asset pair along with its timestamp +// Returns: +// - price: The exchange rate as a big.Int +// - blockTime: Unix timestamp of the block when the price was recorded +// - blockHeight: Height of the block when the price was recorded func (p precompileOracle) queryExchangeRate(proto/nibiru/oracle/v1/query.proto (1)
124-139: Consider aligning field naming with existing messages.The message structure is well-defined with appropriate types and clear documentation. However, for consistency with
QueryExchangeRateResponse, consider renaming thepricefield toexchange_rate.Apply this diff to align the field naming:
message QueryDatedExchangeRateResponse { - string price = 1 [ + string exchange_rate = 1 [ (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec", (gogoproto.nullable) = false ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
x/oracle/types/query.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (5)
CHANGELOG.md(3 hunks)proto/nibiru/oracle/v1/query.proto(2 hunks)x/evm/precompile/oracle.go(1 hunks)x/oracle/keeper/querier.go(1 hunks)x/oracle/keeper/querier_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- CHANGELOG.md
- x/oracle/keeper/querier.go
- x/oracle/keeper/querier_test.go
🔇 Additional comments (3)
x/evm/precompile/oracle.go (1)
84-89: Verify breaking changes in dependent contracts.
The implementation looks good, but this is a breaking change that modifies the return values of the precompile method.
Let's verify the contract dependencies and ABI changes:
proto/nibiru/oracle/v1/query.proto (2)
25-29: LGTM! Well-structured RPC method definition.
The new DatedExchangeRate RPC method is well-defined with:
- Clear purpose in the comment
- Appropriate reuse of the existing request message
- Consistent HTTP endpoint pattern
121-122: LGTM! Standard empty message definition.
Empty message definition follows protobuf best practices for requests that don't require parameters.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2097 +/- ##
=======================================
Coverage ? 64.68%
=======================================
Files ? 272
Lines ? 21393
Branches ? 0
=======================================
Hits ? 13838
Misses ? 6594
Partials ? 961
|
Purpose / Abstract
Summary by CodeRabbit
New Features
DatedExchangeRatefor querying the latest price, timestamp, and block height.queryExchangeRatefunction to return more detailed information, including price, block time, and block height.Bug Fixes
Tests
DatedExchangeRatemethod to ensure accurate retrieval of exchange rates and proper error handling.