PFM: FeePercentage param should default to zero if not set

[nicolaslara]

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

[Lockwarr]

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

[nicolaslara]

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

[agouin]

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

[jtieri]

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?

[nicolaslara]

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 ""

[jtieri]

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