🎨 Simplify the acceptable_assets - #485
Merged
Merged
Conversation
This was referenced May 9, 2025
Member
Author
lrazovic
changed the base branch from
05-08-draft_use_plmc_amount_in_evaluation
to
graphite-base/485
May 9, 2025 15:10
lrazovic
force-pushed
the
graphite-base/485
branch
from
May 9, 2025 15:10
a1105c5 to
382f61e
Compare
lrazovic
force-pushed
the
05-09-feat_simplify_the_acceptable_assets
branch
from
May 9, 2025 15:10
742e30b to
359ca8a
Compare
lrazovic
changed the base branch from
graphite-base/485
to
05-08-feat_macros_update_and_documentation
May 9, 2025 15:11
lrazovic
force-pushed
the
05-09-feat_simplify_the_acceptable_assets
branch
from
May 9, 2025 15:42
359ca8a to
15d088b
Compare
lrazovic
force-pushed
the
05-08-feat_macros_update_and_documentation
branch
from
May 9, 2025 15:44
382f61e to
91611a0
Compare
lrazovic
force-pushed
the
05-09-feat_simplify_the_acceptable_assets
branch
2 times, most recently
from
May 9, 2025 16:52
67962d0 to
a4df891
Compare
Merged
lrazovic
force-pushed
the
05-09-feat_simplify_the_acceptable_assets
branch
3 times, most recently
from
May 12, 2025 11:36
eeada01 to
e7ffa1b
Compare
This was referenced May 12, 2025
lrazovic
marked this pull request as ready for review
May 16, 2025 14:45
There was a problem hiding this comment.
Pull Request Overview
This PR refactors price conversion and asset management by switching to reference-based API calls, removing unused constants, clarifying error messages, and simplifying type constraints.
- Use
&AssetIdin price provider methods and drop theUSD_DECIMALSparameter. - Remove unnecessary
FixedPointNumberbounds and unused imports. - Add
decimals()andall_ids_and_plmc()toAcceptedFundingAsset, and improve error messaging.
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| runtimes/polimec/src/lib.rs | Replaced PLMCToAssetBalance with HereToForeignAsset and updated XCM payment API |
| polimec-common/common/src/lib.rs | Updated ProvideAssetPrice trait docs/signature and added convert_back_to_normal_price |
| polimec-common/common/src/assets.rs | Added decimals() and all_ids_and_plmc() to AcceptedFundingAsset |
| pallets/proxy-bonding/src/mock.rs | Changed get_price to accept &Location |
| pallets/proxy-bonding/src/functions.rs | Updated calculate_fee to use the new get_decimals_aware_price signature |
| pallets/funding/src/types.rs | Removed FixedPointNumber constraint from struct generics |
| pallets/funding/src/tests/runtime_api.rs | Adjusted tests to pass references to get_price |
| pallets/funding/src/tests/misc.rs | Aligned test calls with the reference-based price API |
| pallets/funding/src/tests/3_auction.rs | Refactored test to use &Location and the updated price API |
| pallets/funding/src/tests/2_evaluation.rs | Updated test price calls to use references |
| pallets/funding/src/mock.rs | Changed mock get_price to accept &Location |
| pallets/funding/src/lib.rs | Clarified TooLow/TooHigh error messages |
| pallets/funding/src/instantiator/calculations.rs | Simplified price lookup by removing USD_DECIMALS parameter |
| pallets/funding/src/functions/runtime_api.rs | Removed USD_DECIMALS import and updated get_decimals_aware_price usage |
| pallets/funding/src/functions/mod.rs | Cleaned up imports and removed unused constants |
| pallets/funding/src/functions/misc.rs | Switched to reference-based price calls and dropped USD_DECIMALS |
| pallets/funding/src/functions/2_evaluation.rs | Simplified repeated price retrieval calls |
| integration-tests/src/tests/transaction_payment.rs | Updated asset location usage and price API in integration tests |
| integration-tests/src/tests/otm_edge_cases.rs | Refined reference usage and removed USD_DECIMALS in edge-case tests |
| Cargo.toml | Added redundant_clone lint configuration |
Comments suppressed due to low confidence (2)
pallets/funding/src/functions/misc.rs:53
- [nitpick] Consider accepting
assetas a reference (&AcceptedFundingAsset) rather than by value to avoid unnecessary copying and align with other APIs that take references.
pub fn calculate_funding_asset_amount(
polimec-common/common/src/assets.rs:60
- The order of asset IDs in
all_ids_and_plmcdiffers from the originalall_idsvector; if ordering matters in downstream code, consider preserving the original sequence or documenting the change.
pub fn all_ids_and_plmc() -> [Location; AcceptedFundingAsset::VARIANT_COUNT + 1] {
| let plmc_usd_price = self.execute(|| { | ||
| <PriceProviderOf<T>>::get_decimals_aware_price(Location::here(), USD_DECIMALS, PLMC_DECIMALS).unwrap() | ||
| }); | ||
| let plmc_usd_price = |
There was a problem hiding this comment.
[nitpick] The logic to fetch the PLMC price is repeated in multiple places; extracting this into a helper method (e.g., get_plmc_price()) would reduce duplication and improve clarity.
lrazovic
force-pushed
the
05-08-feat_macros_update_and_documentation
branch
from
May 19, 2025 06:15
91611a0 to
6c2a7dd
Compare
lrazovic
force-pushed
the
05-09-feat_simplify_the_acceptable_assets
branch
from
May 19, 2025 06:15
e7ffa1b to
b5652c4
Compare
dastansam
approved these changes
May 19, 2025
| // 3. Calculate nominal price of PLMC in terms of the target asset. | ||
| // Result is in "units of target_asset per unit of PLMC". | ||
| // TargetAsset/PLMC = (USD/PLMC) / (USD/TargetAsset) | ||
| let price_plmc_in_target_asset_nominal = |
Contributor
There was a problem hiding this comment.
wouldn't this round down a lot of decimals compared to using Perquintill? is it negligible?
lrazovic
force-pushed
the
05-08-feat_macros_update_and_documentation
branch
from
May 19, 2025 06:48
6c2a7dd to
eb49fc8
Compare
lrazovic
force-pushed
the
05-09-feat_simplify_the_acceptable_assets
branch
from
May 19, 2025 06:48
b5652c4 to
2db9c76
Compare
lrazovic
force-pushed
the
05-08-feat_macros_update_and_documentation
branch
from
May 20, 2025 06:21
eb49fc8 to
5abd90d
Compare
lrazovic
force-pushed
the
05-09-feat_simplify_the_acceptable_assets
branch
from
May 20, 2025 06:21
2db9c76 to
c9be272
Compare
Closed
lrazovic
force-pushed
the
05-09-feat_simplify_the_acceptable_assets
branch
from
May 28, 2025 13:35
156f866 to
e22ac65
Compare
lrazovic
force-pushed
the
05-08-feat_macros_update_and_documentation
branch
from
May 28, 2025 13:35
5abd90d to
14b0f58
Compare
lrazovic
force-pushed
the
05-09-feat_simplify_the_acceptable_assets
branch
from
May 28, 2025 13:38
e22ac65 to
378a176
Compare
lrazovic
force-pushed
the
05-08-feat_macros_update_and_documentation
branch
from
May 28, 2025 13:38
14b0f58 to
2fa24d7
Compare
dastansam
force-pushed
the
05-08-feat_macros_update_and_documentation
branch
from
July 9, 2025 06:38
2fa24d7 to
60e9f36
Compare
dastansam
force-pushed
the
05-09-feat_simplify_the_acceptable_assets
branch
from
July 9, 2025 06:38
378a176 to
48db973
Compare
lrazovic
force-pushed
the
05-09-feat_simplify_the_acceptable_assets
branch
from
July 25, 2025 14:19
48db973 to
d4e10e0
Compare
dastansam
force-pushed
the
05-09-feat_simplify_the_acceptable_assets
branch
from
July 25, 2025 21:21
d4e10e0 to
48db973
Compare
Member
Author
lrazovic
changed the base branch from
05-08-feat_macros_update_and_documentation
to
graphite-base/485
August 4, 2025 07:50
lrazovic
force-pushed
the
05-09-feat_simplify_the_acceptable_assets
branch
from
August 4, 2025 07:53
48db973 to
be00d51
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

This pull request introduces several changes aimed at improving code quality, simplifying function calls, and enhancing maintainability across multiple files. The key updates include replacing direct object references with references (
&), removing unused constants, and simplifying type constraints in struct definitions. Additionally, some error messages were clarified for better understanding.Code Quality Improvements:
Replaced direct object usage with references in function calls to improve efficiency and align with Rust's borrowing principles. This change is applied across multiple files, including
pallets/funding/src/functions/misc.rs,integration-tests/src/tests/otm_edge_cases.rs, and others. [1] [2] [3]Removed unused constants such as
USD_DECIMALSfrom imports in several files, includingpallets/funding/src/functions/mod.rsandpallets/funding/src/instantiator/calculations.rs. [1] [2]Simplifications in Struct Definitions:
ProjectMetadataandBiddingTicketSizesby removing theFixedPointNumberconstraint where it was unnecessary. This improves flexibility and reduces complexity. [1] [2]Error Message Enhancements:
Test Updates:
pallets/funding/src/tests/2_evaluation.rsandpallets/funding/src/tests/misc.rs. [1] [2]Mock and Runtime Adjustments:
pallets/funding/src/mock.rs.