Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Staking fixes #912

Merged
merged 31 commits into from
Jul 12, 2018
Merged

Staking fixes #912

merged 31 commits into from
Jul 12, 2018

Conversation

mappum
Copy link
Contributor

@mappum mappum commented Jul 3, 2018

Closes #842
Closes #880

  • Hide grey bar when we previously had 0 bonded to a validator

  • Show inline errors when user tries to bond too much

  • Indicate if a validator is revoked image

  • Fetch validator set when fetching candidates, so we can indicate which candidates are currently validators (closes Validator status still faked by utility script #880)

@mappum mappum requested review from faboweb, NodeGuy and nylira as code owners July 3, 2018 02:21
@codecov
Copy link

codecov bot commented Jul 3, 2018

Codecov Report

Merging #912 into develop will increase coverage by 0.87%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop     #912      +/-   ##
===========================================
+ Coverage    95.15%   96.03%   +0.87%     
===========================================
  Files           82       81       -1     
  Lines         1487     1489       +2     
  Branches        78       78              
===========================================
+ Hits          1415     1430      +15     
+ Misses          59       49      -10     
+ Partials        13       10       -3
Impacted Files Coverage Δ
app/src/renderer/connectors/lcdClientMock.js 94.94% <ø> (ø) ⬆️
app/src/renderer/components/staking/LiDelegate.vue 92.3% <ø> (+0.3%) ⬆️
app/src/renderer/connectors/lcdClient.js 90.47% <ø> (ø) ⬆️
app/src/renderer/vuex/modules/delegation.js 100% <100%> (+10.25%) ⬆️
app/src/renderer/vuex/modules/delegates.js 100% <100%> (ø) ⬆️
app/src/renderer/components/staking/PageBond.vue 99.1% <100%> (+6.93%) ⬆️
...pp/src/renderer/components/staking/PageStaking.vue 97.5% <100%> (+2.62%) ⬆️
... and 1 more

@mappum
Copy link
Contributor Author

mappum commented Jul 3, 2018

I can't reproduce the remaining bugs in #842 with my local testnet (it would probably be easier if the testnet was up). We should probably merge the changes we have in the PR so far, then put the remaining problems into their own ticket(s).

@mappum mappum changed the title WIP: Staking fixes Staking fixes Jul 3, 2018
@faboweb
Copy link
Collaborator

faboweb commented Jul 3, 2018

Pulling in @nylira and @jolesbi for design

@nylira
Copy link
Contributor

nylira commented Jul 4, 2018

The only thing added is the slashed out validator name and "REVOKED", which is simple, but usable. Let's add a TmFormMsg below "REVOKED" validators that explains what revoked means.

Did we solve the question of what delegators can do if they delegated tokens to revoked validator?

@faboweb
Copy link
Collaborator

faboweb commented Jul 4, 2018

@rigelrozanski What can delegators do if they delegated tokens to revoked validator?

@rigelrozanski
Copy link

@faboweb When a validator is revoked it begins the unbonding process. During the unbonding process a delegator can either instantaneously redelegate, OR commit to unbonding. During this unbonding process if the validator is unrevoked and becomes bonded again (if they are in the top 100 validators) then they will automatically bond all the stake associated with that validator as well as all their delegators stake who haven't redelegated or who have NOT committed to unbonding.

The same mechanisms apply after the validator has been unbonding enough and it now fully unbonded, delegators can then opt to commit to unbonding, but since the unbonding period has already passed they will simply get their funds instantaneously

@faboweb
Copy link
Collaborator

faboweb commented Jul 4, 2018

Nice. Thank you for the clarification.

@faboweb
Copy link
Collaborator

faboweb commented Jul 8, 2018

Added the warning. For simplicity I added the warning once, if there are any revoked validators.

@@ -41,13 +39,15 @@ export default ({ dispatch, node }) => {
async getDelegates({ state, dispatch, commit, rootState }) {
commit("setDelegateLoading", true)
let delegates = await node.candidates()
let { validators } = await node.getValidators()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is only happening once. can we use the rpc event to requery the validators when they change?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put this as a follow up #946

@faboweb
Copy link
Collaborator

faboweb commented Jul 9, 2018

Can someone approve that PageBond is fully tested?

Screenshot of local coverage:
image

@okwme
Copy link
Contributor

okwme commented Jul 9, 2018

screenshot 2018-07-09 16 33 09

I got this error trying to unbond after previously bonding to myself on local-testnet

@faboweb
Copy link
Collaborator

faboweb commented Jul 9, 2018

I can't get to the PageBond after bonding:
image

@faboweb
Copy link
Collaborator

faboweb commented Jul 10, 2018

Posted errors are fixed.

Note: Unbonding shares are currently not accessible but will be in future SDK versions.
Let's test the existing code properly and get this merged.

@okwme
Copy link
Contributor

okwme commented Jul 11, 2018

---EDIT----
omg i forgot to pull, lemme try again
---EDIT---

Using local testnet i import my new account and go to staking page.

  1. I see myself, am already selected, click next, put 10 into the input to add 10 atom, click checkbox, click submit.

  2. I'm sent back to validators page, I see my node now has 10 more total votes to equal 110, but under "your votes" is still says 0.

  3. I click next again, looks like i have 0 bonded to this validator (myself), bond 10 more, click checkbox, click submit.

  4. I'm sent back to validators page, I see my node now has 10 more total votes to equal 120, under "your votes" it says 110

  5. I refresh the page and have 120 listed under "your votes"

  6. I click next, in the already bonded input box it says "119.99999999999" with an error below that says "Atoms must contain only numerals"

  7. I take the slider down to 0 so that the unbonding orange slider increases until 119.99999999

  8. I click the checkmark, then click submit and I get the following Error:

Codespace: 4
Code:      203
ABCICode:  262347
Error:     --= Error =--
Message: bad shares provided as input, must be MAX or decimal
Cause: <nil>
T: 0xcb
Msg Traces:
--= /Error =--

=== /ABCI Log ===
  1. I change my unbonding amount to 3.999999 so that my bonded amount is equal to 116

  2. Click submit and get the same error

  3. I increase my bonds to 140, click submit and get the following error:

json: cannot unmarshal number 20.000000000000014 into Go struct field Coin.amount of type int64

@okwme
Copy link
Contributor

okwme commented Jul 11, 2018

ok, landing back in the updated app i still had this strange decimal balance of atoms delegated to myself. Trying to increase them resulted in the json: cannot unmarshal... error above. Making them lower succeeded and took me back to the validator page. Clicking next to adjust again, I tried to take the balance all the way down to 0. This succeeded also.

In the list of all validators (where I am the only one) I now have 0 votes, but also 100% of the votes (because i am the only one i guess). It says NaN under total votes with a full bar.

Click next again I raise my delegation to the full 150 and click submit and get this huge error:

DeliverTx failed: (65537) === ABCI Log ===
Codespace: 1
Code:      1
ABCICode:  65537
Error:     --= Error =--
Message: Recovered: division by zero
stack:
goroutine 57 [running]:
runtime/debug.Stack(0xc425098898, 0x17c39e0, 0x1adb150)
	/usr/local/go/src/runtime/debug/stack.go:24 +0xa7
github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).runTx.func1(0xc42509c2c0)
	/go/src/github.com/cosmos/cosmos-sdk/baseapp/baseapp.go:474 +0x1f4
panic(0x17c39e0, 0x1adb150)
	/usr/local/go/src/runtime/panic.go:502 +0x229
math/big.(*Rat).Quo(0xc425098a08, 0xc425098a58, 0xc425098a98, 0x5)
	/usr/local/go/src/math/big/rat.go:509 +0x167
github.com/cosmos/cosmos-sdk/types.Rat.Quo(0x0, 0xc422f16d80, 0x1, 0x5, 0x0, 0xc422f16db0, 0x0, 0x5, 0x0, 0x0, ...)
	/go/src/github.com/cosmos/cosmos-sdk/types/rational.go:91 +0x94
github.com/cosmos/cosmos-sdk/x/stake.Pool.addTokensBonded(0x32, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xc422f169c0, ...)
	/go/src/github.com/cosmos/cosmos-sdk/x/stake/pool.go:122 +0x150
github.com/cosmos/cosmos-sdk/x/stake.Validator.addTokensFromDel(0xc424893ca0, 0x14, 0x14, 0x1ae7a40, 0xc424893d00, 0x0, 0x2, 0x0, 0xc422f16360, 0x1, ...)
	/go/src/github.com/cosmos/cosmos-sdk/x/stake/validator.go:202 +0x4ac
github.com/cosmos/cosmos-sdk/x/stake.delegate(0x1ae6f40, 0xc423ee5fb0, 0xc424e09040, 0xe, 0x1ae0740, 0xc4200129b0, 0xc420a24480, 0x1ae0740, 0xc420012990, 0x1aed280, ...)
	/go/src/github.com/cosmos/cosmos-sdk/x/stake/handler.go:166 +0x41c
github.com/cosmos/cosmos-sdk/x/stake.handleMsgDelegate(0x1ae6f40, 0xc423ee5fb0, 0xc424e09040, 0xe, 0xc424893a20, 0x14, 0x14, 0xc424893a40, 0x14, 0x14, ...)
	/go/src/github.com/cosmos/cosmos-sdk/x/stake/handler.go:137 +0x40c
github.com/cosmos/cosmos-sdk/x/stake.NewHandler.func1(0x1ae6f40, 0xc423ee5fb0, 0xc424e09040, 0xe, 0x1ae7cc0, 0xc4229abd60, 0x0, 0x0, 0x0, 0x0, ...)
	/go/src/github.com/cosmos/cosmos-sdk/x/stake/handler.go:19 +0x78c
github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).runTx(0xc4200f02a0, 0x2, 0xc4241d9e00, 0xe5, 0xe5, 0x1aded00, 0xc4229abdb0, 0x0, 0x0, 0x0, ...)
	/go/src/github.com/cosmos/cosmos-sdk/baseapp/baseapp.go:537 +0x340
github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).DeliverTx(0xc4200f02a0, 0xc4241d9e00, 0xe5, 0xe5, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/go/src/github.com/cosmos/cosmos-sdk/baseapp/baseapp.go:426 +0x466
github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/abci/client.(*localClient).DeliverTxAsync(0xc420ada480, 0xc4241d9e00, 0xe5, 0xe5, 0x0)
	/go/src/github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/abci/client/local_client.go:76 +0x88
github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/proxy.(*appConnConsensus).DeliverTxAsync(0xc420a9cc80, 0xc4241d9e00, 0xe5, 0xe5, 0xc422771800)
	/go/src/github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/proxy/app_conn.go:73 +0x51
github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/state.execBlockOnProxyApp(0x1ae7640, 0xc420ab0a40, 0x1aea5a0, 0xc420a9cc80, 0xc4229ab950, 0xc423ee48a0, 0x1aedc00, 0xc4200bc4d8, 0x1, 0xc42487e8a0, ...)
	/go/src/github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/state/execution.go:203 +0x735
github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/state.(*BlockExecutor).ApplyBlock(0xc42007e720, 0xc420a1b020, 0x11, 0x66c, 0x4, 0xc424c2de60, 0x14, 0x20, 0x1, 0xc42487e8a0, ...)
	/go/src/github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/state/execution.go:76 +0x12f
github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusState).finalizeCommit(0xc4200d3b80, 0x66d)
	/go/src/github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/consensus/state.go:1267 +0xbb2
