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

Config params not properly parsed #2087

Closed
1 of 5 tasks
mpoke opened this issue Jan 27, 2023 · 2 comments · Fixed by #2092
Closed
1 of 5 tasks

Config params not properly parsed #2087

mpoke opened this issue Jan 27, 2023 · 2 comments · Fixed by #2092
Assignees
Labels
type: bug Issues that need priority attention -- something isn't working

Comments

@mpoke
Copy link
Contributor

mpoke commented Jan 27, 2023

Summary of Bug

Configuration params with the key = [ ] structure are not properly passed. The expected value should be an empty list, however, the outcome is a nil value.

In general, we should be able to differentiate between the following scenarios:

  • no key in the config file
    • Log that default value is used instead
  • key =
    • Log a warning that the config is incorrect. Use default value instead.
  • key = [ ]
    • The value for the key is an empty list
  • key = ["value1", "value2"]
    • The value for the key is this list.

Version

v8, v9


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
  • Is a spike necessary to map out how the issue should be approached?
@mpoke mpoke added the type: bug Issues that need priority attention -- something isn't working label Jan 27, 2023
@mpoke
Copy link
Contributor Author

mpoke commented Jan 27, 2023

Note that a fix for this issue would not be state breaking. Thus, it can be applied after releasing v8 and v9.

@ancazamfir
Copy link

I am ok with what you decide wrt to timeline. But I think for v8 we should document things clearly.

regarding the issue itself, why is the default set to those IBC messages?

bypass-min-fee-msg-types = ["/ibc.core.channel.v1.MsgRecvPacket", "/ibc.core.channel.v1.MsgAcknowledgement", "/ibc.core.client.v1.MsgUpdateClient", ]

imho it should be [].

The current default is not clearly stated in the app.toml comments. I was testing this and assumed that if I removed the bypass-min-fee-msg-types there will be no bypassing. The comments should be updated.

The list is also incomplete from packet relaying pov, should also contain the timeout message types (there are two /ibc.core.client.v1.MsgTimeout and /ibc.core.client.v1.MsgTimeoutOnClose).

The format of these strings requires operators to have intimate knowledge of protobuf Any Uris for IBC messages. There should be some documentation available and kept up to date for these.

@mpoke mpoke added this to the Gaia v8.0.0 milestone Jan 27, 2023
@mpoke mpoke added the scope: docs Improvements or additions to documentation label Jan 27, 2023
@mpoke mpoke removed the scope: docs Improvements or additions to documentation label Jan 27, 2023
@mpoke mpoke removed this from the Gaia v8.0.0 milestone Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Issues that need priority attention -- something isn't working
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants