R4R: Parameter Change Proposal #3880

mossid · 2019-03-14T02:55:09Z

Replaces: #3799
Closes: #3565
Dependent on: #3779

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
Wrote tests
Updated relevant documentation (docs/)
Added entries in PENDING.md with issue #
rereviewed Files changed in the github PR explorer

For Admin Use:

Added appropriate labels to PR (ex. wip, ready-for-review, docs)
Reviewers Assigned
Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

Co-Authored-By: mossid <torecursedivine@gmail.com>

…s/cosmos-sdk into joon/3570-split-proposal-interface

Co-Authored-By: mossid <torecursedivine@gmail.com>

…s/cosmos-sdk into joon/3570-split-proposal-interface

mossid · 2019-03-14T03:37:34Z

Some XXXs are remaining but ready for review for most of the part.

Also, I'm not sure is it safe to modify gov.MsgSubmitProposal + any existing types, I suspect there could be an (un/) marshaling problem.

mossid · 2019-03-14T15:55:28Z

(I'll remove SubmitForm right now which seems like an overgeneralization)

codecov · 2019-03-14T16:57:20Z

Codecov Report

Merging #3880 into develop will decrease coverage by 0.33%.
The diff coverage is 55.78%.

@@             Coverage Diff             @@
##           develop    #3880      +/-   ##
===========================================
- Coverage     60.2%   59.87%   -0.34%     
===========================================
  Files          212      218       +6     
  Lines        15157    15291     +134     
===========================================
+ Hits          9126     9156      +30     
- Misses        5407     5512     +105     
+ Partials       624      623       -1

Co-Authored-By: mossid <torecursedivine@gmail.com>

…-parameter-change-proposal-2

…smos/cosmos-sdk into joon/3565-parameter-change-proposal-2

…-parameter-change-proposal-2

mossid · 2019-04-10T19:53:31Z

Docker related tests are failing but ready for review

fedekunze

some file structuring changes mostly

cmd/gaia/app/app.go

x/gov/client/cli/tx.go

fedekunze · 2019-04-10T20:39:18Z

x/gov/client/cli/tx.go

-				return err
+			content := gov.ContentFromProposalType(proposal.Title, proposal.Description, proposal.Type)
+			if content == nil {
+				return fmt.Errorf("Invalid proposal type %s", proposal.Type)


Ditto. From the line above I'd have expected something like "proposal content is empty"

fedekunze · 2019-04-10T21:06:28Z

x/gov/simulation/msgs.go

-		simulation.RandStringOfLength(r, 5),
-		simulation.RandStringOfLength(r, 5),
-		gov.ProposalTypeText,
+		gov.NewTextProposal(


should we add a parameter change proposal too ?

x/gov/tags/tags.go

fedekunze · 2019-04-10T21:09:22Z

x/params/contents.go

+}
+
+// Constructs new ChangeProposal
+func NewChangeProposal(title string, description string, changes []Change) ChangeProposal {


this should be part of governance

fedekunze · 2019-04-10T21:10:15Z

x/params/contents.go

+}
+
+// Proposal which contains multiple changes on proposals
+type ChangeProposal struct {


also this should be ParameterChangeProposal

It will be params.ParameterChangeProposal.. but I agree it makes more sense to name it ParameterChangeProposal

Is ParamChangeProposal looks good?

x/params/tags/tags.go

golangcibot · 2019-04-12T18:19:48Z

x/params/contents.go

+	if len(c.Subkey) != 0 {
+		subkey = "(" + string(c.Subkey) + ")"
+	}
+	return fmt.Sprintf("{%s%s := %X} (%s)", string(c.Key), subkey, c.Value, c.Space)


unnecessary conversion (from unconvert)

golangcibot · 2019-04-12T18:23:50Z

x/params/contents.go

+	if len(c.Subkey) != 0 {
+		subkey = "(" + string(c.Subkey) + ")"
+	}
+	return fmt.Sprintf("{%s%s := %X} (%s)", string(c.Key), subkey, c.Value, c.Space)


unnecessary conversion (from unconvert)

Co-Authored-By: mossid <torecursedivine@gmail.com>

alessio · 2019-04-16T18:33:52Z

x/gov/client/cli/parse.go

 func parseSubmitProposalFlags() (*proposal, error) {
 	proposal := &proposal{}
 	proposalFile := viper.GetString(flagProposal)

 	if proposalFile == "" {
 		proposal.Title = viper.GetString(flagTitle)
 		proposal.Description = viper.GetString(flagDescription)
-		proposal.Type = govClientUtils.NormalizeProposalType(viper.GetString(flagProposalType))
+		proposal.Type = readProposalType()


Just curious to learn why? Wasn't the inline call enough? I can't find any other place where readProposalType() is used

alexanderbez · 2019-04-18T04:19:17Z

Superseded by #4137.

mossid and others added 19 commits March 1, 2019 19:58

in progress

8fbe541

in progres

19b04db

fix test in progress

4fdabb1

fix test

ec133ef

fix lint

aac536d

Update x/gov/endblocker.go

cd0eb03

Co-Authored-By: mossid <torecursedivine@gmail.com>

Update x/gov/endblocker.go

d6b1e53

Co-Authored-By: mossid <torecursedivine@gmail.com>

address comments

6b13eb7

fix integration test

502bfc9

Merge branch 'joon/3570-split-proposal-interface' of github.com:cosmo…

afa4038

…s/cosmos-sdk into joon/3570-split-proposal-interface

add SoftwareUpgradeProposal

dcea8ef

Update x/gov/endblocker.go

31bda27

Co-Authored-By: mossid <torecursedivine@gmail.com>

Update x/gov/endblocker.go

0e58adb

Co-Authored-By: mossid <torecursedivine@gmail.com>

Merge branch 'joon/3570-split-proposal-interface' of github.com:cosmo…

e2ec4fe

…s/cosmos-sdk into joon/3570-split-proposal-interface

add to codec

90c1d7c

add pending

a03bcc8

Merge branch 'develop' into joon/3570-split-proposal-interface

ea820cf

expose SubmitProposal(), fix ProposalEqual()

c0a556a

fix test, add docs

43dcee0

mossid added the WIP label Mar 14, 2019

mossid marked this pull request as ready for review March 14, 2019 03:38

mossid requested review from cwgoes, ebuchman and rigelrozanski as code owners March 14, 2019 03:38

mossid changed the title ~~WIP: Parameter Change Proposal~~ R4R: Parameter Change Proposal Mar 14, 2019

mossid added ready-for-review and removed WIP labels Mar 14, 2019

mossid and others added 8 commits April 10, 2019 14:40

add test

7a8f90b

rename MsgSubmitParameterChangeProposal -> MsgSubmitProposal

d806275

Apply suggestions from code review

b05ebdb

Co-Authored-By: mossid <torecursedivine@gmail.com>

Merge branch 'develop' of github.com:cosmos/cosmos-sdk into joon/3565…

244ad53

…-parameter-change-proposal-2

unifying msg form, add validatebasic to content

ded0fcf

Merge branch 'joon/3565-parameter-change-proposal-2' of github.com:co…

a7edcbe

…smos/cosmos-sdk into joon/3565-parameter-change-proposal-2

unify msg into gov

0820bea

Merge branch 'develop' of github.com:cosmos/cosmos-sdk into joon/3565…

008b6f2

…-parameter-change-proposal-2

mossid changed the title ~~WIP: Parameter Change Proposal~~ R4R: Parameter Change Proposal Apr 10, 2019

mossid added ready-for-review and removed WIP labels Apr 10, 2019

fedekunze suggested changes Apr 10, 2019

View reviewed changes

add rest/cli, address comments

b098315

golangcibot reviewed Apr 12, 2019

View reviewed changes

mistake

d7056b7

golangcibot reviewed Apr 12, 2019

View reviewed changes

mossid and others added 5 commits April 12, 2019 20:23

gobot

ebdd392

Apply suggestions from code review

901cf44

Co-Authored-By: mossid <torecursedivine@gmail.com>

address comments

a784ee9

rename to ParameterChangeProposal, fix test

6b4cbda

Merge branch 'develop' into joon/3565-parameter-change-proposal-2

93b9dda

mossid requested a review from alessio as a code owner April 16, 2019 18:08

alessio reviewed Apr 16, 2019

View reviewed changes

alexanderbez mentioned this pull request Apr 16, 2019

Param Change Proposal: Part I #4137

Closed

5 tasks

alexanderbez closed this Apr 18, 2019

cwgoes deleted the joon/3565-parameter-change-proposal-2 branch April 18, 2019 09:35

This was referenced Apr 23, 2019

Param Change Proposal #4174

Closed

Param Change Proposal #4206

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

R4R: Parameter Change Proposal #3880

R4R: Parameter Change Proposal #3880

mossid commented Mar 14, 2019 •

edited by alexanderbez

Loading

mossid commented Mar 14, 2019

mossid commented Mar 14, 2019

codecov bot commented Mar 14, 2019 •

edited

Loading

mossid commented Apr 10, 2019

fedekunze left a comment

fedekunze Apr 10, 2019

fedekunze Apr 10, 2019

fedekunze Apr 10, 2019

fedekunze Apr 10, 2019

fedekunze Apr 10, 2019

mossid Apr 12, 2019

mossid Apr 12, 2019

golangcibot Apr 12, 2019

golangcibot Apr 12, 2019

alessio Apr 16, 2019

alexanderbez commented Apr 18, 2019

R4R: Parameter Change Proposal #3880

R4R: Parameter Change Proposal #3880

Conversation

mossid commented Mar 14, 2019 • edited by alexanderbez Loading

mossid commented Mar 14, 2019

mossid commented Mar 14, 2019

codecov bot commented Mar 14, 2019 • edited Loading

Codecov Report

mossid commented Apr 10, 2019

fedekunze left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexanderbez commented Apr 18, 2019

mossid commented Mar 14, 2019 •

edited by alexanderbez

Loading

codecov bot commented Mar 14, 2019 •

edited

Loading