Add token price oracle metrics and stable coins #826
Conversation
WalkthroughAdds Prometheus metrics for last successful update timestamp and update counts; filters inactive tokens and caches token info in the updater; adds stablecoin parsing in Bitget client and example configs; and bumps github.com/morph-l2/go-ethereum pseudo-version across modules and the Makefile. Changes
sequenceDiagram
autonumber
actor Scheduler as Updater
participant UpdaterProcess as Updater.Process
participant Metrics as Prometheus
participant Client as BitgetClient
participant Store as PriceStore
Scheduler->>UpdaterProcess: trigger update cycle
UpdaterProcess->>UpdaterProcess: filterActiveTokens() -> activeIDs, tokenInfoMap
alt no active tokens
UpdaterProcess->>Metrics: LastSuccessfulUpdateTimestamp.Set(now)
UpdaterProcess->>Metrics: UpdatesTotal.WithLabel("skipped").Add(1)
UpdaterProcess-->>Scheduler: return (skipped)
else active tokens exist
UpdaterProcess->>Client: Get prices for activeIDs
loop per token
Client->>Client: if symbol startsWith("$") parse fixed price else fetch exchange price
Client-->>UpdaterProcess: price or error
end
UpdaterProcess->>UpdaterProcess: calculatePriceRatioWithInfo(tokenInfoMap, prices)
alt no updates needed
UpdaterProcess->>Metrics: LastSuccessfulUpdateTimestamp.Set(now)
UpdaterProcess->>Metrics: UpdatesTotal.WithLabel("skipped").Add(1)
UpdaterProcess-->>Scheduler: return (skipped)
else updates applied
UpdaterProcess->>Store: persist updated prices
UpdaterProcess->>Metrics: LastSuccessfulUpdateTimestamp.Set(now)
UpdaterProcess->>Metrics: UpdatesTotal.WithLabel("updated").Add(1)
UpdaterProcess-->>Scheduler: return (updated)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
token-price-oracle/updater/token_price.go (1)
182-187: Align skip metrics with all “no-op” update pathsThis correctly records a successful “skipped” cycle when all price changes are below threshold. However, the earlier
len(tokenIDs) == 0path logs a skipped cycle but returns without touching these metrics, so long periods with no supported tokens would look like the oracle is stalled.Consider treating the
len(tokenIDs) == 0case as a successful “skipped” cycle too, e.g.:- if len(tokenIDs) == 0 { - log.Warn("No tokens to update, skipping price update cycle") - return nil - } + if len(tokenIDs) == 0 { + log.Warn("No tokens to update, skipping price update cycle") + metrics.LastSuccessfulUpdateTimestamp.Set(float64(time.Now().Unix())) + metrics.UpdatesTotal.WithLabelValues("skipped").Inc() + return nil + }This would make the new metrics fully consistent with the comments and with the log messages.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
token-price-oracle/metrics/metrics.go(1 hunks)token-price-oracle/updater/token_price.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
token-price-oracle/updater/token_price.go (1)
token-price-oracle/metrics/metrics.go (2)
LastSuccessfulUpdateTimestamp(32-37)UpdatesTotal(40-46)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (rust)
🔇 Additional comments (3)
token-price-oracle/updater/token_price.go (1)
223-226: Success metrics placement looks correctRecording
LastSuccessfulUpdateTimestampand incrementingUpdatesTotal{type="updated"}only after verifying a non‑nil receipt withStatus == types.ReceiptStatusSuccessfulcleanly reflects fully successful on‑chain update cycles. No changes needed here.token-price-oracle/metrics/metrics.go (2)
29-46: Metric definitions are clear and match intended semantics
LastSuccessfulUpdateTimestampandUpdatesTotal{type="updated"|"skipped"}are well‑named, have clear help text, and align with howupdate()records successful “updated” vs “skipped” cycles. Once you also record metrics in thelen(tokenIDs) == 0skip path (as suggested intoken_price.go), the comment that this gauge includes both updates and skips will be fully accurate across all no‑op cycles.No issues with these definitions from a metrics/observability standpoint.
51-55: Registration ininit()is consistent with existing patternRegistering the new metrics alongside
UpdateErrorsandAccountBalanceviaprometheus.MustRegisterkeeps a single global registry and matches the existing approach in this package. Looks good.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
token-price-oracle/metrics/metrics.go(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (rust)
🔇 Additional comments (4)
token-price-oracle/metrics/metrics.go (4)
5-5: LGTM!The
timepackage import is appropriately added for initializing the timestamp metric.
30-38: LGTM!The metric definition is well-structured. Using a Gauge for timestamps is correct, and the documentation clearly explains that "successful" encompasses both on-chain updates and skipped updates (below threshold).
40-47: LGTM!The CounterVec definition is appropriate for tracking update cycles. The label-based distinction between "updated" and "skipped" events provides good observability granularity.
54-55: LGTM!Metric registration follows the established pattern and correctly uses
MustRegisterto ensure metrics are properly initialized at startup.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Makefile (1)
33-33: Add token-price-oracle to fmt-go target for consistency.The
update_all_modtarget now includestoken-price-oracle(line 33), but thefmt-gotarget (lines 90–102) does not invokego mod tidyfor this module, unlike the other modules (bindings, contracts, node, ops/l2-genesis, ops/tools, oracle, tx-submitter). This is inconsistent, especially since token-price-oracle/go.mod is being updated as part of this PR.Add token-price-oracle to the fmt-go target:
fmt-go: go work sync cd $(PWD)/bindings/ && go mod tidy cd $(PWD)/contracts/ && go mod tidy cd $(PWD)/node/ && go mod tidy cd $(PWD)/ops/l2-genesis/ && go mod tidy cd $(PWD)/ops/tools/ && go mod tidy cd $(PWD)/oracle/ && go mod tidy cd $(PWD)/tx-submitter/ && go mod tidy + cd $(PWD)/token-price-oracle/ && go mod tidy find . -name '*.go' -type f -not -path "./go-ethereum*" -not -name '*.pb.go' | xargs gofmt -w -s find . -name '*.go' -type f -not -path "./go-ethereum*" -not -name '*.pb.go' | xargs misspell -w find . -name '*.go' -type f -not -path "./go-ethereum*" -not -name '*.pb.go' | xargs goimports -w -local $(PWD)/
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
bindings/go.sumis excluded by!**/*.sumcontracts/go.sumis excluded by!**/*.sumgo.work.sumis excluded by!**/*.sumnode/go.sumis excluded by!**/*.sumops/l2-genesis/go.sumis excluded by!**/*.sumops/tools/go.sumis excluded by!**/*.sumoracle/go.sumis excluded by!**/*.sumtoken-price-oracle/go.sumis excluded by!**/*.sumtx-submitter/go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
Makefile(2 hunks)bindings/go.mod(1 hunks)contracts/go.mod(1 hunks)node/go.mod(1 hunks)ops/l2-genesis/go.mod(1 hunks)ops/tools/go.mod(1 hunks)oracle/go.mod(1 hunks)token-price-oracle/go.mod(1 hunks)tx-submitter/go.mod(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: test
- GitHub Check: check
- GitHub Check: test
- GitHub Check: test
- GitHub Check: check
- GitHub Check: test
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (go)
🔇 Additional comments (3)
token-price-oracle/go.mod (1)
11-11: Missing implementation files for token-price-oracle metrics.The go-ethereum dependency is updated consistently; however, the PR objectives reference new Prometheus metrics (token-price-oracle/metrics/metrics.go) and updater changes (token-price-oracle/updater/token_price.go) that are not provided for review.
Ensure that the full PR changeset includes the metrics instrumentation implementation files mentioned in the PR summary, and verify they follow Prometheus best practices for metric naming and registration.
node/go.mod (1)
14-14: Coordinated go-ethereum version bump across modules.Line 14 updates the go-ethereum pseudo-version from v1.10.14-0.20251119080508-d085f8c79a53 to v1.10.14-0.20251203083507-49fa27bcab24, consistent with parallel updates in other module go.mod files. The new version corresponds to the merge of PR #256 ("Fix precompile address") into morph-l2/go-ethereum on December 3, 2025, which standardizes precompile address notation and updates BLS12381 precompile tests in core/vm.
Makefile (1)
2-3: Version alignment looks correct.The ETHEREUM_SUBMODULE_COMMIT_OR_TAG (
49fa27bcab24...) correctly matches the commit hash in ETHEREUM_TARGET_VERSION.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
token-price-oracle/client/bitget_sdk.go(4 hunks)token-price-oracle/env.example(1 hunks)token-price-oracle/local.sh(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: check
- GitHub Check: check
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: check
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (rust)
🔇 Additional comments (7)
token-price-oracle/client/bitget_sdk.go (3)
20-23: LGTM! Well-documented constant.The exported constant is clearly documented with the format and example, making it easy for other parts of the codebase to understand and use stablecoin pricing.
71-74: LGTM! Clear documentation.The documentation clearly explains the stablecoin handling behavior with a concrete example, making it easy for users to understand the feature.
107-121: LGTM! Clean separation of stablecoin and exchange price paths.The code correctly handles both stablecoin and regular token price fetching with appropriate logging for each path. The error handling and logging context are comprehensive.
token-price-oracle/local.sh (2)
11-11: LGTM! Proper bash escaping for stablecoin mapping.The dollar sign is correctly escaped with a backslash to prevent bash variable expansion, ensuring the literal string "3:$1.0" is passed to the application.
19-22: LGTM! Clear and helpful documentation.The comments provide clear guidance on the token mapping format for both regular tokens and stablecoins, with the important reminder about bash escaping.
token-price-oracle/env.example (2)
21-25: LGTM! Comprehensive format documentation with good examples.The documentation clearly explains both regular token and stablecoin formats, with realistic examples including an off-peg stablecoin ($0.9999). The format is consistent with the implementation in bitget_sdk.go and local.sh.
27-28: LGTM! Consistent format across price feed providers.The Binance mapping example follows the same format as Bitget, including stablecoin support, ensuring consistency and making it easy for users to enable multiple price feeds.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
token-price-oracle/updater/token_price.go (1)
303-366: Add guard for token decimals exceeding 18 to prevent unsigned arithmetic underflowThe decimal adjustment calculation has an unsigned arithmetic underflow vulnerability:
decimalAdjustment := new(big.Int).Exp(big.NewInt(10), big.NewInt(int64(18-tokenDecimals)), nil)Since
tokenDecimalsisuint8, the expression18 - tokenDecimalsuses unsigned arithmetic. If any token hasdecimals > 18, this underflows to a large positive value (e.g.,18 - 19 = 255in uint8), causing the exponent to be10^255instead of a negative scaling factor. This would grossly inflate the price ratio.Add a guard to enforce the constraint:
if tokenDecimals > 18 { return nil, fmt.Errorf("unsupported token decimals %d for token %d (must be <= 18)", tokenDecimals, tokenID) } decimalAdjustmentExp := int64(18) - int64(tokenDecimals) decimalAdjustment := new(big.Int).Exp(big.NewInt(10), big.NewInt(decimalAdjustmentExp), nil)This ensures the subtraction happens in signed space and fails explicitly if an invalid token is encountered.
🧹 Nitpick comments (2)
token-price-oracle/updater/token_price.go (2)
189-194: Clarify whether “no tokens / no active tokens” should count as successful skipped cycles in metricsYou correctly record metrics for:
- “threshold skip” cycles (
len(tokenIDsToUpdate) == 0) as"skipped", and- successful on-chain updates as
"updated".However, earlier short-circuit paths:
- Line 109–112:
len(tokenIDs) == 0- Line 116–119:
len(activeTokenIDs) == 0return
nilwithout touchingLastSuccessfulUpdateTimestamporUpdatesTotal. If the intent oflast_successful_update_timestampandupdates_total{type="skipped"}is to reflect every successful invocation ofupdate(including cycles where there is literally nothing to do), these branches will make the metrics look stale even though the updater is healthy.Consider treating these branches as successful skipped cycles, e.g. by calling a small helper like:
func recordUpdateCycle(result string) { metrics.LastSuccessfulUpdateTimestamp.Set(float64(time.Now().Unix())) metrics.UpdatesTotal.WithLabelValues(result).Inc() }and using
result="skipped"in those early-return cases as well.Also applies to: 197-202, 231-235
245-252: TokenInfo cache and filterActiveTokens helper are generally well-structuredThe TokenInfo cache and
filterActiveTokenshelper are structured cleanly and make the later update path easier to reason about. Logging inactive tokens and the final summary ("Filtered tokens by active status") is useful operationally.Two small, optional refinements you might consider:
Log level for per-token inactive logs
For large registries with many inactive entries, the per-tokenlog.Info("Token is inactive, skipping price update", ...)could get noisy. If this is expected to be common in production, dropping that toDebugand relying on the aggregate"Filtered tokens by active status"info log would reduce log volume.Observability for GetTokenInfo failures
Currently,GetTokenInfofailures only emit a warning and silently drop the token. If these failures are meaningful in your environment, you might want to hook them into a dedicated metric (e.g.,token_info_errors_total) so they’re visible in Prometheus as well as logs.Both are non-blocking; the current implementation is functionally correct.
Also applies to: 253-301
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
token-price-oracle/updater/token_price.go(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
token-price-oracle/updater/token_price.go (2)
token-price-oracle/metrics/metrics.go (2)
LastSuccessfulUpdateTimestamp(33-38)UpdatesTotal(41-47)token-price-oracle/client/price_feed.go (1)
TokenPrice(12-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: test
- GitHub Check: check
- GitHub Check: test
- GitHub Check: test
- GitHub Check: check
- GitHub Check: test
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
token-price-oracle/updater/token_price.go (1)
114-127: Active-token prefiltering and TokenInfo-based price ratio look solidPrefiltering inactive tokens before calling
GetBatchTokenPricesand reusingtokenInfoMapincalculatePriceRatioWithInfois a good optimization and avoids redundant contract calls. IndexingtokenInfoMap[tokenID]is safe given the nil-check insidecalculatePriceRatioWithInfo, and the skip-on-error behavior is reasonable here.Also applies to: 130-138
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
token-price-oracle/client/bitget_sdk.go (1)
90-124: Stablecoin price path looks correct; consider small helper for reuse.Logic correctly:
- Detects
$-prefixed symbols.- Parses the fixed price, validates it’s > 0, and wraps parse errors with context.
- Avoids Bitget API calls while still logging and returning a
TokenPriceconsistent with the non-stablecoin path.If this pattern is reused elsewhere, you could extract a small helper like
parseStablecoinPrice(symbol string) (*big.Float, error)to centralize parsing/validation and make unit testing easier, but it’s not required for correctness here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
token-price-oracle/client/bitget_sdk.go(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: check
- GitHub Check: test
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (rust)
🔇 Additional comments (3)
token-price-oracle/client/bitget_sdk.go (3)
11-11: Import usage is correct and minimal.The new
stringsimport is used appropriately for prefix detection and trimming, with no unused imports introduced.
21-23: Stablecoin prefix constant is well-documented and scoped.Exporting
StablecoinPrefix = "$"with a clear format comment is good for keeping env/config parsing consistent across packages.
71-75: API doc comment accurately reflects new stablecoin behavior.The added explanation of the
$<price>stablecoin format matches the implementation and helps callers understand how to configure token maps.
curryxbo
changed the title
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.