feat(custom-rpc): improve environment RPC #4154
Conversation
acdibble
requested review from
niklasnatter and
zoheb391
and removed request for
a team
WEB-402 Get minimum swap amount from RPC instead of hardcoding them
Right now we hardcode the minimum deposit amount and minimum swap amount in the code of the SDK: These value will get outdated eventually and it will need a new version of the npm package to update them. Instead, we should fetch the amounts from the backend which reads them from the state chain using an RPC call. The backend already supports reading the values via the |
Codecov Report
@@ Coverage Diff @@
## main #4154 +/- ##
======================================
- Coverage 72% 72% -0%
======================================
Files 378 379 +1
Lines 61335 61477 +142
Branches 61335 61477 +142
======================================
+ Hits 43993 44053 +60
- Misses 15068 15122 +54
- Partials 2274 2302 +28
... and 6 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| @@ -246,6 +246,7 @@ impl ToHumanreadableAddress for PolkadotAccountId { | |||
|
|
|||
| #[cfg(feature = "std")] | |||
| #[derive(Debug, Clone, Serialize, Deserialize)] | |||
| #[serde(untagged)] | |||
There was a problem hiding this comment.
@acdibble Note this type is also used in get_open_swap_channels in the LP Api...
@AlastairHolmes I can't remember if we decided on anything regarding this?
There was a problem hiding this comment.
I feel the untagged is a bad idea (Because of the deserialization danger), and that we should instead of the ChainMap solution we discussed.
There was a problem hiding this comment.
And I'd prefer not using it and waiting until we introduce the ChainMap, which can alternatively don't pretty quickly.
There was a problem hiding this comment.
I guess using an unimplemented Deserialize is ok, for now. But the ChainMap I think is better.
There was a problem hiding this comment.
I've added a linear issue to do so: https://linear.app/chainflip/issue/PRO-940/introduce-chainmap-struct-into-the-sc-to-ensure-compile-checking-of-sc
There was a problem hiding this comment.
Yeah we discussed it in person a bit and there's no use case for deserializing a human-readable address using this because more context is required besides the address itself. Even if we could parse addresses using this deserialization method, we would still have to convert it to another type before it can be used with any other methods (or change those methods' signatures to accept this type).
There was a problem hiding this comment.
Or if we do have a use case for it it's outside the scope of this PR since currently we only ever serialize this type.
acdibble
force-pushed
the
feat/environment-rpc
branch
from
1d9f31f
to
c2e819d
Compare
acdibble
force-pushed
the
feat/environment-rpc
branch
from
c2e819d
to
076c45e
Compare
… feat/environment-rpc
| { | ||
| match self { | ||
| // JS numbers are 64-bit floats, so we need to use a string for numbers larger than 2^53 | ||
| &Self::Number(n) if n >= 2u64.pow(53) => U256::from(n).serialize(serializer), |
There was a problem hiding this comment.
This should probably be raised as an issue with parity 😅 ...
There was a problem hiding this comment.
Was thinking the same, but not sure if it would be too breaking.
Pull Request
Closes WEB-472
Closes WEB-402
Checklist
Please conduct a thorough self-review before opening the PR.
Summary
We never used the old environment RPC so I just gutted it and replaced it with useful stuff.
I tried to group the information in the RPC by pallet and created individual RPCs for each of those pallets.