feat!: bypass-min-fee migration #2352

yaruwangway · 2023-04-03T14:14:55Z

Description

Create v10 upgrade handler and migration scripts adding bypass-min-fee-msg-types and MaxBypassMinFeeMsgGasUsage to the global fee module params.

query the params, chain inited by gaiad v9:
gaiad q globalfee minimum-gas-prices
curl http://localhost:1317/gaia/globalfee/v1beta1/minimum_gas_prices
query the params, chain inited by gaiad v10:

gaiad q globalfee params 

bypass_min_fee_msg_types:
- /ibc.core.channel.v1.MsgRecvPacket
- /ibc.core.channel.v1.MsgAcknowledgement
- /ibc.core.client.v1.MsgUpdateClient
- /ibc.core.channel.v1.MsgTimeout
- /ibc.core.channel.v1.MsgTimeoutOnClose
max_total_bypass_min_fee_msg_gas_usage: "1000000"
minimum_gas_prices:
- amount: ""

curl http://localhost:1317/gaia/globalfee/v1beta1/params
{
  "params": {
    "minimum_gas_prices": [
      {
        "denom": "stake",
        "amount": "0.002000000000000000"
      }
    ],
    "bypass_min_fee_msg_types": [
      "/ibc.core.channel.v1.MsgRecvPacket",
      "/ibc.core.channel.v1.MsgAcknowledgement",
      "/ibc.core.client.v1.MsgUpdateClient",
      "/ibc.core.channel.v1.MsgTimeout",
      "/ibc.core.channel.v1.MsgTimeoutOnClose"
    ],
    "max_total_bypass_min_fee_msg_gas_usage": "1000000"
  }
}%

This PR partially closes #2366 and closes #2457.

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

confirmed the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
confirmed all author checklist items have been addressed
reviewed state machine logic
reviewed API design and naming
reviewed documentation is accurate
reviewed tests and test coverage

…g_gas_usage to globalfee params

yaruwangway · 2023-04-04T16:08:47Z

CHANGELOG.md

@@ -35,6 +35,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 # Changelog

 ## [Unreleased]
+* Add `bypass-min-fee-msg-types` and `maxTotalBypassMinFeeMsgGagUsage` to globalfee params (state breaking and API breaking!!!).


add issue and pr link here

proto/gaia/globalfee/v1beta1/query.proto

x/globalfee/types/params.go

x/globalfee/migrations/v2/migration_test.go

proto/gaia/globalfee/v1beta1/query.proto

mpoke

Great work. Just some nits.

tests/e2e/query.go

.github/workflows/test.yml

contrib/single-node.sh

p-offtermatt

Great work! Don't think I have enough context to give an approval, but I looked over the changes and they look good to me. Just one tiny nitpick in a bash script.

Fix nitpick Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com>

MSalopek

Most of the changes are due to updating the module version from cosmos/gaia/v9... to cosmos/gaia/v10.

Other than that, the actual migration logic is minimal.

Approval!

MSalopek · 2023-05-16T12:36:14Z

x/globalfee/migrations/v2/migration.go

