feat(`network`): param-change apply to genesis #3208

aljo242 · 2022-11-30T14:57:02Z

Closes #2899
Closes #3156

Adds check to verify param change before it is broadcast to the network.

ignite/pkg/jsonfile/jsonfile.go

ignite/services/network/networkchain/prepare.go

lumtis

Looks good, running some tests right now

ignite/services/network/networkchain/prepare.go

ignite/pkg/cosmosutil/genesis/genesis.go

ignite/services/network/networktypes/genesisinformation.go

lumtis · 2022-12-01T21:53:38Z

@aljo242
It seems like I get some trouble when using value as JSON

ignite n request change-param 7 crisis constant_fee "{\"denom\":\"bar\",\"amount\":\"500\"}" --from alice

ignite n request approve 7 5
Source code fetched
Blockchain set up
Requests format verified
Blockchain initialized
Genesis initialized
Genesis built
Request(s) not valid: Error: error reading GenesisDoc at /var/folders/6n/c3hb98tx2sjdxbz0cg5fy2gm0000gn/T/708038360/config/genesis.json: invalid character 'd' after object key:value pair. Make sure that you have correctly migrated all Tendermint consensus params, please see the chain migration guide at https://github.com/cosmos/cosmos-sdk/blob/main/UPGRADING.md for more info
Usage:
  exampled validate-genesis [file] [flags]

Am I using the correct format for JSON value? "{\"denom\":\"bar\",\"amount\":\"500\"}"

Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com>

lumtis · 2022-12-05T23:04:13Z

This Tx will still fail because this is not a param but that's ok:

Yes, actually

lumtis

It seems there is now an issue with string value

ignite n request change-param 11 staking bond_denom bar --from alice
Source code fetched
Blockchain set up
Genesis initialized
Invalid parameter change requested: Error: error reading GenesisDoc at /Users/lucas/spn/11/config/genesis.json: invalid character 'b' looking for beginning of value. Make sure that you have correctly migrated all Tendermint consensus params, please see the chain migration guide at https://github.com/cosmos/cosmos-sdk/blob/main/UPGRADING.md for more info
Usage:
  exampled validate-genesis [file] [flags]

Flags:
  -h, --help   help for validate-genesis

Global Flags:
      --home string         directory for config and data (default "/Users/lucas/.example")
      --log_format string   The logging format (json|plain) (default "plain")
      --log_level string    The logging level (trace|debug|info|warn|error|fatal|panic) (default "info")
      --trace               print out full stack trace on errors

: exit status 1

aljo242 · 2022-12-05T23:13:58Z

It seems there is now an issue with string value

ignite n request change-param 11 staking bond_denom bar --from alice
Source code fetched
Blockchain set up
Genesis initialized
Invalid parameter change requested: Error: error reading GenesisDoc at /Users/lucas/spn/11/config/genesis.json: invalid character 'b' looking for beginning of value. Make sure that you have correctly migrated all Tendermint consensus params, please see the chain migration guide at https://github.com/cosmos/cosmos-sdk/blob/main/UPGRADING.md for more info
Usage:
  exampled validate-genesis [file] [flags]

Flags:
  -h, --help   help for validate-genesis

Global Flags:
      --home string         directory for config and data (default "/Users/lucas/.example")
      --log_format string   The logging format (json|plain) (default "plain")
      --log_level string    The logging level (trace|debug|info|warn|error|fatal|panic) (default "info")
      --trace               print out full stack trace on errors

: exit status 1

Yes, you just have to wrap the string in quotations:

ignite n request change-param 11 staking bond_denom \"bar\" --from alice

Its because we are treating everything as a []byte internally and from the cmd level.

lumtis · 2022-12-05T23:20:01Z

Ok thanks, I was surprised as I used it without the quotes in previous tests

lumtis · 2022-12-06T21:59:10Z

@Pantani can you take some time to have a look at this one?

ignite/pkg/cosmosutil/genesis/genesis.go

Pantani · 2022-12-07T10:54:27Z

ignite/pkg/jsonfile/jsonfile_test.go

@@ -179,6 +182,16 @@ func TestJSONFile_Update(t *testing.T) {
 				),
 			},
 		},
