Feat: RPC lp total balances #4951

marcellorigotti · 2024-06-11T08:51:20Z

Pull Request

Checklist

Please conduct a thorough self-review before opening the PR.

I am confident that the code works.
I have updated documentation where appropriate.

Summary

Created a new RPC (cf_lp_total_balances) which exposes the total balances of an LP (free_balances + open_order_balances + boosted_balances)

Non-Breaking changes

engine-runner-bin/build.rs

codecov · 2024-06-11T09:29:45Z

Codecov Report

Attention: Patch coverage is 1.75439% with 168 lines in your changes missing coverage. Please review.

Project coverage is 71%. Comparing base (5db2502) to head (844a437).
Report is 1 commits behind head on main.

Files	Patch %	Lines
state-chain/primitives/src/chains/assets.rs	0%	67 Missing ⚠️
state-chain/pallets/cf-pools/src/lib.rs	0%	29 Missing ⚠️
state-chain/pallets/cf-ingress-egress/src/lib.rs	0%	22 Missing ⚠️
state-chain/chains/src/benchmarking_value.rs	0%	18 Missing ⚠️
state-chain/custom-rpc/src/lib.rs	0%	15 Missing ⚠️
state-chain/runtime/src/lib.rs	0%	0 Missing and 14 partials ⚠️
state-chain/traits/src/liquidity.rs	0%	3 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff           @@
##            main   #4951    +/-   ##
======================================
- Coverage     72%     71%    -0%     
======================================
  Files        434     434            
  Lines      75556   75583    +27     
  Branches   75556   75583    +27     
======================================
- Hits       54048   53849   -199     
- Misses     18660   18867   +207     
- Partials    2848    2867    +19

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

state-chain/pallets/cf-ingress-egress/src/lib.rs

state-chain/runtime/src/lib.rs

kylezs

otherwise boost stuff looks good, didn't look at the other stuff

marcellorigotti · 2024-06-13T10:11:03Z

Actually, I just realized I am not keeping into account range_orders here! Will update this!

dandanlen · 2024-06-13T10:13:31Z

state-chain/primitives/src/chains/assets.rs

@@ -389,6 +389,13 @@ macro_rules! assets {
 					})
 				}
 			}
