Initialization of POS chain #1373

mossid · 2018-06-25T23:54:08Z

Updated all relevant documentation in docs
Updated all code comments where relevant
Wrote tests
Updated CHANGELOG.md
Updated Gaia/Examples
Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

Closes: #959

pass lint

codecov · 2018-06-26T00:09:32Z

Codecov Report

Merging #1373 into develop will decrease coverage by 0.04%.
The diff coverage is 70.58%.

@@             Coverage Diff             @@
##           develop    #1373      +/-   ##
===========================================
- Coverage     64.2%   64.16%   -0.05%     
===========================================
  Files          115      115              
  Lines         6834     6837       +3     
===========================================
- Hits          4388     4387       -1     
- Misses        2192     2196       +4     
  Partials       254      254

mossid · 2018-06-26T00:14:29Z

Ready for review. Do not merge it before #1119

ebuchman · 2018-06-26T16:05:32Z

Oh - can we get a test for this ?

alexanderbez · 2018-06-26T20:05:02Z

x/stake/genesis.go

@@ -32,7 +33,7 @@ func DefaultGenesisState() GenesisState {
 }

 // InitGenesis - store genesis parameters
-func InitGenesis(ctx sdk.Context, k Keeper, data GenesisState) {
+func InitGenesis(ctx sdk.Context, k Keeper, data GenesisState) (vals []abci.Validator) {


Is there a reason to use a named return var when we're not using that actual named variable?

pass lint apply requests fix test

cwgoes · 2018-07-04T04:24:52Z

Sorry for the delay @mossid - this needs a rebase now.

pass lint apply requests fix test fix abci dep

rigelrozanski · 2018-07-04T23:37:58Z

baseapp/baseapp.go

@@ -296,7 +296,7 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC
 	if app.initChainer == nil {
 		return
 	}
-	app.initChainer(app.deliverState.ctx, req) // no error
+	res = app.initChainer(app.deliverState.ctx, req) // no error


delete the comment // no error? why do we even have that here

rigelrozanski · 2018-07-04T23:39:36Z

x/stake/genesis.go

 )

 // InitGenesis sets the pool and parameters for the provided keeper and
 // initializes the IntraTxCounter. For each validator in data, it sets that
 // validator in the keeper along with manually setting the indexes. In
 // addition, it also sets any delegations found in data. Finally, it updates
 // the bonded validators.
-func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) {
+func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) (res []abci.Validator) {


function comment should be updated to reflect these changes.

If I understand this correctly we want to pass back the abci validators so that we do not need to have the validators twice in the genesis file? Tendermint will just read the validators from response init chain?

rigelrozanski

Cool - I guess tendermint is already ready to take in the validator set directly from response of init chain?

If so, as a part of this PR, we should update the Genesis file creation to nolonger write the abci validators to the config file here:https://github.com/cosmos/cosmos-sdk/blob/develop/server/init.go#L132

mossid · 2018-07-13T01:34:33Z

Found that TestValidatorsQuery was failing, the reason was:

It calls InitializeTestLCD with nValidators=2
The function allocates 10 power to the first validator(who runs the node) and 1 power to the rest so it supposed to be run normally without the rest of the validators.
But new InitChain now returns a new validator set, which is called by GaiaAppGenState, which allocates equal power to the all validators(https://github.com/cosmos/cosmos-sdk/blob/master/cmd/gaia/app/genesis.go#L175)
So only 50% of voting power available, node not starts, test fails.

I'm changing TestValidatorsQuery to use only one validator, but I think we should change the equal validator power allocating part, at least for the test codes.

pass lint apply requests fix test fix abci dep fix TestValidatorsQuery rm mistake file apply requests fix lint

mossid · 2018-07-13T04:14:09Z

Ready for review.

cwgoes

Is Tendermint ready for this? Mostly LGTM; req'd two changes.

cwgoes · 2018-07-13T17:53:02Z

client/lcd/lcd_test.go

@@ -359,25 +359,20 @@ func TestTxs(t *testing.T) {
 }

 func TestValidatorsQuery(t *testing.T) {
-	cleanup, pks, port := InitializeTestLCD(t, 2, []sdk.AccAddress{})
+	cleanup, pks, port := InitializeTestLCD(t, 1, []sdk.AccAddress{})


Why can't we initialize with 2 validators?

cwgoes · 2018-07-13T17:54:13Z

x/stake/genesis.go

@@ -41,7 +44,13 @@ func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) error
 	}

 	keeper.UpdateBondedValidatorsFull(ctx)
-	return nil
+
+	vals := keeper.GetAllValidators(ctx)


We want all the bonded validators, not all validators - I think you want GetValidatorsBonded - there may be more than 100 genesis validator candidates

cwgoes · 2018-07-13T17:55:26Z

Also needs changelog entry and update to docs if applicable.

rigelrozanski · 2018-07-14T19:32:58Z

@mossid There was some conflicting work done here and with share mechanism removal I think? Let's attempt to rebase here - Also TestValidatorsQuery bug could maybe be separated into a mini PR for fast merge to split up the fixes within this PR. Otherwise I'd like to try to merge this in soon, as soon as rebased - lookin' good

rigelrozanski · 2018-07-18T18:47:05Z

Can we add a test to this PR which ensures that if there > MaxValidators in the genesis file only the top 100 make it into Tendermint as outlined by #1710

pass lint apply requests fix test fix abci dep fix TestValidatorsQuery rm mistake file apply requests fix lint in progress apply requests

cwgoes · 2018-07-19T01:29:41Z

x/stake/keeper/key.go

@@ -85,12 +84,18 @@ func getValidatorPowerRank(validator types.Validator, pool types.Pool) []byte {
 	counterBytes := make([]byte, 2)
 	binary.BigEndian.PutUint16(counterBytes, ^uint16(validator.BondIntraTxCounter)) // invert counter (first txns have priority)

-	return append(append(append(append(
+	lenBytes := make([]byte, binary.MaxVarintLen64)


Instead merge #1724 to fix this.

cwgoes

utACK, thanks @mossid!

Test has been added.

ebuchman · 2018-07-19T17:10:49Z

Note tendermint/tendermint#2015

mossid requested a review from ebuchman as a code owner June 25, 2018 23:54

mossid force-pushed the joon/959-pos-init branch from c1f1618 to 6c9bc69 Compare June 25, 2018 23:58

mossid added a commit that referenced this pull request Jun 25, 2018

Merge pull request #1373: Initialization of POS chain

6c9bc69

pass lint

mossid added the ready-for-review label Jun 26, 2018

ebuchman previously approved these changes Jun 26, 2018

View reviewed changes

alexanderbez reviewed Jun 26, 2018

View reviewed changes

mossid dismissed ebuchman’s stale review via d3dcd5c June 26, 2018 23:29

mossid force-pushed the joon/959-pos-init branch from 6c9bc69 to d3dcd5c Compare June 26, 2018 23:29

mossid force-pushed the joon/959-pos-init branch from d3dcd5c to 7eb96db Compare June 27, 2018 02:10

mossid force-pushed the joon/959-pos-init branch from 7eb96db to a9e86ee Compare June 27, 2018 02:23

mossid added a commit that referenced this pull request Jun 27, 2018

Merge pull request #1373: Initialization of POS chain

a9e86ee

pass lint apply requests fix test

cwgoes mentioned this pull request Jun 30, 2018

client/lcd: fix tests #1491

Merged

cwgoes changed the base branch from develop to unstable July 3, 2018 21:12

cwgoes changed the base branch from unstable to develop July 4, 2018 19:26

mossid force-pushed the joon/959-pos-init branch from 0c8281b to a148093 Compare July 4, 2018 22:56

mossid requested review from cwgoes and rigelrozanski as code owners July 4, 2018 22:56

mossid added a commit that referenced this pull request Jul 4, 2018

Merge pull request #1373: Initialization of POS chain

a148093

pass lint apply requests fix test fix abci dep

rigelrozanski reviewed Jul 4, 2018

View reviewed changes

rigelrozanski previously requested changes Jul 4, 2018

View reviewed changes

ebuchman changed the base branch from develop to master July 11, 2018 00:28

mossid force-pushed the joon/959-pos-init branch from a148093 to df0079e Compare July 13, 2018 01:37

mossid force-pushed the joon/959-pos-init branch from df0079e to bed8be3 Compare July 13, 2018 02:16

mossid force-pushed the joon/959-pos-init branch from bed8be3 to 9f12e67 Compare July 13, 2018 03:47

mossid added a commit that referenced this pull request Jul 13, 2018

Merge pull request #1373: Initialization of POS chain

9f12e67

pass lint apply requests fix test fix abci dep fix TestValidatorsQuery rm mistake file apply requests fix lint

cwgoes suggested changes Jul 13, 2018

View reviewed changes

ebuchman changed the base branch from master to develop July 16, 2018 15:53

mossid force-pushed the joon/959-pos-init branch from 9f12e67 to 323dbce Compare July 17, 2018 04:33

This was referenced Jul 17, 2018

Ensure > 100 genesis validator candidates are handled correctly #1710

Closed

R4R: Simulation framework, including staking simulation #1620

Merged

Merge pull request #1373: Initialization of POS chain

0469710

pass lint apply requests fix test fix abci dep fix TestValidatorsQuery rm mistake file apply requests fix lint in progress apply requests

mossid force-pushed the joon/959-pos-init branch from 686df73 to 0469710 Compare July 19, 2018 01:04

cwgoes reviewed Jul 19, 2018

View reviewed changes

revert key.go

b3cecf0

cwgoes approved these changes Jul 19, 2018

View reviewed changes

cwgoes added 2 commits July 19, 2018 08:27

Update PENDING.md

70e555f

Merge branch 'develop' into joon/959-pos-init

53a3cd3

cwgoes merged commit aa52541 into develop Jul 19, 2018

cwgoes deleted the joon/959-pos-init branch July 19, 2018 06:39

MrXJC mentioned this pull request Jul 24, 2018

Can't start the gaiad by the previous command #1804

Closed

rigelrozanski mentioned this pull request Jul 25, 2018

WIP fix gen-tx #1824

Merged

9 tasks

mossid mentioned this pull request Sep 14, 2018

InitializeTestLCD timeouts when nValidators > 1 #2339

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initialization of POS chain #1373

Initialization of POS chain #1373

mossid commented Jun 25, 2018 •

edited by cwgoes

Loading

codecov bot commented Jun 26, 2018 •

edited

Loading

mossid commented Jun 26, 2018

ebuchman commented Jun 26, 2018

alexanderbez Jun 26, 2018

cwgoes commented Jul 4, 2018

rigelrozanski Jul 4, 2018

rigelrozanski Jul 4, 2018

rigelrozanski left a comment

mossid commented Jul 13, 2018

mossid commented Jul 13, 2018

cwgoes left a comment

cwgoes Jul 13, 2018

cwgoes Jul 13, 2018

cwgoes commented Jul 13, 2018

rigelrozanski commented Jul 14, 2018

rigelrozanski commented Jul 18, 2018

cwgoes Jul 19, 2018

cwgoes left a comment

ebuchman commented Jul 19, 2018

Initialization of POS chain #1373

Initialization of POS chain #1373

Conversation

mossid commented Jun 25, 2018 • edited by cwgoes Loading

codecov bot commented Jun 26, 2018 • edited Loading

Codecov Report

mossid commented Jun 26, 2018

ebuchman commented Jun 26, 2018

alexanderbez Jun 26, 2018

Choose a reason for hiding this comment

cwgoes commented Jul 4, 2018

rigelrozanski Jul 4, 2018

Choose a reason for hiding this comment

rigelrozanski Jul 4, 2018

Choose a reason for hiding this comment

rigelrozanski left a comment

Choose a reason for hiding this comment

mossid commented Jul 13, 2018

mossid commented Jul 13, 2018

cwgoes left a comment

Choose a reason for hiding this comment

cwgoes Jul 13, 2018

Choose a reason for hiding this comment

cwgoes Jul 13, 2018

Choose a reason for hiding this comment

cwgoes commented Jul 13, 2018

rigelrozanski commented Jul 14, 2018

rigelrozanski commented Jul 18, 2018

cwgoes Jul 19, 2018

Choose a reason for hiding this comment

cwgoes left a comment

Choose a reason for hiding this comment

ebuchman commented Jul 19, 2018

mossid commented Jun 25, 2018 •

edited by cwgoes

Loading

codecov bot commented Jun 26, 2018 •

edited

Loading