Implement a new pallet services-payment-price-oracle to be able to store TANSSI/STAR token price#1477
Implement a new pallet services-payment-price-oracle to be able to store TANSSI/STAR token price#1477
services-payment-price-oracle to be able to store TANSSI/STAR token price#1477Conversation
📝 WalkthroughWalkthroughAdds a new Services Payment Price Oracle pallet and runtime-api, integrates it into Dancelight and Starlight runtimes (config, proxy routing, weights, benches, runtime APIs), provides root-gated price-setting and cost-calculation helpers, plus tests, mocks, workspace manifests, and TypeScript API augmentations. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,200,255,0.5)
participant Admin as Admin / Sudo
participant Runtime as Runtime
participant Oracle as ServicesPaymentPriceOracle
participant Storage as TokenPriceStorage
participant CostProv as CostProvider
participant API as RuntimeAPI
end
Admin->>Runtime: Sudo::call(set_token_price(price))
Runtime->>Oracle: dispatch set_token_price(price)
Oracle->>Storage: write TokenPriceUsd
Oracle-->>Runtime: emit PriceUpdated
CostProv->>Oracle: calculate_block_production_cost()
Oracle->>Storage: read TokenPriceUsd
Oracle->>Oracle: compute scale factor & usd_to_tokens
Oracle-->>CostProv: return block cost (tokens)
CostProv->>Oracle: calculate_collator_assignment_cost()
Oracle->>Storage: read TokenPriceUsd
Oracle->>Oracle: compute scale factor & usd_to_tokens
Oracle-->>CostProv: return session cost (tokens)
API->>Oracle: token_price_usd()
Oracle->>Storage: read TokenPriceUsd
Oracle-->>API: return FixedU128 or None
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
evgeny-s
added
breaking
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@pallets/services-payment-price-oracle/runtime-api/src/lib.rs`:
- Around line 32-38: Docs claim the functions may return None but the signatures
for block_cost and collator_assignment_cost currently return Balance; change
their return types to Option<Balance> in the runtime API (update fn block_cost()
-> Option<Balance> and fn collator_assignment_cost() -> Option<Balance>) and
then update all corresponding implementations and callers to handle
Some(balance)/None (or alternatively, if you prefer the fixed-cost fallback
behavior, update the docstrings to describe the fallback instead of "None").
Ensure you update every implementation of block_cost and
collator_assignment_cost and any runtime-to-runtime-api call sites to match the
new Option<Balance> signature.
In `@pallets/services-payment-price-oracle/src/benchmarking.rs`:
- Around line 34-38: The benchmark macro is referencing a nonexistent ExtBuilder
in the mock module; update the impl_benchmark_test_suite invocation to use the
existing test externalities helper instead (replace
crate::mock::ExtBuilder.build() with the crate::mock::new_test_ext() call), or
alternatively add an ExtBuilder type in crate::mock that exposes a build()
method; modify the invocation around impl_benchmark_test_suite, referencing the
symbols impl_benchmark_test_suite, crate::mock::new_test_ext, and ExtBuilder as
appropriate so the suite compiles under runtime-benchmarks.
🧹 Nitpick comments (3)
pallets/services-payment-price-oracle/src/weights.rs (1)
17-17: Resolve the TODO in this generated file.If the file is already generated, remove the TODO or replace it with regeneration instructions.
chains/orchestrator-relays/runtime/dancelight/src/lib.rs (2)
1013-1019: Address the TODO comment for tests.The
SudoOraclePriceproxy filter implementation looks correct and follows the same pattern asSudoValidatorManagement. Consider tracking the TODO for test coverage.Do you want me to open a GitHub issue to track adding tests for the
SudoOraclePriceproxy type?
1755-1773: Consider accounting for read weight in cost providers.The cost providers correctly implement fallback behavior when the oracle price is unavailable. However, both return
Weight::zero()even thoughcalculate_block_production_cost()andcalculate_collator_assignment_cost()perform a storage read ofTokenPriceUsd.♻️ Suggested improvement
pub struct BlockProductionCost<Runtime>(PhantomData<Runtime>); impl ProvideBlockProductionCost<Runtime> for BlockProductionCost<Runtime> { fn block_cost(_para_id: &ParaId) -> (u128, Weight) { // Try to get cost from oracle, fallback to fixed cost if not available let cost = pallet_services_payment_price_oracle::Pallet::<Runtime>::calculate_block_production_cost() .unwrap_or(FIXED_BLOCK_PRODUCTION_COST); - (cost, Weight::zero()) + (cost, <Runtime as frame_system::Config>::DbWeight::get().reads(1)) } }
|
@coderabbitai |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 11954 🏁 Script executed: Length of output: 18051 🏁 Script executed: Length of output: 18505 🏁 Script executed: Length of output: 7287 🏁 Script executed: Length of output: 3932 🏁 Script executed: Length of output: 18950 🏁 Script executed: Length of output: 993 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 967 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 4234 🏁 Script executed: Length of output: 1075 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 3953
🔴 Critical Security Issues1. No Price Bounds ValidationLocation: The pallet only validates that price is non-zero, but doesn't enforce minimum or maximum bounds. This creates significant economic risks:
Recommendation: Add reasonable bounds validation: pub const MIN_ACCEPTABLE_PRICE: u128 = FixedU128::from_u32(0.001).into_inner(); // $0.001
pub const MAX_ACCEPTABLE_PRICE: u128 = FixedU128::from_u32(1000).into_inner(); // $1000
ensure!(
price.into_inner() >= MIN_ACCEPTABLE_PRICE && price.into_inner() <= MAX_ACCEPTABLE_PRICE,
Error::<T>::PriceOutOfBounds
);🟠 High Security Issues2. No Rate Limiting or Price Change CapsLocation: Price can be updated multiple times per block with no restrictions on:
This allows a compromised sudo account to cause economic chaos by rapidly changing prices or making extreme adjustments. Recommendation: Implement price change constraints: // Maximum % change per update (e.g., 20%)
const MAX_PRICE_CHANGE_PERCENT: u32 = 20;
// Validate price change if previous price exists
if let Some(old_price) = TokenPriceUsd::<T>::get() {
let change_ratio = if price > old_price {
price.saturating_sub(old_price).checked_div(&old_price)
} else {
old_price.saturating_sub(price).checked_div(&old_price)
};
ensure!(
change_ratio <= FixedU128::from_u32(MAX_PRICE_CHANGE_PERCENT) / FixedU128::from_u32(100),
Error::<T>::PriceChangeTooLarge
);
}3. Integer Overflow Risk in
|
WASM runtime size check:Compared to target branchdancebox runtime: 1900 KB (no changes) ✅ flashbox runtime: 1124 KB (no changes) ✅ dancelight runtime: 2680 KB (+24 KB) 🚨 starlight runtime: 2584 KB (no changes) 🚨 container chain template simple runtime: 1516 KB (no changes) ✅ container chain template frontier runtime: 1848 KB (no changes) ✅ |
|
/cmd generate-ts-api |
Coverage Report@@ Coverage Diff @@
## master evgeny-token-price-oracle +/- ##
=============================================================
- Coverage 73.85% 73.83% -0.02%
+ Files 568 573 +5
+ Lines 82793 82938 +145
=============================================================
+ Hits 61141 61235 +94
+ Misses 21652 21703 +51
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@chains/orchestrator-relays/runtime/dancelight/src/lib.rs`:
- Around line 920-921: The SudoOraclePrice proxy filter currently allows any
ServicesPaymentPriceOracle call under sudo; limit it to only the intended
extrinsic by changing the SudoOraclePrice match to allow
ServicesPaymentPriceOracle::set_token_price(_) and deny all other
ServicesPaymentPriceOracle variants (reference the SudoOraclePrice variant and
ServicesPaymentPriceOracle enum and the set_token_price function); update the
proxy/permission check logic where SudoOraclePrice is used to enforce this
specific variant check and add unit tests covering acceptance of set_token_price
and rejection of other ServicesPaymentPriceOracle calls (add tests for the TODO
at the set_token_price location).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@chains/orchestrator-relays/runtime/starlight/src/lib.rs`:
- Around line 1017-1022: The ProxyType::SudoOraclePrice branch currently allows
any RuntimeCall::ServicesPaymentPriceOracle via the Sudo wrapper; narrow this to
only permit the specific set_token_price extrinsic by changing the inner match
on x to explicitly match RuntimeCall::ServicesPaymentPriceOracle(...) for the
set_token_price variant (the exact variant name used in the pallet), instead of
a wildcard matches!(), so only RuntimeCall::Sudo(pallet_sudo::Call::sudo { call:
ref x }) where x is ServicesPaymentPriceOracle::set_token_price is allowed.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@pallets/services-payment-price-oracle/src/lib.rs`:
- Around line 156-165: The genesis builder for GenesisConfig::build currently
accepts initial_price without bounds checking; change it to enforce configured
min/max bounds by comparing FixedU128::from_inner(price) against the pallet's
configured limits (e.g., T::MinPriceUsd::get() and T::MaxPriceUsd::get() or the
appropriate associated constants/ storage getters) and only call
TokenPriceUsd::<T>::put(fixed_price) if fixed_price is within these bounds; if
out of range, fail fast via an assert/panic or skip setting and document the
behavior so genesis cannot silently install an out‑of‑range price.
- Around line 323-335: The current conversion in usd_to_tokens multiplies
usd_amount by FixedU128::DIV which can overflow; replace that manual scaling
with FixedU128::checked_from_rational to build usd_scaled safely (e.g.,
FixedU128::checked_from_rational(usd_amount, 10u128.pow(USD_DECIMALS))). Then
perform tokens_fixed = usd_scaled.checked_div(&price) as before and convert to
integer tokens using
into_inner().checked_mul(one_token)?.checked_div(FixedU128::DIV) — this avoids
the usd_amount * FixedU128::DIV overflow while preserving the same scaling logic
in the usd_to_tokens flow.
♻️ Duplicate comments (2)
chains/orchestrator-relays/runtime/dancelight/src/lib.rs (1)
1022-1026: TightenSudoOraclePriceto only allowset_token_price.
Current filter grants allServicesPaymentPriceOraclecalls under sudo, which is overly broad for future extrinsics.🔒 Suggested restriction
ProxyType::SudoOraclePrice => match c { RuntimeCall::Sudo(pallet_sudo::Call::sudo { call: ref x }) => { - matches!(x.as_ref(), &RuntimeCall::ServicesPaymentPriceOracle(..)) + matches!( + x.as_ref(), + &RuntimeCall::ServicesPaymentPriceOracle( + pallet_services_payment_price_oracle::Call::set_token_price { .. } + ) + ) } _ => false, },Also please cover the TODO with proxy‑filter tests.
chains/orchestrator-relays/runtime/starlight/src/lib.rs (1)
1017-1022: Overly broad proxy filter allows anyServicesPaymentPriceOraclecall.The
SudoOraclePriceproxy type matches any call toServicesPaymentPriceOracle, not justset_token_price. If future extrinsics are added to this pallet, they would automatically be allowed through this proxy without explicit approval.Restrict the proxy to only the intended
set_token_pricecall:🔒 Proposed fix to restrict proxy filter
ProxyType::SudoOraclePrice => match c { RuntimeCall::Sudo(pallet_sudo::Call::sudo { call: ref x }) => { - matches!(x.as_ref(), &RuntimeCall::ServicesPaymentPriceOracle(..)) + matches!( + x.as_ref(), + &RuntimeCall::ServicesPaymentPriceOracle( + pallet_services_payment_price_oracle::Call::set_token_price { .. } + ) + ) } _ => false, },
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@test/suites/dev-tanssi-relay/pallet-services-payment-price-oracle/test_pallet_services_payment_price_oracle.ts`:
- Around line 149-163: The test computes belowMinPriceValue from minTokenPrice
but when minTokenPrice === 1 the value equals the min (not below), making the
test ineffective; update the logic around belowMinPriceValue in the
test_pallet_services_payment_price_oracle.ts test: if minPriceConst > 1n keep
minPriceConst - 1n and assert PriceOutOfBounds on setTokenPrice, but if
minPriceConst <= 1n set belowMinPriceValue to 0n and either expect the pallet's
zero-price rejection (adjust the expected error from PriceOutOfBounds to the
zero-price error) or assert the appropriate behavior (skip the bound-check
assertion) after calling setTokenPrice; ensure you still construct belowMinPrice
via polkadotJs.createType("FixedU128", belowMinPriceValue) and use the same sudo
dispatch flow and retrieveSudoDispatchErrors to verify the correct error event.
...nssi-relay/pallet-services-payment-price-oracle/test_pallet_services_payment_price_oracle.ts
Outdated
Show resolved
Hide resolved
…evgeny-token-price-oracle
1 participant
Summary
This PR implements a new pallet that encapsulates the token price storage functionality as well as the price calculation for Tanssi services like block production cost and collator assignment.
Changes
pallet-services-payment-price-oracle, that stores the token_price (TokenPriceUsd), that can be updated via the extrinsic with sudo origin. It calculates the service cost based on fixed monthly cost (FixedMonthlyServicesCostUsd), keeping the ratio via configurationReferenceBlockCostandReferenceSessionCost.SudoOraclePrice, which guards all the extrinsics besides the one that updates the token price.block_costandcollator_assignment_costfunction. It calculates the cost based on the token price; it falls back to fixed prices otherwise (old behavior).Implementation Plan
Improvements (for future)