Problem: Node operator cannot vote on a ValidatorElection (#2428)

* Problem: Node operator cannot vote on a ValidatorElection

Solution: Implement validator election voting spec

* Problem: Incorrent code comments

Solution: Update comments with correct context

* Problem: Delegated vote not casted back to election

Solution: Update test to cast votes back to election id and assert their validity

bigchaindb/__init__.py

@@ -90,7 +90,9 @@ _config = copy.deepcopy(config)
 from bigchaindb.common.transaction import Transaction  # noqa
 from bigchaindb import models                          # noqa
 from bigchaindb.upsert_validator import ValidatorElection  # noqa
+from bigchaindb.upsert_validator import ValidatorElectionVote  # noqa
 
 Transaction.register_type(Transaction.CREATE, models.Transaction)
 Transaction.register_type(Transaction.TRANSFER, models.Transaction)
 Transaction.register_type(ValidatorElection.VALIDATOR_ELECTION, ValidatorElection)
+Transaction.register_type(ValidatorElectionVote.VALIDATOR_ELECTION_VOTE, ValidatorElectionVote)

bigchaindb/common/schema/__init__.py

@@ -34,6 +34,9 @@ _, TX_SCHEMA_TRANSFER = _load_schema('transaction_transfer_' +
 _, TX_SCHEMA_VALIDATOR_ELECTION = _load_schema('transaction_validator_election_' +
                                                TX_SCHEMA_VERSION)
 
+_, TX_SCHEMA_VALIDATOR_ELECTION_VOTE = _load_schema('transaction_validator_election_vote_' +
+                                                    TX_SCHEMA_VERSION)
+
 
 def _validate_schema(schema, body):
     """Validate data against a schema"""

bigchaindb/common/schema/transaction_v2.0.yaml

@@ -59,6 +59,7 @@ definitions:
     - CREATE
     - TRANSFER
     - VALIDATOR_ELECTION
+    - VALIDATOR_ELECTION_VOTE
   asset:
     type: object
     additionalProperties: false

bigchaindb/common/schema/transaction_validator_election_vote_v2.0.yaml

@@ -0,0 +1,27 @@
+---
+"$schema": "http://json-schema.org/draft-04/schema#"
+type: object
+title: Validator Election Vote Schema - Vote on a validator set change
+required:
+- operation
+- outputs
+properties:
+  operation: "VALIDATOR_ELECTION_VOTE"
+  outputs:
+    type: array
+    items:
+      "$ref": "#/definitions/output"
+definitions:
+  output:
+    type: object
+    properties:
+      condition:
+        type: object
+        required:
+          - uri
+        properties:
+          uri:
+            type: string
+            pattern: "^ni:///sha-256;([a-zA-Z0-9_-]{0,86})[?]\
+              (fpt=ed25519-sha-256(&)?|cost=[0-9]+(&)?|\
+              subtypes=ed25519-sha-256(&)?){2,3}$"

bigchaindb/common/transaction.py

@@ -18,6 +18,7 @@ from sha3 import sha3_256
 
 from bigchaindb.common.crypto import PrivateKey, hash_data
 from bigchaindb.common.exceptions import (KeypairMismatchException,
+                                          InputDoesNotExist, DoubleSpend,
                                           InvalidHash, InvalidSignature,
                                           AmountError, AssetIdMismatch,
                                           ThresholdTooDeep)
@@ -523,11 +524,11 @@ class Transaction(object):
         # Asset payloads for 'CREATE' operations must be None or
         # dicts holding a `data` property. Asset payloads for 'TRANSFER'
         # operations must be dicts holding an `id` property.
-        if (operation == Transaction.CREATE and
+        if (operation == self.CREATE and
                 asset is not None and not (isinstance(asset, dict) and 'data' in asset)):
             raise TypeError(('`asset` must be None or a dict holding a `data` '
                              " property instance for '{}' Transactions".format(operation)))
-        elif (operation == Transaction.TRANSFER and
+        elif (operation == self.TRANSFER and
                 not (isinstance(asset, dict) and 'id' in asset)):
             raise TypeError(('`asset` must be a dict holding an `id` property '
                              "for 'TRANSFER' Transactions".format(operation)))
@@ -555,9 +556,9 @@ class Transaction(object):
         structure containing relevant information for storing them in
         a UTXO set, and performing validation.
         """
-        if self.operation == Transaction.CREATE:
+        if self.operation == self.CREATE:
             self._asset_id = self._id
-        elif self.operation == Transaction.TRANSFER:
+        elif self.operation == self.TRANSFER:
             self._asset_id = self.asset['id']
         return (UnspentOutput(
             transaction_id=self._id,
@@ -649,6 +650,31 @@ class Transaction(object):
         (inputs, outputs) = cls.validate_create(tx_signers, recipients, asset, metadata)
         return cls(cls.CREATE, {'data': asset}, inputs, outputs, metadata)
 
+    @classmethod
+    def validate_transfer(cls, inputs, recipients, asset_id, metadata):
+        if not isinstance(inputs, list):
+            raise TypeError('`inputs` must be a list instance')
+        if len(inputs) == 0:
+            raise ValueError('`inputs` must contain at least one item')
+        if not isinstance(recipients, list):
+            raise TypeError('`recipients` must be a list instance')
+        if len(recipients) == 0:
+            raise ValueError('`recipients` list cannot be empty')
+
+        outputs = []
+        for recipient in recipients:
+            if not isinstance(recipient, tuple) or len(recipient) != 2:
+                raise ValueError(('Each `recipient` in the list must be a'
+                                  ' tuple of `([<list of public keys>],'
+                                  ' <amount>)`'))
+            pub_keys, amount = recipient
+            outputs.append(Output.generate(pub_keys, amount))
+
+        if not isinstance(asset_id, str):
+            raise TypeError('`asset_id` must be a string')
+
+        return (deepcopy(inputs), outputs)
+
     @classmethod
     def transfer(cls, inputs, recipients, asset_id, metadata=None):
         """A simple way to generate a `TRANSFER` transaction.
@@ -688,28 +714,7 @@ class Transaction(object):
             Returns:
                 :class:`~bigchaindb.common.transaction.Transaction`
         """
-        if not isinstance(inputs, list):
+        (inputs, outputs) = cls.validate_transfer(inputs, recipients, asset_id, metadata)
-            raise TypeError('`inputs` must be a list instance')
-        if len(inputs) == 0:
-            raise ValueError('`inputs` must contain at least one item')
-        if not isinstance(recipients, list):
-            raise TypeError('`recipients` must be a list instance')
-        if len(recipients) == 0:
-            raise ValueError('`recipients` list cannot be empty')
-
-        outputs = []
-        for recipient in recipients:
-            if not isinstance(recipient, tuple) or len(recipient) != 2:
-                raise ValueError(('Each `recipient` in the list must be a'
-                                  ' tuple of `([<list of public keys>],'
-                                  ' <amount>)`'))
-            pub_keys, amount = recipient
-            outputs.append(Output.generate(pub_keys, amount))
-
-        if not isinstance(asset_id, str):
-            raise TypeError('`asset_id` must be a string')
-
-        inputs = deepcopy(inputs)
         return cls(cls.TRANSFER, {'id': asset_id}, inputs, outputs, metadata)
 
     def __eq__(self, other):
@@ -954,7 +959,7 @@ class Transaction(object):
             #       greatly, as we do not have to check against `None` values.
             return self._inputs_valid(['dummyvalue'
                                        for _ in self.inputs])
-        elif self.operation == Transaction.TRANSFER:
+        elif self.operation == self.TRANSFER:
             return self._inputs_valid([output.fulfillment.condition_uri
                                        for output in outputs])
         else:
@@ -1098,8 +1103,8 @@ class Transaction(object):
         tx = Transaction._remove_signatures(self.to_dict())
         return Transaction._to_str(tx)
 
-    @staticmethod
+    @classmethod
-    def get_asset_id(transactions):
+    def get_asset_id(cls, transactions):
         """Get the asset id from a list of :class:`~.Transactions`.
 
         This is useful when we want to check if the multiple inputs of a
@@ -1123,7 +1128,7 @@ class Transaction(object):
             transactions = [transactions]
 
         # create a set of the transactions' asset ids
-        asset_ids = {tx.id if tx.operation == Transaction.CREATE
+        asset_ids = {tx.id if tx.operation == tx.CREATE
                      else tx.asset['id']
                      for tx in transactions}
 
@@ -1242,3 +1247,56 @@ class Transaction(object):
     @classmethod
     def validate_schema(cls, tx):
         pass
+
+    def validate_transfer_inputs(self, bigchain, current_transactions=[]):
+        # store the inputs so that we can check if the asset ids match
+        input_txs = []
+        input_conditions = []
+        for input_ in self.inputs:
+            input_txid = input_.fulfills.txid
+            input_tx = bigchain.get_transaction(input_txid)
+
+            if input_tx is None:
+                for ctxn in current_transactions:
+                    if ctxn.id == input_txid:
+                        input_tx = ctxn
+
+            if input_tx is None:
+                raise InputDoesNotExist("input `{}` doesn't exist"
+                                        .format(input_txid))
+
+            spent = bigchain.get_spent(input_txid, input_.fulfills.output,
+                                       current_transactions)
+            if spent:
+                raise DoubleSpend('input `{}` was already spent'
+                                  .format(input_txid))
+
+            output = input_tx.outputs[input_.fulfills.output]
+            input_conditions.append(output)
+            input_txs.append(input_tx)
+
+        # Validate that all inputs are distinct
+        links = [i.fulfills.to_uri() for i in self.inputs]
+        if len(links) != len(set(links)):
+            raise DoubleSpend('tx "{}" spends inputs twice'.format(self.id))
+
+        # validate asset id
+        asset_id = self.get_asset_id(input_txs)
+        if asset_id != self.asset['id']:
+            raise AssetIdMismatch(('The asset id of the input does not'
+                                   ' match the asset id of the'
+                                   ' transaction'))
+
+        input_amount = sum([input_condition.amount for input_condition in input_conditions])
+        output_amount = sum([output_condition.amount for output_condition in self.outputs])
+
+        if output_amount != input_amount:
+            raise AmountError(('The amount used in the inputs `{}`'
+                               ' needs to be same as the amount used'
+                               ' in the outputs `{}`')
+                              .format(input_amount, output_amount))
+
+        if not self.inputs_valid(input_conditions):
+            raise InvalidSignature('Transaction signature is invalid.')
+
+        return True

bigchaindb/models.py

@@ -1,7 +1,4 @@
-from bigchaindb.common.exceptions import (InvalidSignature, DoubleSpend,
+from bigchaindb.common.exceptions import (InvalidSignature,
-                                          InputDoesNotExist,
-                                          TransactionNotInValidBlock,
-                                          AssetIdMismatch, AmountError,
                                           DuplicateTransaction)
 from bigchaindb.common.transaction import Transaction
 from bigchaindb.common.utils import (validate_txn_obj, validate_key)
@@ -32,65 +29,13 @@ class Transaction(Transaction):
             if bigchain.get_transaction(self.to_dict()['id']) or duplicates:
                 raise DuplicateTransaction('transaction `{}` already exists'
                                            .format(self.id))
-        elif self.operation == Transaction.TRANSFER:
-            # store the inputs so that we can check if the asset ids match
-            input_txs = []
-            for input_ in self.inputs:
-                input_txid = input_.fulfills.txid
-                input_tx, status = bigchain.\
-                    get_transaction(input_txid, include_status=True)
-
-                if input_tx is None:
-                    for ctxn in current_transactions:
-                        # assume that the status as valid for previously validated
-                        # transactions in current round
-                        if ctxn.id == input_txid:
-                            input_tx = ctxn
-                            status = bigchain.TX_VALID
-
-                if input_tx is None:
-                    raise InputDoesNotExist("input `{}` doesn't exist"
-                                            .format(input_txid))
-
-                if status != bigchain.TX_VALID:
-                    raise TransactionNotInValidBlock(
-                        'input `{}` does not exist in a valid block'.format(
-                            input_txid))
-
-                spent = bigchain.get_spent(input_txid, input_.fulfills.output,
-                                           current_transactions)
-                if spent:
-                    raise DoubleSpend('input `{}` was already spent'
-                                      .format(input_txid))
-
-                output = input_tx.outputs[input_.fulfills.output]
-                input_conditions.append(output)
-                input_txs.append(input_tx)
-
-            # Validate that all inputs are distinct
-            links = [i.fulfills.to_uri() for i in self.inputs]
-            if len(links) != len(set(links)):
-                raise DoubleSpend('tx "{}" spends the same output more than once'.format(self.id))
-
-            # validate asset id
-            asset_id = Transaction.get_asset_id(input_txs)
-            if asset_id != self.asset['id']:
-                raise AssetIdMismatch(('The asset id of the input does not'
-                                       ' match the asset id of the'
-                                       ' transaction'))
-
-            input_amount = sum([input_condition.amount for input_condition in input_conditions])
-            output_amount = sum([output_condition.amount for output_condition in self.outputs])
-
-            if output_amount != input_amount:
-                raise AmountError(('The amount used in the inputs `{}`'
-                                   ' needs to be same as the amount used'
-                                   ' in the outputs `{}`')
-                                  .format(input_amount, output_amount))
 
             if not self.inputs_valid(input_conditions):
                 raise InvalidSignature('Transaction signature is invalid.')
 
+        elif self.operation == Transaction.TRANSFER:
+            self.validate_transfer_inputs(bigchain, current_transactions)
+
         return self
 
     @classmethod

bigchaindb/upsert_validator/__init__.py

@@ -1,2 +1,3 @@
 
 from bigchaindb.upsert_validator.validator_election import ValidatorElection # noqa
+from bigchaindb.upsert_validator.validator_election_vote import ValidatorElectionVote # noqa

bigchaindb/upsert_validator/validator_election_vote.py

@@ -0,0 +1,65 @@
+import base58
+
+from bigchaindb.common.transaction import Transaction
+from bigchaindb.common.schema import (_validate_schema,
+                                      TX_SCHEMA_COMMON,
+                                      TX_SCHEMA_TRANSFER,
+                                      TX_SCHEMA_VALIDATOR_ELECTION_VOTE)
+
+
+class ValidatorElectionVote(Transaction):
+
+    VALIDATOR_ELECTION_VOTE = 'VALIDATOR_ELECTION_VOTE'
+    # NOTE: This class inherits TRANSFER txn type. The `TRANSFER` property is
+    # overriden to re-use methods from parent class
+    TRANSFER = VALIDATOR_ELECTION_VOTE
+    ALLOWED_OPERATIONS = (VALIDATOR_ELECTION_VOTE,)
+
+    def validate(self, bigchain, current_transactions=[]):
+        """Validate election vote transaction
+        NOTE: There are no additional validity conditions on casting votes i.e.
+        a vote is just a valid TRANFER transaction
+
+        For more details refer BEP-21: https://github.com/bigchaindb/BEPs/tree/master/21
+
+        Args:
+            bigchain (BigchainDB): an instantiated bigchaindb.lib.BigchainDB object.
+
+        Returns:
+            `True` if the election vote is valid
+
+        Raises:
+            ValidationError: If the election vote is invalid
+        """
+        self.validate_transfer_inputs(bigchain, current_transactions)
+        return self
+
+    @classmethod
+    def to_public_key(cls, election_id):
+        return base58.b58encode(bytes.fromhex(election_id))
+
+    @classmethod
+    def generate(cls, inputs, recipients, election_id, metadata=None):
+        (inputs, outputs) = cls.validate_transfer(inputs, recipients, election_id, metadata)
+        election_vote = cls(cls.VALIDATOR_ELECTION_VOTE, {'id': election_id}, inputs, outputs, metadata)
+        cls.validate_schema(election_vote.to_dict(), skip_id=True)
+        return election_vote
+
+    @classmethod
+    def validate_schema(cls, tx, skip_id=False):
+        """Validate the validator election vote transaction. Since `VALIDATOR_ELECTION_VOTE` extends `TRANFER`
+           transaction, all the validations for `CREATE` transaction should be inherited
+        """
+        if not skip_id:
+            cls.validate_id(tx)
+        _validate_schema(TX_SCHEMA_COMMON, tx)
+        _validate_schema(TX_SCHEMA_TRANSFER, tx)
+        _validate_schema(TX_SCHEMA_VALIDATOR_ELECTION_VOTE, tx)
+
+    @classmethod
+    def create(cls, tx_signers, recipients, metadata=None, asset=None):
+        raise NotImplementedError
+
+    @classmethod
+    def transfer(cls, tx_signers, recipients, metadata=None, asset=None):
+        raise NotImplementedError

tests/conftest.py

@@ -647,6 +647,17 @@ def node_key(node_keys):
     return key_pair_from_ed25519_key(key_from_base64(priv))
 
 
+@pytest.fixture
+def ed25519_node_keys(node_keys):
+    (pub, priv) = list(node_keys.items())[0]
+    node_keys_dict = {}
+    for pub, priv in node_keys.items():
+        key = key_pair_from_ed25519_key(key_from_base64(priv))
+        node_keys_dict[key.public_key] = key
+
+    return node_keys_dict
+
+
 @pytest.fixture(scope='session')
 def node_keys():
     return {'zL/DasvKulXZzhSNFwx4cLRXKkSM9GPK7Y0nZ4FEylM=':

tests/upsert_validator/conftest.py

@@ -1,5 +1,7 @@
 import pytest
 
+from bigchaindb.upsert_validator import ValidatorElection
+
 
 @pytest.fixture
 def b_mock(b, network_validators):
@@ -30,3 +32,11 @@ def mock_get_validators(network_validators):
         return validators
 
     return validator_set
+
+
+@pytest.fixture
+def valid_election(b_mock, node_key, new_validator):
+    voters = ValidatorElection.recipients(b_mock)
+    return ValidatorElection.generate([node_key.public_key],
+                                      voters,
+                                      new_validator, None).sign([node_key.private_key])

tests/upsert_validator/test_validator_election_vote.py

@@ -0,0 +1,80 @@
+import pytest
+
+from bigchaindb.upsert_validator import ValidatorElectionVote
+from bigchaindb.common.exceptions import AmountError
+
+
+pytestmark = [pytest.mark.tendermint, pytest.mark.bdb]
+
+
+def test_upsert_validator_valid_election_vote(b_mock, valid_election, ed25519_node_keys):
+    b_mock.store_bulk_transactions([valid_election])
+
+    input0 = valid_election.to_inputs()[0]
+    votes = valid_election.outputs[0].amount
+    public_key0 = input0.owners_before[0]
+    key0 = ed25519_node_keys[public_key0]
+
+    election_pub_key = ValidatorElectionVote.to_public_key(valid_election.id)
+
+    vote = ValidatorElectionVote.generate([input0],
+                                          [([election_pub_key], votes)],
+                                          election_id=valid_election.id)\
+                                .sign([key0.private_key])
+    assert vote.validate(b_mock)
+
+
+def test_upsert_validator_delegate_election_vote(b_mock, valid_election, ed25519_node_keys):
+    from bigchaindb.common.crypto import generate_key_pair
+
+    alice = generate_key_pair()
+
+    b_mock.store_bulk_transactions([valid_election])
+
+    input0 = valid_election.to_inputs()[0]
+    votes = valid_election.outputs[0].amount
+    public_key0 = input0.owners_before[0]
+    key0 = ed25519_node_keys[public_key0]
+
+    delegate_vote = ValidatorElectionVote.generate([input0],
+                                                   [([alice.public_key], 3), ([key0.public_key], votes-3)],
+                                                   election_id=valid_election.id)\
+                                         .sign([key0.private_key])
+
+    assert delegate_vote.validate(b_mock)
+
+    b_mock.store_bulk_transactions([delegate_vote])
+    election_pub_key = ValidatorElectionVote.to_public_key(valid_election.id)
+
+    alice_votes = delegate_vote.to_inputs()[0]
+    alice_casted_vote = ValidatorElectionVote.generate([alice_votes],
+                                                       [([election_pub_key], 3)],
+                                                       election_id=valid_election.id)\
+                                             .sign([alice.private_key])
+    assert alice_casted_vote.validate(b_mock)
+
+    key0_votes = delegate_vote.to_inputs()[1]
+    key0_casted_vote = ValidatorElectionVote.generate([key0_votes],
+                                                      [([election_pub_key], votes-3)],
+                                                      election_id=valid_election.id)\
+                                            .sign([key0.private_key])
+    assert key0_casted_vote.validate(b_mock)
+
+
+def test_upsert_validator_invalid_election_vote(b_mock, valid_election, ed25519_node_keys):
+    b_mock.store_bulk_transactions([valid_election])
+
+    input0 = valid_election.to_inputs()[0]
+    votes = valid_election.outputs[0].amount
+    public_key0 = input0.owners_before[0]
+    key0 = ed25519_node_keys[public_key0]
+
+    election_pub_key = ValidatorElectionVote.to_public_key(valid_election.id)
+
+    vote = ValidatorElectionVote.generate([input0],
+                                          [([election_pub_key], votes+1)],
+                                          election_id=valid_election.id)\
+                                .sign([key0.private_key])
+
+    with pytest.raises(AmountError):
+        assert vote.validate(b_mock)