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

go-kosu: implement validator removal and subcommand #190

Merged
merged 5 commits into from Jul 30, 2019

Conversation

@gchaincl
Copy link
Contributor

gchaincl commented Jul 25, 2019

Overview

Implements validator removal and `kosu-cli tx update-validator sub-command. Useful to test this scenario.

Description

Given an incoming Validator Tx we check the amount:
- if == 0: use the corresponding validator's power to confirm the tx and update the balance with 0
- if != 0: use the tx amount as confirmation power.

validators updated with 0 balance will be removed from the store

Testing

To test this, a new command has been introduced: kosu-cli tx update-validator <block> <pubkey> <address> <power> [flags]:

  1. start the testnet
make testnet
  1. check the current validator set:
curl localhost:8000/validators | jq '.result.validators[].voting_power'
  1. Update a validator power
./kosu-cli --abci http://localhost:8000 --home ./testnet/node0 tx update-validator 1 r9dq+UvR+DzAmKhuo8VTk+i4OuSLA4uUPxVmKI1ucas= address 6000000000000000000
  1. check the current validator set
curl localhost:8000/validators | jq '.result.validators[].voting_power'
  1. Remove the validator from set
./kosu-cli --abci http://localhost:8000 --home ./testnet/node0 tx update-validator 2 r9dq+UvR+DzAmKhuo8VTk+i4OuSLA4uUPxVmKI1ucas= address 0
  1. check the current validator set
curl localhost:8000/validators | jq '.result.validators[].voting_power'
@gchaincl gchaincl requested a review from hrharder Jul 25, 2019
@gchaincl

This comment has been minimized.

Copy link
Contributor Author

gchaincl commented Jul 25, 2019

@hrharder I'm tagging this as a WIP cause there's still some details that I'd like to discuss with you.

@gchaincl gchaincl force-pushed the go-kosu/rm-validators branch from 1fff438 to 499f3af Jul 25, 2019
@hrharder

This comment has been minimized.

Copy link
Member

hrharder commented Jul 25, 2019

Great, this looks good so far Gus! I will play around with it today.

@gchaincl

This comment has been minimized.

Copy link
Contributor Author

gchaincl commented Jul 25, 2019

cool, let me know how it goes

@hrharder

This comment has been minimized.

Copy link
Member

hrharder commented Jul 25, 2019

go-kosu: please

Looks like the git-gods listened, all appears to work! Great job :)

Copy link
Member

hrharder left a comment

This looks good to me, the only outstanding issue I see is dealing with confirmed witness Tx's. We'll have to figure out the best way to safely prune them.

Perhaps during execution of each new WitnessTx, we check if there are any in the store that are more than N blocks old, and if so, we can assume they will never be confirmed (or already have been) and can delete... Just an idea. We can discuss elsewhere.

@gchaincl gchaincl removed the WIP label Jul 30, 2019
@gchaincl

This comment has been minimized.

Copy link
Contributor Author

gchaincl commented Jul 30, 2019

@hrharder do you think we should implement witnesstx pruning before merging?
IMO we can open a new PR when we define the behavior we want

@hrharder

This comment has been minimized.

Copy link
Member

hrharder commented Jul 30, 2019

I agree, let's merge this in and delegate that to a new PR.

@hrharder hrharder merged commit 6f1d153 into master Jul 30, 2019
1 check passed
1 check passed
continuous-integration/drone/pr Build is passing
Details
@hrharder hrharder deleted the go-kosu/rm-validators branch Jul 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.