Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C4GT] Asset: Added test for InvalidAssetType Issue #374 #427

Closed

Conversation

hardik-pratap-singh
Copy link
Contributor

Fixes #374
Added tests for InvalidAssetType for pallet/asset
@vatsa287 @amarts

Signed-off-by: hardik-pratap-singh <21bcs090@iiitdmj.ac.in>
@hardik-pratap-singh
Copy link
Contributor Author

Hey @vatsa287 @amarts !
All checks have passed..
Please review it.

Copy link
Member

@vatsa287 vatsa287 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hardik-pratap-singh Even though the tests have passed, I doubt that else branch is actually being executed.
Since initially asset type has been set to MF. And in if condition it is being checked if type is of any 3 valid types. So if part is executed and not else.
So the error code is not being checked.
Please add a log and check.

@hardik-pratap-singh
Copy link
Contributor Author

@hardik-pratap-singh Even though the tests have passed, I doubt that else branch is actually being executed. Since initially asset type has been set to MF. And in if condition it is being checked if type is of any 3 valid types. So if part is executed and not else. So the error code is not being checked. Please add a log and check.

@vatsa287 ! I was also thinking the same. But everywhere I can find asset_type being initialized with MF. How can I actually get the asset_type ?

@vatsa287
Copy link
Member

@hardik-pratap-singh Even though the tests have passed, I doubt that else branch is actually being executed. Since initially asset type has been set to MF. And in if condition it is being checked if type is of any 3 valid types. So if part is executed and not else. So the error code is not being checked. Please add a log and check.

@vatsa287 ! I was also thinking the same. But everywhere I can find asset_type being initialized with MF. How can I actually get the asset_type ?

Check pallets/src/types.rs where we define AssetTypeOf to be ,

pub enum AssetTypeOf {
	ART,
	BOND,
	MF,
}

@hardik-pratap-singh
Copy link
Contributor Author

@hardik-pratap-singh Even though the tests have passed, I doubt that else branch is actually being executed. Since initially asset type has been set to MF. And in if condition it is being checked if type is of any 3 valid types. So if part is executed and not else. So the error code is not being checked. Please add a log and check.

@vatsa287 ! I was also thinking the same. But everywhere I can find asset_type being initialized with MF. How can I actually get the asset_type ?

Check pallets/src/types.rs where we define AssetTypeOf to be ,

pub enum AssetTypeOf {
	ART,
	BOND,
	MF,
}

I wanted to ask that all of the values i.e. asset_desc , asset_tag , asset_meta , asset_qty , asset_value and asset_type are being hard coded right ? Or we are getting them dynamically from somewhere ?
If we don't have to hard code the value just like we did for asset_type as MF, what can we write for asset_type ?
Currently we have this : let asset_type = AssetTypeOf::MF

We can add a new asset type called Invalid and pass AssetTypeOf::Invalid to get the else part in function. But I don't think that will be a good practice.

@hardik-pratap-singh
Copy link
Contributor Author

@vatsa287 Can you help me with it ?

@hardik-pratap-singh
Copy link
Contributor Author

Hey @vatsa287 @NiranjanAP @amarts !
I am stuck at this issue.

Can you please guide me ?

@vatsa287
Copy link
Member

vatsa287 commented May 6, 2024

@hardik-pratap-singh Regarding this, It is not possible to send a value/option which is not defined in the enum of valid asset types.
Moreover there are checks present in SDK through with the data is in bound to the chain. And on the chain too there is validity check present.
Since RUST is a compiled time language you cannot pass AssetTypeOf::Invalid in tests too which will throw it is not a part of enum declared.

@vatsa287 vatsa287 closed this Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[C4GT] Asset: Add tests for InvalidAssetType
2 participants