Staking Implementation #540

rigelrozanski · 2018-03-01T16:28:35Z

pre-MVP staking logic

    ____________========
   /      o          | |
 /                    ||
 v v v v               |
  ^ ^ ^ ^              |
  \........            |

codecov · 2018-03-01T16:47:23Z

Codecov Report

Merging #540 into develop will decrease coverage by 3.98%.
The diff coverage is 47.63%.

@@             Coverage Diff             @@
##           develop     #540      +/-   ##
===========================================
- Coverage     63.3%   59.32%   -3.99%     
===========================================
  Files           44       62      +18     
  Lines         2055     3255    +1200     
===========================================
+ Hits          1301     1931     +630     
- Misses         639     1183     +544     
- Partials       115      141      +26

adrianbrink · 2018-03-22T14:00:34Z

What's the message for Unbonding as a Candidate?

cwgoes · 2018-03-26T14:53:14Z

x/stake/handler.go

+	if msg.Description.Details != "" {
+		candidate.Description.Details = msg.Description.Details
+	}
+


Do we care about candidates having the ability to clear previously set metadata?

opening an issue

cwgoes · 2018-03-26T14:57:22Z

x/stake/keeper.go

+	}
+	store := ctx.KVStore(k.storeKey)
+	b := store.Get(ParamKey)
+	if b == nil {


Desirable to move store initialization logic into a single function for clarity? Also avoids strange results were a store to return nil incorrectly.

created an issue, I think this can wait until after the MVP though #724

cwgoes · 2018-03-26T15:01:06Z

x/stake/types.go

+		Inflation:         sdk.NewRat(7, 100),
+	}
+}
+


Desirable to load default staking params / pool params from a config file? That way clients can customize without recompiling.

same as previous response above ^

cwgoes · 2018-03-26T15:09:51Z

x/stake/handler.go

+// Perform all the actions required to bond tokens to a delegator bond from their account
+func (k Keeper) BondCoins(ctx sdk.Context, bond DelegatorBond, candidate Candidate, amount sdk.Coin) sdk.Error {
+
+	_, err := k.coinKeeper.SubtractCoins(ctx, bond.DelegatorAddr, sdk.Coins{amount})


Spec says coins should be transferred to pool account - is that account not necessary to track explicitly?

Discussed; spec should change @milosevic - no longer a need for a pool account.

cwgoes · 2018-03-26T15:13:36Z

x/stake/pool.go

+		receivedGlobalShares = k.addTokensUnbonded(ctx, amount)
+	}
+	candidate.Assets = candidate.Assets.Add(receivedGlobalShares)
+


Spec defines issuedShares and receivedGlobalShares as two different variables, but the latter is used nowhere else in the spec - are they supposed to be the same (as they are here)?

they are not the same here - can go over with you tomorrow

cwgoes · 2018-03-26T15:25:57Z

x/stake/handler.go

+	bond.Shares = bond.Shares.Sub(shares)
+
+	// get pubKey candidate
+	candidate, found := k.GetCandidate(ctx, msg.CandidateAddr)


Do we want to also check candidate existence in CheckTx calls?

yes - resolved

cwgoes · 2018-03-26T15:31:27Z

x/stake/handler.go