github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusState).tryFinalizeCommit(0xc4200d3b80, 0x66d)
	/go/src/github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/consensus/state.go:1198 +0x468
github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusState).enterCommit.func1(0xc4200d3b80, 0x0, 0x66d)
	/go/src/github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/consensus/state.go:1146 +0x98
github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusState).enterCommit(0xc4200d3b80, 0x66d, 0x0)
	/go/src/github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/consensus/state.go:1175 +0x802
github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusState).addVote(0xc4200d3b80, 0xc423db8460, 0x0, 0x0, 0xc4223f4318, 0xc4223f4320, 0xc42509db08)
	/go/src/github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/consensus/state.go:1532 +0xbb4
github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusState).tryAddVote(0xc4200d3b80, 0xc423db8460, 0x0, 0x0, 0xc42509db58, 0x16be249)
	/go/src/github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/consensus/state.go:1390 +0x56
github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusState).handleMsg(0xc4200d3b80, 0x17fb760, 0xc4239286e0, 0x0, 0x0)
	/go/src/github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/consensus/state.go:607 +0x643
github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine(0xc4200d3b80, 0x0)
	/go/src/github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/consensus/state.go:564 +0x5a5
created by github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/consensus.(*ConsensusState).OnStart
	/go/src/github.com/cosmos/cosmos-sdk/vendor/github.com/tendermint/tendermint/consensus/state.go:272 +0x140

