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

[WIP] Problem: Validators cannot vote on proposals (BEP-18) #44

Merged
merged 5 commits into from
Jul 9, 2018

Conversation

vrde
Copy link
Contributor

@vrde vrde commented May 22, 2018

Solution: write a transactional election protocol.

@vrde vrde force-pushed the stateful-upsert-validator branch 3 times, most recently from 64d9f6d to 7684871 Compare May 22, 2018 12:31
18/README.md Outdated

## Rationale
<!--The rationale fleshes out the specification by describing what motivated the design and why particular design decisions were made. It should describe alternate designs that were considered and related work, e.g. how the feature is supported in other languages. The rationale may also provide evidence of consensus within the community, and should discuss important objections or concerns raised during discussion.-->
Another idea to implement this protocol is to use a multi signature transaction.
Copy link
Contributor

Choose a reason for hiding this comment

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

The Rationale section should explain why the above design was chosen rather than some other design. In this case, you should explain why a multi-signature transaction wasn't used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'll add it soon.

18/README.md Outdated

## Implementation
<!--The implementations must be completed before any BEP is given status "stable", but it need not be completed before the BEP is accepted. While there is merit to the approach of reaching consensus on the BEP and rationale before writing code, the principle of "rough consensus and running code" is still useful when it comes to resolving many discussions of API details.-->

Copy link
Contributor

Choose a reason for hiding this comment

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

You can say that this section can be filled in once there is an implementation.

18/README.md Outdated

An Election is a transaction representing the matter of change, and some Vote tokens. The Initiator issues a `CREATE` transaction with the `amount` set to the total number of Validators, and transfer one vote token per Validator using the transaction `outputs`.

At this point the Election starts. Independently, and asynchronously, each Validator can spend its Vote Token to an Election Address to show agreement on the matter of change. The Election Address is the `id` of the first `CREATE` transaction. Once a Vote Token has been transferred to that address, it is not possible to transfer it again, because there private key is not known.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand how you can transfer/spend an election token to the 'id' of a CREATE transaction. That will be a hash, but in BigchainDB, one transfers/spends outputs to public keys, not hashes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a tricky part I should explain more. Last night I wrote a note about that.

A public key can be generated from a private key, or initialized with 32 bytes (256 bits). The id of a transaction is a 256 bit long hash. So we can use ids as seeds to initialize a public key 🍾. I need to expand this section to explain this process. Maybe we can have a quick chat with @kansi and you. It can be an informal BEP, so it can be reused in other BEPs.

@kansi
Copy link
Contributor

kansi commented May 23, 2018

After discussing this spec with @vrde following issue should be addressed i.e.

The CREATE transaction proposed in the current spec allocates one vote to each node in the network. This implies that all the nodes in the network have an equal influence on the election. Another factor which should be taken in account is power of the nodes i.e. each node in the network has a power (integer) associated to it which is taken into account when calculating if the majority (>2/3) concensus has been achieved. This power should be reflected in the CREATE transaction i.e. each validator should be allocated a number of votes equal to their power in the network. So, the CREATE transaction is only included/commited in a block if each of nodes in the network aggree to the number of votes allocated to the each of nodes for the election. Furthermore, when votes are being casted using TRANSFER transaction each node should cross verify the number of votes a given node is allowed to cast by comparing the current validator and the CREATE transaction. Any mismatch should result in the vote being discarded.

18/README.md Outdated
"Election Address"
]
}
]
Copy link
Contributor

Choose a reason for hiding this comment

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

Comma is missing

18/README.md Outdated
1. `type`: must contain the string `election`.
2. `name`: name of the election.
3. `version`: version number.
4. `matter`: a human readable, short paragraph on the matter of change.
Copy link
Contributor

Choose a reason for hiding this comment

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

The field is not specified in the schema below.

18/README.md Outdated

## Motivation
<!--The motivation is critical for BEPs that want to change the BigchainDB protocol. It should clearly explain why the existing protocol BEP is inadequate to address the problem that the BEP solves. BEP submissions without sufficient motivation may be rejected outright.-->
Changing the shape of a BigchainDB Network at runtime is an important requirement. While this BEP does not address this issue directly, it wants to solve the limitations we had with [Upsert Validators][BEP-3] by giving a tool that can be used this and other situations.
Copy link
Contributor

Choose a reason for hiding this comment

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

it wants to solve the limitations we had with [Upsert Validators][BEP-3]

I would add a more detailed explanation of the limitations.

As I see them:

  • Since Validator Set changes are not themselves expressed as transactions but are included into the metadata of an arbitrary block, all the nodes have to add a validator simultaneously - if less than the majority of the nodes attempt to include a validator, the block fails. Not only the attempt to add a validator fails but all the transactions in the block have to go through the consensus process again. The proposed algorithm, on the other hand, consists of the individual transactions each node can verify without making itself a decision to include a validator.
  • Nodes fail to replay the blocks committed by the validators they do not know yet. The Invalid commit (last validator set size) vs (number of precommits) problem.

18/README.md Outdated

## Motivation
<!--The motivation is critical for BEPs that want to change the BigchainDB protocol. It should clearly explain why the existing protocol BEP is inadequate to address the problem that the BEP solves. BEP submissions without sufficient motivation may be rejected outright.-->
Changing the shape of a BigchainDB Network at runtime is an important requirement. While this BEP does not address this issue directly, it wants to solve the limitations we had with [Upsert Validators (BEP-3)][BEP-3] by giving a tool that can be used this and other situations. BEP-3 addresses a really interesting use case: adding a Member to a running Network. Since changes in the Validator Set are not themselves expressed as transactions but are included into the metadata of an arbitrary block, all the nodes have to add a validator simultaneously—if less than the majority of the nodes attempt to include a Validator, the block fails and the whole Network can stuck. Since those changes are **not** stored in the blockchain itself, a new Node that needs to sync with the Network will fail in validating blocks from a specific *height* on. The *height* will match the one of the first block voted by a new Validator that was **not** included in the `genesis.json` file (and Tendermint will fail with `Invalid commit (last validator set size) vs (number of precommits)`).
Copy link
Contributor

Choose a reason for hiding this comment

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

.. whole Network can get stuck ..

18/README.md Outdated
@@ -15,7 +15,9 @@ This specification introduces the new concept of **Election**. An Election is an

## Motivation
<!--The motivation is critical for BEPs that want to change the BigchainDB protocol. It should clearly explain why the existing protocol BEP is inadequate to address the problem that the BEP solves. BEP submissions without sufficient motivation may be rejected outright.-->
Changing the shape of a BigchainDB Network at runtime is an important requirement. While this BEP does not address this issue directly, it wants to solve the limitations we had with [Upsert Validators][BEP-3] by giving a tool that can be used this and other situations.
Changing the shape of a BigchainDB Network at runtime is an important requirement. While this BEP does not address this issue directly, it wants to solve the limitations we had with [Upsert Validators (BEP-3)][BEP-3] by giving a tool that can be used this and other situations. BEP-3 addresses a really interesting use case: adding a Member to a running Network. Since changes in the Validator Set are not themselves expressed as transactions but are included into the metadata of an arbitrary block, all the nodes have to add a validator simultaneously—if less than the majority of the nodes attempt to include a Validator, the block fails and the whole Network can stuck. Since those changes are **not** stored in the blockchain itself, a new Node that needs to sync with the Network will fail in validating blocks from a specific *height* on. The *height* will match the one of the first block voted by a new Validator that was **not** included in the `genesis.json` file (and Tendermint will fail with `Invalid commit (last validator set size) vs (number of precommits)`).
Copy link
Contributor

Choose a reason for hiding this comment

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

all the nodes have to add a validator simultaneously—if less than the majority of the nodes attempt to include a Validator, the block fails and the whole Network can stuck. Since those changes are not stored in the blockchain itself

I would put it slightly differently:

  1. all the nodes have to agree on the validator set updates;
  2. if the majority agrees on the update, the majority can further post transactions but all other nodes who did not agree on the update get stuck.

If the network is small, there might be not enough nodes to form a majority and thus the whole network might be stuck but it's fine - it just reflects the disagreements between the nodes on the validator topic.

Since those changes are not stored in the blockchain itself, a new Node that needs to sync with the Network will fail in validating blocks from a specific height on.

They are actually stored in the blockchain. If the node does not accept the validator, this behaviour is expected. If the node does accept the validator, it needs to determine the height when to include it upon replay. Basically, that's the main problem the election approach solves - determines the height when to include the new validator.

18/README.md Outdated

