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

change bypass-min-fee-types parsing; change defaults #2092

Merged
merged 5 commits into from
Mar 17, 2023

Conversation

MSalopek
Copy link
Contributor

@MSalopek MSalopek commented Jan 27, 2023

Closes: #2087

Parsing of bypass-min-fee-types is changed to allow operators to use empty bypass list.

The default was changed to be an empty list (no bypass allowed).

Removing the bypass-min-fee-types from app.toml will cause the default list of types to be used.

Validation was added so strings not corresponding to any proto message types will cause a panic (since this is a misconfiguration, manual action should be required)

Logging was added to inform operators about bypass usage.

Behaviour

no key in the config file

  • use defaults; log that default values were applied

key =

  • node panics; config cannot be parsed

key = [ ]

  • The value for the key is an empty list

key = ["value1", "value2"]

  • the value for the key is this list.

@MSalopek MSalopek self-assigned this Jan 27, 2023
@sonarcloud
Copy link

sonarcloud bot commented Jan 27, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@codecov
Copy link

codecov bot commented Jan 27, 2023

Codecov Report

Merging #2092 (b54f455) into main (483fb4d) will decrease coverage by 0.44%.
The diff coverage is 50.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2092      +/-   ##
==========================================
- Coverage   83.84%   83.40%   -0.44%     
==========================================
  Files          22       22              
  Lines        1541     1555      +14     
==========================================
+ Hits         1292     1297       +5     
- Misses        201      207       +6     
- Partials       48       51       +3     
Impacted Files Coverage Δ
app/params/config.go 100.00% <ø> (ø)
app/app.go 73.36% <50.00%> (-2.30%) ⬇️

@yaruwangway yaruwangway mentioned this pull request Feb 16, 2023
11 tasks
@MSalopek MSalopek marked this pull request as ready for review March 7, 2023 15:41
@MSalopek MSalopek changed the title [Draft] change bypass-min-fee-types parsing; change defaults change bypass-min-fee-types parsing; change defaults Mar 7, 2023
cmd/gaiad/cmd/root.go Outdated Show resolved Hide resolved
@yaruwangway
Copy link
Contributor

we can add some e2e tests, let me know if you want me to add it

@yaruwangway
Copy link
Contributor

this pr will be finally refactored. please see this issue.
#2228 (comment)

but lets still get it merged first, in case any decision changes later

@yaruwangway
Copy link
Contributor

changelog need to be added before merge.

CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com>
@yaruwangway yaruwangway merged commit 4badbcd into main Mar 17, 2023
@yaruwangway yaruwangway deleted the 2087-bypass-config-parser branch March 17, 2023 14:51
mergify bot pushed a commit that referenced this pull request Mar 17, 2023
* change bypass-min-fee-types parsing; change defaults

* revert default message registration

* docs: add changelog for fixing bypass parsing

* Update CHANGELOG.md

Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com>

---------

Co-authored-by: Yaru Wang <yaru@informal.systems>
Co-authored-by: yaruwangway <69694322+yaruwangway@users.noreply.github.com>
Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com>
(cherry picked from commit 4badbcd)

# Conflicts:
#	CHANGELOG.md
sainoe added a commit that referenced this pull request Mar 21, 2023
…#2311)

* change bypass-min-fee-types parsing; change defaults (#2092)

* change bypass-min-fee-types parsing; change defaults

* revert default message registration

* docs: add changelog for fixing bypass parsing

* Update CHANGELOG.md

Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com>

---------

Co-authored-by: Yaru Wang <yaru@informal.systems>
Co-authored-by: yaruwangway <69694322+yaruwangway@users.noreply.github.com>
Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com>
(cherry picked from commit 4badbcd)

# Conflicts:
#	CHANGELOG.md

* resolve conflict in changelog

* debug upgrade script

* bump cosmovisor

---------

Co-authored-by: MSalopek <35486649+MSalopek@users.noreply.github.com>
Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com>
@sainoe sainoe mentioned this pull request Apr 14, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Config params not properly parsed
3 participants