+	}
+
+	// deduct shares from the candidate
+	if candidate.Liabilities.IsZero() {


Per spec, this check should be for candidate.Assets (globalStakeShares)

conceptually makes more sense yes - although I think both should work

cwgoes · 2018-03-26T17:07:23Z

x/stake/msg.go

+type MsgEditCandidacy struct {
+	Description
+	CandidateAddr sdk.Address `json:"address"`
+}


Spec also includes provision for changingcommission

commission not yet implemented

cwgoes · 2018-03-26T17:21:12Z

x/stake/handler.go

+	}
+
+	// subtract bond tokens from delegator bond
+	if bond.Shares.LT(shares) { // bond shares < msg shares


Check redundant? Either bond.Shares.GT(shares) is true or shares == bond.Shares

cool was partially redundant, moved stuff around to make clear / remove redundancy

rigelrozanski · 2018-03-27T17:56:28Z

Many issues: #713

... ...

ebuchman · 2018-03-04T20:07:26Z

x/stake/handler.go

+//_______________________________________________________________________
+
+// DelegatedProofOfStake - interface to enforce delegation stake
+type delegatedProofOfStake interface {


rigelrozanski requested a review from ebuchman as a code owner March 1, 2018 16:28

rigelrozanski mentioned this pull request Mar 1, 2018

WIP: Staking module #508

Closed

ebuchman mentioned this pull request Mar 4, 2018

Move staking module into new sdk #345

Closed

adrianbrink changed the title ~~WIP Staking~~ WIP: Staking Mar 12, 2018

adrianbrink changed the title ~~WIP: Staking~~ WIP: Staking Implementation Mar 12, 2018

rigelrozanski force-pushed the rigel/staking branch 6 times, most recently from 355a1dc to 6eb2a12 Compare March 20, 2018 13:56

martindale mentioned this pull request Mar 22, 2018

Sunny/staking #665

Closed

rigelrozanski force-pushed the rigel/staking branch from fbb9b83 to c5b6490 Compare March 23, 2018 16:15

cwgoes reviewed Mar 26, 2018

View reviewed changes

rigelrozanski force-pushed the rigel/staking branch from c2acefb to 8c7b126 Compare March 26, 2018 17:05

cwgoes reviewed Mar 26, 2018

View reviewed changes

rigelrozanski force-pushed the rigel/staking branch from df41b76 to 41e009f Compare March 28, 2018 01:04

rigelrozanski changed the title ~~WIP: Staking Implementation~~ Staking Implementation Mar 28, 2018

rigelrozanski and others added 26 commits March 28, 2018 19:01

gettin' stakin' tests up to snuff

3f0d7ed

tests compile

acc5bda

name and pass rebase fixes

d439f5c

gitignore

b8cdf05

testing staking

c1d8aef

... ...

working sunny staking refactor

93173e9

sunny refactor compiling

13835f0

Rational -> Rat

7da2789

rational to Exposed Fields

db0a275

staking passing, low test coverage

a4ab2fc

keeper tests/revisions

3a01167

expose some keeper functions, tick cleanup

5ba2970

major keeper revisions

27e0bbc

borken validator keeper tests

c79263f

remove term voting power from candidates

59562cd

rebase fixes

4e45d73

msg_tests

c1cb53c

validatebasic tests cleanup

1df21e0

dep update

e25b780

... ...

fix validator keeper functionality, add testing

e5199f0

stake handler nolonger function of keeper

cbbba2c

cwgoes comments addressed

b2c5814

fixes for SignBuildBroadcast in staking

18c512e

revert to develop docs

cabdbfd

revert examples/basecoin

91d3838

Allow clearing candidacy fields (closes #723)

aa4bd05

rigelrozanski force-pushed the rigel/staking branch from e965e9b to aa4bd05 Compare March 28, 2018 17:02

ebuchman approved these changes Mar 28, 2018

View reviewed changes

ebuchman merged commit 2e5943e into develop Mar 28, 2018

ebuchman deleted the rigel/staking branch March 28, 2018 17:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Staking Implementation #540

Staking Implementation #540

rigelrozanski commented Mar 1, 2018 •

edited

Loading

codecov bot commented Mar 1, 2018 •

edited

Loading

adrianbrink commented Mar 22, 2018

cwgoes Mar 26, 2018

rigelrozanski Mar 28, 2018

rigelrozanski Mar 28, 2018

cwgoes Mar 26, 2018

rigelrozanski Mar 28, 2018

cwgoes Mar 26, 2018

rigelrozanski Mar 28, 2018

cwgoes Mar 26, 2018

cwgoes Mar 26, 2018

cwgoes Mar 26, 2018 •

edited

Loading

rigelrozanski Mar 28, 2018

cwgoes Mar 26, 2018

rigelrozanski Mar 28, 2018

cwgoes Mar 26, 2018

rigelrozanski Mar 26, 2018

cwgoes Mar 26, 2018

rigelrozanski Mar 26, 2018

cwgoes Mar 26, 2018

rigelrozanski Mar 28, 2018

rigelrozanski commented Mar 27, 2018

ebuchman Mar 4, 2018

Staking Implementation #540

Staking Implementation #540

Conversation

rigelrozanski commented Mar 1, 2018 • edited Loading

codecov bot commented Mar 1, 2018 • edited Loading

Codecov Report

adrianbrink commented Mar 22, 2018

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

cwgoes Mar 26, 2018 • edited Loading

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

rigelrozanski commented Mar 27, 2018

Choose a reason for hiding this comment

rigelrozanski commented Mar 1, 2018 •

edited

Loading

codecov bot commented Mar 1, 2018 •

edited

Loading

cwgoes Mar 26, 2018 •

edited

Loading