Cause: <nil>
T: 0x1
Msg Traces:
--= /Error =--

=== /ABCI Log ===

@okwme
Copy link
Contributor

okwme commented Jul 11, 2018

new local chain install with fresh seeds and screenshots:
first view:
screenshot 2018-07-11 17 38 30
staking page:
screenshot 2018-07-11 17 38 23
bonding page:
screenshot 2018-07-11 17 38 39
increase:
screenshot 2018-07-11 17 38 51
submit:
screenshot 2018-07-11 17 38 58
bond again:
screenshot 2018-07-11 17 39 08
increase again:
screenshot 2018-07-11 17 39 20
submit:
screenshot 2018-07-11 17 39 26
hit refresh (notice"Your Votes" increases by 100):
screenshot 2018-07-11 17 39 44
bond again and we got funny stuff:
screenshot 2018-07-11 17 39 55
increase:
screenshot 2018-07-11 17 40 05
error:
screenshot 2018-07-11 17 40 11
decrease:
screenshot 2018-07-11 17 40 24
error:
screenshot 2018-07-11 17 40 28
decrease all:
screenshot 2018-07-11 17 40 36
error:
screenshot 2018-07-11 17 40 40
wallet:
screenshot 2018-07-11 17 41 05

@faboweb
Copy link
Collaborator