+// "/ibc.core.channel.v1.MsgTimeout",
+// "/ibc.core.channel.v1.MsgTimeoutOnClose"] as default and
+// add MaxTotalBypassMinFeeMsgGasUsage that is set 1_000_000 as default.
+func MigrateStore(ctx sdk.Context, globalfeeSubspace paramtypes.Subspace) error {


Note: this is the actual migration logic. Most of other files are boilerplate changes due to module renaming from cosmos/gaia/v9... to cosmos/gaia/v10

MSalopek · 2023-05-16T12:39:16Z

x/globalfee/migrations/v2/migration.go

+		MaxTotalBypassMinFeeMsgGasUsage: defaultParams.MaxTotalBypassMinFeeMsgGasUsage,
+	}
+
+	if !globalfeeSubspace.HasKeyTable() {


Note: if keytable does not exist - create it

mpoke

Approval

shaspitz · 2023-05-22T18:31:49Z

x/globalfee/migrations/v2/migration.go

+	}
+
+	if !globalfeeSubspace.HasKeyTable() {
+		globalfeeSubspace.WithKeyTable(types.ParamKeyTable())


@yaruwangway @MSalopek I don't think this line does anything. Correct me if I'm wrong. WithKeyTable seems to return a modified subspace that is thrown out here

i think it should already registered keytable, this is not necessary here. but it does not harm to recheck. keytable regissters the kv, and the validation methods.

We should use the returned value tho right?

Suggested change

globalfeeSubspace.WithKeyTable(types.ParamKeyTable())

newSubspace := globalfeeSubspace.WithKeyTable(types.ParamKeyTable())

fixed in this PR, thank you! #2524

Yaru Wang and others added 8 commits April 3, 2023 15:52

feat: update proto files

7a20fb0

feat: update proto-docs

92eb6f1

refactor: update query request and response

94906df

refactor: add bypass-min-fee-msg-types and max_total_bypass_minFee_ms…

15e0bc8

…g_gas_usage to globalfee params

test: update test

4e7478e

feat: add migration

a244a85

docs: update changelog

edd0e51

add BypassMinFeeMsgTypes param to store

c9b9b98

yaruwangway force-pushed the yaru/bypass-fee-migration branch from 2f327ae to edd0e51 Compare April 3, 2023 15:09

yaruwangway mentioned this pull request Apr 4, 2023

Plan: Global Fee module refactor #2270

Closed

Yaru Wang and others added 6 commits April 4, 2023 12:27

feat: add validateBypassMinFeeMsgTypes

e7b49a5

update fee decorator and global fee genesis

c57f7e9

merge main, fix tests

7ed0455

merge main

671cf76

test: fix e2e test

97b7e7b

test: add migration_test

1be66af

yaruwangway commented Apr 4, 2023

View reviewed changes

Yaru Wang and others added 2 commits April 4, 2023 18:12

chore: add comments

1a42713

remove bypass from config

784dce9

sainoe reviewed Apr 5, 2023

View reviewed changes

proto/gaia/globalfee/v1beta1/query.proto Outdated Show resolved Hide resolved

Yaru Wang added 2 commits April 5, 2023 10:34

fix: proto typo

e5b3b01

skip bypass test

3fbdbfe

yaruwangway commented Apr 5, 2023

View reviewed changes

x/globalfee/types/params.go Outdated Show resolved Hide resolved

x/globalfee/migrations/v2/migration_test.go Outdated Show resolved Hide resolved

sainoe added 3 commits April 5, 2023 15:49

save

6f33945

update ditrosless image url in Dockerfiles

be046e3

fix typo in makefil

ee64b8a

yaruwangway changed the title ~~bypass-min-fee migration~~ feat: !(state-breaking) bypass-min-fee migration Apr 6, 2023

sainoe added 3 commits April 6, 2023 15:48

Merge branch 'yaru/bypass-fee-migration' into sainoe/bypass-msg

0bb684d

Merge branch 'main' into sainoe/bypass-msg

a91a954

save

3383e37

sainoe and others added 2 commits May 5, 2023 16:34

clean e2e tests

478f042

merge main

2f70daa

yaruwangway marked this pull request as ready for review May 12, 2023 13:34

Yaru Wang added 5 commits May 12, 2023 16:02

fix QueryParamsResponse

9b59050

chore: remove duplicates

33d8644

docs: update proto doc

bd6c551

fix: globalfee params query

2bfa5ee

fix: e2e test, query globalfee params

350f106

MSalopek reviewed May 15, 2023

View reviewed changes

proto/gaia/globalfee/v1beta1/query.proto Show resolved Hide resolved

Yaru Wang added 3 commits May 16, 2023 11:06

update module to v10

a93f703

fix lint

f3aacb0

docs: update changelog

bc30c63

mpoke reviewed May 16, 2023

View reviewed changes

tests/e2e/query.go Outdated Show resolved Hide resolved

.github/workflows/test.yml Outdated Show resolved Hide resolved

contrib/single-node.sh Outdated Show resolved Hide resolved

Yaru Wang added 4 commits May 16, 2023 12:17

docs: update changelog links

1e7f613

update according to comments

362e1a2

test: upgrade fromVersion v9.1.0

8b24cf6

Merge branch 'main' into yaru/bypass-fee-migration

ad07053

p-offtermatt reviewed May 16, 2023

View reviewed changes

contrib/single-node.sh Outdated Show resolved Hide resolved

p-offtermatt reviewed May 16, 2023

View reviewed changes

MSalopek and others added 2 commits May 16, 2023 14:43

Update contrib/single-node.sh

c641a46

Fix nitpick Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com>

docs: update changelog about API endpoint change

91a08e9

MSalopek approved these changes May 16, 2023

View reviewed changes

MSalopek requested a review from mpoke May 16, 2023 13:48

mpoke approved these changes May 17, 2023

View reviewed changes

yaruwangway merged commit d425834 into main May 17, 2023

yaruwangway deleted the yaru/bypass-fee-migration branch May 17, 2023 09:45

shaspitz mentioned this pull request May 18, 2023

Discrepancy between v10 release branch and main #2505

Closed

yaruwangway mentioned this pull request May 22, 2023

Feat: tidy bypass-msg in config #2369

Closed

18 tasks

shaspitz reviewed May 22, 2023

View reviewed changes

sainoe mentioned this pull request May 24, 2023

tidy the bypas-min-fee-msg-types in config.toml #2368

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: bypass-min-fee migration #2352

feat!: bypass-min-fee migration #2352

yaruwangway commented Apr 3, 2023 •

edited

Loading

yaruwangway Apr 4, 2023

mpoke left a comment

p-offtermatt left a comment

MSalopek left a comment

MSalopek May 16, 2023

MSalopek May 16, 2023

mpoke left a comment

shaspitz May 22, 2023

yaruwangway May 22, 2023

shaspitz May 22, 2023

yaruwangway May 23, 2023

	globalfeeSubspace.WithKeyTable(types.ParamKeyTable())
	newSubspace := globalfeeSubspace.WithKeyTable(types.ParamKeyTable())

feat!: bypass-min-fee migration #2352

feat!: bypass-min-fee migration #2352

Conversation

yaruwangway commented Apr 3, 2023 • edited Loading

Description

Author Checklist

Reviewers Checklist

yaruwangway Apr 4, 2023

Choose a reason for hiding this comment

mpoke left a comment

Choose a reason for hiding this comment

p-offtermatt left a comment

Choose a reason for hiding this comment

MSalopek left a comment

Choose a reason for hiding this comment

MSalopek May 16, 2023

Choose a reason for hiding this comment

MSalopek May 16, 2023

Choose a reason for hiding this comment

mpoke left a comment

Choose a reason for hiding this comment

shaspitz May 22, 2023

Choose a reason for hiding this comment

yaruwangway May 22, 2023

Choose a reason for hiding this comment

shaspitz May 22, 2023

Choose a reason for hiding this comment

yaruwangway May 23, 2023

Choose a reason for hiding this comment

yaruwangway commented Apr 3, 2023 •

edited

Loading