Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

[0.9.40] Add query_account_balances runtime api for pallet assets. #199

Closed
hbulgarini opened this issue May 29, 2023 · 4 comments
Closed
Assignees

Comments

@hbulgarini
Copy link
Contributor

During release 0.9.40, the query_account_balances runtime api for querying the assets balances were implemented.

We can have the details here:
paritytech/substrate#13352

This is what it has to be added in the runtime (please check the assets_common):

	impl assets_common::runtime_api::FungiblesApi<
		Block,
		AccountId,
	> for Runtime
	{
		fn query_account_balances(account: AccountId) -> Result<xcm::VersionedMultiAssets, assets_common::runtime_api::FungiblesAccessError> {
			use assets_common::fungible_conversion::{convert, convert_balance};
			Ok([
				// collect pallet_balance
				{
					let balance = Balances::free_balance(account.clone());
					if balance > 0 {
						vec![convert_balance::<KsmLocation, Balance>(balance)?]
					} else {
						vec![]
					}
				},
				// collect pallet_assets (TrustBackedAssets)
				convert::<_, _, _, _, TrustBackedAssetsConvertedConcreteId>(
					Assets::account_balances(account.clone())
						.iter()
						.filter(|(_, balance)| balance > &0)
				)?,
				// collect pallet_assets (ForeignAssets)
				convert::<_, _, _, _, ForeignAssetsConvertedConcreteId>(
					ForeignAssets::account_balances(account)
						.iter()
						.filter(|(_, balance)| balance > &0)
				)?,
				// collect ... e.g. other tokens
			].concat().into())
		}
	}

CC: @stiiifff @valentinfernandez1

@hbulgarini hbulgarini added this to the Trappist M2 / XCM v3 milestone May 29, 2023
@hbulgarini
Copy link
Contributor Author

hbulgarini commented May 29, 2023

In addition to this i have noticed that the assets-common, contains a more interesting MultiAssetConverter/convert_ref implemention that the one we have in our xcm-primitives.

I see a couple of improvements in the asset-common as:

  • Better error management.
  • It returns the amount in addition to the asset id.

This should also be considered in the asset-registry pallet that we are implementing.

Taking into account that modifying the asset-registry is a different topic that adding this runtime api, i think that this enhancement should be applied in a different issue after this task is done. Thinking out loud, i see a period of time where both XCM primitives will cohexist until the xcm-primitives is removed or replaced.

@valentinfernandez1
Copy link
Contributor

valentinfernandez1 commented May 29, 2023

I like this implementation. We would have to remove this part since we don't have ForeignAssets.

....
// collect pallet_assets (ForeignAssets)
convert::<_, _, _, _, ForeignAssetsConvertedConcreteId>(
    ForeignAssets::account_balances(account)
        .iter()
        .filter(|(_, balance)| balance > &0)
)?,

But everything else looks good to me. I can add it today.

In regards to your second comment, why do you consider that both convert_ref should coexist in the same file??

@hbulgarini
Copy link
Contributor Author

In regards to your second comment, why do you consider that both convert_ref should coexist in the same file??

Basically because i'm not 100% sure which changes are needed over the asset registry by using the new convert_ref function. In case we need to change something (if it's technically feasible), i would apply these new impls from assets_common and implement the runtime api and once that is ready in a different PR i would modify the old impls by the new ones. However i saw that in your draft PR #202 you have to modify the FungiblesAdapter, so probably everything should be changed in one PR.

@stiiifff
Copy link
Contributor

@valentinfernandez1 Closing this as I believe this was done, right ? Re-open if not.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants