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

Add simple "Set Validators" module #64

Closed
6 of 7 tasks
ethanfrey opened this issue Jun 1, 2018 · 7 comments
Closed
6 of 7 tasks

Add simple "Set Validators" module #64

ethanfrey opened this issue Jun 1, 2018 · 7 comments
Assignees

Comments

@ethanfrey
Copy link
Contributor

ethanfrey commented Jun 1, 2018

This is a minimal version of #32, with a few purposes:

  • to make sure the whole stack (incl. tendermint) can properly update validators on a running network
  • to provide a tool to easily add/remove validators on testnet (switch nodes from non-validator to validator)
  • and as a learning experience writing a weave module

Requirements:

  • There should be a new module with a new transaction type. It receives a list of the Validator diff that will be sent on EndBlock and passes it through verbatim. (So basically a client sends the diff that we then pass on to update validator power)
  • There should be one key set in the genesis file that is the only key that can authorize this transaction (pubkey/address)
  • Unit test the go code behaves as desired
  • Integration test that EndBlock message is proper format for tendermint, and validators are dynamically updated.

Steps (WIP):

  • Figure out a protocol and implement it via protobuf
  • Implement initializer for key
  • Implement business logic
  • Wrap BL into an extension interface (decorator)
  • Unit-test this using goconvey/gomega+ginkgo
  • Add a genesis key for an address/pubkey and it's handler
  • Integration tests and last touch-ups
@ruseinov
Copy link
Contributor

ruseinov commented Jun 9, 2018

@ethanfrey ready to do this, going to try my hand

@ethanfrey
Copy link
Contributor Author

Cool.

The first test (unit testable) is that the difference from the transaction is emitted in End Block, look there for the format they wish.

After that, we have to adjust the bytes of PubKey for the validator. I think they are go-amino encoded public key. Which is more of less the 32 byte public key, along with a 4 byte prefix identifying it as a ed25519 public key.

The server can just pass the bytes without validation if you want to start, later minimal validation. Once you got it unit tested, we can hook it up to tendermint and I will help on the client side test of what actual bytes to put here to add a new validator node.

Eg. Query node for its key, do some formatting, build the TX, see validator set properly Update.

@ruseinov
Copy link
Contributor

@ethanfrey please take a look at steps and see if they make sense

@ruseinov
Copy link
Contributor

ruseinov commented Jun 28, 2018

@ethanfrey question about protocol:
So the protobuf definition of the abci.Validator struct looks like this:

// Validator
message Validator {
  bytes address = 1;
  PubKey pub_key = 2 [(gogoproto.nullable)=false];
  int64 power = 3;
}

But we are going to receive another one from tendermint rest api:

// Info about the node's validator
type ValidatorInfo struct {
	Address     cmn.HexBytes  `json:"address"`
	PubKey      crypto.PubKey `json:"pub_key"`
	VotingPower int64         `json:"voting_power"`
}

In the client we will need to depend on https://github.com/tendermint/tendermint/blob/master/rpc/core/types/responses.go#L72) and then convert to abci.Validator and send to us.

@ruseinov
Copy link
Contributor

ruseinov commented Jun 28, 2018

@ethanfrey I have also stumbled upon protoc issues:
while executing make prototools I've noticed that none of these:

	@go install ./vendor/github.com/gogo/protobuf/protoc-gen-gogofast
	@go install ./vendor/github.com/gogo/protobuf/protoc-gen-gogofaster
	@go install ./vendor/github.com/gogo/protobuf/protoc-gen-gogoslick

Are vendored, but we seem to use protoc-gen-gogofaster.
I'm going to install protoc-gen-gogofast by hand for now, but let me know if you want me to get this vendored. Although I'm not sure why not just go get -u ..

@ruseinov
Copy link
Contributor

@ethanfrey While I was looking at cash implementation I've noticed the following:
we wrap orm.Bucket into another struct to create one method, that is actually pretty common as it's basically an upsert.

What do you think about turning orm.Bucket into interface (that also adds that method in both interface and it's original implementation)? That would allow us to get rid of the extra duplicated code.

In any case, I will proceed with the extension and then we can decide whether we want to refacfor common code into something reusable or not.

@ethanfrey
Copy link
Contributor Author

ethanfrey commented Jun 28, 2018

The problem is the method I am adding is basically a type-safe check that is different for each instance.

I would like to abstract this more and have less boilerplate.

After the validator PR, if you want to propose a concrete refactor, I will check it out. Basically this is how I try to enforce Bucket<T> where it only holds data of type T.

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

No branches or pull requests

2 participants