go-bitsong icon indicating copy to clipboard operation
go-bitsong copied to clipboard

Default Params value

Open giorgionocera opened this issue 3 years ago • 2 comments

In the nft module, when the DefaultGenesisState method is called, it returns default data for the genesis, which includes all attributes for the genesis at default zero values. In such a scenario, the Params attribute is associated with the results of the DefaultParams method https://github.com/bitsongofficial/go-bitsong/blob/84691c819214a1f9b9dd844ea2b49b7ccc8ece1b/x/nft/genesis.go#L9-L18

The IssuePrice parameter is defined in the NewParams method https://github.com/bitsongofficial/go-bitsong/blob/84691c819214a1f9b9dd844ea2b49b7ccc8ece1b/x/nft/types/params.go#L25-L30

but it is not defined in the DefaultParams method https://github.com/bitsongofficial/go-bitsong/blob/84691c819214a1f9b9dd844ea2b49b7ccc8ece1b/x/nft/types/params.go#L37-L40

Is this an intentional lack?

I think it is due to the fact that DefaultParams returns an empty Params object which includes the IssuePrice in its definition, so its value is automatically set to zero. If so, I agree that it is not an error, but maybe, explicit the quantity it could increase the readability, moreover by adding a default zero value as it is done in the fantoken module

https://github.com/bitsongofficial/go-bitsong/blob/84691c819214a1f9b9dd844ea2b49b7ccc8ece1b/x/fantoken/types/params.go#L43-L50

What do you think about it?

giorgionocera avatar Sep 29 '22 06:09 giorgionocera

I agree to set the DefaultParams even with zero

 func DefaultParams() Params { 
 	return Params{ 
 		IssueFee: sdk.NewCoin(sdk.DefaultBondDenom, sdk.ZeroInt()),
 	} 
 } 

angelorc avatar Sep 29 '22 10:09 angelorc

Ok

ryusmo avatar Sep 30 '22 17:09 ryusmo