From 61cbd129766918492771e213791ed603791e07c7 Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Tue, 10 Jul 2018 02:38:09 -0400 Subject: [PATCH 01/13] cleanup logic in updateValidator --- x/stake/keeper/validator.go | 120 +++++++++++++++++++++--------------- 1 file changed, 69 insertions(+), 51 deletions(-) diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go index 6962ca28a1..b1a0ebe866 100644 --- a/x/stake/keeper/validator.go +++ b/x/stake/keeper/validator.go @@ -199,78 +199,96 @@ func (k Keeper) ClearTendermintUpdates(ctx sdk.Context) { func (k Keeper) UpdateValidator(ctx sdk.Context, validator types.Validator) types.Validator { store := ctx.KVStore(k.storeKey) pool := k.GetPool(ctx) - ownerAddr := validator.Owner + oldValidator, oldFound := k.GetValidator(ctx, validator.Owner) - // always update the main list ordered by owner address before exiting - defer func() { - k.SetValidator(ctx, validator) - }() + validator = k.updateForRevoking(ctx, oldFound, oldValidator, validator) + powerIncreasing := k.getPowerIncreasing(ctx, oldFound, oldValidator, validator) + validator.BondHeight, validator.BondIntraTxCounter = k.getBondDetails(ctx, oldFound, oldValidator, validator) + valPower := k.updateValidatorPower(ctx, oldFound, oldValidator, validator, pool) + cliffPower := k.GetCliffValidatorPower(ctx) - // retrieve the old validator record - oldValidator, oldFound := k.GetValidator(ctx, ownerAddr) + switch { + // if already bonded and power increasing only need to update tendermint + case powerIncreasing && !validator.Revoked && + (oldFound && oldValidator.Status() == sdk.Bonded): - if validator.Revoked && oldValidator.Status() == sdk.Bonded { - validator = k.unbondValidator(ctx, validator) + bz := k.cdc.MustMarshalBinary(validator.ABCIValidator()) + store.Set(GetTendermintUpdatesKey(validator.Owner), bz) + + // if was unbonded/or is a new validator - and the new power is less than the cliff validator + case cliffPower != nil && + (!oldFound || (oldFound && oldValidator.Status() == sdk.Unbonded)) && + bytes.Compare(valPower, cliffPower) == -1: //(valPower < cliffPower + // skip to completion + + default: + // update the validator set for this validator + updatedVal := k.UpdateBondedValidators(ctx, validator) + if updatedVal.Owner != nil { // updates to validator occurred to be updated + validator = updatedVal + } else { + + // if decreased in power but still bonded, update Tendermint validator + // (if updatedVal is set, the validator has changed bonding status) + stillBonded := oldFound && oldValidator.Status() == sdk.Bonded + if stillBonded && oldValidator.PoolShares.Bonded().GT(validator.PoolShares.Bonded()) { + bz := k.cdc.MustMarshalBinary(validator.ABCIValidator()) + store.Set(GetTendermintUpdatesKey(validator.Owner), bz) + } + } + } + + k.SetValidator(ctx, validator) + return validator +} + +func (k Keeper) updateForRevoking(ctx sdk.Context, oldFound bool, oldValidator, newValidator types.Validator) types.Validator { + if newValidator.Revoked && oldFound && oldValidator.Status() == sdk.Bonded { + newValidator = k.unbondValidator(ctx, newValidator) // need to also clear the cliff validator spot because the revoke has // opened up a new spot which will be filled when // updateValidatorsBonded is called k.clearCliffValidator(ctx) } + return newValidator +} - powerIncreasing := false - if oldFound && oldValidator.PoolShares.Bonded().LT(validator.PoolShares.Bonded()) { - powerIncreasing = true +func (k Keeper) getPowerIncreasing(ctx sdk.Context, oldFound bool, oldValidator, newValidator types.Validator) bool { + if oldFound && oldValidator.PoolShares.Bonded().LT(newValidator.PoolShares.Bonded()) { + return true } + return false +} + +func (k Keeper) getBondDetails(ctx sdk.Context, oldFound bool, oldValidator, + newValidator types.Validator) (height int64, intraTxCounter int16) { // if already a validator, copy the old block height and counter, else set them if oldFound && oldValidator.Status() == sdk.Bonded { - validator.BondHeight = oldValidator.BondHeight - validator.BondIntraTxCounter = oldValidator.BondIntraTxCounter - } else { - validator.BondHeight = ctx.BlockHeight() - counter := k.GetIntraTxCounter(ctx) - validator.BondIntraTxCounter = counter - k.SetIntraTxCounter(ctx, counter+1) + height = oldValidator.BondHeight + intraTxCounter = oldValidator.BondIntraTxCounter + return } + height = ctx.BlockHeight() + counter := k.GetIntraTxCounter(ctx) + intraTxCounter = counter + k.SetIntraTxCounter(ctx, counter+1) + return +} + +func (k Keeper) updateValidatorPower(ctx sdk.Context, oldFound bool, oldValidator, + newValidator types.Validator, pool types.Pool) (valPower []byte) { + store := ctx.KVStore(k.storeKey) // update the list ordered by voting power if oldFound { store.Delete(GetValidatorsByPowerIndexKey(oldValidator, pool)) } - valPower := GetValidatorsByPowerIndexKey(validator, pool) - store.Set(valPower, validator.Owner) + valPower = GetValidatorsByPowerIndexKey(newValidator, pool) + store.Set(valPower, newValidator.Owner) - // efficiency case: - // if already bonded and power increasing only need to update tendermint - if powerIncreasing && !validator.Revoked && oldValidator.Status() == sdk.Bonded { - bz := k.cdc.MustMarshalBinary(validator.ABCIValidator()) - store.Set(GetTendermintUpdatesKey(ownerAddr), bz) - return validator - } - - // efficiency case: - // if was unbonded/or is a new validator - and the new power is less than the cliff validator - cliffPower := k.GetCliffValidatorPower(ctx) - if cliffPower != nil && - (!oldFound || (oldFound && oldValidator.Status() == sdk.Unbonded)) && - bytes.Compare(valPower, cliffPower) == -1 { //(valPower < cliffPower - return validator - } - - // update the validator set for this validator - updatedVal := k.UpdateBondedValidators(ctx, validator) - if updatedVal.Owner != nil { // updates to validator occurred to be updated - validator = updatedVal - } - // if decreased in power but still bonded, update Tendermint validator - // (if updatedVal is set, the validator has changed bonding status) - stillBonded := oldFound && oldValidator.Status() == sdk.Bonded && updatedVal.Owner == nil - if stillBonded && oldValidator.PoolShares.Bonded().GT(validator.PoolShares.Bonded()) { - bz := k.cdc.MustMarshalBinary(validator.ABCIValidator()) - store.Set(GetTendermintUpdatesKey(ownerAddr), bz) - } - return validator + return valPower } // Update the validator group and kick out any old validators. In addition this From 12932de40f89844923f38b6b3f85bbc6fa99994e Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Tue, 10 Jul 2018 10:23:31 -0400 Subject: [PATCH 02/13] further cleanup, also reorg updateBondedValidators --- x/stake/keeper/validator.go | 107 ++++++++++++++++++++---------------- 1 file changed, 59 insertions(+), 48 deletions(-) diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go index b1a0ebe866..2cc1c2d452 100644 --- a/x/stake/keeper/validator.go +++ b/x/stake/keeper/validator.go @@ -215,9 +215,14 @@ func (k Keeper) UpdateValidator(ctx sdk.Context, validator types.Validator) type bz := k.cdc.MustMarshalBinary(validator.ABCIValidator()) store.Set(GetTendermintUpdatesKey(validator.Owner), bz) - // if was unbonded/or is a new validator - and the new power is less than the cliff validator + // if is a new validator and the new power is less than the cliff validator + case cliffPower != nil && !oldFound && + bytes.Compare(valPower, cliffPower) == -1: //(valPower < cliffPower + // skip to completion + + // if was unbonded and the new power is less than the cliff validator case cliffPower != nil && - (!oldFound || (oldFound && oldValidator.Status() == sdk.Unbonded)) && + (oldFound && oldValidator.Status() == sdk.Unbonded) && bytes.Compare(valPower, cliffPower) == -1: //(valPower < cliffPower // skip to completion @@ -299,11 +304,13 @@ func (k Keeper) updateValidatorPower(ctx sdk.Context, oldFound bool, oldValidato // The correct subset is retrieved by iterating through an index of the // validators sorted by power, stored using the ValidatorsByPowerIndexKey. // Simultaneously the current validator records are updated in store with the -// ValidatorsBondedIndexKey. This store is used to determine if a validator is a -// validator without needing to iterate over the subspace as we do in +// ValidatorsBondedIndexKey. This store is used to determine if a validator is +// a validator without needing to iterate over the subspace as we do in // GetValidators. // -// Optionally also return the validator from a retrieve address if the validator has been bonded +// Optionally also return the validator from a retrieve address if the +// validator has been bonded +// // nolint: gocyclo // TODO: Remove the above golint func (k Keeper) UpdateBondedValidators(ctx sdk.Context, @@ -311,30 +318,20 @@ func (k Keeper) UpdateBondedValidators(ctx sdk.Context, store := ctx.KVStore(k.storeKey) - kickCliffValidator := false oldCliffValidatorAddr := k.GetCliffValidator(ctx) - - // add the actual validator power sorted store maxValidators := k.GetParams(ctx).MaxValidators - iterator := sdk.KVStoreReversePrefixIterator(store, ValidatorsByPowerIndexKey) // largest to smallest bondedValidatorsCount := 0 - var validator types.Validator + var validator, validatorToBond types.Validator + + iterator := sdk.KVStoreReversePrefixIterator(store, ValidatorsByPowerIndexKey) // largest to smallest for { if !iterator.Valid() || bondedValidatorsCount > int(maxValidators-1) { - - // TODO benchmark if we should read the current power and not write if it's the same - if bondedValidatorsCount == int(maxValidators) { // is cliff validator - k.setCliffValidator(ctx, validator, k.GetPool(ctx)) - } else if len(oldCliffValidatorAddr) > 0 { - k.clearCliffValidator(ctx) - } break } // either retrieve the original validator from the store, or under the - // situation that this is the "new validator" just use the validator - // provided because it has not yet been updated in the main validator - // store + // situation that this is the "affected validator" just use the + // validator provided because it has not yet been updated in the store ownerAddr := iterator.Value() if bytes.Equal(ownerAddr, affectedValidator.Owner) { validator = affectedValidator @@ -346,38 +343,48 @@ func (k Keeper) UpdateBondedValidators(ctx sdk.Context, } } - // if not previously a validator (and unrevoked), - // kick the cliff validator / bond this new validator - if validator.Status() != sdk.Bonded && !validator.Revoked { - kickCliffValidator = true - - validator = k.bondValidator(ctx, validator) - if bytes.Equal(ownerAddr, affectedValidator.Owner) { - updatedVal = validator - } - } - + // increment bondedValidatorsCount / get the validator to bond if !validator.Revoked { - bondedValidatorsCount++ - } else { - if validator.Status() == sdk.Bonded { - panic(fmt.Sprintf("revoked validator cannot be bonded, address: %v\n", ownerAddr)) + if validator.Status() != sdk.Bonded { + validatorToBond = validator } + bondedValidatorsCount++ + + // sanity check + } else if validator.Status() == sdk.Bonded { + panic(fmt.Sprintf("revoked validator cannot be bonded, address: %v\n", ownerAddr)) } iterator.Next() } iterator.Close() - // perform the actual kicks - if oldCliffValidatorAddr != nil && kickCliffValidator { - validator, found := k.GetValidator(ctx, oldCliffValidatorAddr) - if !found { - panic(fmt.Sprintf("validator record not found for address: %v\n", oldCliffValidatorAddr)) - } - k.unbondValidator(ctx, validator) + // clear or set the cliff validator + if bondedValidatorsCount == int(maxValidators) { + k.setCliffValidator(ctx, validator, k.GetPool(ctx)) + } else if len(oldCliffValidatorAddr) > 0 { + k.clearCliffValidator(ctx) } + // perform the validator swap for any new validators to add + if len(validatorToBond.Owner) > 0 { + + // unbond the cliff validator + if oldCliffValidatorAddr != nil { + cliffVal, found := k.GetValidator(ctx, oldCliffValidatorAddr) + if !found { + panic(fmt.Sprintf("validator record not found for address: %v\n", oldCliffValidatorAddr)) + } + k.unbondValidator(ctx, cliffVal) + + } + + // bond the new validator + validator = k.bondValidator(ctx, validatorToBond) + if bytes.Equal(validator.Owner, affectedValidator.Owner) { + updatedVal = validator + } + } return } @@ -395,17 +402,14 @@ func (k Keeper) UpdateBondedValidatorsFull(ctx sdk.Context) { } iterator.Close() - // add the actual validator power sorted store + oldCliffValidatorAddr := k.GetCliffValidator(ctx) maxValidators := k.GetParams(ctx).MaxValidators - iterator = sdk.KVStoreReversePrefixIterator(store, ValidatorsByPowerIndexKey) // largest to smallest bondedValidatorsCount := 0 + + iterator = sdk.KVStoreReversePrefixIterator(store, ValidatorsByPowerIndexKey) // largest to smallest var validator types.Validator for { if !iterator.Valid() || bondedValidatorsCount > int(maxValidators-1) { - - if bondedValidatorsCount == int(maxValidators) { // is cliff validator - k.setCliffValidator(ctx, validator, k.GetPool(ctx)) - } break } @@ -443,6 +447,13 @@ func (k Keeper) UpdateBondedValidatorsFull(ctx sdk.Context) { } iterator.Close() + // clear or set the cliff validator + if bondedValidatorsCount == int(maxValidators) { + k.setCliffValidator(ctx, validator, k.GetPool(ctx)) + } else if len(oldCliffValidatorAddr) > 0 { + k.clearCliffValidator(ctx) + } + // perform the actual kicks kickOutValidators(k, ctx, toKickOut) return From 053f4e955b7819372eb5432bd2d16625a7ed6c4a Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Tue, 10 Jul 2018 14:25:56 -0400 Subject: [PATCH 03/13] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 222f52e4ab..dabd90a514 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,6 +99,7 @@ IMPROVEMENTS * [stake] keeper always loads the store (instead passing around which doesn't really boost efficiency) * [stake] edit-validator changes now can use the keyword [do-not-modify] to not modify unspecified `--flag` (aka won't set them to `""` value) * [stake] offload more generic functionality from the handler into the keeper +* [stake] clearer staking logic * [types] added common tag constants * [keys] improve error message when deleting non-existent key * [gaiacli] improve error messages on `send` and `account` commands From e8e5e8c6007e83bf6d01f906018d0e2176ea482b Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Tue, 10 Jul 2018 18:36:57 -0400 Subject: [PATCH 04/13] bez updates --- x/stake/keeper/validator.go | 52 +++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go index 2cc1c2d452..60177aa6e0 100644 --- a/x/stake/keeper/validator.go +++ b/x/stake/keeper/validator.go @@ -191,9 +191,11 @@ func (k Keeper) ClearTendermintUpdates(ctx sdk.Context) { //___________________________________________________________________________ -// perfom all the nessisary steps for when a validator changes its power -// updates all validator stores as well as tendermint update store -// may kick out validators if new validator is entering the bonded validator group +// Perfom all the nessisary steps for when a validator changes its power. This +// function updates all validator stores as well as tendermint update store. +// It may kick out validators if new validator is entering the bonded validator +// group. +// // nolint: gocyclo // TODO: Remove above nolint, function needs to be simplified func (k Keeper) UpdateValidator(ctx sdk.Context, validator types.Validator) types.Validator { @@ -203,7 +205,7 @@ func (k Keeper) UpdateValidator(ctx sdk.Context, validator types.Validator) type validator = k.updateForRevoking(ctx, oldFound, oldValidator, validator) powerIncreasing := k.getPowerIncreasing(ctx, oldFound, oldValidator, validator) - validator.BondHeight, validator.BondIntraTxCounter = k.getBondDetails(ctx, oldFound, oldValidator, validator) + validator.BondHeight, validator.BondIntraTxCounter = k.bondIncrement(ctx, oldFound, oldValidator, validator) valPower := k.updateValidatorPower(ctx, oldFound, oldValidator, validator, pool) cliffPower := k.GetCliffValidatorPower(ctx) @@ -228,8 +230,8 @@ func (k Keeper) UpdateValidator(ctx sdk.Context, validator types.Validator) type default: // update the validator set for this validator - updatedVal := k.UpdateBondedValidators(ctx, validator) - if updatedVal.Owner != nil { // updates to validator occurred to be updated + updatedVal, updated := k.UpdateBondedValidators(ctx, validator) + if updated { // updates to validator occurred to be updated validator = updatedVal } else { @@ -266,7 +268,8 @@ func (k Keeper) getPowerIncreasing(ctx sdk.Context, oldFound bool, oldValidator, return false } -func (k Keeper) getBondDetails(ctx sdk.Context, oldFound bool, oldValidator, +// get the bond height and incremented intra-tx counter +func (k Keeper) bondIncrement(ctx sdk.Context, oldFound bool, oldValidator, newValidator types.Validator) (height int64, intraTxCounter int16) { // if already a validator, copy the old block height and counter, else set them @@ -296,25 +299,22 @@ func (k Keeper) updateValidatorPower(ctx sdk.Context, oldFound bool, oldValidato return valPower } -// Update the validator group and kick out any old validators. In addition this -// function adds (or doesn't add) a validator which has updated its bonded -// tokens to the validator group. -> this validator is specified through the -// updatedValidatorAddr term. +// Update the bonded validator group based on a change to the validator +// affectedValidator. This function potentially adds the affectedValidator to +// the bonded validator group which kicks out the cliff validator. Under this +// situation this function returns the updated affectedValidator. // -// The correct subset is retrieved by iterating through an index of the -// validators sorted by power, stored using the ValidatorsByPowerIndexKey. -// Simultaneously the current validator records are updated in store with the -// ValidatorsBondedIndexKey. This store is used to determine if a validator is -// a validator without needing to iterate over the subspace as we do in -// GetValidators. -// -// Optionally also return the validator from a retrieve address if the -// validator has been bonded +// The correct bonded subset of validators is retrieved by iterating through an +// index of the validators sorted by power, stored using the +// ValidatorsByPowerIndexKey. Simultaneously the current validator records are +// updated in store with the ValidatorsBondedIndexKey. This store is used to +// determine if a validator is a validator without needing to iterate over all +// validators. // // nolint: gocyclo // TODO: Remove the above golint func (k Keeper) UpdateBondedValidators(ctx sdk.Context, - affectedValidator types.Validator) (updatedVal types.Validator) { + affectedValidator types.Validator) (updatedVal types.Validator, updated bool) { store := ctx.KVStore(k.storeKey) @@ -322,6 +322,7 @@ func (k Keeper) UpdateBondedValidators(ctx sdk.Context, maxValidators := k.GetParams(ctx).MaxValidators bondedValidatorsCount := 0 var validator, validatorToBond types.Validator + newValidatorBonded := false iterator := sdk.KVStoreReversePrefixIterator(store, ValidatorsByPowerIndexKey) // largest to smallest for { @@ -347,6 +348,7 @@ func (k Keeper) UpdateBondedValidators(ctx sdk.Context, if !validator.Revoked { if validator.Status() != sdk.Bonded { validatorToBond = validator + newValidatorBonded = true } bondedValidatorsCount++ @@ -366,8 +368,8 @@ func (k Keeper) UpdateBondedValidators(ctx sdk.Context, k.clearCliffValidator(ctx) } - // perform the validator swap for any new validators to add - if len(validatorToBond.Owner) > 0 { + // swap the cliff validator for a new validator if the affected validator was bonded + if newValidatorBonded { // unbond the cliff validator if oldCliffValidatorAddr != nil { @@ -382,10 +384,10 @@ func (k Keeper) UpdateBondedValidators(ctx sdk.Context, // bond the new validator validator = k.bondValidator(ctx, validatorToBond) if bytes.Equal(validator.Owner, affectedValidator.Owner) { - updatedVal = validator + return validator, true } } - return + return Validator{}, false } // full update of the bonded validator set, many can be added/kicked From 561eda3fc337b6cbeeaae80f435d7c0190c0d19b Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Tue, 10 Jul 2018 18:39:52 -0400 Subject: [PATCH 05/13] typo --- x/stake/keeper/validator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go index 60177aa6e0..db10ebd362 100644 --- a/x/stake/keeper/validator.go +++ b/x/stake/keeper/validator.go @@ -387,7 +387,7 @@ func (k Keeper) UpdateBondedValidators(ctx sdk.Context, return validator, true } } - return Validator{}, false + return types.Validator{}, false } // full update of the bonded validator set, many can be added/kicked From 9eeb47517d02129dc5b1c5955a933579ec5acb5c Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Tue, 10 Jul 2018 19:27:39 -0400 Subject: [PATCH 06/13] cwgoes comments --- x/stake/keeper/validator.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go index db10ebd362..e96a1330b5 100644 --- a/x/stake/keeper/validator.go +++ b/x/stake/keeper/validator.go @@ -228,7 +228,11 @@ func (k Keeper) UpdateValidator(ctx sdk.Context, validator types.Validator) type bytes.Compare(valPower, cliffPower) == -1: //(valPower < cliffPower // skip to completion + // default case - validator was either: + // a) not-bonded and now has power-rank greater than cliff validator + // b) bonded and now has decreased in power default: + // update the validator set for this validator updatedVal, updated := k.UpdateBondedValidators(ctx, validator) if updated { // updates to validator occurred to be updated From cba7fd7ee831ab1b690d2414fec91d6674e7c09d Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Tue, 10 Jul 2018 17:59:07 -0700 Subject: [PATCH 07/13] gov enums marshal to String --- CHANGELOG.md | 2 + client/lcd/lcd_test.go | 60 ++++---- cmd/gaia/cli_test/cli_test.go | 18 +-- x/gov/client/cli/tx.go | 12 +- x/gov/client/rest/rest.go | 75 +++------- x/gov/depositsvotes.go | 156 +++++++++++---------- x/gov/errors.go | 9 +- x/gov/keeper.go | 6 +- x/gov/msgs.go | 44 ++---- x/gov/msgs_test.go | 2 +- x/gov/proposals.go | 252 ++++++++++++++++++++++------------ 11 files changed, 335 insertions(+), 301 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a4c6acf7b..f7a12d87fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,8 @@ BREAKING CHANGES * [lcd] Switch key creation output to return bech32 * [lcd] Removed shorthand CLI flags (`a`, `c`, `n`, `o`) * [gaiad] genesis transactions now use bech32 addresses / pubkeys +* [gov] VoteStatus renamed to ProposalStatus +* [gov] VoteOption, ProposalType, and ProposalStatus all marshal to string form in JSON DEPRECATED * [cli] Deprecated `--name` flag in commands that send txs, in favor of `--from` diff --git a/client/lcd/lcd_test.go b/client/lcd/lcd_test.go index b5800ca292..d4ca9d0d38 100644 --- a/client/lcd/lcd_test.go +++ b/client/lcd/lcd_test.go @@ -443,7 +443,7 @@ func TestSubmitProposal(t *testing.T) { // query proposal proposal := getProposal(t, port, proposalID) - require.Equal(t, "Test", proposal.Title) + require.Equal(t, "Test", proposal.GetTitle()) } func TestDeposit(t *testing.T) { @@ -465,7 +465,7 @@ func TestDeposit(t *testing.T) { // query proposal proposal := getProposal(t, port, proposalID) - require.Equal(t, "Test", proposal.Title) + require.Equal(t, "Test", proposal.GetTitle()) // create SubmitProposal TX resultTx = doDeposit(t, port, seed, name, password, addr, proposalID) @@ -473,7 +473,7 @@ func TestDeposit(t *testing.T) { // query proposal proposal = getProposal(t, port, proposalID) - require.True(t, proposal.TotalDeposit.IsEqual(sdk.Coins{sdk.NewCoin("steak", 10)})) + require.True(t, proposal.GetTotalDeposit().IsEqual(sdk.Coins{sdk.NewCoin("steak", 10)})) // query deposit deposit := getDeposit(t, port, proposalID, addr) @@ -499,7 +499,7 @@ func TestVote(t *testing.T) { // query proposal proposal := getProposal(t, port, proposalID) - require.Equal(t, "Test", proposal.Title) + require.Equal(t, "Test", proposal.GetTitle()) // create SubmitProposal TX resultTx = doDeposit(t, port, seed, name, password, addr, proposalID) @@ -507,7 +507,7 @@ func TestVote(t *testing.T) { // query proposal proposal = getProposal(t, port, proposalID) - require.Equal(t, gov.StatusToString(gov.StatusVotingPeriod), proposal.Status) + require.Equal(t, gov.StatusVotingPeriod, proposal.GetStatus()) // create SubmitProposal TX resultTx = doVote(t, port, seed, name, password, addr, proposalID) @@ -515,7 +515,7 @@ func TestVote(t *testing.T) { vote := getVote(t, port, proposalID, addr) require.Equal(t, proposalID, vote.ProposalID) - require.Equal(t, gov.VoteOptionToString(gov.OptionYes), vote.Option) + require.Equal(t, gov.OptionYes, vote.Option) } func TestUnrevoke(t *testing.T) { @@ -576,31 +576,31 @@ func TestProposalsQuery(t *testing.T) { // Test query all proposals proposals := getProposalsAll(t, port) - require.Equal(t, proposalID1, (proposals[0]).ProposalID) - require.Equal(t, proposalID2, (proposals[1]).ProposalID) - require.Equal(t, proposalID3, (proposals[2]).ProposalID) + require.Equal(t, proposalID1, (proposals[0]).GetProposalID()) + require.Equal(t, proposalID2, (proposals[1]).GetProposalID()) + require.Equal(t, proposalID3, (proposals[2]).GetProposalID()) // Test query deposited by addr1 proposals = getProposalsFilterDepositer(t, port, addr) - require.Equal(t, proposalID1, (proposals[0]).ProposalID) + require.Equal(t, proposalID1, (proposals[0]).GetProposalID()) // Test query deposited by addr2 proposals = getProposalsFilterDepositer(t, port, addr2) - require.Equal(t, proposalID2, (proposals[0]).ProposalID) - require.Equal(t, proposalID3, (proposals[1]).ProposalID) + require.Equal(t, proposalID2, (proposals[0]).GetProposalID()) + require.Equal(t, proposalID3, (proposals[1]).GetProposalID()) // Test query voted by addr1 proposals = getProposalsFilterVoter(t, port, addr) - require.Equal(t, proposalID2, (proposals[0]).ProposalID) - require.Equal(t, proposalID3, (proposals[1]).ProposalID) + require.Equal(t, proposalID2, (proposals[0]).GetProposalID()) + require.Equal(t, proposalID3, (proposals[1]).GetProposalID()) // Test query voted by addr2 proposals = getProposalsFilterVoter(t, port, addr2) - require.Equal(t, proposalID3, (proposals[0]).ProposalID) + require.Equal(t, proposalID3, (proposals[0]).GetProposalID()) // Test query voted and deposited by addr1 proposals = getProposalsFilterVoterDepositer(t, port, addr, addr) - require.Equal(t, proposalID2, (proposals[0]).ProposalID) + require.Equal(t, proposalID2, (proposals[0]).GetProposalID()) } //_____________________________________________________________________________ @@ -838,68 +838,68 @@ func getValidators(t *testing.T, port string) []stakerest.StakeValidatorOutput { return validators } -func getProposal(t *testing.T, port string, proposalID int64) gov.ProposalRest { +func getProposal(t *testing.T, port string, proposalID int64) gov.Proposal { res, body := Request(t, port, "GET", fmt.Sprintf("/gov/proposals/%d", proposalID), nil) require.Equal(t, http.StatusOK, res.StatusCode, body) - var proposal gov.ProposalRest + var proposal gov.Proposal err := cdc.UnmarshalJSON([]byte(body), &proposal) require.Nil(t, err) return proposal } -func getDeposit(t *testing.T, port string, proposalID int64, depositerAddr sdk.AccAddress) gov.DepositRest { +func getDeposit(t *testing.T, port string, proposalID int64, depositerAddr sdk.AccAddress) gov.Deposit { res, body := Request(t, port, "GET", fmt.Sprintf("/gov/proposals/%d/deposits/%s", proposalID, depositerAddr), nil) require.Equal(t, http.StatusOK, res.StatusCode, body) - var deposit gov.DepositRest + var deposit gov.Deposit err := cdc.UnmarshalJSON([]byte(body), &deposit) require.Nil(t, err) return deposit } -func getVote(t *testing.T, port string, proposalID int64, voterAddr sdk.AccAddress) gov.VoteRest { +func getVote(t *testing.T, port string, proposalID int64, voterAddr sdk.AccAddress) gov.Vote { res, body := Request(t, port, "GET", fmt.Sprintf("/gov/proposals/%d/votes/%s", proposalID, voterAddr), nil) require.Equal(t, http.StatusOK, res.StatusCode, body) - var vote gov.VoteRest + var vote gov.Vote err := cdc.UnmarshalJSON([]byte(body), &vote) require.Nil(t, err) return vote } -func getProposalsAll(t *testing.T, port string) []gov.ProposalRest { +func getProposalsAll(t *testing.T, port string) []gov.Proposal { res, body := Request(t, port, "GET", "/gov/proposals", nil) require.Equal(t, http.StatusOK, res.StatusCode, body) - var proposals []gov.ProposalRest + var proposals []gov.Proposal err := cdc.UnmarshalJSON([]byte(body), &proposals) require.Nil(t, err) return proposals } -func getProposalsFilterDepositer(t *testing.T, port string, depositerAddr sdk.AccAddress) []gov.ProposalRest { +func getProposalsFilterDepositer(t *testing.T, port string, depositerAddr sdk.AccAddress) []gov.Proposal { res, body := Request(t, port, "GET", fmt.Sprintf("/gov/proposals?depositer=%s", depositerAddr), nil) require.Equal(t, http.StatusOK, res.StatusCode, body) - var proposals []gov.ProposalRest + var proposals []gov.Proposal err := cdc.UnmarshalJSON([]byte(body), &proposals) require.Nil(t, err) return proposals } -func getProposalsFilterVoter(t *testing.T, port string, voterAddr sdk.AccAddress) []gov.ProposalRest { +func getProposalsFilterVoter(t *testing.T, port string, voterAddr sdk.AccAddress) []gov.Proposal { res, body := Request(t, port, "GET", fmt.Sprintf("/gov/proposals?voter=%s", voterAddr), nil) require.Equal(t, http.StatusOK, res.StatusCode, body) - var proposals []gov.ProposalRest + var proposals []gov.Proposal err := cdc.UnmarshalJSON([]byte(body), &proposals) require.Nil(t, err) return proposals } -func getProposalsFilterVoterDepositer(t *testing.T, port string, voterAddr, depositerAddr sdk.AccAddress) []gov.ProposalRest { +func getProposalsFilterVoterDepositer(t *testing.T, port string, voterAddr, depositerAddr sdk.AccAddress) []gov.Proposal { res, body := Request(t, port, "GET", fmt.Sprintf("/gov/proposals?depositer=%s&voter=%s", depositerAddr, voterAddr), nil) require.Equal(t, http.StatusOK, res.StatusCode, body) - var proposals []gov.ProposalRest + var proposals []gov.Proposal err := cdc.UnmarshalJSON([]byte(body), &proposals) require.Nil(t, err) return proposals diff --git a/cmd/gaia/cli_test/cli_test.go b/cmd/gaia/cli_test/cli_test.go index 38a347b241..645a6c276a 100644 --- a/cmd/gaia/cli_test/cli_test.go +++ b/cmd/gaia/cli_test/cli_test.go @@ -183,8 +183,8 @@ func TestGaiaCLISubmitProposal(t *testing.T) { require.Equal(t, int64(45), fooAcc.GetCoins().AmountOf("steak").Int64()) proposal1 := executeGetProposal(t, fmt.Sprintf("gaiacli gov query-proposal --proposalID=1 --output=json %v", flags)) - require.Equal(t, int64(1), proposal1.ProposalID) - require.Equal(t, gov.StatusToString(gov.StatusDepositPeriod), proposal1.Status) + require.Equal(t, int64(1), proposal1.GetProposalID()) + require.Equal(t, gov.StatusDepositPeriod, proposal1.GetStatus()) executeWrite(t, fmt.Sprintf("gaiacli gov deposit %v --depositer=%s --deposit=10steak --proposalID=1 --from=foo", flags, fooAddr), pass) tests.WaitForNextHeightTM(port) @@ -192,15 +192,15 @@ func TestGaiaCLISubmitProposal(t *testing.T) { fooAcc = executeGetAccount(t, fmt.Sprintf("gaiacli account %s %v", fooAddr, flags)) require.Equal(t, int64(35), fooAcc.GetCoins().AmountOf("steak").Int64()) proposal1 = executeGetProposal(t, fmt.Sprintf("gaiacli gov query-proposal --proposalID=1 --output=json %v", flags)) - require.Equal(t, int64(1), proposal1.ProposalID) - require.Equal(t, gov.StatusToString(gov.StatusVotingPeriod), proposal1.Status) + require.Equal(t, int64(1), proposal1.GetProposalID()) + require.Equal(t, gov.StatusVotingPeriod, proposal1.GetStatus()) executeWrite(t, fmt.Sprintf("gaiacli gov vote %v --proposalID=1 --voter=%s --option=Yes --from=foo", flags, fooAddr), pass) tests.WaitForNextHeightTM(port) vote := executeGetVote(t, fmt.Sprintf("gaiacli gov query-vote --proposalID=1 --voter=%s --output=json %v", fooAddr, flags)) require.Equal(t, int64(1), vote.ProposalID) - require.Equal(t, gov.VoteOptionToString(gov.OptionYes), vote.Option) + require.Equal(t, gov.OptionYes, vote.Option) } //___________________________________________________________________________________ @@ -288,18 +288,18 @@ func executeGetValidator(t *testing.T, cmdStr string) stake.Validator { return validator } -func executeGetProposal(t *testing.T, cmdStr string) gov.ProposalRest { +func executeGetProposal(t *testing.T, cmdStr string) gov.Proposal { out := tests.ExecuteT(t, cmdStr) - var proposal gov.ProposalRest + var proposal gov.Proposal cdc := app.MakeCodec() err := cdc.UnmarshalJSON([]byte(out), &proposal) require.NoError(t, err, "out %v\n, err %v", out, err) return proposal } -func executeGetVote(t *testing.T, cmdStr string) gov.VoteRest { +func executeGetVote(t *testing.T, cmdStr string) gov.Vote { out := tests.ExecuteT(t, cmdStr) - var vote gov.VoteRest + var vote gov.Vote cdc := app.MakeCodec() err := cdc.UnmarshalJSON([]byte(out), &vote) require.NoError(t, err, "out %v\n, err %v", out, err) diff --git a/x/gov/client/cli/tx.go b/x/gov/client/cli/tx.go index 1c5c11b71e..8369c99273 100644 --- a/x/gov/client/cli/tx.go +++ b/x/gov/client/cli/tx.go @@ -48,7 +48,7 @@ func GetCmdSubmitProposal(cdc *wire.Codec) *cobra.Command { return err } - proposalType, err := gov.StringToProposalType(strProposalType) + proposalType, err := gov.ProposalTypeFromString(strProposalType) if err != nil { return err } @@ -145,7 +145,7 @@ func GetCmdVote(cdc *wire.Codec) *cobra.Command { option := viper.GetString(flagOption) - byteVoteOption, err := gov.StringToVoteOption(option) + byteVoteOption, err := gov.VoteOptionFromString(option) if err != nil { return err } @@ -158,7 +158,7 @@ func GetCmdVote(cdc *wire.Codec) *cobra.Command { return err } - fmt.Printf("Vote[Voter:%s,ProposalID:%d,Option:%s]", bechVoter, msg.ProposalID, gov.VoteOptionToString(msg.Option)) + fmt.Printf("Vote[Voter:%s,ProposalID:%d,Option:%s]", bechVoter, msg.ProposalID, msg.Option) // build and sign the transaction, then broadcast to Tendermint ctx := context.NewCoreContextFromViper().WithDecoder(authcmd.GetAccountDecoder(cdc)) @@ -195,8 +195,7 @@ func GetCmdQueryProposal(storeName string, cdc *wire.Codec) *cobra.Command { var proposal gov.Proposal cdc.MustUnmarshalBinary(res, &proposal) - proposalRest := gov.ProposalToRest(proposal) - output, err := wire.MarshalJSONIndent(cdc, proposalRest) + output, err := wire.MarshalJSONIndent(cdc, proposal) if err != nil { return err } @@ -232,8 +231,7 @@ func GetCmdQueryVote(storeName string, cdc *wire.Codec) *cobra.Command { var vote gov.Vote cdc.MustUnmarshalBinary(res, &vote) - voteRest := gov.VoteToRest(vote) - output, err := wire.MarshalJSONIndent(cdc, voteRest) + output, err := wire.MarshalJSONIndent(cdc, vote) if err != nil { return err } diff --git a/x/gov/client/rest/rest.go b/x/gov/client/rest/rest.go index 3e121d4d41..1473045871 100644 --- a/x/gov/client/rest/rest.go +++ b/x/gov/client/rest/rest.go @@ -36,24 +36,24 @@ func RegisterRoutes(ctx context.CoreContext, r *mux.Router, cdc *wire.Codec) { } type postProposalReq struct { - BaseReq baseReq `json:"base_req"` - Title string `json:"title"` // Title of the proposal - Description string `json:"description"` // Description of the proposal - ProposalType string `json:"proposal_type"` // Type of proposal. Initial set {PlainTextProposal, SoftwareUpgradeProposal} - Proposer string `json:"proposer"` // Address of the proposer - InitialDeposit sdk.Coins `json:"initial_deposit"` // Coins to add to the proposal's deposit + BaseReq baseReq `json:"base_req"` + Title string `json:"title"` // Title of the proposal + Description string `json:"description"` // Description of the proposal + ProposalType gov.ProposalKind `json:"proposal_type"` // Type of proposal. Initial set {PlainTextProposal, SoftwareUpgradeProposal} + Proposer sdk.AccAddress `json:"proposer"` // Address of the proposer + InitialDeposit sdk.Coins `json:"initial_deposit"` // Coins to add to the proposal's deposit } type depositReq struct { - BaseReq baseReq `json:"base_req"` - Depositer string `json:"depositer"` // Address of the depositer - Amount sdk.Coins `json:"amount"` // Coins to add to the proposal's deposit + BaseReq baseReq `json:"base_req"` + Depositer sdk.AccAddress `json:"depositer"` // Address of the depositer + Amount sdk.Coins `json:"amount"` // Coins to add to the proposal's deposit } type voteReq struct { - BaseReq baseReq `json:"base_req"` - Voter string `json:"voter"` // address of the voter - Option string `json:"option"` // option from OptionSet chosen by the voter + BaseReq baseReq `json:"base_req"` + Voter sdk.AccAddress `json:"voter"` // address of the voter + Option gov.VoteOption `json:"option"` // option from OptionSet chosen by the voter } func postProposalHandlerFn(cdc *wire.Codec, ctx context.CoreContext) http.HandlerFunc { @@ -68,20 +68,8 @@ func postProposalHandlerFn(cdc *wire.Codec, ctx context.CoreContext) http.Handle return } - proposer, err := sdk.AccAddressFromBech32(req.Proposer) - if err != nil { - writeErr(&w, http.StatusBadRequest, err.Error()) - return - } - - proposalTypeByte, err := gov.StringToProposalType(req.ProposalType) - if err != nil { - writeErr(&w, http.StatusBadRequest, err.Error()) - return - } - // create the message - msg := gov.NewMsgSubmitProposal(req.Title, req.Description, proposalTypeByte, proposer, req.InitialDeposit) + msg := gov.NewMsgSubmitProposal(req.Title, req.Description, req.ProposalType, req.Proposer, req.InitialDeposit) err = msg.ValidateBasic() if err != nil { writeErr(&w, http.StatusBadRequest, err.Error()) @@ -117,19 +105,12 @@ func depositHandlerFn(cdc *wire.Codec, ctx context.CoreContext) http.HandlerFunc if err != nil { return } - if !req.BaseReq.baseReqValidate(w) { return } - depositer, err := sdk.AccAddressFromBech32(req.Depositer) - if err != nil { - writeErr(&w, http.StatusBadRequest, err.Error()) - return - } - // create the message - msg := gov.NewMsgDeposit(depositer, proposalID, req.Amount) + msg := gov.NewMsgDeposit(req.Depositer, proposalID, req.Amount) err = msg.ValidateBasic() if err != nil { writeErr(&w, http.StatusBadRequest, err.Error()) @@ -165,25 +146,12 @@ func voteHandlerFn(cdc *wire.Codec, ctx context.CoreContext) http.HandlerFunc { if err != nil { return } - if !req.BaseReq.baseReqValidate(w) { return } - voter, err := sdk.AccAddressFromBech32(req.Voter) - if err != nil { - writeErr(&w, http.StatusBadRequest, err.Error()) - return - } - - voteOptionByte, err := gov.StringToVoteOption(req.Option) - if err != nil { - writeErr(&w, http.StatusBadRequest, err.Error()) - return - } - // create the message - msg := gov.NewMsgVote(voter, proposalID, voteOptionByte) + msg := gov.NewMsgVote(req.Voter, proposalID, req.Option) err = msg.ValidateBasic() if err != nil { writeErr(&w, http.StatusBadRequest, err.Error()) @@ -225,8 +193,7 @@ func queryProposalHandlerFn(cdc *wire.Codec) http.HandlerFunc { var proposal gov.Proposal cdc.MustUnmarshalBinary(res, &proposal) - proposalRest := gov.ProposalToRest(proposal) - output, err := wire.MarshalJSONIndent(cdc, proposalRest) + output, err := wire.MarshalJSONIndent(cdc, proposal) if err != nil { w.WriteHeader(http.StatusBadRequest) w.Write([]byte(err.Error())) @@ -291,8 +258,7 @@ func queryDepositHandlerFn(cdc *wire.Codec) http.HandlerFunc { var deposit gov.Deposit cdc.MustUnmarshalBinary(res, &deposit) - depositRest := gov.DepositToRest(deposit) - output, err := wire.MarshalJSONIndent(cdc, depositRest) + output, err := wire.MarshalJSONIndent(cdc, deposit) if err != nil { w.WriteHeader(http.StatusBadRequest) w.Write([]byte(err.Error())) @@ -358,8 +324,7 @@ func queryVoteHandlerFn(cdc *wire.Codec) http.HandlerFunc { var vote gov.Vote cdc.MustUnmarshalBinary(res, &vote) - voteRest := gov.VoteToRest(vote) - output, err := wire.MarshalJSONIndent(cdc, voteRest) + output, err := wire.MarshalJSONIndent(cdc, vote) if err != nil { w.WriteHeader(http.StatusBadRequest) w.Write([]byte(err.Error())) @@ -411,7 +376,7 @@ func queryProposalsWithParameterFn(cdc *wire.Codec) http.HandlerFunc { var maxProposalID int64 cdc.MustUnmarshalBinary(res, &maxProposalID) - matchingProposals := []gov.ProposalRest{} + matchingProposals := []gov.Proposal{} for proposalID := int64(0); proposalID < maxProposalID; proposalID++ { if voterAddr != nil { @@ -435,7 +400,7 @@ func queryProposalsWithParameterFn(cdc *wire.Codec) http.HandlerFunc { var proposal gov.Proposal cdc.MustUnmarshalBinary(res, &proposal) - matchingProposals = append(matchingProposals, gov.ProposalToRest(proposal)) + matchingProposals = append(matchingProposals, proposal) } output, err := wire.MarshalJSONIndent(cdc, matchingProposals) diff --git a/x/gov/depositsvotes.go b/x/gov/depositsvotes.go index 5ec5651897..19ed97f69d 100644 --- a/x/gov/depositsvotes.go +++ b/x/gov/depositsvotes.go @@ -1,19 +1,11 @@ package gov import ( + "encoding/json" + "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" -) - -// Type that represents VoteOption as a byte -type VoteOption = byte - -//nolint -const ( - OptionEmpty VoteOption = 0x00 - OptionYes VoteOption = 0x01 - OptionAbstain VoteOption = 0x02 - OptionNo VoteOption = 0x03 - OptionNoWithVeto VoteOption = 0x04 + "github.com/pkg/errors" ) // Vote @@ -30,9 +22,80 @@ type Deposit struct { Amount sdk.Coins `json:"amount"` // Deposit amount } -// ProposalTypeToString for pretty prints of ProposalType -func VoteOptionToString(option VoteOption) string { - switch option { +// Type that represents VoteOption as a byte +type VoteOption byte + +//nolint +const ( + OptionEmpty VoteOption = 0x00 + OptionYes VoteOption = 0x01 + OptionAbstain VoteOption = 0x02 + OptionNo VoteOption = 0x03 + OptionNoWithVeto VoteOption = 0x04 +) + +// String to proposalType byte. Returns ff if invalid. +func VoteOptionFromString(str string) (VoteOption, error) { + switch str { + case "Yes": + return OptionYes, nil + case "Abstain": + return OptionAbstain, nil + case "No": + return OptionNo, nil + case "NoWithVeto": + return OptionNoWithVeto, nil + default: + return VoteOption(0xff), errors.Errorf("'%s' is not a valid vote option", str) + } +} + +// Is defined VoteOption +func validVoteOption(option VoteOption) bool { + if option == OptionYes || + option == OptionAbstain || + option == OptionNo || + option == OptionNoWithVeto { + return true + } + return false +} + +// Marshal needed for protobuf compatibility +func (vo VoteOption) Marshal() ([]byte, error) { + return []byte{byte(vo)}, nil +} + +// Unmarshal needed for protobuf compatibility +func (vo *VoteOption) Unmarshal(data []byte) error { + *vo = VoteOption(data[0]) + return nil +} + +// Marshals to JSON using string +func (vo VoteOption) MarshalJSON() ([]byte, error) { + return json.Marshal(vo.String()) +} + +// Unmarshals from JSON assuming Bech32 encoding +func (vo *VoteOption) UnmarshalJSON(data []byte) error { + var s string + err := json.Unmarshal(data, &s) + if err != nil { + return nil + } + + bz2, err := VoteOptionFromString(s) + if err != nil { + return err + } + *vo = bz2 + return nil +} + +// Turns VoteOption byte to String +func (vo VoteOption) String() string { + switch vo { case OptionYes: return "Yes" case OptionAbstain: @@ -46,63 +109,12 @@ func VoteOptionToString(option VoteOption) string { } } -func validVoteOption(option VoteOption) bool { - if option == OptionYes || - option == OptionAbstain || - option == OptionNo || - option == OptionNoWithVeto { - return true - } - return false -} - -// String to proposalType byte. Returns ff if invalid. -func StringToVoteOption(str string) (VoteOption, sdk.Error) { - switch str { - case "Yes": - return OptionYes, nil - case "Abstain": - return OptionAbstain, nil - case "No": - return OptionNo, nil - case "NoWithVeto": - return OptionNoWithVeto, nil +// For Printf / Sprintf, returns bech32 when using %s +func (vo VoteOption) Format(s fmt.State, verb rune) { + switch verb { + case 's': + s.Write([]byte(fmt.Sprintf("%s", vo.String()))) default: - return VoteOption(0xff), ErrInvalidVote(DefaultCodespace, str) - } -} - -//----------------------------------------------------------- -// REST - -// Rest Deposits -type DepositRest struct { - Depositer sdk.AccAddress `json:"depositer"` // address of the depositer - ProposalID int64 `json:"proposal_id"` // proposalID of the proposal - Amount sdk.Coins `json:"option"` -} - -// Turn any Deposit to a DepositRest -func DepositToRest(deposit Deposit) DepositRest { - return DepositRest{ - Depositer: deposit.Depositer, - ProposalID: deposit.ProposalID, - Amount: deposit.Amount, - } -} - -// Rest Votes -type VoteRest struct { - Voter sdk.AccAddress `json:"voter"` // address of the voter - ProposalID int64 `json:"proposal_id"` // proposalID of the proposal - Option string `json:"option"` -} - -// Turn any Vote to a VoteRest -func VoteToRest(vote Vote) VoteRest { - return VoteRest{ - Voter: vote.Voter, - ProposalID: vote.ProposalID, - Option: VoteOptionToString(vote.Option), + s.Write([]byte(fmt.Sprintf("%v", byte(vo)))) } } diff --git a/x/gov/errors.go b/x/gov/errors.go index e1f2f27937..0825e00f8c 100644 --- a/x/gov/errors.go +++ b/x/gov/errors.go @@ -20,6 +20,7 @@ const ( CodeInvalidProposalType sdk.CodeType = 8 CodeInvalidVote sdk.CodeType = 9 CodeInvalidGenesis sdk.CodeType = 10 + CodeInvalidProposalStatus sdk.CodeType = 11 ) //---------------------------------------- @@ -53,12 +54,12 @@ func ErrInvalidDescription(codespace sdk.CodespaceType, description string) sdk. return sdk.NewError(codespace, CodeInvalidDescription, fmt.Sprintf("Proposal Desciption '%s' is not valid", description)) } -func ErrInvalidProposalType(codespace sdk.CodespaceType, strProposalType string) sdk.Error { - return sdk.NewError(codespace, CodeInvalidProposalType, fmt.Sprintf("Proposal Type '%s' is not valid", strProposalType)) +func ErrInvalidProposalType(codespace sdk.CodespaceType, proposalType ProposalKind) sdk.Error { + return sdk.NewError(codespace, CodeInvalidProposalType, fmt.Sprintf("Proposal Type '%s' is not valid", proposalType)) } -func ErrInvalidVote(codespace sdk.CodespaceType, strOption string) sdk.Error { - return sdk.NewError(codespace, CodeInvalidVote, fmt.Sprintf("'%s' is not a valid voting option", strOption)) +func ErrInvalidVote(codespace sdk.CodespaceType, voteOption VoteOption) sdk.Error { + return sdk.NewError(codespace, CodeInvalidVote, fmt.Sprintf("'%v' is not a valid voting option", voteOption)) } func ErrInvalidGenesis(codespace sdk.CodespaceType, msg string) sdk.Error { diff --git a/x/gov/keeper.go b/x/gov/keeper.go index d966a78a2f..b60404b8ce 100644 --- a/x/gov/keeper.go +++ b/x/gov/keeper.go @@ -48,7 +48,7 @@ func (keeper Keeper) WireCodec() *wire.Codec { // Proposals // Creates a NewProposal -func (keeper Keeper) NewTextProposal(ctx sdk.Context, title string, description string, proposalType byte) Proposal { +func (keeper Keeper) NewTextProposal(ctx sdk.Context, title string, description string, proposalType ProposalKind) Proposal { proposalID, err := keeper.getNewProposalID(ctx) if err != nil { return nil @@ -165,8 +165,8 @@ func (keeper Keeper) AddVote(ctx sdk.Context, proposalID int64, voterAddr sdk.Ac return ErrInactiveProposal(keeper.codespace, proposalID) } - if option != OptionYes && option != OptionAbstain && option != OptionNo && option != OptionNoWithVeto { - return ErrInvalidVote(keeper.codespace, VoteOptionToString(option)) + if !validVoteOption(option) { + return ErrInvalidVote(keeper.codespace, option) } vote := Vote{ diff --git a/x/gov/msgs.go b/x/gov/msgs.go index 5b38861eef..5d85f689e5 100644 --- a/x/gov/msgs.go +++ b/x/gov/msgs.go @@ -41,7 +41,7 @@ func (msg MsgSubmitProposal) ValidateBasic() sdk.Error { return ErrInvalidDescription(DefaultCodespace, msg.Description) // TODO: Proper Error } if !validProposalType(msg.ProposalType) { - return ErrInvalidProposalType(DefaultCodespace, ProposalTypeToString(msg.ProposalType)) + return ErrInvalidProposalType(DefaultCodespace, msg.ProposalType) } if len(msg.Proposer) == 0 { return sdk.ErrInvalidAddress(msg.Proposer.String()) @@ -56,7 +56,7 @@ func (msg MsgSubmitProposal) ValidateBasic() sdk.Error { } func (msg MsgSubmitProposal) String() string { - return fmt.Sprintf("MsgSubmitProposal{%v, %v, %v, %v}", msg.Title, msg.Description, ProposalTypeToString(msg.ProposalType), msg.InitialDeposit) + return fmt.Sprintf("MsgSubmitProposal{%s, %s, %s, %v}", msg.Title, msg.Description, msg.ProposalType, msg.InitialDeposit) } // Implements Msg. @@ -66,19 +66,7 @@ func (msg MsgSubmitProposal) Get(key interface{}) (value interface{}) { // Implements Msg. func (msg MsgSubmitProposal) GetSignBytes() []byte { - b, err := msgCdc.MarshalJSON(struct { - Title string `json:"title"` - Description string `json:"description"` - ProposalType string `json:"proposal_type"` - Proposer sdk.AccAddress `json:"proposer"` - InitialDeposit sdk.Coins `json:"deposit"` - }{ - Title: msg.Title, - Description: msg.Description, - ProposalType: ProposalTypeToString(msg.ProposalType), - Proposer: msg.Proposer, - InitialDeposit: msg.InitialDeposit, - }) + b, err := msgCdc.MarshalJSON(msg) if err != nil { panic(err) } @@ -127,7 +115,7 @@ func (msg MsgDeposit) ValidateBasic() sdk.Error { } func (msg MsgDeposit) String() string { - return fmt.Sprintf("MsgDeposit{%v=>%v: %v}", msg.Depositer, msg.ProposalID, msg.Amount) + return fmt.Sprintf("MsgDeposit{%s=>%v: %v}", msg.Depositer, msg.ProposalID, msg.Amount) } // Implements Msg. @@ -137,15 +125,7 @@ func (msg MsgDeposit) Get(key interface{}) (value interface{}) { // Implements Msg. func (msg MsgDeposit) GetSignBytes() []byte { - b, err := msgCdc.MarshalJSON(struct { - ProposalID int64 `json:"proposalID"` - Depositer sdk.AccAddress `json:"proposer"` - Amount sdk.Coins `json:"deposit"` - }{ - ProposalID: msg.ProposalID, - Depositer: msg.Depositer, - Amount: msg.Amount, - }) + b, err := msgCdc.MarshalJSON(msg) if err != nil { panic(err) } @@ -185,13 +165,13 @@ func (msg MsgVote) ValidateBasic() sdk.Error { return ErrUnknownProposal(DefaultCodespace, msg.ProposalID) } if !validVoteOption(msg.Option) { - return ErrInvalidVote(DefaultCodespace, VoteOptionToString(msg.Option)) + return ErrInvalidVote(DefaultCodespace, msg.Option) } return nil } func (msg MsgVote) String() string { - return fmt.Sprintf("MsgVote{%v - %v}", msg.ProposalID, msg.Option) + return fmt.Sprintf("MsgVote{%v - %s}", msg.ProposalID, msg.Option) } // Implements Msg. @@ -201,15 +181,7 @@ func (msg MsgVote) Get(key interface{}) (value interface{}) { // Implements Msg. func (msg MsgVote) GetSignBytes() []byte { - b, err := msgCdc.MarshalJSON(struct { - ProposalID int64 `json:"proposalID"` - Voter sdk.AccAddress `json:"voter"` - Option string `json:"option"` - }{ - ProposalID: msg.ProposalID, - Voter: msg.Voter, - Option: VoteOptionToString(msg.Option), - }) + b, err := msgCdc.MarshalJSON(msg) if err != nil { panic(err) } diff --git a/x/gov/msgs_test.go b/x/gov/msgs_test.go index fd526c7fa7..186b8365ba 100644 --- a/x/gov/msgs_test.go +++ b/x/gov/msgs_test.go @@ -22,7 +22,7 @@ func TestMsgSubmitProposal(t *testing.T) { _, addrs, _, _ := mock.CreateGenAccounts(1, sdk.Coins{}) tests := []struct { title, description string - proposalType byte + proposalType ProposalKind proposerAddr sdk.AccAddress initialDeposit sdk.Coins expectPass bool diff --git a/x/gov/proposals.go b/x/gov/proposals.go index c81d61e21d..64ffe6bfa2 100644 --- a/x/gov/proposals.go +++ b/x/gov/proposals.go @@ -1,27 +1,14 @@ package gov import ( + "encoding/json" + "fmt" + + "github.com/pkg/errors" + sdk "github.com/cosmos/cosmos-sdk/types" ) -// Type that represents Status as a byte -type VoteStatus = byte - -// Type that represents Proposal Type as a byte -type ProposalKind = byte - -//nolint -const ( - StatusDepositPeriod VoteStatus = 0x01 - StatusVotingPeriod VoteStatus = 0x02 - StatusPassed VoteStatus = 0x03 - StatusRejected VoteStatus = 0x04 - - ProposalTypeText ProposalKind = 0x01 - ProposalTypeParameterChange ProposalKind = 0x02 - ProposalTypeSoftwareUpgrade ProposalKind = 0x03 -) - //----------------------------------------------------------- // Proposal interface type Proposal interface { @@ -37,8 +24,8 @@ type Proposal interface { GetProposalType() ProposalKind SetProposalType(ProposalKind) - GetStatus() VoteStatus - SetStatus(VoteStatus) + GetStatus() ProposalStatus + SetStatus(ProposalStatus) GetSubmitBlock() int64 SetSubmitBlock(int64) @@ -73,7 +60,7 @@ type TextProposal struct { Description string `json:"description"` // Description of the proposal ProposalType ProposalKind `json:"proposal_type"` // Type of proposal. Initial set {PlainTextProposal, SoftwareUpgradeProposal} - Status VoteStatus `json:"string"` // Status of the Proposal {Pending, Active, Passed, Rejected} + Status ProposalStatus `json:"string"` // Status of the Proposal {Pending, Active, Passed, Rejected} SubmitBlock int64 `json:"submit_block"` // Height of the block where TxGovSubmitProposal was included TotalDeposit sdk.Coins `json:"total_deposit"` // Current deposit on this proposal. Initial value is set at InitialDeposit @@ -93,8 +80,8 @@ func (tp TextProposal) GetDescription() string { return tp.D func (tp *TextProposal) SetDescription(description string) { tp.Description = description } func (tp TextProposal) GetProposalType() ProposalKind { return tp.ProposalType } func (tp *TextProposal) SetProposalType(proposalType ProposalKind) { tp.ProposalType = proposalType } -func (tp TextProposal) GetStatus() VoteStatus { return tp.Status } -func (tp *TextProposal) SetStatus(status VoteStatus) { tp.Status = status } +func (tp TextProposal) GetStatus() ProposalStatus { return tp.Status } +func (tp *TextProposal) SetStatus(status ProposalStatus) { tp.Status = status } func (tp TextProposal) GetSubmitBlock() int64 { return tp.SubmitBlock } func (tp *TextProposal) SetSubmitBlock(submitBlock int64) { tp.SubmitBlock = submitBlock } func (tp TextProposal) GetTotalDeposit() sdk.Coins { return tp.TotalDeposit } @@ -104,12 +91,82 @@ func (tp *TextProposal) SetVotingStartBlock(votingStartBlock int64) { tp.VotingStartBlock = votingStartBlock } -// Current Active Proposals +//----------------------------------------------------------- +// ProposalQueue type ProposalQueue []int64 -// ProposalTypeToString for pretty prints of ProposalType -func ProposalTypeToString(proposalType ProposalKind) string { - switch proposalType { +//----------------------------------------------------------- +// ProposalKind + +// Type that represents Proposal Type as a byte +type ProposalKind byte + +//nolint +const ( + ProposalTypeText ProposalKind = 0x01 + ProposalTypeParameterChange ProposalKind = 0x02 + ProposalTypeSoftwareUpgrade ProposalKind = 0x03 +) + +// String to proposalType byte. Returns ff if invalid. +func ProposalTypeFromString(str string) (ProposalKind, error) { + switch str { + case "Text": + return ProposalTypeText, nil + case "ParameterChange": + return ProposalTypeParameterChange, nil + case "SoftwareUpgrade": + return ProposalTypeSoftwareUpgrade, nil + default: + return ProposalKind(0xff), errors.Errorf("'%s' is not a valid proposal type", str) + } +} + +// is defined ProposalType? +func validProposalType(pt ProposalKind) bool { + if pt == ProposalTypeText || + pt == ProposalTypeParameterChange || + pt == ProposalTypeSoftwareUpgrade { + return true + } + return false +} + +// Marshal needed for protobuf compatibility +func (pt ProposalKind) Marshal() ([]byte, error) { + return []byte{byte(pt)}, nil +} + +// Unmarshal needed for protobuf compatibility +func (pt *ProposalKind) Unmarshal(data []byte) error { + *pt = ProposalKind(data[0]) + return nil +} + +// Marshals to JSON using string +func (pt ProposalKind) MarshalJSON() ([]byte, error) { + return json.Marshal(pt.String()) +} + +// Unmarshals from JSON assuming Bech32 encoding +func (pt *ProposalKind) UnmarshalJSON(data []byte) error { + var s string + err := json.Unmarshal(data, &s) + if err != nil { + return nil + } + + bz2, err := ProposalTypeFromString(s) + if err != nil { + return err + } + *pt = bz2 + return nil +} + +// Turns VoteOption byte to String +func (pt ProposalKind) String() string { + switch pt { case 0x00: return "Text" case 0x01: @@ -121,31 +178,91 @@ func ProposalTypeToString(proposalType ProposalKind) string { } } -func validProposalType(proposalType ProposalKind) bool { - if proposalType == ProposalTypeText || - proposalType == ProposalTypeParameterChange || - proposalType == ProposalTypeSoftwareUpgrade { +// For Printf / Sprintf, returns bech32 when using %s +func (pt ProposalKind) Format(s fmt.State, verb rune) { + switch verb { + case 's': + s.Write([]byte(fmt.Sprintf("%s", pt.String()))) + default: + s.Write([]byte(fmt.Sprintf("%v", pt))) + } +} + +//----------------------------------------------------------- +// ProposalStatus + +// Type that represents Proposal Status as a byte +type ProposalStatus byte + +//nolint +const ( + StatusDepositPeriod ProposalStatus = 0x01 + StatusVotingPeriod ProposalStatus = 0x02 + StatusPassed ProposalStatus = 0x03 + StatusRejected ProposalStatus = 0x04 +) + +// ProposalStatusToString turns a string into a ProposalStatus +func ProposalStatusFromString(str string) (ProposalStatus, error) { + switch str { + case "DepositPeriod": + return StatusDepositPeriod, nil + case "VotingPeriod": + return StatusVotingPeriod, nil + case "Passed": + return StatusPassed, nil + case "Rejected": + return StatusRejected, nil + default: + return ProposalStatus(0xff), errors.Errorf("'%s' is not a valid proposal status", str) + } +} + +// is defined ProposalType? +func validProposalStatus(status ProposalStatus) bool { + if status == StatusDepositPeriod || + status == StatusVotingPeriod || + status == StatusPassed || + status == StatusRejected { return true } return false } -// String to proposalType byte. Returns ff if invalid. -func StringToProposalType(str string) (ProposalKind, sdk.Error) { - switch str { - case "Text": - return ProposalTypeText, nil - case "ParameterChange": - return ProposalTypeParameterChange, nil - case "SoftwareUpgrade": - return ProposalTypeSoftwareUpgrade, nil - default: - return ProposalKind(0xff), ErrInvalidProposalType(DefaultCodespace, str) - } +// Marshal needed for protobuf compatibility +func (status ProposalStatus) Marshal() ([]byte, error) { + return []byte{byte(status)}, nil } -// StatusToString for pretty prints of Status -func StatusToString(status VoteStatus) string { +// Unmarshal needed for protobuf compatibility +func (status *ProposalStatus) Unmarshal(data []byte) error { + *status = ProposalStatus(data[0]) + return nil +} + +// Marshals to JSON using string +func (status ProposalStatus) MarshalJSON() ([]byte, error) { + return json.Marshal(status.String()) +} + +// Unmarshals from JSON assuming Bech32 encoding +func (status *ProposalStatus) UnmarshalJSON(data []byte) error { + var s string + err := json.Unmarshal(data, &s) + if err != nil { + return nil + } + + bz2, err := ProposalStatusFromString(s) + if err != nil { + return err + } + *status = bz2 + return nil +} + +// Turns VoteStatus byte to String +func (status ProposalStatus) String() string { switch status { case StatusDepositPeriod: return "DepositPeriod" @@ -160,45 +277,12 @@ func StatusToString(status VoteStatus) string { } } -// StatusToString for pretty prints of Status -func StringToStatus(status string) VoteStatus { - switch status { - case "DepositPeriod": - return StatusDepositPeriod - case "VotingPeriod": - return StatusVotingPeriod - case "Passed": - return StatusPassed - case "Rejected": - return StatusRejected +// For Printf / Sprintf, returns bech32 when using %s +func (status ProposalStatus) Format(s fmt.State, verb rune) { + switch verb { + case 's': + s.Write([]byte(fmt.Sprintf("%s", status.String()))) default: - return VoteStatus(0xff) - } -} - -//----------------------------------------------------------- -// Rest Proposals -type ProposalRest struct { - ProposalID int64 `json:"proposal_id"` // ID of the proposal - Title string `json:"title"` // Title of the proposal - Description string `json:"description"` // Description of the proposal - ProposalType string `json:"proposal_type"` // Type of proposal. Initial set {PlainTextProposal, SoftwareUpgradeProposal} - Status string `json:"string"` // Status of the Proposal {Pending, Active, Passed, Rejected} - SubmitBlock int64 `json:"submit_block"` // Height of the block where TxGovSubmitProposal was included - TotalDeposit sdk.Coins `json:"total_deposit"` // Current deposit on this proposal. Initial value is set at InitialDeposit - VotingStartBlock int64 `json:"voting_start_block"` // Height of the block where MinDeposit was reached. -1 if MinDeposit is not reached -} - -// Turn any Proposal to a ProposalRest -func ProposalToRest(proposal Proposal) ProposalRest { - return ProposalRest{ - ProposalID: proposal.GetProposalID(), - Title: proposal.GetTitle(), - Description: proposal.GetDescription(), - ProposalType: ProposalTypeToString(proposal.GetProposalType()), - Status: StatusToString(proposal.GetStatus()), - SubmitBlock: proposal.GetSubmitBlock(), - TotalDeposit: proposal.GetTotalDeposit(), - VotingStartBlock: proposal.GetVotingStartBlock(), + s.Write([]byte(fmt.Sprintf("%v", status))) } } From fc4c563e292f7bbdd8ceaf2c2cc549503e0b2ee2 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Tue, 10 Jul 2018 19:56:09 -0700 Subject: [PATCH 08/13] keys: Keybase.Update no longer asks for newpass if oldpass is incorrect Achieved by refactoring the parameter newpass as follows: * (newpass string) -> (getNewpass func() (string, error)) Closes #1629 --- CHANGELOG.md | 8 ++++++++ client/keys/update.go | 24 +++++++++++++----------- crypto/keys/keybase.go | 9 +++++++-- crypto/keys/keybase_test.go | 10 ++++++---- crypto/keys/types.go | 2 +- 5 files changed, 35 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44fa7290ef..bfb5523d52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## Pending + +BREAKING CHANGES +- [keys] Keybase.Update function now takes in a function to get the newpass, rather than the password itself + +BUG FIXES +- [keys] \#1629 - updating password no longer asks for a new password when the first entered password was incorrect + ## 0.20.0 *July 10th, 2018* diff --git a/client/keys/update.go b/client/keys/update.go index 0308b89bdb..78a81bf0e6 100644 --- a/client/keys/update.go +++ b/client/keys/update.go @@ -26,23 +26,23 @@ func runUpdateCmd(cmd *cobra.Command, args []string) error { name := args[0] buf := client.BufferStdin() + kb, err := GetKeyBase() + if err != nil { + return err + } oldpass, err := client.GetPassword( "Enter the current passphrase:", buf) if err != nil { return err } - newpass, err := client.GetCheckPassword( - "Enter the new passphrase:", - "Repeat the new passphrase:", buf) - if err != nil { - return err + + getNewpass := func() (string, error) { + return client.GetCheckPassword( + "Enter the new passphrase:", + "Repeat the new passphrase:", buf) } - kb, err := GetKeyBase() - if err != nil { - return err - } - err = kb.Update(name, oldpass, newpass) + err = kb.Update(name, oldpass, getNewpass) if err != nil { return err } @@ -81,8 +81,10 @@ func UpdateKeyRequestHandler(w http.ResponseWriter, r *http.Request) { return } + getNewpass := func() (string, error) { return m.NewPassword, nil } + // TODO check if account exists and if password is correct - err = kb.Update(name, m.OldPassword, m.NewPassword) + err = kb.Update(name, m.OldPassword, getNewpass) if err != nil { w.WriteHeader(401) w.Write([]byte(err.Error())) diff --git a/crypto/keys/keybase.go b/crypto/keys/keybase.go index 7cf43eea67..80de47d3df 100644 --- a/crypto/keys/keybase.go +++ b/crypto/keys/keybase.go @@ -330,8 +330,9 @@ func (kb dbKeybase) Delete(name, passphrase string) error { // encrypted. // // oldpass must be the current passphrase used for encryption, -// newpass will be the only valid passphrase from this time forward. -func (kb dbKeybase) Update(name, oldpass, newpass string) error { +// getNewpass is a function to get the passphrase to permanently replace +// the current passphrase +func (kb dbKeybase) Update(name, oldpass string, getNewpass func() (string, error)) error { info, err := kb.Get(name) if err != nil { return err @@ -343,6 +344,10 @@ func (kb dbKeybase) Update(name, oldpass, newpass string) error { if err != nil { return err } + newpass, err := getNewpass() + if err != nil { + return err + } kb.writeLocalKey(key, name, newpass) return nil default: diff --git a/crypto/keys/keybase_test.go b/crypto/keys/keybase_test.go index c910849865..1ca4317922 100644 --- a/crypto/keys/keybase_test.go +++ b/crypto/keys/keybase_test.go @@ -167,9 +167,10 @@ func TestSignVerify(t *testing.T) { } func assertPassword(t *testing.T, cstore Keybase, name, pass, badpass string) { - err := cstore.Update(name, badpass, pass) + getNewpass := func() (string, error) { return pass, nil } + err := cstore.Update(name, badpass, getNewpass) require.NotNil(t, err) - err = cstore.Update(name, pass, pass) + err = cstore.Update(name, pass, getNewpass) require.Nil(t, err, "%+v", err) } @@ -265,12 +266,13 @@ func TestAdvancedKeyManagement(t *testing.T) { assertPassword(t, cstore, n1, p1, p2) // update password requires the existing password - err = cstore.Update(n1, "jkkgkg", p2) + getNewpass := func() (string, error) { return p2, nil } + err = cstore.Update(n1, "jkkgkg", getNewpass) require.NotNil(t, err) assertPassword(t, cstore, n1, p1, p2) // then it changes the password when correct - err = cstore.Update(n1, p1, p2) + err = cstore.Update(n1, p1, getNewpass) require.NoError(t, err) // p2 is now the proper one! assertPassword(t, cstore, n1, p2, p1) diff --git a/crypto/keys/types.go b/crypto/keys/types.go index 0e77725c31..74cedf9199 100644 --- a/crypto/keys/types.go +++ b/crypto/keys/types.go @@ -34,7 +34,7 @@ type Keybase interface { CreateOffline(name string, pubkey crypto.PubKey) (info Info, err error) // The following operations will *only* work on locally-stored keys - Update(name, oldpass, newpass string) error + Update(name, oldpass string, getNewpass func() (string, error)) error Import(name string, armor string) (err error) ImportPubKey(name string, armor string) (err error) Export(name string) (armor string, err error) From e400e83c43d6ec0b1f938d0fd8218bdba380e0c9 Mon Sep 17 00:00:00 2001 From: Fabian Weber Date: Thu, 12 Jul 2018 15:28:28 +0200 Subject: [PATCH 09/13] fix key import creating random account --- client/keys/add.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/client/keys/add.go b/client/keys/add.go index 1bd13dd0a2..d462db1c06 100644 --- a/client/keys/add.go +++ b/client/keys/add.go @@ -161,6 +161,7 @@ func printCreate(info keys.Info, seed string) { type NewKeyBody struct { Name string `json:"name"` Password string `json:"password"` + Seed string `json:"seed"` } // add new key REST handler @@ -205,7 +206,11 @@ func AddNewKeyRequestHandler(w http.ResponseWriter, r *http.Request) { } // create account - info, mnemonic, err := kb.CreateMnemonic(m.Name, keys.English, m.Password, keys.Secp256k1) + seed := m.Seed + if seed == "" { + seed = getSeed(keys.Secp256k1) + } + info, err := kb.CreateKey(m.Name, seed, m.Password) if err != nil { w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(err.Error())) @@ -219,7 +224,7 @@ func AddNewKeyRequestHandler(w http.ResponseWriter, r *http.Request) { return } - keyOutput.Seed = mnemonic + keyOutput.Seed = seed bz, err := json.Marshal(keyOutput) if err != nil { From f8f46b2b8c670d53649a5c160b992d0aa27e6c69 Mon Sep 17 00:00:00 2001 From: Fabian Weber Date: Thu, 12 Jul 2018 16:19:20 +0200 Subject: [PATCH 10/13] added test for account importing --- client/lcd/lcd_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/client/lcd/lcd_test.go b/client/lcd/lcd_test.go index 48c1529c57..8d1a030a05 100644 --- a/client/lcd/lcd_test.go +++ b/client/lcd/lcd_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/spf13/viper" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" cryptoKeys "github.com/cosmos/cosmos-sdk/crypto/keys" @@ -52,7 +53,7 @@ func TestKeys(t *testing.T) { newPassword := "0987654321" // add key - jsonStr := []byte(fmt.Sprintf(`{"name":"%s", "password":"%s"}`, newName, newPassword)) + jsonStr := []byte(fmt.Sprintf(`{"name":"%s", "password":"%s", "seed":"%s"}`, newName, newPassword, seed)) res, body = Request(t, port, "POST", "/keys", jsonStr) require.Equal(t, http.StatusOK, res.StatusCode, body) @@ -64,6 +65,11 @@ func TestKeys(t *testing.T) { _, err = sdk.AccAddressFromBech32(addr2Bech32) require.NoError(t, err, "Failed to return a correct bech32 address") + // test if created account is the correct account + expectedInfo, _ := GetKB(t).CreateKey(newName, seed, newPassword) + expectedAccount := sdk.AccAddress(expectedInfo.GetPubKey().Address().Bytes()) + assert.Equal(t, expectedAccount.String(), addr2Bech32) + // existing keys res, body = Request(t, port, "GET", "/keys", nil) require.Equal(t, http.StatusOK, res.StatusCode, body) From c3bbb37df3e37c8c3903a00a9587f98f18af9902 Mon Sep 17 00:00:00 2001 From: Fabian Weber Date: Thu, 12 Jul 2018 16:22:26 +0200 Subject: [PATCH 11/13] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4adaecaa78..6396fe5861 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -151,6 +151,7 @@ BUG FIXES * \#1287 - prevent zero power validators at genesis * [x/stake] fix bug when unbonding/redelegating using `--shares-percent` * \#1010 - two validators can't bond with the same pubkey anymore +* [lcd] importing an account would create a random account ## 0.19.0 From 8b9f5396de2831347dfcfea932e60e84286613d6 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Thu, 12 Jul 2018 19:45:09 +0200 Subject: [PATCH 12/13] Move changelog entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6396fe5861..7e57b8a324 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ FEATURES BUG FIXES * [keys] \#1629 - updating password no longer asks for a new password when the first entered password was incorrect +* [lcd] importing an account would create a random account ## 0.20.0 @@ -151,7 +152,6 @@ BUG FIXES * \#1287 - prevent zero power validators at genesis * [x/stake] fix bug when unbonding/redelegating using `--shares-percent` * \#1010 - two validators can't bond with the same pubkey anymore -* [lcd] importing an account would create a random account ## 0.19.0 From 41dd01906ae46aa20ecdbfbebbcf8f76c4f87cf4 Mon Sep 17 00:00:00 2001 From: Jeremiah Andrews Date: Thu, 12 Jul 2018 13:03:18 -0700 Subject: [PATCH 13/13] Merge PR #1654: Wait for 2 blocks attempt --- cmd/gaia/cli_test/cli_test.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/cmd/gaia/cli_test/cli_test.go b/cmd/gaia/cli_test/cli_test.go index 645a6c276a..e37e4d5219 100644 --- a/cmd/gaia/cli_test/cli_test.go +++ b/cmd/gaia/cli_test/cli_test.go @@ -49,7 +49,7 @@ func TestGaiaCLISend(t *testing.T) { defer proc.Stop(false) tests.WaitForTMStart(port) - tests.WaitForNextHeightTM(port) + tests.WaitForNextNBlocksTM(2, port) fooAddr, _ := executeGetAddrPK(t, fmt.Sprintf("gaiacli keys show foo --output=json --home=%s", gaiacliHome)) barAddr, _ := executeGetAddrPK(t, fmt.Sprintf("gaiacli keys show bar --output=json --home=%s", gaiacliHome)) @@ -58,7 +58,7 @@ func TestGaiaCLISend(t *testing.T) { require.Equal(t, int64(50), fooAcc.GetCoins().AmountOf("steak").Int64()) executeWrite(t, fmt.Sprintf("gaiacli send %v --amount=10steak --to=%s --from=foo", flags, barAddr), pass) - tests.WaitForNextHeightTM(port) + tests.WaitForNextNBlocksTM(2, port) barAcc := executeGetAccount(t, fmt.Sprintf("gaiacli account %s %v", barAddr, flags)) require.Equal(t, int64(10), barAcc.GetCoins().AmountOf("steak").Int64()) @@ -67,7 +67,7 @@ func TestGaiaCLISend(t *testing.T) { // test autosequencing executeWrite(t, fmt.Sprintf("gaiacli send %v --amount=10steak --to=%s --from=foo", flags, barAddr), pass) - tests.WaitForNextHeightTM(port) + tests.WaitForNextNBlocksTM(2, port) barAcc = executeGetAccount(t, fmt.Sprintf("gaiacli account %s %v", barAddr, flags)) require.Equal(t, int64(20), barAcc.GetCoins().AmountOf("steak").Int64()) @@ -76,7 +76,7 @@ func TestGaiaCLISend(t *testing.T) { // test memo executeWrite(t, fmt.Sprintf("gaiacli send %v --amount=10steak --to=%s --from=foo --memo 'testmemo'", flags, barAddr), pass) - tests.WaitForNextHeightTM(port) + tests.WaitForNextNBlocksTM(2, port) barAcc = executeGetAccount(t, fmt.Sprintf("gaiacli account %s %v", barAddr, flags)) require.Equal(t, int64(30), barAcc.GetCoins().AmountOf("steak").Int64()) @@ -101,14 +101,14 @@ func TestGaiaCLICreateValidator(t *testing.T) { defer proc.Stop(false) tests.WaitForTMStart(port) - tests.WaitForNextHeightTM(port) + tests.WaitForNextNBlocksTM(2, port) fooAddr, _ := executeGetAddrPK(t, fmt.Sprintf("gaiacli keys show foo --output=json --home=%s", gaiacliHome)) barAddr, barPubKey := executeGetAddrPK(t, fmt.Sprintf("gaiacli keys show bar --output=json --home=%s", gaiacliHome)) barCeshPubKey := sdk.MustBech32ifyValPub(barPubKey) executeWrite(t, fmt.Sprintf("gaiacli send %v --amount=10steak --to=%s --from=foo", flags, barAddr), pass) - tests.WaitForNextHeightTM(port) + tests.WaitForNextNBlocksTM(2, port) barAcc := executeGetAccount(t, fmt.Sprintf("gaiacli account %s %v", barAddr, flags)) require.Equal(t, int64(10), barAcc.GetCoins().AmountOf("steak").Int64()) @@ -124,7 +124,7 @@ func TestGaiaCLICreateValidator(t *testing.T) { cvStr += fmt.Sprintf(" --moniker=%v", "bar-vally") executeWrite(t, cvStr, pass) - tests.WaitForNextHeightTM(port) + tests.WaitForNextNBlocksTM(2, port) barAcc = executeGetAccount(t, fmt.Sprintf("gaiacli account %s %v", barAddr, flags)) require.Equal(t, int64(8), barAcc.GetCoins().AmountOf("steak").Int64(), "%v", barAcc) @@ -142,7 +142,7 @@ func TestGaiaCLICreateValidator(t *testing.T) { success := executeWrite(t, unbondStr, pass) require.True(t, success) - tests.WaitForNextHeightTM(port) + tests.WaitForNextNBlocksTM(2, port) /* // this won't be what we expect because we've only started unbonding, haven't completed barAcc = executeGetAccount(t, fmt.Sprintf("gaiacli account %v %v", barCech, flags)) @@ -169,7 +169,7 @@ func TestGaiaCLISubmitProposal(t *testing.T) { defer proc.Stop(false) tests.WaitForTMStart(port) - tests.WaitForNextHeightTM(port) + tests.WaitForNextNBlocksTM(2, port) fooAddr, _ := executeGetAddrPK(t, fmt.Sprintf("gaiacli keys show foo --output=json --home=%s", gaiacliHome)) @@ -177,7 +177,7 @@ func TestGaiaCLISubmitProposal(t *testing.T) { require.Equal(t, int64(50), fooAcc.GetCoins().AmountOf("steak").Int64()) executeWrite(t, fmt.Sprintf("gaiacli gov submit-proposal %v --proposer=%s --deposit=5steak --type=Text --title=Test --description=test --from=foo", flags, fooAddr), pass) - tests.WaitForNextHeightTM(port) + tests.WaitForNextNBlocksTM(2, port) fooAcc = executeGetAccount(t, fmt.Sprintf("gaiacli account %s %v", fooAddr, flags)) require.Equal(t, int64(45), fooAcc.GetCoins().AmountOf("steak").Int64()) @@ -187,7 +187,7 @@ func TestGaiaCLISubmitProposal(t *testing.T) { require.Equal(t, gov.StatusDepositPeriod, proposal1.GetStatus()) executeWrite(t, fmt.Sprintf("gaiacli gov deposit %v --depositer=%s --deposit=10steak --proposalID=1 --from=foo", flags, fooAddr), pass) - tests.WaitForNextHeightTM(port) + tests.WaitForNextNBlocksTM(2, port) fooAcc = executeGetAccount(t, fmt.Sprintf("gaiacli account %s %v", fooAddr, flags)) require.Equal(t, int64(35), fooAcc.GetCoins().AmountOf("steak").Int64()) @@ -196,7 +196,7 @@ func TestGaiaCLISubmitProposal(t *testing.T) { require.Equal(t, gov.StatusVotingPeriod, proposal1.GetStatus()) executeWrite(t, fmt.Sprintf("gaiacli gov vote %v --proposalID=1 --voter=%s --option=Yes --from=foo", flags, fooAddr), pass) - tests.WaitForNextHeightTM(port) + tests.WaitForNextNBlocksTM(2, port) vote := executeGetVote(t, fmt.Sprintf("gaiacli gov query-vote --proposalID=1 --voter=%s --output=json %v", fooAddr, flags)) require.Equal(t, int64(1), vote.ProposalID)