## Abstract
<!-- The abstract is a short (~200 word) description of the technical issue being addressed. -->
This specification introduces the new concept of **Election**. An Election is an asynchronous process that when successful triggers Network wide changes synchronously (i.e. at the same *block height*). An Election is started by any Validator in the Network, called **Initiatior**. The Election itself and all cast Votes are transactions, hence stored in the blockchain. This enables new Validators to replay Network changes incrementally, while syncing.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: maybe a , after successful, triggers network wide changes

18/README.md Outdated

## Motivation
<!--The motivation is critical for BEPs that want to change the BigchainDB protocol. It should clearly explain why the existing protocol BEP is inadequate to address the problem that the BEP solves. BEP submissions without sufficient motivation may be rejected outright.-->
Changing the shape of a BigchainDB Network at runtime is an important requirement. While this BEP does not address this issue directly, it wants to solve the limitations we had with [Upsert Validators (BEP-3)][BEP-3] by giving a tool that can be used this and other situations. BEP-3 addresses a really interesting use case: adding a Member to a running Network. Since changes in the Validator Set are not themselves expressed as transactions but are included into the metadata of an arbitrary block, all the nodes have to add a validator simultaneously—if less than the majority of the nodes attempt to include a Validator, the block fails and the whole Network can stuck. Since those changes are **not** stored in the blockchain itself, a new Node that needs to sync with the Network will fail in validating blocks from a specific *height* on. The *height* will match the one of the first block voted by a new Validator that was **not** included in the `genesis.json` file (and Tendermint will fail with `Invalid commit (last validator set size) vs (number of precommits)`).
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a reference to BEP/19 here, I feel I can, understand the motivation more clearly when I understand whats happening in BEP/19 and over here it is kind of skimmed .

@ldmberman
Copy link
Contributor

I have some concerns about implementing proposals and votes via assets compared to new transaction types.

As of now:

  • users have almost full control over the asset structure - it's completely up to them what they store inside the "data" field;
  • BigchainDB only makes sense of the transaction version, there is no such thing as an asset version.

Here I am trying to ponder about what we will face if we change either of those.

Do we validate proposal transactions upon their placement? If yes, we need to put additional effort into implementing asset validation and we further restrict what users can put into their assets. If not, we lose the convenient feature of reporting about the invalid proposals early on.

In any case, since assets are parts of transactions, introducing the election mechanism requires a new version of the transaction spec - it needs to be documented there that certain assets gain special treatment.

Versioning itself poses another question - if we support multiversioned transactions, when do we increase the transaction version and when do we increment the election asset version? The whole topic is quite vague at the moment, but IMO those versions might be connected and we may end up writing validators that check for a cartesian product of transaction and asset versions to apply different validation.

I understand the desire to build on top of what we already have but on the other hand network changes is a fundamental entity so it is worth making them first-class citizens of the system.

I think, it's possible to implement and manage it both ways, but due to the abovementioned I am personally in favor of the new transaction types, like NETWORK_UPDATE and NETWORK_UPDATE_VOTE.

What do you think guys?

@ttmc
Copy link
Contributor

ttmc commented May 31, 2018

I like the suggestion of @ldmberman to have two new transaction types (in addition to the current CREATE and TRANSFER), such as NETWORK_UPDATE and NETWORK_UPDATE_VOTE.

That would mean creating a new version of the BigchainDB Transaction Spec, i.e. v3, which I'm reluctant to do at this point in time, since BigchainDB 2.0 assumes v2 and BigchainDB 2.0 is almost in Beta.

Maybe we can go with using v2 transactions, and special assets, for now (in BigchainDB 2.0). Then we will have time to do a proper design of v3 transactions.

@ldmberman
Copy link
Contributor

@ttmc a new transaction type is a backwards-compatible spec change so we can do 2.1. Also, we need to add a new transaction spec version even if we go on with special assets.

@ttmc ttmc changed the title [WIP] Problem: Validators cannot vote on proposals [WIP] Problem: Validators cannot vote on proposals (BEP-18) Jun 14, 2018
@kansi
Copy link
Contributor

kansi commented Jun 18, 2018

I concur i.e. it would be good to solve this problem via new transaction types i.e. NETWORK_UPDATE and NETWORK_UPDATE_VOTE. So a summary of what changes should be done is as follows,

  • Extend CREATE transaction sepc and define NETWORK_UPDATE transaction sepc and write the corresponding BEP. Some tasks here would be to write spec for asset.data, restrict crypto-conditions to ed25519 etc.
  • Extend TRANSFER transaction spec and define NETWORK_UPDATE_VOTE transaction sepc and write the corresponding BEP.
  • Once we settle on the above-described specs, then refactor/add validation logic for new proposed sepcs in the server code.
  • Once the server has been updated the drivers should be updated to allow the creation of these new transaction types (js, python).

