refactor: Consolidate FungibleStrAssetType and FungibleObjAssetType#596
Merged
Conversation
Both represent the same underlying structure. There was an extra nested "Fungible" in ObjType as well that added unnecessary complexity.
andrew-ifrita
force-pushed
the
fix/fungible-fungible
branch
from
ef1b9f5 to
666b4a9
Compare
…nd FungibleMultiAsset
andrew-ifrita
force-pushed
the
fix/fungible-fungible
branch
from
18a2dd8 to
d9f8628
Compare
TarikGul
reviewed
Jun 18, 2025
TarikGul
reviewed
Jun 18, 2025
TarikGul
approved these changes
Jun 18, 2025
TarikGul
left a comment
TarikGul
left a comment
Member
There was a problem hiding this comment.
Overall nice refactor, just one small suggestion.
That all being said, I was pretty thorough in PR and looking for the actual use of the Fungible Object in the SDK and polkadot-js. I couldn't find anything so I think its safe to say we can definitely remove it.
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
2 participants
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.
Consolidate
FungibleStrAssetTypeandFungibleObjAssetTypeintoFungibleAssetTypeBoth represent the same underlying structure.
There was an extra nested "Fungible" in ObjType as well that added unnecessary complexity.
edit: also consolidate
XcmAssetandXcmMultiAssetintoFungibleAssetandFungibleMultiAssetReview Notes
I don't think this is a Chesterton's Fence but can I get a slightly more thorough review on this one? There is no underlying Asset struct in
polkadot-sdkwith a nestedFungiblelikeFungibleObjAssethad. I could not find any reasonable explanation for the separation of the Obj and Str types. All tests continue to pass after the changes.