Feature: Add config option for fee granters #1634
Conversation
fkneeland-figure
requested review from
adizere,
ancazamfir and
romac
as code owners
|
A nice and simple fix... As an aside this was tested using a relayer instance for the Provenance --> Cosmoshub IBC channel. |
There was a problem hiding this comment.
Thank you for you contribution!
We may adjust the changelog in your PR to adhere to our process (we follow this guideline for maintaining the changelog). I also left a nit suggestion. Feel free to fix the nit & changelog, otherwise one of the core maintainers will do it within ~1 day.
Cheers!
relayer/src/chain/cosmos.rs
Outdated
| @@ -440,6 +440,11 @@ impl CosmosSdkChain { | |||
| self.config.max_gas.unwrap_or(DEFAULT_MAX_GAS) | |||
| } | |||
|
|
|||
| /// Get the fee granter address | |||
| fn fee_grant(&self) -> &str { | |||
There was a problem hiding this comment.
Could we name this method similarly to the config.toml option, specifically fee_granter ?
|
@adizere Thanks for the review! I ran the script for the changelog and updated the name as you suggested. |
|
Question: Would it be sensible for your use-case if we make pub fee_granter: Option<String>,instead of pub fee_granter: String,? Motivation: This would simplify a bit the life of relayer operators who do not need to employ this new config.toml field, because they can simply skip defining it (and thus maintain a more concise config.toml for their operation). I think the change I'm proposing should not impact your use-case, but not sure if I'm missing anything. |
…/ibc-rs into fred/allow_fee_granters
Some fmt & clippy fixes.
|
I document the config.toml option in 16fc6a2 but left that segment of the configuration file commented-out. The reason for keeping this option a bit hidden is that it may be a source of misconfiguration. In particular, we don't validate the string in any way, and I suspect it should be a bech32-valid account identifier, but unclear how (if) we can validate that on Hermes side. Currently, using a random string such as Error message when using invalid fee granter account
|
There was a problem hiding this comment.
Many thanks for your contribution @fkneeland-figure and @iramiller.
|
Thanks! @adizere |
* Add config option for fee granters * update changelog * add fee_granter to config * make granter name consistent * reset changelog and fmt * updated changelog * use optional type for fee_granter * Documented `fee_granter` in config.toml. Some fmt & clippy fixes. Co-authored-by: Adi Seredinschi <adi@informal.systems>
Closes: #1633
Description
I updated the way the relayer handles fee to add a fee granter if it is specified in the config.toml. If the fee_granter is unspecified it will be set to an empty string as it currently is.
PR author checklist:
unclog.docs/).Reviewer checklist:
Files changedin the GitHub PR explorer.