+		{


Can we add a test case for:

WithKeyValueByte( "app_state.crisis.params.constant_fee ", []bytes("newValue"), ),

Maybe this case should fail because it will miss the "

I don't think I understand. Why would this test fail?

Because the value is a string and don't have the " in the begin and the end of the value, the JSON will break

How we handle to add string to JSON:

https://github.com/ignite/cli/pull/3208/files#diff-480ca1c42bb9467c5454780d5c8c887e218b1de339e6b77c58aa907725512179R203-R208

but it's a corner case. We don't need to cover if you think don't make sense

But in this specific test nothing will fail. We are just updating some generic []byte value here and there is no way to test if it is valid until we run validate-genesis later. So here at least, []byte("newValue") and []byte("\"newValue\"")are valid.

ignite/services/network/networkchain/prepare.go

ignite/services/network/networkchain/request.go

Co-authored-by: Danilo Pantani <danpantani@gmail.com>

Pantani

LGMT

aljo242 · 2022-12-08T15:18:15Z

@jeronimoalbi will merge if you approve

* update spn * import * fix * update mocks * rename test * feat(`network`): `param-change` (#3142) * utd * add command * typo * add test * changelog * format * Update ignite/cmd/network_request_param_change.go Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com> * Update changelog.md Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com> Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com> * refactor: rename command `param-change` into `change-param` (#3169) * change name * rename file * chore: upgrade `spn` (#3177) * gomod * gosum * endpoints (#3196) * format * request list * feat: add `ParamChange` to `ignite n request list/show` (#3233) * chore: go formatting (#3232) Co-authored-by: aljo242 <aljo242@users.noreply.github.com> * request list * format * ascii Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: aljo242 <aljo242@users.noreply.github.com> * format * feat(`network`): param-change apply to genesis (#3208) * stub out * prepare logic * fix test * Update ignite/services/network/networkchain/prepare.go Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com> * refactor WithKeyValueByte * add detail to error message * const format string * format * add integration test and fix * add check to command * format * finish check code * improve error msg * wip * fix * format * remove incorrect test * Update ignite/pkg/cosmosutil/genesis/genesis.go Co-authored-by: Danilo Pantani <danpantani@gmail.com> * rename according to review * optimize appylParamChanges * basic ModuleParamField test * format * revert errors Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com> Co-authored-by: Danilo Pantani <danpantani@gmail.com> * add test * add test * flesh out genesis information tests * format Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: aljo242 <aljo242@users.noreply.github.com> Co-authored-by: Danilo Pantani <danpantani@gmail.com>

* update spn * import * fix * update mocks * rename test * feat(`network`): `param-change` (ignite#3142) * utd * add command * typo * add test * changelog * format * Update ignite/cmd/network_request_param_change.go Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com> * Update changelog.md Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com> Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com> * refactor: rename command `param-change` into `change-param` (ignite#3169) * change name * rename file * chore: upgrade `spn` (ignite#3177) * gomod * gosum * endpoints (ignite#3196) * format * request list * feat: add `ParamChange` to `ignite n request list/show` (ignite#3233) * chore: go formatting (ignite#3232) Co-authored-by: aljo242 <aljo242@users.noreply.github.com> * request list * format * ascii Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: aljo242 <aljo242@users.noreply.github.com> * format * feat(`network`): param-change apply to genesis (ignite#3208) * stub out * prepare logic * fix test * Update ignite/services/network/networkchain/prepare.go Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com> * refactor WithKeyValueByte * add detail to error message * const format string * format * add integration test and fix * add check to command * format * finish check code * improve error msg * wip * fix * format * remove incorrect test * Update ignite/pkg/cosmosutil/genesis/genesis.go Co-authored-by: Danilo Pantani <danpantani@gmail.com> * rename according to review * optimize appylParamChanges * basic ModuleParamField test * format * revert errors Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com> Co-authored-by: Danilo Pantani <danpantani@gmail.com> * add test * add test * flesh out genesis information tests * format Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: aljo242 <aljo242@users.noreply.github.com> Co-authored-by: Danilo Pantani <danpantani@gmail.com>

aljo242 added 3 commits November 30, 2022 09:19

stub out

c6d347a

prepare logic

843e47e

Merge branch 'main' into feat/param-change-genesis

7c12ea1

aljo242 changed the base branch from main to chore/update-spn November 30, 2022 14:57

aljo242 changed the title ~~feat(network: param-change apply to genesis~~ feat(network): param-change apply to genesis Nov 30, 2022

Merge branch 'chore/update-spn' into feat/param-change-genesis

6a9db14

aljo242 added the skip-changelog Don't check changelog for new entries label Nov 30, 2022

fix test

d758610

aljo242 mentioned this pull request Dec 1, 2022

Network: Support Param Change requests #2899

Closed

2 tasks

Merge branch 'chore/update-spn' into feat/param-change-genesis

55bc316

aljo242 marked this pull request as ready for review December 1, 2022 16:33

aljo242 requested review from ilgooz, lumtis, jeronimoalbi, tbruyelle and fadeev as code owners December 1, 2022 16:33

Merge branch 'chore/update-spn' into feat/param-change-genesis

58a9fc8

tbruyelle reviewed Dec 1, 2022

View reviewed changes

ignite/pkg/jsonfile/jsonfile.go Outdated Show resolved Hide resolved

tbruyelle reviewed Dec 1, 2022

View reviewed changes

ignite/services/network/networkchain/prepare.go Outdated Show resolved Hide resolved

tbruyelle reviewed Dec 1, 2022

View reviewed changes

ignite/services/network/networkchain/prepare.go Outdated Show resolved Hide resolved

lumtis reviewed Dec 1, 2022

View reviewed changes

ignite/services/network/networkchain/prepare.go Outdated Show resolved Hide resolved

ignite/pkg/cosmosutil/genesis/genesis.go Outdated Show resolved Hide resolved

ignite/services/network/networktypes/genesisinformation.go Show resolved Hide resolved

Alex Johnson and others added 8 commits December 2, 2022 09:57

Update ignite/services/network/networkchain/prepare.go

4ecd64d

Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com>

refactor WithKeyValueByte

5963952

add detail to error message

429b8ad

const format string

1b1f200

format

3cc3184

add integration test and fix

3dba4fe

add check to command

4fd7dd2

format

4020211

aljo242 requested review from tbruyelle, Pantani and lumtis December 5, 2022 20:39

lumtis reviewed Dec 5, 2022

View reviewed changes

lumtis approved these changes Dec 5, 2022

View reviewed changes

lumtis mentioned this pull request Dec 6, 2022

Upgrade spn version #3111

Closed

2 tasks

Merge branch 'chore/update-spn' into feat/param-change-genesis

77e7bee

Pantani requested changes Dec 7, 2022

View reviewed changes

Alex Johnson and others added 6 commits December 7, 2022 08:44

Update ignite/pkg/cosmosutil/genesis/genesis.go

6512582

Co-authored-by: Danilo Pantani <danpantani@gmail.com>

rename according to review

93283c9

optimize appylParamChanges

0ba352c

basic ModuleParamField test

186b01a

format

462f3a4

revert errors

7f85936

aljo242 requested a review from Pantani December 7, 2022 14:12

Merge branch 'chore/update-spn' into feat/param-change-genesis

c66f491

tbruyelle approved these changes Dec 8, 2022

View reviewed changes

Pantani approved these changes Dec 8, 2022

View reviewed changes

Merge branch 'chore/update-spn' into feat/param-change-genesis

e59b9a6

jeronimoalbi approved these changes Dec 8, 2022

View reviewed changes

aljo242 merged commit 629920f into chore/update-spn Dec 8, 2022

aljo242 deleted the feat/param-change-genesis branch December 8, 2022 16:04

aljo242 mentioned this pull request Dec 8, 2022

Validate param-change values in network #3156

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(`network`): param-change apply to genesis #3208

feat(`network`): param-change apply to genesis #3208

aljo242 commented Nov 30, 2022 •

edited

Loading

lumtis left a comment

lumtis commented Dec 1, 2022

lumtis commented Dec 5, 2022

lumtis left a comment

aljo242 commented Dec 5, 2022 •

edited

Loading

lumtis commented Dec 5, 2022

lumtis commented Dec 6, 2022

Pantani Dec 7, 2022

aljo242 Dec 7, 2022

Pantani Dec 8, 2022

Pantani Dec 8, 2022 •

edited

Loading

Pantani Dec 8, 2022

aljo242 Dec 8, 2022 •

edited

Loading

Pantani left a comment

aljo242 commented Dec 8, 2022

@@ @@ -179,6 +182,16 @@ func TestJSONFile_Update(t *testing.T) { @@
               				),
               			},
               		},
+              		{

feat(network): param-change apply to genesis #3208

feat(network): param-change apply to genesis #3208

Conversation

aljo242 commented Nov 30, 2022 • edited Loading

lumtis left a comment

Choose a reason for hiding this comment

lumtis commented Dec 1, 2022

lumtis commented Dec 5, 2022

lumtis left a comment

Choose a reason for hiding this comment

aljo242 commented Dec 5, 2022 • edited Loading

lumtis commented Dec 5, 2022

lumtis commented Dec 6, 2022

Pantani Dec 7, 2022

Choose a reason for hiding this comment

aljo242 Dec 7, 2022

Choose a reason for hiding this comment

Pantani Dec 8, 2022

Choose a reason for hiding this comment

Pantani Dec 8, 2022 • edited Loading

Choose a reason for hiding this comment

Pantani Dec 8, 2022

Choose a reason for hiding this comment

aljo242 Dec 8, 2022 • edited Loading

Choose a reason for hiding this comment

Pantani left a comment

Choose a reason for hiding this comment

aljo242 commented Dec 8, 2022

feat(`network`): param-change apply to genesis #3208

feat(`network`): param-change apply to genesis #3208

aljo242 commented Nov 30, 2022 •

edited

Loading

aljo242 commented Dec 5, 2022 •

edited

Loading

Pantani Dec 8, 2022 •

edited

Loading

aljo242 Dec 8, 2022 •

edited

Loading