This approach is much elaborate than the current one and its been already suggested that beta releases should be feature freezed so I don't think this approach can be implemented before stable release. So I see two options here,

  • We rollback the upsert-validator feature altogether as the current implementation doesn't work that smoothly and work on the above approach after the stable release.

  • Have the current implementation via asset.data and later upgrade to above-suggested approach.

@vrde @ttmc @ldmberman let me know which approach would be preferable.

@ldmberman
Copy link
Contributor

Insteand of rolling upsert-validator back, we can consider including the complete current set of validators into every EndBlock to make the current implementation usable.

The implementation with asset.data also requires an update of the transaction spec.

@ttmc
Copy link
Contributor

ttmc commented Jun 18, 2018

The implementation with asset.data also requires an update of the transaction spec.

The "BigchainDB Transactions Spec v2" allows asset.data to be almost any JSON (with a few limitations imposed by MongoDB). If we give special meanings to some values of asset.data, then I'm not sure that belongs in the "transactions spec"; it seems out of scope. It can go in other specs, such as BEP-18.

@ldmberman
Copy link
Contributor

If we give special meanings to some values of asset.data, then I'm not sure that belongs in the "transactions spec"; it seems out of scope.

@ttmc how do you decide what to include into the transaction spec? Why should the spec describe the special meaning of version, outputs, inputs, and asset keys but not describe the special meaning of the asset.data.type and asset.data.version keys?

@vrde vrde force-pushed the stateful-upsert-validator branch 2 times, most recently from 3cb7f5f to a8ceb68 Compare July 9, 2018 01:07
@vrde vrde force-pushed the stateful-upsert-validator branch from a8ceb68 to ce94075 Compare July 9, 2018 01:08
18/README.md Outdated
9. Return true.
2. If `asset.data.id` is **not** a valid Election, return.
3. If the Election has less than ⅔ of positive votes, return.
4. If the Election has been already implemented, return.
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about replacing has been already implemented with already has more than ⅔ of positive votes, for clarity?

It would then expand into 3 bullet points instead of 2:

- If the Election including the current vote has less than ⅔ of positive votes, return.
- If the Election excluding the current vote has more than ⅔ of positive votes, return. It has been concluded already.
- If the Election excluding the current vote has less than ⅔ of positive votes and including the current vote has more than ⅔ of positive votes, conclude the election.

18/README.md Outdated

The basic idea is to formalize the concept of an Election storing its data in a [BigchainDB Transaction Spec v2][BEP-13].
The basic idea is to formalize the concept of an Election storing its data in a [BigchainDB Transaction Spec v2 (BEP-13)][BEP-13].
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we mention the need for a new version of the transaction spec here?

18/README.md Outdated
"version": "2.0"
}
```
### Constraints
Copy link
Contributor

Choose a reason for hiding this comment

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

I think, it makes sense to put this into the About Election finality section - the text there seems to be a perfect introduction into what we explain here.


1. Elections can have intersections. In other words, the following is possible:

```election 1 creation height < election 2 creation height < election 1 conclusion height < election 2 conclusion height```
Copy link
Contributor

Choose a reason for hiding this comment

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

I was not right about the intersections - in case 2 elections do not update the validator set, they can interleave even in the first approach. So, the second approach is just about surviving validator set updates.

18/README.md Outdated
5. Each Validator is represented in `outputs`.
6. Each entry in `outputs` can be spent by only one Validator, and the amount attached to it is equal to the power of that Validator.

**Note: any change in the Validator Set will make old Elections invalid.**
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems to belong to the What is a valid Vote? section. The vote is valid if it transfers some vote tokens to a valid election and the validator set has not changed since the election was placed. Does it sound good?

Copy link
Contributor

Choose a reason for hiding this comment

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

Side note:

The vote is valid if it transfers some vote tokens to a valid election and the validator set has not changed since the election was placed

The vote could be a valid vote even if it doesn't transfer to a valid election but to another (non) validator

@ldmberman ldmberman merged commit f8881c5 into bigchaindb:master Jul 9, 2018
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.

5 participants