ready-for-review: fixed donotmodify bug #1827
Conversation
sunnya97
requested review from
cwgoes,
ebuchman and
rigelrozanski
as code owners
sunnya97
changed the title
x/stake/types/validator.go
Outdated
| @@ -271,16 +271,16 @@ func NewDescription(moniker, identity, website, details string) Description { | |||
| // UpdateDescription updates the fields of a given description. An error is | |||
| // returned if the resulting description contains an invalid length. | |||
| func (d Description) UpdateDescription(d2 Description) (Description, sdk.Error) { | |||
| if d.Moniker == doNotModifyDescVal { | |||
| if d2.Moniker == doNotModifyDescVal { | |||
There was a problem hiding this comment.
I thought the point of this code is that if it is "[do-not-modify]" it can't be altered.
This now allows the description to be modified even if its currently "[do-not-modify]"
This is presuming that d2 is the updated desc (the variable should be renamed regardless imo, to make the code + godoc clearer).
There was a problem hiding this comment.
@ValarDragon you got it backwards - the point of [do-not-modify] is exclusively for the flag usage, not supposed to ever be stored in actual descriptions. How Sunny modified this here corrects the code to its original intention
x/stake/types/validator.go
Outdated
| d2.Moniker = d.Moniker | ||
| } | ||
| if d.Identity == doNotModifyDescVal { | ||
| if d2.Identity == doNotModifyDescVal { |
There was a problem hiding this comment.
Can we export doNotModifyDescVal?
There was a problem hiding this comment.
agreed no reason for this to be unexported - I can muster up some situations where maybe folks will want access to this
x/stake/client/cli/flags.go
Outdated
| fsDescription.String(FlagMoniker, "", "validator name") | ||
| fsDescription.String(FlagIdentity, "", "optional identity signature (ex. UPort or Keybase)") | ||
| fsDescription.String(FlagWebsite, "", "optional website") | ||
| fsDescription.String(FlagDetails, "", "optional details") |
There was a problem hiding this comment.
This change only makes sense in the context of these flags being used in create-validator. In the context of edit-validator the old default values make sense
There was a problem hiding this comment.
Wait, but if you default it to [do-not-modify], then how do I reset a field to empty using the cli?
There was a problem hiding this comment.
The default for validator creation is an empty string "", the default for the flags for edit-candidacy is [do-not-modify] which just means that if you do not include the flag, it will not edit the original field you had in there. If you want to reset a field to the empty string then something like gaiacli stake edit-validator --moniker="" should work
Codecov Report
@@ Coverage Diff @@
## develop #1827 +/- ##
========================================
Coverage 63.44% 63.44%
========================================
Files 117 117
Lines 6937 6937
========================================
Hits 4401 4401
Misses 2281 2281
Partials 255 255 |
closes #1740
docs/)PENDING.mdcmd/gaiaandexamples/For Admin Use: