PFM: FeePercentage param should default to zero if not set about ibc-apps HOT 6 CLOSED

Hey! While exploring the PFM, also noticed that the ModuleName has a typo packetfowardmiddleware -> packetforwardmiddleware, would be nice to be renamed if it's an easy fix

Yes, we realized this after the first release of the middleware unfortunately, so it was already adopted with the typo by some chains. To prevent migrations, we decided to live with the typo.

https://github.com/strangelove-ventures/packet-forward-middleware/commits/f520372571b26f23e7ce4ba9925efbd1366fb0b6/router/types/keys.go

from ibc-apps.

Hey! While exploring the PFM, also noticed that the ModuleName has a typo packetfowardmiddleware -> packetforwardmiddleware, would be nice to be renamed if it's an easy fix

from ibc-apps.

yeah, I noticed that too, but thought I'd ignore it to avoid having to migrate the params to the new key.

from ibc-apps.

At the moment, if the param is not set (i.e.: set to "") txs with forwarding will fail since https://github.com/cosmos/ibc-apps/blob/main/middleware/packet-forward-middleware/router/keeper/keeper.go#L214 can't convert it to a Dec.

Adding sensible defaults should be an easy fix

I took a look at this and it seemed like we are using a default value of 0 and also doing some validation to ensure that the param value can be type coerced into the sdk.Dec type properly see

Is there something i'm overlooking here?

from ibc-apps.

That looks good to me to right now. I can't remember the details here, but I think it was happening for us on a testnet when the parameter was not set on genesis or set to "". IIRC the validation occurs when setting the param, but if it never was initialized, it would somehow get ""

from ibc-apps.

Thanks that bit of context actually helped me find what I think the issue could be then.

In module.go we are not calling ValidateGenesis in the InitGenesis call. I'll get a PR opened right now

from ibc-apps.