feat(backend): /stats endpoint #567
Conversation
refactor(backend:rpc): return price as big decimal
fix: proper compile_error invocation
Karrq
added
B3-backendnoteworthy
santikaplan
requested review from
HermanObst
and removed request for
ffarall
HermanObst
left a comment
HermanObst
left a comment
There was a problem hiding this comment.
Awesome :).
Just some comments to discuss, mostly im curious to check that we have some integration test checking that the backend is getting correct info from the payment stream price runtime API
| strictEqual(mspNodePeerId.toString(), userApi.shConsts.NODE_INFOS.msp1.expectedPeerId); | ||
| }); | ||
|
|
||
| it("Should be able to retrieve current price per giga unit per tick", async () => { |
There was a problem hiding this comment.
This is deleted because now the backend directly use runtime API?
There was a problem hiding this comment.
Dont we still need to test that calling the runtime API from the backend is correctly working?
There was a problem hiding this comment.
Yeah the suite test/suites/backend/payment-streams.test.ts was the one supossed to test this but after your comments I checked it out and it wasn't testing much at all. Just refactored it to better test this functionality
| let total_bytes = stats | ||
| .capacity | ||
| .total_bytes | ||
| .parse::<BigDecimal>() | ||
| .expect("total_bytes to be a number"); | ||
| let available_bytes = stats | ||
| .capacity | ||
| .available_bytes | ||
| .parse::<BigDecimal>() | ||
| .expect("available_bytes to be a number"); | ||
| let used_bytes = stats | ||
| .capacity | ||
| .used_bytes | ||
| .parse::<BigDecimal>() | ||
| .expect("used_bytes to be a number"); | ||
|
|
||
| assert!( | ||
| total_bytes.is_positive(), | ||
| "Total bytes should always be positive" | ||
| ); | ||
| assert!( | ||
| available_bytes <= total_bytes, | ||
| "Available bytes should never be more than total bytes" | ||
| ); | ||
| assert!( | ||
| used_bytes <= total_bytes, | ||
| "Used bytes should never be more than total bytes" | ||
| ); | ||
| assert_eq!( | ||
| used_bytes + available_bytes, | ||
| total_bytes, | ||
| "Available + used bytes should always equal total bytes" | ||
| ); |
There was a problem hiding this comment.
Can we also check the other stats fields?
activeUsers: number;
lastCapacityChange: string;
valuePropsAmount: string;
bucketsAmount: string;
There was a problem hiding this comment.
We can't check much but I'll add some basic checks for them, sure. active_users the only thing that I would be able to test is that it's a number greater or equal to 0 but the type boundary already gives us that info so the compiler complains
backend/lib/src/runtime.rs
Outdated
| // if #[cfg(all(feature = "solochain", ...))] { | ||
| // compile_error!("Please select only 1 runtime"); | ||
| // } else |
There was a problem hiding this comment.
This was commented on purpose?
There was a problem hiding this comment.
Hmm that is leftover work from Fran, I'll remove the comment
| pub last_capacity_change: String, | ||
| #[serde(rename = "valuePropsAmount")] | ||
| pub value_props_amount: u64, | ||
| pub value_props_amount: String, | ||
| #[serde(rename = "bucketsAmount")] | ||
| pub buckets_amount: u64, | ||
| pub buckets_amount: String, | ||
| } | ||
|
|
||
| #[derive(Debug, Serialize)] | ||
| pub struct Capacity { | ||
| #[serde(rename = "totalBytes")] | ||
| pub total_bytes: u64, | ||
| pub total_bytes: String, | ||
| #[serde(rename = "availableBytes")] | ||
| pub available_bytes: u64, | ||
| pub available_bytes: String, | ||
| #[serde(rename = "usedBytes")] | ||
| pub used_bytes: u64, | ||
| pub used_bytes: String, |
There was a problem hiding this comment.
why string and not unsigned?
There was a problem hiding this comment.
We changed this as we were losing precision otherwise when sending these values through JSON, I believe
| pub async fn get_current_price_per_giga_unit_per_tick(&self) -> RpcResult<BigDecimal> { | ||
| debug!(target: "rpc::client::get_current_price_per_giga_unit_per_tick", "RPC call: get_current_price_per_giga_unit_per_tick"); | ||
|
|
||
| self.call_no_params(methods::CURRENT_PRICE).await | ||
| self.call_runtime_api::<runtime_apis::GetCurrentPricePerGigaUnitPerTick>(()) | ||
| .await | ||
| .map(|price| price.into()) | ||
| } |
There was a problem hiding this comment.
IIUC this method is not being tested, right?
This PR changes the backend to return the actual values for the
/statsendpoint.It does so by making a RPC call to the configured provider and reading the storage of the chain for the provider.
The PR also adds utilities to make runtime api calls (obsoleting and thus removing the RPC method for the payment stream's price per giga unit) and also for storage reads.
Another important change is that the backend now depends on the runtime, this was done to provide the appropriate type definitions and storage key encoding.
An alternative approach was using subxt, which allows for dynamic scale decoding, by fetching the runtime metadata "just in time" and processing it that way. Unfortunately, subxt still requires the backend to be "implicitly" dependant on the runtime (via the client's Config).
Nevertheless, the subxt approach is present in commit
b66af47369f5660e6de19a099a188cf2a9f4bc7fif we decide to switch back to that instead.