faboweb commented Jul 11, 2018 via email

@faboweb
Copy link
Collaborator

faboweb commented Jul 12, 2018

Apparently the SDK sends staked shares in some weir rational format i.e. 890000000000034/150000000000 This could cause the decimals showing.

@faboweb
Copy link
Collaborator

faboweb commented Jul 12, 2018

cosmos/cosmos-sdk#1637

@faboweb
Copy link
Collaborator

faboweb commented Jul 12, 2018

I am seeing similar problems with the stake calculation:
Starting of with 0 bond
Validator has 100 steak
Bonding 10
Bonding 10
Validator has 120 steak, I have 11

Data:
/stake/validators

[{"owner":"cosmosvaladdr1znx55r09uks9kreft3rrn22j3cpf6mh5kt3n4j","pub_key":"cosmosvalpub1zcjduc3q9gax50xwp8h8fw22nm8d2qkk8fx8sh7pv6r3g3galwptfzmdj66squ49n5","revoked":false,"pool_shares":{"status":2,"amount":"120"},"delegator_shares":"120/11","description":{"moniker":"local","identity":"","website":"","details":""},"bond_height":0,"bond_intra_tx_counter":0,"proposer_reward_pool":null,"commission":"0","commission_max":"0","commission_change_rate":"0","commission_change_today":"0","prev_bonded_shares":"0"}]

/stake/cosmosaccaddr1znx55r09uks9kreft3rrn22j3cpf6mh5s6yymt/bonding_status/cosmosvaladdr1znx55r09uks9kreft3rrn22j3cpf6mh5kt3n4j

{"delegator_addr":"14CD4A0DE5E5A05B0F295C4639A9528E029D6EF4","validator_addr":"14CD4A0DE5E5A05B0F295C4639A9528E029D6EF4","shares":"120/11","height":217}

Pulling in @rigelrozanski for debugging.

@okwme
Copy link
Contributor

okwme commented Jul 12, 2018

so we decided to merge this and make a new issue right?

@faboweb
Copy link
Collaborator

faboweb commented Jul 12, 2018

If you approve the code. ;)

Copy link
Contributor

@okwme okwme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅

@okwme okwme merged commit 75d16a6 into develop Jul 12, 2018
@okwme okwme deleted the matt/842-staking-fixes branch July 12, 2018 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validator status still faked by utility script Staking UI problems
5 participants