+
+			impl scale_info::prelude::ops::Add<AssetMap<crate::AssetAmount>> for AssetMap<crate::AssetAmount> {


nit: it's a bit strange to import this via scale_info...

Suggested change

impl scale_info::prelude::ops::Add<AssetMap<crate::AssetAmount>> for AssetMap<crate::AssetAmount> {

impl core::ops::Add<AssetMap<crate::AssetAmount>> for AssetMap<crate::AssetAmount> {

dandanlen

I seem to remember @AlastairHolmes mentioning that LP balances might no be accurately reflected by the pool_orders method. Might be worth him taking a look too.

state-chain/traits/src/liquidity.rs

state-chain/primitives/src/chains/assets.rs

dandanlen

Looks fine to me - as mentioned above it's worth @AlastairHolmes taking a look to make sure the pool info is accurate.

state-chain/runtime/src/lib.rs

state-chain/pallets/cf-pools/src/lib.rs

AlastairHolmes · 2024-06-17T14:40:57Z

I seem to remember @AlastairHolmes mentioning that LP balances might no be accurately reflected by the pool_orders method. Might be worth him taking a look too.

The internal state on polkadot.js isn't right, but these functions are correct.

AlastairHolmes · 2024-06-17T14:42:53Z

state-chain/pallets/cf-pools/src/lib.rs

+			for ask in pool_orders.limit_orders.asks {
+				*result.index_mut(base_asset) = result
+					.index(base_asset)
+					.saturating_add(ask.sell_amount.saturated_into::<u128>());


u128 -> AssetAmount

AlastairHolmes · 2024-06-17T14:44:47Z

state-chain/primitives/src/chains/assets.rs

@@ -389,6 +389,37 @@ macro_rules! assets {
 					})
 				}
 			}
+
+			impl<N: sp_arithmetic::traits::Saturating> sp_arithmetic::traits::Saturating for AssetMap<N> {


AlastairHolmes · 2024-06-17T14:51:20Z

state-chain/custom-rpc/src/lib.rs

+			.map_err(to_rpc_error)
+			.and_then(|result| {
+				result
+					.map(|free_balances| free_balances.map(Into::into))


Suggested change

.map(|free_balances| free_balances.map(Into::into))

.map(|total_balances| total_balances.map(Into::into))

AlastairHolmes · 2024-06-17T14:52:40Z

state-chain/runtime/src/lib.rs

@@ -1262,6 +1262,23 @@ impl_runtime_apis! {
 		fn cf_free_balances(account_id: AccountId) -> Result<AssetMap<AssetAmount>, DispatchErrorWithMessage> {
 			LiquidityProvider::free_balances(&account_id).map_err(Into::into)
 		}
+		fn cf_lp_total_balances(account_id: AccountId) -> Result<AssetMap<AssetAmount>, DispatchErrorWithMessage> {
+			let free_balances = LiquidityProvider::free_balances(&account_id).map_err(Into::<DispatchErrorWithMessage>::into)?;


You don't need this map_err and Into, you can just use ? I believe.

AlastairHolmes · 2024-06-17T14:55:48Z

state-chain/pallets/cf-ingress-egress/src/lib.rs

+
+impl<T: Config<I>, I: 'static> BoostApi for Pallet<T, I> {
+	type AccountId = T::AccountId;
+	fn boost_pool_account_balances(who: &Self::AccountId) -> Vec<(Asset, AssetAmount)> {


This could use AssetMap, and AssetMap::from_fn

There are maps for each chain

marcellorigotti · 2024-06-21T13:36:56Z

addressed the latest comments, PR should be ready now 👍

…-ccm-checker * origin/wip/solana-api: Feat/solana ccm gas limit (#5048) feat: return early if no client ready on retry limited requests (#5057) feature/PRO-1378/validator-gas-refund (#4962) chore: fix bug in merge that caused tests to fail chore: build binaries on Mac M2 Runner 🍎 (#5005) chore: remove unused spec versioned migration util (#5054) test: update integration tests feat(docker images): add `chainflip-node` probe scripts 🩺 (#5033) Feat: Make FoK constants configurable (#5024) fix: cp .so files to /usr/lib (#5051) chore: run gas_limit test by default fix: allow validators to deregister once they no longer have relevant key material (#5045) Feat: RPC lp total balances (#4951) fix: rpath name on 1.4.5 dylib (#5046) chore: remove cp dylibs from post check (#5044) feat: add primary/secondary label to RPC_RETRIER_TOTAL_REQUESTS metric (#5015) feat: smart retrier endpoint selection (#4984) refactor: move collected network fee to swapping pallet (#5014) feat: consistent broker api address strings (#5030) # Conflicts: # foreign-chains/solana/sol-prim/src/consts.rs # state-chain/chains/src/sol/api.rs # state-chain/pallets/cf-swapping/src/lib.rs # state-chain/pallets/cf-swapping/src/mock.rs # state-chain/runtime/src/lib.rs

marcellorigotti requested a review from dandanlen as a code owner June 11, 2024 08:51

kylezs reviewed Jun 11, 2024

View reviewed changes

engine-runner-bin/build.rs Outdated Show resolved Hide resolved

marcellorigotti force-pushed the feature/pro-1418 branch from 8bc505b to 7be1a16 Compare June 11, 2024 08:53

marcellorigotti changed the title ~~Feature/pro 1418~~ Feat: RPC lp total balances Jun 11, 2024

kylezs reviewed Jun 11, 2024

View reviewed changes

state-chain/pallets/cf-ingress-egress/src/lib.rs Outdated Show resolved Hide resolved

state-chain/runtime/src/lib.rs Outdated Show resolved Hide resolved

kylezs reviewed Jun 11, 2024

View reviewed changes

dandanlen reviewed Jun 13, 2024

View reviewed changes

state-chain/traits/src/liquidity.rs Outdated Show resolved Hide resolved

state-chain/traits/src/liquidity.rs Outdated Show resolved Hide resolved

state-chain/primitives/src/chains/assets.rs Outdated Show resolved Hide resolved

dandanlen reviewed Jun 14, 2024

View reviewed changes

state-chain/runtime/src/lib.rs Outdated Show resolved Hide resolved

state-chain/pallets/cf-pools/src/lib.rs Outdated Show resolved Hide resolved

dandanlen requested a review from AlastairHolmes June 14, 2024 11:23

AlastairHolmes reviewed Jun 17, 2024

View reviewed changes

marcellorigotti force-pushed the feature/pro-1418 branch from 99d3dc3 to 3eedcb4 Compare June 21, 2024 13:33

AlastairHolmes approved these changes Jul 16, 2024

View reviewed changes

marcellorigotti added 9 commits July 16, 2024 11:13

cf_lp_total_balances

3ce3c21

comments

1c7eccc

comments

70acea2

add additional parameter to cf_pool_orders (filled_orders: boolean)

8ebb0aa

cf_lp_total_balances

35f5ebc

fmt

81d1480

switch to Vec<(Asset, AssetAmount)>

92c24a8

use saturating_add, saturating_into

f0efe34

fix imports

1ec942f

marcellorigotti and others added 11 commits July 16, 2024 11:16

fix open_order_balances

cdafe1d

addressed comments

44e5a0c

fmt

6b9b754

feat: impl Saturating for AssetMap

60850dd

add range_orders to open_order_balances

37c82f0

address comments

a41f4a4

addressed comments

e203b5e

use collect() directly to create the AssetMap

05357e4

add ChainAssetMap for Chain trait

622caf7

fix import

c8cf40f

update benchmark values & fix imports

844a437

marcellorigotti force-pushed the feature/pro-1418 branch from 3eedcb4 to 844a437 Compare July 16, 2024 09:42

marcellorigotti added this pull request to the merge queue Jul 16, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 16, 2024

marcellorigotti added this pull request to the merge queue Jul 16, 2024

Merged via the queue into main with commit bbc7649 Jul 16, 2024
47 checks passed

marcellorigotti deleted the feature/pro-1418 branch July 16, 2024 13:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: RPC lp total balances #4951

Feat: RPC lp total balances #4951

marcellorigotti commented Jun 11, 2024

codecov bot commented Jun 11, 2024 •

edited

Loading

kylezs left a comment

marcellorigotti commented Jun 13, 2024

dandanlen Jun 13, 2024

dandanlen left a comment

dandanlen left a comment

AlastairHolmes commented Jun 17, 2024 •

edited

Loading

AlastairHolmes Jun 17, 2024

AlastairHolmes Jun 17, 2024

AlastairHolmes Jun 17, 2024

AlastairHolmes Jun 17, 2024

AlastairHolmes Jun 17, 2024

AlastairHolmes Jun 17, 2024

marcellorigotti commented Jun 21, 2024

	impl scale_info::prelude::ops::Add<AssetMap<crate::AssetAmount>> for AssetMap<crate::AssetAmount> {
	impl core::ops::Add<AssetMap<crate::AssetAmount>> for AssetMap<crate::AssetAmount> {

	.map(\|free_balances\| free_balances.map(Into::into))
	.map(\|total_balances\| total_balances.map(Into::into))

Feat: RPC lp total balances #4951

Feat: RPC lp total balances #4951

Conversation

marcellorigotti commented Jun 11, 2024

Pull Request

Checklist

Summary

Non-Breaking changes

codecov bot commented Jun 11, 2024 • edited Loading

Codecov Report

kylezs left a comment

Choose a reason for hiding this comment

marcellorigotti commented Jun 13, 2024

dandanlen Jun 13, 2024

Choose a reason for hiding this comment

dandanlen left a comment

Choose a reason for hiding this comment

dandanlen left a comment

Choose a reason for hiding this comment

AlastairHolmes commented Jun 17, 2024 • edited Loading

AlastairHolmes Jun 17, 2024

Choose a reason for hiding this comment

AlastairHolmes Jun 17, 2024

Choose a reason for hiding this comment

AlastairHolmes Jun 17, 2024

Choose a reason for hiding this comment

AlastairHolmes Jun 17, 2024

Choose a reason for hiding this comment

AlastairHolmes Jun 17, 2024

Choose a reason for hiding this comment

AlastairHolmes Jun 17, 2024

Choose a reason for hiding this comment

marcellorigotti commented Jun 21, 2024

codecov bot commented Jun 11, 2024 •

edited

Loading

AlastairHolmes commented Jun 17, 2024 •

edited

Loading